diff mbox series

[v2,1/6] dt-bindings: can: sja1000: Convert to json-schema

Message ID 20220703104705.341070-2-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series Add support for RZ/N1 SJA1000 CAN controller | expand

Checks

Context Check Description
netdev/tree_selection success Guessed tree name to be net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers success CCed 11 of 11 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: DT binding docs and includes should be a separate patch. See: Documentation/devicetree/bindings/submitting-patches.rst WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Biju Das July 3, 2022, 10:47 a.m. UTC 
Convert the NXP SJA1000 CAN Controller Device Tree binding
documentation to json-schema.

Update the example to match reality.

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v1->v2:
 * Moved $ref: can-controller.yaml# to top along with if conditional to
   avoid multiple mapping issues with the if conditional in the subsequent
   patch.
---
 .../bindings/net/can/nxp,sja1000.yaml         | 102 ++++++++++++++++++
 .../devicetree/bindings/net/can/sja1000.txt   |  58 ----------
 2 files changed, 102 insertions(+), 58 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/can/sja1000.txt

Comments

Rob Herring July 3, 2022, 6 p.m. UTC | #1 
On Sun, 03 Jul 2022 11:47:00 +0100, Biju Das wrote:
> Convert the NXP SJA1000 CAN Controller Device Tree binding
> documentation to json-schema.
> 
> Update the example to match reality.
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v1->v2:
>  * Moved $ref: can-controller.yaml# to top along with if conditional to
>    avoid multiple mapping issues with the if conditional in the subsequent
>    patch.
> ---
>  .../bindings/net/can/nxp,sja1000.yaml         | 102 ++++++++++++++++++
>  .../devicetree/bindings/net/can/sja1000.txt   |  58 ----------
>  2 files changed, 102 insertions(+), 58 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
>  delete mode 100644 Documentation/devicetree/bindings/net/can/sja1000.txt
> 

Running 'make dtbs_check' with the schema in this patch gives the
following warnings. Consider if they are expected or the schema is
incorrect. These may not be new warnings.

Note that it is not yet a requirement to have 0 warnings for dtbs_check.
This will change in the future.

Full log is available here: https://patchwork.ozlabs.org/patch/


can@4,0: nxp,tx-output-config:0:0: 22 is not one of [1, 2, 4, 6, 8, 16, 32, 48]
	arch/arm/boot/dts/imx27-phytec-phycore-rdk.dtb

can@4,0: 'reg-io-width' is a required property
	arch/arm/boot/dts/imx27-phytec-phycore-rdk.dtb
Biju Das July 3, 2022, 6:54 p.m. UTC | #2 
Hi Rob,

Thanks for the feedback.

> Subject: Re: [PATCH v2 1/6] dt-bindings: can: sja1000: Convert to json-
> schema
> 
> On Sun, 03 Jul 2022 11:47:00 +0100, Biju Das wrote:
> > Convert the NXP SJA1000 CAN Controller Device Tree binding
> > documentation to json-schema.
> >
> > Update the example to match reality.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> > v1->v2:
> >  * Moved $ref: can-controller.yaml# to top along with if conditional
> > to    avoid multiple mapping issues with the if conditional in the
> subsequent
> >    patch.
> > ---
> >  .../bindings/net/can/nxp,sja1000.yaml         | 102
> ++++++++++++++++++
> >  .../devicetree/bindings/net/can/sja1000.txt   |  58 ----------
> >  2 files changed, 102 insertions(+), 58 deletions(-)  create mode
> > 100644 Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
> >  delete mode 100644
> > Documentation/devicetree/bindings/net/can/sja1000.txt
> >
> 
> Running 'make dtbs_check' with the schema in this patch gives the
> following warnings. Consider if they are expected or the schema is
> incorrect. These may not be new warnings.
> 
> Note that it is not yet a requirement to have 0 warnings for dtbs_check.
> This will change in the future.
>

OK.
 
> 
> can@4,0: nxp,tx-output-config:0:0: 22 is not one of [1, 2, 4, 6, 8, 16,
> 32, 48]
> 	arch/arm/boot/dts/imx27-phytec-phycore-rdk.dtb

Looks like this property is not enum, as combination possible with tx-output-config values for TX0 and TX1. 

22 = 6 + 16 -> TX0 push-pull + TX1 pull-down

I will remove enums definition from the next version.
which will fix this error. 

Also please let me know, is there any better way to handle
combination of enum values like above.

> 
> can@4,0: 'reg-io-width' is a required property
> 	arch/arm/boot/dts/imx27-phytec-phycore-rdk.dtb

I have added this as per original documentation[1], it is ignored only for
Technologic. But after checking [2] none of them except Technologic is using reg-io
I will send V3 with fixing this.

[1] snippet from original bindings.
Optional properties:
reg-io-width : Specify the size (in bytes) of the IO accesses that
	should be performed on the device.  Valid value is 1, 2 or 4.
	This property is ignored for technologic version.
	Default to 1 (8 bits).

