diff mbox series

[1/3] dt-bindings: soc: samsung: exynos-speedy: Document SPEEDY host controller bindings

Message ID 20241212-speedy-v1-1-544ad7bcfb6a@gmail.com (mailing list archive)
State New
Headers show
Series Add Samsung SPEEDY serial bus host controller driver | expand

Commit Message

Markuss Broks Dec. 12, 2024, 9:09 p.m. UTC
Add the schema for the Samsung SPEEDY serial bus host controller.
The bus has 4 bit wide addresses for addressing devices
and 8 bit wide register addressing. Each register is also 8
bit long, so the address can be 0-f (hexadecimal), node name
for child device follows the format: node_name@[0-f].

Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz>
Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz>
Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
---
 .../bindings/soc/samsung/exynos-speedy.yaml        | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)

Comments

Rob Herring (Arm) Dec. 12, 2024, 10:30 p.m. UTC | #1
On Thu, 12 Dec 2024 23:09:01 +0200, Markuss Broks wrote:
> Add the schema for the Samsung SPEEDY serial bus host controller.
> The bus has 4 bit wide addresses for addressing devices
> and 8 bit wide register addressing. Each register is also 8
> bit long, so the address can be 0-f (hexadecimal), node name
> for child device follows the format: node_name@[0-f].
> 
> Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz>
> Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../bindings/soc/samsung/exynos-speedy.yaml        | 78 ++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: ignoring, error in schema: properties: clock-names
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:compatible: [{'items': [{'enum': ['samsung,exynos9810-speedy']}, {'const': 'samsung,exynos-speedy'}]}] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:clock-names: [{'const': 'pclk'}] is not of type 'object', 'boolean'
	from schema $id: http://json-schema.org/draft-07/schema#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:compatible: [{'items': [{'enum': ['samsung,exynos9810-speedy']}, {'const': 'samsung,exynos-speedy'}]}] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml: properties:clock-names: [{'const': 'pclk'}] is not of type 'object', 'boolean'
	from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000: failed to match any schema with compatible: ['samsung,exynos9810-speedy', 'samsung-exynos-speedy']
Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000: failed to match any schema with compatible: ['samsung,exynos9810-speedy', 'samsung-exynos-speedy']
Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000/pmic@0: failed to match any schema with compatible: ['samsung,s2mps18-pmic']
Documentation/devicetree/bindings/soc/samsung/exynos-speedy.example.dtb: /example-0/speedy@141c0000/regulator@1: failed to match any schema with compatible: ['samsung,s2mps18-regulator']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241212-speedy-v1-1-544ad7bcfb6a@gmail.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Krzysztof Kozlowski Dec. 13, 2024, 7:40 a.m. UTC | #2
On 12/12/2024 22:09, Markuss Broks wrote:
> Add the schema for the Samsung SPEEDY serial bus host controller.
> The bus has 4 bit wide addresses for addressing devices
> and 8 bit wide register addressing. Each register is also 8
> bit long, so the address can be 0-f (hexadecimal), node name
> for child device follows the format: node_name@[0-f].


This wasn't tested so limited review.

A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz>
> Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz>
> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
> ---
>  .../bindings/soc/samsung/exynos-speedy.yaml        | 78 ++++++++++++++++++++++

Filename must match compatible.

>  1 file changed, 78 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung Exynos SPEEDY serial bus host controller

Speedy or SPEEDY?

> +
> +maintainers:
> +  - Markuss Broks <markuss.broks@gmail.com>
> +
> +description:
> +  Samsung SPEEDY is a proprietary Samsung serial 1-wire bus.

1-wire? But not compatible with w1 (onwire)?

> +  It is used on various Samsung Exynos chips. The bus can
> +  address at most 4 bit (16) devices. The devices on the bus
> +  have 8 bit long register line, and the registers are also
> +  8 bit long each. It is typically used for communicating with
> +  Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips,
> +  such as RF parts.
> +
> +properties:
> +  compatible:
> +    - items:
> +        - enum:
> +            - samsung,exynos9810-speedy
> +        - const: samsung,exynos-speedy

Drop last compatible and use only SoC specific.

> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    - const: pclk

Drop clock-names, not needed for one entry.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - "#address-cells"
> +  - "#size-cells"

You do not have them in the properties, anyway required goes before
additionalProperties

> +
> +patternProperties:
> +  "^[a-z][a-z0-9]*@[0-9a-f]$":

That's odd regex. Look at other bus bindings.

> +    type: object
> +    additionalProperties: true
> +
> +    properties:
> +      reg:
> +        maxItems: 1

maximum: 15

