diff mbox series

[v4,1/2] dt-bindings: net: Add ICSSG Ethernet Driver bindings

Message ID 20230206060708.3574472-2-danishanwar@ti.com (mailing list archive)
State New, archived
Headers show
Series Introduce ICSSG based ethernet Driver | expand

Commit Message

MD Danish Anwar Feb. 6, 2023, 6:07 a.m. UTC
From: Puranjay Mohan <p-mohan@ti.com>

Add a YAML binding document for the ICSSG Programmable real time unit
based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs
to interface the PRUs and load/run the firmware for supporting ethernet
functionality.

Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
Signed-off-by: Md Danish Anwar <danishanwar@ti.com>
---
 .../bindings/net/ti,icssg-prueth.yaml         | 179 ++++++++++++++++++
 1 file changed, 179 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml

Comments

Krzysztof Kozlowski Feb. 6, 2023, 7:50 a.m. UTC | #1
On 06/02/2023 07:07, MD Danish Anwar wrote:
> From: Puranjay Mohan <p-mohan@ti.com>
> 
> Add a YAML binding document for the ICSSG Programmable real time unit
> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs

You add a binding for the hardware, not for driver.

> to interface the PRUs and load/run the firmware for supporting ethernet
> functionality.

Subject: drop second/last, redundant "driver bindings". The
"dt-bindings" prefix is already stating that these are bindings.

> 
> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
> Signed-off-by: Md Danish Anwar <danishanwar@ti.com>
> ---
>  .../bindings/net/ti,icssg-prueth.yaml         | 179 ++++++++++++++++++
>  1 file changed, 179 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> new file mode 100644
> index 000000000000..e4dee01a272a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> @@ -0,0 +1,179 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments ICSSG PRUSS Ethernet
> +
> +maintainers:
> +  - Md Danish Anwar <danishanwar@ti.com>
> +
> +description:
> +  Ethernet based on the Programmable Real-Time
> +  Unit and Industrial Communication Subsystem.
> +
> +allOf:
> +  - $ref: /schemas/remoteproc/ti,pru-consumer.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,am654-icssg-prueth  # for AM65x SoC family
> +
> +  ti,sram:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to MSMC SRAM node
> +
> +  dmas:
> +    maxItems: 10
> +
> +  dma-names:
> +    items:
> +      - const: tx0-0
> +      - const: tx0-1
> +      - const: tx0-2
> +      - const: tx0-3
> +      - const: tx1-0
> +      - const: tx1-1
> +      - const: tx1-2
> +      - const: tx1-3
> +      - const: rx0
> +      - const: rx1
> +
> +  ethernet-ports:

Bring some order or logic in the order of the properties. Keep the
ethernet-ports as last property.

> +    type: object
> +    additionalProperties: false

Blank line

> +    properties:
> +      '#address-cells':
> +        const: 1
> +      '#size-cells':
> +        const: 0
> +
> +    patternProperties:
> +      ^port@[0-1]$:
> +        type: object
> +        description: ICSSG PRUETH external ports
> +

Drop blank line

> +        $ref: ethernet-controller.yaml#
> +

Drop blank line

> +        unevaluatedProperties: false
> +
> +        properties:
> +          reg:
> +            items:
> +              - enum: [0, 1]
> +            description: ICSSG PRUETH port number
> +
> +          interrupts-extended:

Just "interrupts"
> +            maxItems: 1
> +
> +          ti,syscon-rgmii-delay:
> +            items:
> +              - items:
> +                  - description: phandle to system controller node
> +                  - description: The offset to ICSSG control register
> +            $ref: /schemas/types.yaml#/definitions/phandle-array
> +            description:
> +              phandle to system controller node and register offset
> +              to ICSSG control register for RGMII transmit delay
> +
> +        required:
> +          - reg