[2]
biju@biju-VirtualBox:~/linux-next$ fgrep -r "sja1000" arch
arch/powerpc/boot/dts/digsy_mtc.dts:			compatible = "nxp,sja1000";
arch/powerpc/boot/dts/digsy_mtc.dts:			compatible = "nxp,sja1000";
arch/powerpc/boot/dts/socrates.dts:			compatible = "philips,sja1000";
Binary file arch/arm/boot/dts/imx51-ts4800.dtb matches
arch/arm/boot/dts/imx51-ts4800.dts:			compatible = "technologic,sja1000";
arch/arm/boot/dts/imx27-phytec-phycore-rdk.dts:		compatible = "nxp,sja1000";

Cheers,
Biju
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
new file mode 100644
index 000000000000..3232ce7e2642
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/can/nxp,sja1000.yaml
@@ -0,0 +1,102 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/can/nxp,sja1000.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Memory mapped SJA1000 CAN controller from NXP (formerly Philips)
+
+maintainers:
+  - Wolfgang Grandegger <wg@grandegger.com>
+
+allOf:
+  - $ref: can-controller.yaml#
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: technologic,sja1000
+    then:
+      required:
+        - reg-io-width
+
+properties:
+  compatible:
+    oneOf:
+      - description: NXP SJA1000 CAN Controller
+        const: nxp,sja1000
+      - description: Technologic Systems SJA1000 CAN Controller
+        const: technologic,sja1000
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reg-io-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: I/O register width (in bytes) implemented by this device
+    default: 1
+    enum: [ 1, 2, 4 ]
+
+  nxp,external-clock-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 16000000
+    description: |
+      Frequency of the external oscillator clock in Hz.
+      The internal clock frequency used by the SJA1000 is half of that value.
+
+  nxp,tx-output-mode:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0, 1, 2, 3 ]
+    default: 1
+    description: |
+      operation mode of the TX output control logic.  Valid values are:
+        <0x0> : bi-phase output mode
+        <0x1> : normal output mode (default)
+        <0x2> : test output mode
+        <0x3> : clock output mode
+
+  nxp,tx-output-config:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [ 0x01, 0x02, 0x04, 0x06, 0x08, 0x10, 0x20, 0x30 ]
+    default: 0x02
+    description: |
+      TX output pin configuration.  Valid values are:
+        <0x01> : TX0 invert
+        <0x02> : TX0 pull-down (default)
+        <0x04> : TX0 pull-up
+        <0x06> : TX0 push-pull
+        <0x08> : TX1 invert
+        <0x10> : TX1 pull-down
+        <0x20> : TX1 pull-up
+        <0x30> : TX1 push-pull
+
+  nxp,clock-out-frequency:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      clock frequency in Hz on the CLKOUT pin.
+      If not specified or if the specified value is 0, the CLKOUT pin
+      will be disabled.
+
+  nxp,no-comparator-bypass:
+    type: boolean
+    description: Allows to disable the CAN input comparator.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    can@1a000 {
+            compatible = "technologic,sja1000";
+            reg = <0x1a000 0x100>;
+            interrupts = <1>;
+            nxp,external-clock-frequency = <24000000>;
+    };
diff --git a/Documentation/devicetree/bindings/net/can/sja1000.txt b/Documentation/devicetree/bindings/net/can/sja1000.txt
deleted file mode 100644
index ac3160eca96a..000000000000
--- a/Documentation/devicetree/bindings/net/can/sja1000.txt
+++ /dev/null
@@ -1,58 +0,0 @@ 
-Memory mapped SJA1000 CAN controller from NXP (formerly Philips)
-
-Required properties:
-
-- compatible : should be one of "nxp,sja1000", "technologic,sja1000".
-
-- reg : should specify the chip select, address offset and size required
-	to map the registers of the SJA1000. The size is usually 0x80.
-
-- interrupts: property with a value describing the interrupt source
-	(number and sensitivity) required for the SJA1000.
-
-Optional properties:
-
-- reg-io-width : Specify the size (in bytes) of the IO accesses that
-	should be performed on the device.  Valid value is 1, 2 or 4.
-	This property is ignored for technologic version.
-	Default to 1 (8 bits).
-
-- nxp,external-clock-frequency : Frequency of the external oscillator
-	clock in Hz. Note that the internal clock frequency used by the
-	SJA1000 is half of that value. If not specified, a default value
-	of 16000000 (16 MHz) is used.
-
-- nxp,tx-output-mode : operation mode of the TX output control logic:
-	<0x0> : bi-phase output mode
-	<0x1> : normal output mode (default)
-	<0x2> : test output mode
-	<0x3> : clock output mode
-
-- nxp,tx-output-config : TX output pin configuration:
-	<0x01> : TX0 invert
-	<0x02> : TX0 pull-down (default)
-	<0x04> : TX0 pull-up
-	<0x06> : TX0 push-pull
-	<0x08> : TX1 invert
-	<0x10> : TX1 pull-down
-	<0x20> : TX1 pull-up
-	<0x30> : TX1 push-pull
-
-- nxp,clock-out-frequency : clock frequency in Hz on the CLKOUT pin.
-	If not specified or if the specified value is 0, the CLKOUT pin
-	will be disabled.
-
-- nxp,no-comparator-bypass : Allows to disable the CAN input comparator.
-
-For further information, please have a look to the SJA1000 data sheet.
-
-Examples:
-
-can@3,100 {
-	compatible = "nxp,sja1000";
-	reg = <3 0x100 0x80>;
-	interrupts = <2 0>;
-	interrupt-parent = <&mpic>;
-	nxp,external-clock-frequency = <16000000>;
-};
-