diff mbox series

[v2,1/4] dt-bindings: net: Add bindings for AX88796C SPI Ethernet Adapter

Message ID 20201002192210.19967-2-l.stelmach@samsung.com (mailing list archive)
State Not Applicable
Headers show
Series AX88796C SPI Ethernet Adapter | expand

Commit Message

Lukasz Stelmach Oct. 2, 2020, 7:22 p.m. UTC
Add bindings for AX88796C SPI Ethernet Adapter.

Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
---
 .../bindings/net/asix,ax88796c-spi.yaml       | 76 +++++++++++++++++++
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 2 files changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml

Comments

Krzysztof Kozlowski Oct. 3, 2020, 10:09 a.m. UTC | #1
On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>
> Add bindings for AX88796C SPI Ethernet Adapter.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  .../bindings/net/asix,ax88796c-spi.yaml       | 76 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> new file mode 100644
> index 000000000000..50a488d59dbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/asix,ax88796c-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASIX AX88796C SPI Ethernet Adapter
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#

Order of top-level entries please:
1. id, schema
2. title
3. maintainers
4. description
and then allOf. See example-schema.yaml.

> +
> +description: |
> +  ASIX AX88796C is an Ethernet controller with a built in PHY. This
> +  describes SPI mode of the chip.
> +
> +  The node for this driver must be a child node of a SPI controller, hence
> +  all mandatory properties described in ../spi/spi-bus.txt must be specified.
> +
> +maintainers:
> +  - Łukasz Stelmach <l.stelmach@samsung.com>
> +
> +properties:
> +  compatible:
> +    const: asix,ax99796c-spi
> +
> +  reg:
> +    description:
> +      SPI device address.

Skip description, it's trivial.

> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 40000000
> +
> +  interrupts:
> +    description:
> +     GPIO interrupt to which the chip is connected.

Skip the description. It's trivial and might be not accurate (does not
have to be a GPIO).

> +    maxItems: 1
> +
> +  interrupt-parrent:
> +    description:
> +      A phandle of an interrupt controller.

Skip description.

> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      A GPIO line handling reset of the chip. As the line is active low,
> +      it should be marked GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  local-mac-address: true
> +
> +  mac-address: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - interrupts
> +  - interrupt-parrent
> +  - reset-gpios

Additional properties false.

> +
> +examples:
> +  # Artik5 eval board
> +  - |
> +    ax88796c@0 {
> +        compatible = "asix,ax88796c";
> +        local-mac-address = [00 00 00 00 00 00]; /* Filled in by a bootloader */
> +        interrupt-parent = <&gpx2>;
> +        interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +        spi-max-frequency = <40000000>;
> +        reg = <0x0>;
> +        reset-gpios = <&gpe0 2 GPIO_ACTIVE_LOW>;
> +        controller-data {
> +            samsung,spi-feedback-delay = <2>;
> +        };
> +    };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> index 2baee2c817c1..5ce5c4a43735 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> @@ -117,6 +117,8 @@ patternProperties:
>      description: Asahi Kasei Corp.
>    "^asc,.*":
>      description: All Sensors Corporation
> +  "^asix,.*":
> +    description: ASIX Electronics Corporation

Separate patch please.

Best regards,
Krzysztof

>    "^aspeed,.*":
>      description: ASPEED Technology Inc.
>    "^asus,.*":
> --
> 2.26.2
>
Rob Herring (Arm) Oct. 5, 2020, 1:59 p.m. UTC | #2
On Fri, 02 Oct 2020 21:22:07 +0200, Łukasz Stelmach wrote:
> Add bindings for AX88796C SPI Ethernet Adapter.
> 
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  .../bindings/net/asix,ax88796c-spi.yaml       | 76 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> 


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

Error: Documentation/devicetree/bindings/net/asix,ax88796c-spi.example.dts:23.29-30 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:342: Documentation/devicetree/bindings/net/asix,ax88796c-spi.example.dt.yaml] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1366: dt_binding_check] Error 2


See https://patchwork.ozlabs.org/patch/1376051

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

pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade

Please check and re-submit.
Krzysztof Kozlowski Oct. 5, 2020, 2:01 p.m. UTC | #3
On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
>
> Add bindings for AX88796C SPI Ethernet Adapter.
>
> Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> ---
>  .../bindings/net/asix,ax88796c-spi.yaml       | 76 +++++++++++++++++++
>  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
>  2 files changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> new file mode 100644
> index 000000000000..50a488d59dbf
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> @@ -0,0 +1,76 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/asix,ax88796c-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ASIX AX88796C SPI Ethernet Adapter
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#
> +
> +description: |
> +  ASIX AX88796C is an Ethernet controller with a built in PHY. This
> +  describes SPI mode of the chip.
> +
> +  The node for this driver must be a child node of a SPI controller, hence
> +  all mandatory properties described in ../spi/spi-bus.txt must be specified.
> +
> +maintainers:
> +  - Łukasz Stelmach <l.stelmach@samsung.com>
> +
> +properties:
> +  compatible:
> +    const: asix,ax99796c-spi
> +
> +  reg:
> +    description:
> +      SPI device address.
> +    maxItems: 1
> +
> +  spi-max-frequency:
> +    maximum: 40000000
> +
> +  interrupts:
> +    description:
> +     GPIO interrupt to which the chip is connected.
> +    maxItems: 1
> +
> +  interrupt-parrent:
> +    description:
> +      A phandle of an interrupt controller.
> +    maxItems: 1
> +
> +  reset-gpios:
> +    description:
> +      A GPIO line handling reset of the chip. As the line is active low,
> +      it should be marked GPIO_ACTIVE_LOW.
> +    maxItems: 1
> +
> +  local-mac-address: true
> +
> +  mac-address: true
> +
> +required:
> +  - compatible
> +  - reg
> +  - spi-max-frequency
> +  - interrupts
> +  - interrupt-parrent
> +  - reset-gpios
> +
> +examples:
> +  # Artik5 eval board
> +  - |
> +    ax88796c@0 {
> +        compatible = "asix,ax88796c";
> +        local-mac-address = [00 00 00 00 00 00]; /* Filled in by a bootloader */
> +        interrupt-parent = <&gpx2>;
> +        interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> +        spi-max-frequency = <40000000>;
> +        reg = <0x0>;
> +        reset-gpios = <&gpe0 2 GPIO_ACTIVE_LOW>;

This and IRQ won't compile without defines, which Rob's robot just
pointed out. It seems you did not test the bindings you post.

Best regards,
Krzysztof
Rob Herring (Arm) Oct. 5, 2020, 2:03 p.m. UTC | #4
On Sat, Oct 03, 2020 at 12:09:55PM +0200, Krzysztof Kozlowski wrote:
> On Fri, 2 Oct 2020 at 21:22, Łukasz Stelmach <l.stelmach@samsung.com> wrote:
> >
> > Add bindings for AX88796C SPI Ethernet Adapter.
> >
> > Signed-off-by: Łukasz Stelmach <l.stelmach@samsung.com>
> > ---
> >  .../bindings/net/asix,ax88796c-spi.yaml       | 76 +++++++++++++++++++
> >  .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
> >  2 files changed, 78 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> > new file mode 100644
> > index 000000000000..50a488d59dbf
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
> > @@ -0,0 +1,76 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/asix,ax88796c-spi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ASIX AX88796C SPI Ethernet Adapter
> > +
> > +allOf:
> > +  - $ref: ethernet-controller.yaml#
> 
> Order of top-level entries please:
> 1. id, schema
> 2. title
> 3. maintainers
> 4. description
> and then allOf. See example-schema.yaml.
> 
> > +
> > +description: |
> > +  ASIX AX88796C is an Ethernet controller with a built in PHY. This
> > +  describes SPI mode of the chip.
> > +
> > +  The node for this driver must be a child node of a SPI controller, hence
> > +  all mandatory properties described in ../spi/spi-bus.txt must be specified.

Did you read spi-bus.txt?

> > +
> > +maintainers:
> > +  - Łukasz Stelmach <l.stelmach@samsung.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: asix,ax99796c-spi

'spi' is implied by the bus the device is on, so drop.

> > +
> > +  reg:
> > +    description:
> > +      SPI device address.
> 
> Skip description, it's trivial.
> 
> > +    maxItems: 1
> > +
> > +  spi-max-frequency:
> > +    maximum: 40000000
> > +
> > +  interrupts:
> > +    description:
> > +     GPIO interrupt to which the chip is connected.
> 
> Skip the description. It's trivial and might be not accurate (does not
> have to be a GPIO).
> 
> > +    maxItems: 1
> > +
> > +  interrupt-parrent:

Typo. But you don't need to list interrupt-parent.

> > +    description:
> > +      A phandle of an interrupt controller.
> 
> Skip description.

> 
> > +    maxItems: 1
> > +
> > +  reset-gpios:
> > +    description:
> > +      A GPIO line handling reset of the chip. As the line is active low,
> > +      it should be marked GPIO_ACTIVE_LOW.
> > +    maxItems: 1
> > +
> > +  local-mac-address: true
> > +
> > +  mac-address: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - spi-max-frequency
> > +  - interrupts
> > +  - interrupt-parrent
> > +  - reset-gpios
> 
> Additional properties false.
> 
> > +
> > +examples:
> > +  # Artik5 eval board
> > +  - |
> > +    ax88796c@0 {

ethernet@0

> > +        compatible = "asix,ax88796c";
> > +        local-mac-address = [00 00 00 00 00 00]; /* Filled in by a bootloader */
> > +        interrupt-parent = <&gpx2>;
> > +        interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
> > +        spi-max-frequency = <40000000>;
> > +        reg = <0x0>;
> > +        reset-gpios = <&gpe0 2 GPIO_ACTIVE_LOW>;
> > +        controller-data {

Not documented.

> > +            samsung,spi-feedback-delay = <2>;
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > index 2baee2c817c1..5ce5c4a43735 100644
> > --- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > +++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
> > @@ -117,6 +117,8 @@ patternProperties:
> >      description: Asahi Kasei Corp.
> >    "^asc,.*":
> >      description: All Sensors Corporation
> > +  "^asix,.*":
> > +    description: ASIX Electronics Corporation
> 
> Separate patch please.
> 
> Best regards,
> Krzysztof
> 
> >    "^aspeed,.*":
> >      description: ASPEED Technology Inc.
> >    "^asus,.*":
> > --
> > 2.26.2
> >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
new file mode 100644
index 000000000000..50a488d59dbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/asix,ax88796c-spi.yaml
@@ -0,0 +1,76 @@ 
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/asix,ax88796c-spi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASIX AX88796C SPI Ethernet Adapter
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+description: |
+  ASIX AX88796C is an Ethernet controller with a built in PHY. This
+  describes SPI mode of the chip.
+
+  The node for this driver must be a child node of a SPI controller, hence
+  all mandatory properties described in ../spi/spi-bus.txt must be specified.
+
+maintainers:
+  - Łukasz Stelmach <l.stelmach@samsung.com>
+
+properties:
+  compatible:
+    const: asix,ax99796c-spi
+
+  reg:
+    description:
+      SPI device address.
+    maxItems: 1
+
+  spi-max-frequency:
+    maximum: 40000000
+
+  interrupts:
+    description:
+     GPIO interrupt to which the chip is connected.
+    maxItems: 1
+
+  interrupt-parrent:
+    description:
+      A phandle of an interrupt controller.
+    maxItems: 1
+
+  reset-gpios:
+    description:
+      A GPIO line handling reset of the chip. As the line is active low,
+      it should be marked GPIO_ACTIVE_LOW.
+    maxItems: 1
+
+  local-mac-address: true
+
+  mac-address: true
+
+required:
+  - compatible
+  - reg
+  - spi-max-frequency
+  - interrupts
+  - interrupt-parrent
+  - reset-gpios
+
+examples:
+  # Artik5 eval board
+  - |
+    ax88796c@0 {
+        compatible = "asix,ax88796c";
+        local-mac-address = [00 00 00 00 00 00]; /* Filled in by a bootloader */
+        interrupt-parent = <&gpx2>;
+        interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+        spi-max-frequency = <40000000>;
+        reg = <0x0>;
+        reset-gpios = <&gpe0 2 GPIO_ACTIVE_LOW>;
+        controller-data {
+            samsung,spi-feedback-delay = <2>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2baee2c817c1..5ce5c4a43735 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -117,6 +117,8 @@  patternProperties:
     description: Asahi Kasei Corp.
   "^asc,.*":
     description: All Sensors Corporation
+  "^asix,.*":
+    description: ASIX Electronics Corporation
   "^aspeed,.*":
     description: ASPEED Technology Inc.
   "^asus,.*":