> +
> +    required:
> +      - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    speedy0: speedy@141c0000 {

Drop unused label.

> +      compatible = "samsung,exynos9810-speedy",
> +                   "samsung-exynos-speedy";
> +      reg = <0x141c0000 0x2000>;
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +

No resources? No clocks? No interrupts?



Best regards,
Krzysztof
Markuss Broks Dec. 13, 2024, 9:47 a.m. UTC | #3
Hi Krzysztof,

On 12/13/24 9:40 AM, Krzysztof Kozlowski wrote:
> On 12/12/2024 22:09, Markuss Broks wrote:
>> Add the schema for the Samsung SPEEDY serial bus host controller.
>> The bus has 4 bit wide addresses for addressing devices
>> and 8 bit wide register addressing. Each register is also 8
>> bit long, so the address can be 0-f (hexadecimal), node name
>> for child device follows the format: node_name@[0-f].
>
> This wasn't tested so limited review.
>
> A nit, subject: drop second/last, redundant "bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> See also:
> https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18
>
>> Co-developed-by: Maksym Holovach <nergzd@nergzd723.xyz>
>> Signed-off-by: Maksym Holovach <nergzd@nergzd723.xyz>
>> Signed-off-by: Markuss Broks <markuss.broks@gmail.com>
>> ---
>>   .../bindings/soc/samsung/exynos-speedy.yaml        | 78 ++++++++++++++++++++++
> Filename must match compatible.
>
>>   1 file changed, 78 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
>> @@ -0,0 +1,78 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Samsung Exynos SPEEDY serial bus host controller
> Speedy or SPEEDY?
Technically it's an acronym (Serial Protocol in an EffEctive Digital 
waY), but we could agree on if we use the capitalized or uncapitalised 
version and use it consistently throughout.
>
>> +
>> +maintainers:
>> +  - Markuss Broks <markuss.broks@gmail.com>
>> +
>> +description:
>> +  Samsung SPEEDY is a proprietary Samsung serial 1-wire bus.
> 1-wire? But not compatible with w1 (onwire)?
Nope, I suppose this requires more clarification, as explained in the 
previous letter, there are several differences between the protocols, 
looking at the Samsung patent. [1]
>
>> +  It is used on various Samsung Exynos chips. The bus can
>> +  address at most 4 bit (16) devices. The devices on the bus
>> +  have 8 bit long register line, and the registers are also
>> +  8 bit long each. It is typically used for communicating with
>> +  Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips,
>> +  such as RF parts.
>> +
>> +properties:
>> +  compatible:
>> +    - items:
>> +        - enum:
>> +            - samsung,exynos9810-speedy
>> +        - const: samsung,exynos-speedy
> Drop last compatible and use only SoC specific.
Makes sense, for some reason I didn't realise it doesn't make much sense.
>
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +  clock-names:
>> +    - const: pclk
> Drop clock-names, not needed for one entry.
>
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - "#address-cells"
>> +  - "#size-cells"
> You do not have them in the properties, anyway required goes before
> additionalProperties
>
>> +
>> +patternProperties:
>> +  "^[a-z][a-z0-9]*@[0-9a-f]$":
> That's odd regex. Look at other bus bindings.
Okay, I'll look into it.
>
>> +    type: object
>> +    additionalProperties: true
>> +
>> +    properties:
>> +      reg:
>> +        maxItems: 1
> maximum: 15
>
>> +
>> +    required:
>> +      - reg
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    speedy0: speedy@141c0000 {
> Drop unused label.
>
>> +      compatible = "samsung,exynos9810-speedy",
>> +                   "samsung-exynos-speedy";
>> +      reg = <0x141c0000 0x2000>;
>> +      #address-cells = <1>;
>> +      #size-cells = <0>;
>> +
> No resources? No clocks? No interrupts?
Will extend the example.
>
>
>
> Best regards,
> Krzysztof

- Markuss


[1] https://patents.google.com/patent/US9882711B1/en
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
new file mode 100644
index 0000000000000000000000000000000000000000..304b322a74ea70f23d8c072b44b6ca86b7cc807f
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/samsung/exynos-speedy.yaml
@@ -0,0 +1,78 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/samsung/exynos-speedy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Exynos SPEEDY serial bus host controller
+
+maintainers:
+  - Markuss Broks <markuss.broks@gmail.com>
+
+description:
+  Samsung SPEEDY is a proprietary Samsung serial 1-wire bus.
+  It is used on various Samsung Exynos chips. The bus can
+  address at most 4 bit (16) devices. The devices on the bus
+  have 8 bit long register line, and the registers are also
+  8 bit long each. It is typically used for communicating with
+  Samsung PMICs (s2mps17, s2mps18, ...) and other Samsung chips,
+  such as RF parts.
+
+properties:
+  compatible:
+    - items:
+        - enum:
+            - samsung,exynos9810-speedy
+        - const: samsung,exynos-speedy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    - const: pclk
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - "#address-cells"
+  - "#size-cells"
+
+patternProperties:
+  "^[a-z][a-z0-9]*@[0-9a-f]$":
+    type: object
+    additionalProperties: true
+
+    properties:
+      reg:
+        maxItems: 1
+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    speedy0: speedy@141c0000 {
+      compatible = "samsung,exynos9810-speedy",
+                   "samsung-exynos-speedy";
+      reg = <0x141c0000 0x2000>;
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      pmic@0 {
+        compatible = "samsung,s2mps18-pmic";
+        reg = <0x0>;
+      };
+
+      regulator@1 {
+        compatible = "samsung,s2mps18-regulator";
+        reg = <0x1>;
+      };
+    };