required for ethernet-ports - at least one port is required, isn't it?
> +
> +  ti,mii-g-rt:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      phandle to MII_G_RT module's syscon regmap.
> +
> +  ti,mii-rt:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description: |
> +      phandle to MII_RT module's syscon regmap
> +
> +  interrupts:
> +    maxItems: 2
> +    description: |
> +      Interrupt specifiers to TX timestamp IRQ.
> +
> +  interrupt-names:
> +    items:
> +      - const: tx_ts0
> +      - const: tx_ts1
> +
> +required:
> +  - compatible
> +  - ti,sram
> +  - dmas
> +  - dma-names
> +  - ethernet-ports
> +  - ti,mii-g-rt
> +  - interrupts
> +  - interrupt-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    /* Example k3-am654 base board SR2.0, dual-emac */
> +    pruss2_eth: ethernet {
> +        compatible = "ti,am654-icssg-prueth";
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&icssg2_rgmii_pins_default>;
> +        ti,sram = <&msmc_ram>;
> +
> +        ti,prus = <&pru2_0>, <&rtu2_0>, <&tx_pru2_0>,
> +                  <&pru2_1>, <&rtu2_1>, <&tx_pru2_1>;
> +        firmware-name = "ti-pruss/am65x-pru0-prueth-fw.elf",
> +                        "ti-pruss/am65x-rtu0-prueth-fw.elf",
> +                        "ti-pruss/am65x-txpru0-prueth-fw.elf",
> +                        "ti-pruss/am65x-pru1-prueth-fw.elf",
> +                        "ti-pruss/am65x-rtu1-prueth-fw.elf",
> +                        "ti-pruss/am65x-txpru1-prueth-fw.elf";
> +        ti,pruss-gp-mux-sel = <2>,      /* MII mode */
> +                              <2>,
> +                              <2>,
> +                              <2>,      /* MII mode */
> +                              <2>,
> +                              <2>;
> +        dmas = <&main_udmap 0xc300>, /* egress slice 0 */
> +               <&main_udmap 0xc301>, /* egress slice 0 */
> +               <&main_udmap 0xc302>, /* egress slice 0 */
> +               <&main_udmap 0xc303>, /* egress slice 0 */
> +               <&main_udmap 0xc304>, /* egress slice 1 */
> +               <&main_udmap 0xc305>, /* egress slice 1 */
> +               <&main_udmap 0xc306>, /* egress slice 1 */
> +               <&main_udmap 0xc307>, /* egress slice 1 */
> +               <&main_udmap 0x4300>, /* ingress slice 0 */
> +               <&main_udmap 0x4301>; /* ingress slice 1 */
> +        dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
> +                    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
> +                    "rx0", "rx1";
> +        ti,mii-g-rt = <&icssg2_mii_g_rt>;
> +        interrupts = <24 0 2>, <25 1 3>;

Aren't you open-coding some IRQ flags?

> +        interrupt-names = "tx_ts0", "tx_ts1";
> +        ethernet-ports {


Best regards,
Krzysztof
Anwar, Md Danish Feb. 6, 2023, 10:39 a.m. UTC | #2
Hi Krzysztof,

Thanks for the comments.

On 06/02/23 13:20, Krzysztof Kozlowski wrote:
> On 06/02/2023 07:07, MD Danish Anwar wrote:
>> From: Puranjay Mohan <p-mohan@ti.com>
>>
>> Add a YAML binding document for the ICSSG Programmable real time unit
>> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs
> 
> You add a binding for the hardware, not for driver.
> 
>> to interface the PRUs and load/run the firmware for supporting ethernet
>> functionality.
> 
> Subject: drop second/last, redundant "driver bindings". The
> "dt-bindings" prefix is already stating that these are bindings.
> 

Sure I will modify the subject and commit description.

>>
>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>> Signed-off-by: Md Danish Anwar <danishanwar@ti.com>
>> ---
>>  .../bindings/net/ti,icssg-prueth.yaml         | 179 ++++++++++++++++++
>>  1 file changed, 179 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
>> new file mode 100644
>> index 000000000000..e4dee01a272a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
>> @@ -0,0 +1,179 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments ICSSG PRUSS Ethernet
>> +
>> +maintainers:
>> +  - Md Danish Anwar <danishanwar@ti.com>
>> +
>> +description:
>> +  Ethernet based on the Programmable Real-Time
>> +  Unit and Industrial Communication Subsystem.
>> +
>> +allOf:
>> +  - $ref: /schemas/remoteproc/ti,pru-consumer.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - ti,am654-icssg-prueth  # for AM65x SoC family
>> +
>> +  ti,sram:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description:
>> +      phandle to MSMC SRAM node
>> +
>> +  dmas:
>> +    maxItems: 10
>> +
>> +  dma-names:
>> +    items:
>> +      - const: tx0-0
>> +      - const: tx0-1
>> +      - const: tx0-2
>> +      - const: tx0-3
>> +      - const: tx1-0
>> +      - const: tx1-1
>> +      - const: tx1-2
>> +      - const: tx1-3
>> +      - const: rx0
>> +      - const: rx1
>> +
>> +  ethernet-ports:
> 
> Bring some order or logic in the order of the properties. Keep the
> ethernet-ports as last property.
> 

Sure, I will re-arrange them.

>> +    type: object
>> +    additionalProperties: false
> 
> Blank line
> 
>> +    properties:
>> +      '#address-cells':
>> +        const: 1
>> +      '#size-cells':
>> +        const: 0
>> +
>> +    patternProperties:
>> +      ^port@[0-1]$:
>> +        type: object
>> +        description: ICSSG PRUETH external ports
>> +

At least one ethernet port is required. Should I add the below line here for this?

   minItems: 1

>> Drop blank line
> 
>> +        $ref: ethernet-controller.yaml#
>> +
> 
> Drop blank line
> 
>> +        unevaluatedProperties: false
>> +
>> +        properties:
>> +          reg:
>> +            items:
>> +              - enum: [0, 1]
>> +            description: ICSSG PRUETH port number
>> +
>> +          interrupts-extended:
> 
> Just "interrupts"

I will drop / add blank lines and change this to just "interrupts".

>> +            maxItems: 1
>> +
>> +          ti,syscon-rgmii-delay:
>> +            items:
>> +              - items:
>> +                  - description: phandle to system controller node
>> +                  - description: The offset to ICSSG control register
>> +            $ref: /schemas/types.yaml#/definitions/phandle-array
>> +            description:
>> +              phandle to system controller node and register offset
>> +              to ICSSG control register for RGMII transmit delay
>> +
>> +        required:
>> +          - reg
> 
> required for ethernet-ports - at least one port is required, isn't it?

Yes, at least one ethernet port is required. Should I add "minItems: 1" in
patternProperties section or should I add a new required section in
patternProperties and mention something like port@0 as required?

>> +
>> +  ti,mii-g-rt:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: |
>> +      phandle to MII_G_RT module's syscon regmap.
>> +
>> +  ti,mii-rt:
>> +    $ref: /schemas/types.yaml#/definitions/phandle
>> +    description: |
>> +      phandle to MII_RT module's syscon regmap
>> +
>> +  interrupts:
>> +    maxItems: 2
>> +    description: |
>> +      Interrupt specifiers to TX timestamp IRQ.
>> +
>> +  interrupt-names:
>> +    items:
>> +      - const: tx_ts0
>> +      - const: tx_ts1
>> +
>> +required:
>> +  - compatible
>> +  - ti,sram
>> +  - dmas
>> +  - dma-names
>> +  - ethernet-ports
>> +  - ti,mii-g-rt
>> +  - interrupts
>> +  - interrupt-names
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    /* Example k3-am654 base board SR2.0, dual-emac */
>> +    pruss2_eth: ethernet {
>> +        compatible = "ti,am654-icssg-prueth";
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&icssg2_rgmii_pins_default>;
>> +        ti,sram = <&msmc_ram>;
>> +
>> +        ti,prus = <&pru2_0>, <&rtu2_0>, <&tx_pru2_0>,
>> +                  <&pru2_1>, <&rtu2_1>, <&tx_pru2_1>;
>> +        firmware-name = "ti-pruss/am65x-pru0-prueth-fw.elf",
>> +                        "ti-pruss/am65x-rtu0-prueth-fw.elf",
>> +                        "ti-pruss/am65x-txpru0-prueth-fw.elf",
>> +                        "ti-pruss/am65x-pru1-prueth-fw.elf",
>> +                        "ti-pruss/am65x-rtu1-prueth-fw.elf",
>> +                        "ti-pruss/am65x-txpru1-prueth-fw.elf";
>> +        ti,pruss-gp-mux-sel = <2>,      /* MII mode */
>> +                              <2>,
>> +                              <2>,
>> +                              <2>,      /* MII mode */
>> +                              <2>,
>> +                              <2>;
>> +        dmas = <&main_udmap 0xc300>, /* egress slice 0 */
>> +               <&main_udmap 0xc301>, /* egress slice 0 */
>> +               <&main_udmap 0xc302>, /* egress slice 0 */
>> +               <&main_udmap 0xc303>, /* egress slice 0 */
>> +               <&main_udmap 0xc304>, /* egress slice 1 */
>> +               <&main_udmap 0xc305>, /* egress slice 1 */
>> +               <&main_udmap 0xc306>, /* egress slice 1 */
>> +               <&main_udmap 0xc307>, /* egress slice 1 */
>> +               <&main_udmap 0x4300>, /* ingress slice 0 */
>> +               <&main_udmap 0x4301>; /* ingress slice 1 */
>> +        dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
>> +                    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
>> +                    "rx0", "rx1";
>> +        ti,mii-g-rt = <&icssg2_mii_g_rt>;
>> +        interrupts = <24 0 2>, <25 1 3>;
> 
> Aren't you open-coding some IRQ flags?
> 

These values are not open coded here. These values are the expected values for
interrupt node.

Here,

Cell 1 -> PRU System event number
Cell 2 -> PRU channel
Cell 3 -> PRU host_event (target)

This is documented in detail in
Documentation/devicetree/bindings/interrupt-controller/ti,pruss-intc.yaml

I also missed mentioning "interrupt-parent" in example section. I will add
interrupt-parent in next revision.

interrupt-parent = <&icssg2_intc>;

>> +        interrupt-names = "tx_ts0", "tx_ts1";
>> +        ethernet-ports {
> 
> 
> Best regards,
> Krzysztof
>
Krzysztof Kozlowski Feb. 6, 2023, 10:41 a.m. UTC | #3
On 06/02/2023 11:39, Md Danish Anwar wrote:
>>> +    properties:
>>> +      '#address-cells':
>>> +        const: 1
>>> +      '#size-cells':
>>> +        const: 0
>>> +
>>> +    patternProperties:
>>> +      ^port@[0-1]$:
>>> +        type: object
>>> +        description: ICSSG PRUETH external ports
>>> +
> 
> At least one ethernet port is required. Should I add the below line here for this?
> 
>    minItems: 1

You need after the patternProperties:
    anyOf:
      - required:
          - port@0
      - required:
          - port@1

> 
> 

Best regards,
Krzysztof
Rob Herring (Arm) Feb. 6, 2023, 1:46 p.m. UTC | #4
On Mon, 06 Feb 2023 11:37:07 +0530, MD Danish Anwar wrote:
> From: Puranjay Mohan <p-mohan@ti.com>
> 
> Add a YAML binding document for the ICSSG Programmable real time unit
> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs
> to interface the PRUs and load/run the firmware for supporting ethernet
> functionality.
> 
> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
> Signed-off-by: Md Danish Anwar <danishanwar@ti.com>
> ---
>  .../bindings/net/ti,icssg-prueth.yaml         | 179 ++++++++++++++++++
>  1 file changed, 179 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.example.dtb: ethernet: False schema does not allow {'compatible': ['ti,am654-icssg-prueth'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], 'ti,sram': [[4294967295]], 'ti,prus': [[4294967295, 4294967295, 4294967295, 4294967295, 4294967295, 4294967295]], 'firmware-name': ['ti-pruss/am65x-pru0-prueth-fw.elf', 'ti-pruss/am65x-rtu0-prueth-fw.elf', 'ti-pruss/am65x-txpru0-prueth-fw.elf', 'ti-pruss/am65x-pru1-prueth-fw.elf', 'ti-pruss/am65x-rtu1-prueth-fw.elf', 'ti-pruss/am65x-txpru1-prueth-fw.elf'], 'ti,pruss-gp-mux-sel': [[2, 2, 2, 2, 2, 2]], 'dmas': [[4294967295, 49920], [4294967295, 49921], [4294967295, 49922], [4294967295, 49923], [4294967295, 49924], [4294967295, 49925], [4294967295, 49926], [4294967295, 49927], [4294967295, 17152], [4294967295, 17153]], 'dma-names': ['tx0-0', 'tx0-1', 'tx0-2', 'tx0-3', 'tx1-0', 'tx1-1', 'tx1-2', 'tx1-3', 'rx0', 'rx1'], 'ti,mii-g-rt': [[4294967295]]
 , 'interrupts': [[24, 0, 2], [25, 1, 3]], 'interrupt-names': ['tx_ts0', 'tx_ts1'], 'ethernet-ports': {'#address-cells': [[1]], '#size-cells': [[0]], 'port@0': {'reg': [[0]], 'phy-handle': [[4294967295]], 'phy-mode': ['rgmii-id'], 'interrupts-extended': [[4294967295, 24]], 'ti,syscon-rgmii-delay': [[4294967295, 16672]], 'local-mac-address': [[0, 0, 0, 0, 0, 0]]}, 'port@1': {'reg': [[1]], 'phy-handle': [[4294967295]], 'phy-mode': ['rgmii-id'], 'interrupts-extended': [[4294967295, 25]], 'ti,syscon-rgmii-delay': [[4294967295, 16676]], 'local-mac-address': [[0, 0, 0, 0, 0, 0]]}}, '$nodename': ['ethernet']}
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.example.dtb: ethernet: Unevaluated properties are not allowed ('firmware-name', 'ti,prus', 'ti,pruss-gp-mux-sel' were unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230206060708.3574472-2-danishanwar@ti.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.
Anwar, Md Danish Feb. 7, 2023, 5 a.m. UTC | #5
Hi Rob,

On 06/02/23 19:16, Rob Herring wrote:
> 
> On Mon, 06 Feb 2023 11:37:07 +0530, MD Danish Anwar wrote:
>> From: Puranjay Mohan <p-mohan@ti.com>
>>
>> Add a YAML binding document for the ICSSG Programmable real time unit
>> based Ethernet driver. This driver uses the PRU and PRUSS consumer APIs
>> to interface the PRUs and load/run the firmware for supporting ethernet
>> functionality.
>>
>> Signed-off-by: Puranjay Mohan <p-mohan@ti.com>
>> Signed-off-by: Md Danish Anwar <danishanwar@ti.com>
>> ---
>>  .../bindings/net/ti,icssg-prueth.yaml         | 179 ++++++++++++++++++
>>  1 file changed, 179 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
>>
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> ./Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml: Unable to find schema file matching $id: http://devicetree.org/schemas/remoteproc/ti,pru-consumer.yaml

This error is coming because this series depends on series [1]. The
ti,pru-consumer.yaml is upstreamed through series [1]. The series is approved
and will hopefully be merged soon.

[1] https://lore.kernel.org/all/20230106121046.886863-1-danishanwar@ti.com/

> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.example.dtb: ethernet: False schema does not allow {'compatible': ['ti,am654-icssg-prueth'], 'pinctrl-names': ['default'], 'pinctrl-0': [[4294967295]], 'ti,sram': [[4294967295]], 'ti,prus': [[4294967295, 4294967295, 4294967295, 4294967295, 4294967295, 4294967295]], 'firmware-name': ['ti-pruss/am65x-pru0-prueth-fw.elf', 'ti-pruss/am65x-rtu0-prueth-fw.elf', 'ti-pruss/am65x-txpru0-prueth-fw.elf', 'ti-pruss/am65x-pru1-prueth-fw.elf', 'ti-pruss/am65x-rtu1-prueth-fw.elf', 'ti-pruss/am65x-txpru1-prueth-fw.elf'], 'ti,pruss-gp-mux-sel': [[2, 2, 2, 2, 2, 2]], 'dmas': [[4294967295, 49920], [4294967295, 49921], [4294967295, 49922], [4294967295, 49923], [4294967295, 49924], [4294967295, 49925], [4294967295, 49926], [4294967295, 49927], [4294967295, 17152], [4294967295, 17153]], 'dma-names': ['tx0-0', 'tx0-1', 'tx0-2', 'tx0-3', 'tx1-0', 'tx1-1', 'tx1-2', 'tx1-3', 'rx0', 'rx1'], 'ti,mii-g-rt': [[429!
>  4967295]], 'interrupts': [[24, 0, 2], [25, 1, 3]], 'interrupt-names': ['tx_ts0', 'tx_ts1'], 'ethernet-ports': {'#address-cells': [[1]], '#size-cells': [[0]], 'port@0': {'reg': [[0]], 'phy-handle': [[4294967295]], 'phy-mode': ['rgmii-id'], 'interrupts-extended': [[4294967295, 24]], 'ti,syscon-rgmii-delay': [[4294967295, 16672]], 'local-mac-address': [[0, 0, 0, 0, 0, 0]]}, 'port@1': {'reg': [[1]], 'phy-handle': [[4294967295]], 'phy-mode': ['rgmii-id'], 'interrupts-extended': [[4294967295, 25]], 'ti,syscon-rgmii-delay': [[4294967295, 16676]], 'local-mac-address': [[0, 0, 0, 0, 0, 0]]}}, '$nodename': ['ethernet']}
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.example.dtb: ethernet: Unevaluated properties are not allowed ('firmware-name', 'ti,prus', 'ti,pruss-gp-mux-sel' were unexpected)
> 	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230206060708.3574472-2-danishanwar@ti.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.
>
Anwar, Md Danish Feb. 7, 2023, 5:07 a.m. UTC | #6
Hi Krzysztof,

On 06/02/23 16:11, Krzysztof Kozlowski wrote:
> On 06/02/2023 11:39, Md Danish Anwar wrote:
>>>> +    properties:
>>>> +      '#address-cells':
>>>> +        const: 1
>>>> +      '#size-cells':
>>>> +        const: 0
>>>> +
>>>> +    patternProperties:
>>>> +      ^port@[0-1]$:
>>>> +        type: object
>>>> +        description: ICSSG PRUETH external ports
>>>> +
>>
>> At least one ethernet port is required. Should I add the below line here for this?
>>
>>    minItems: 1
> 
> You need after the patternProperties:
>     anyOf:
>       - required:
>           - port@0
>       - required:
>           - port@1
> 


Is this correct?

  ethernet-ports:
    type: object
    additionalProperties: false

    properties:
      '#address-cells':
        const: 1
      '#size-cells':
        const: 0

    patternProperties:
      ^port@[0-1]$:
        type: object
        description: ICSSG PRUETH external ports
        $ref: ethernet-controller.yaml#
        unevaluatedProperties: false

        properties:
          reg:
            items:
              - enum: [0, 1]
            description: ICSSG PRUETH port number

          interrupts:
            maxItems: 1

          ti,syscon-rgmii-delay:
            items:
              - items:
                  - description: phandle to system controller node
                  - description: The offset to ICSSG control register
            $ref: /schemas/types.yaml#/definitions/phandle-array
            description:
              phandle to system controller node and register offset
              to ICSSG control register for RGMII transmit delay

        required:
          - reg
    anyOf:
      - required:
          - port@0
      - required:
          - port@1

Adding anyOf just below patternProperties was throwing error, so I added anyOf
after the end of patternProperties. Please let me know if this looks OK.

>>
>>
> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
new file mode 100644
index 000000000000..e4dee01a272a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/ti,icssg-prueth.yaml
@@ -0,0 +1,179 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/ti,icssg-prueth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments ICSSG PRUSS Ethernet
+
+maintainers:
+  - Md Danish Anwar <danishanwar@ti.com>
+
+description:
+  Ethernet based on the Programmable Real-Time
+  Unit and Industrial Communication Subsystem.
+
+allOf:
+  - $ref: /schemas/remoteproc/ti,pru-consumer.yaml#
+
+properties:
+  compatible:
+    enum:
+      - ti,am654-icssg-prueth  # for AM65x SoC family
+
+  ti,sram:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to MSMC SRAM node
+
+  dmas:
+    maxItems: 10
+
+  dma-names:
+    items:
+      - const: tx0-0
+      - const: tx0-1
+      - const: tx0-2
+      - const: tx0-3
+      - const: tx1-0
+      - const: tx1-1
+      - const: tx1-2
+      - const: tx1-3
+      - const: rx0
+      - const: rx1
+
+  ethernet-ports:
+    type: object
+    additionalProperties: false
+    properties:
+      '#address-cells':
+        const: 1
+      '#size-cells':
+        const: 0
+
+    patternProperties:
+      ^port@[0-1]$:
+        type: object
+        description: ICSSG PRUETH external ports
+
+        $ref: ethernet-controller.yaml#
+
+        unevaluatedProperties: false
+
+        properties:
+          reg:
+            items:
+              - enum: [0, 1]
+            description: ICSSG PRUETH port number
+
+          interrupts-extended:
+            maxItems: 1
+
+          ti,syscon-rgmii-delay:
+            items:
+              - items:
+                  - description: phandle to system controller node
+                  - description: The offset to ICSSG control register
+            $ref: /schemas/types.yaml#/definitions/phandle-array
+            description:
+              phandle to system controller node and register offset
+              to ICSSG control register for RGMII transmit delay
+
+        required:
+          - reg
+
+  ti,mii-g-rt:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      phandle to MII_G_RT module's syscon regmap.
+
+  ti,mii-rt:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description: |
+      phandle to MII_RT module's syscon regmap
+
+  interrupts:
+    maxItems: 2
+    description: |
+      Interrupt specifiers to TX timestamp IRQ.
+
+  interrupt-names:
+    items:
+      - const: tx_ts0
+      - const: tx_ts1
+
+required:
+  - compatible
+  - ti,sram
+  - dmas
+  - dma-names
+  - ethernet-ports
+  - ti,mii-g-rt
+  - interrupts
+  - interrupt-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    /* Example k3-am654 base board SR2.0, dual-emac */
+    pruss2_eth: ethernet {
+        compatible = "ti,am654-icssg-prueth";
+        pinctrl-names = "default";
+        pinctrl-0 = <&icssg2_rgmii_pins_default>;
+        ti,sram = <&msmc_ram>;
+
+        ti,prus = <&pru2_0>, <&rtu2_0>, <&tx_pru2_0>,
+                  <&pru2_1>, <&rtu2_1>, <&tx_pru2_1>;
+        firmware-name = "ti-pruss/am65x-pru0-prueth-fw.elf",
+                        "ti-pruss/am65x-rtu0-prueth-fw.elf",
+                        "ti-pruss/am65x-txpru0-prueth-fw.elf",
+                        "ti-pruss/am65x-pru1-prueth-fw.elf",
+                        "ti-pruss/am65x-rtu1-prueth-fw.elf",
+                        "ti-pruss/am65x-txpru1-prueth-fw.elf";
+        ti,pruss-gp-mux-sel = <2>,      /* MII mode */
+                              <2>,
+                              <2>,
+                              <2>,      /* MII mode */
+                              <2>,
+                              <2>;
+        dmas = <&main_udmap 0xc300>, /* egress slice 0 */
+               <&main_udmap 0xc301>, /* egress slice 0 */
+               <&main_udmap 0xc302>, /* egress slice 0 */
+               <&main_udmap 0xc303>, /* egress slice 0 */
+               <&main_udmap 0xc304>, /* egress slice 1 */
+               <&main_udmap 0xc305>, /* egress slice 1 */
+               <&main_udmap 0xc306>, /* egress slice 1 */
+               <&main_udmap 0xc307>, /* egress slice 1 */
+               <&main_udmap 0x4300>, /* ingress slice 0 */
+               <&main_udmap 0x4301>; /* ingress slice 1 */
+        dma-names = "tx0-0", "tx0-1", "tx0-2", "tx0-3",
+                    "tx1-0", "tx1-1", "tx1-2", "tx1-3",
+                    "rx0", "rx1";
+        ti,mii-g-rt = <&icssg2_mii_g_rt>;
+        interrupts = <24 0 2>, <25 1 3>;
+        interrupt-names = "tx_ts0", "tx_ts1";
+        ethernet-ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            pruss2_emac0: port@0 {
+                reg = <0>;
+                phy-handle = <&pruss2_eth0_phy>;
+                phy-mode = "rgmii-id";
+                interrupts-extended = <&icssg2_intc 24>;
+                ti,syscon-rgmii-delay = <&scm_conf 0x4120>;
+                /* Filled in by bootloader */
+                local-mac-address = [00 00 00 00 00 00];
+            };
+
+            pruss2_emac1: port@1 {
+                reg = <1>;
+                phy-handle = <&pruss2_eth1_phy>;
+                phy-mode = "rgmii-id";
+                interrupts-extended = <&icssg2_intc 25>;
+                ti,syscon-rgmii-delay = <&scm_conf 0x4124>;
+                /* Filled in by bootloader */
+                local-mac-address = [00 00 00 00 00 00];
+            };
+        };
+    };