diff mbox series

[net-next] dt-bindings: net: xilinx: document xilinx emaclite driver binding

Message ID 1653031473-21032-1-git-send-email-radhey.shyam.pandey@amd.com (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series [net-next] dt-bindings: net: xilinx: document xilinx emaclite driver binding | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
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 10 of 10 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/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning 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

Pandey, Radhey Shyam May 20, 2022, 7:24 a.m. UTC 
Add basic description for the xilinx emaclite driver DT bindings.

Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
Changes since RFC:
- Add ethernet-controller yaml reference.
- 4 space indent for DTS example.
---
 .../bindings/net/xlnx,emaclite.yaml           | 63 +++++++++++++++++++
 1 file changed, 63 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/xlnx,emaclite.yaml

Comments

Krzysztof Kozlowski May 21, 2022, 3:43 p.m. UTC | #1 
On 20/05/2022 09:24, Radhey Shyam Pandey wrote:
> Add basic description for the xilinx emaclite driver DT bindings.
> 
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> ---
> Changes since RFC:
> - Add ethernet-controller yaml reference.
> - 4 space indent for DTS example.
> ---
>  .../bindings/net/xlnx,emaclite.yaml           | 63 +++++++++++++++++++
>  1 file changed, 63 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> new file mode 100644
> index 000000000000..6105122ad583
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> @@ -0,0 +1,63 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/xlnx,emaclite.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Xilinx Emaclite Ethernet controller
> +
> +allOf:
> +  - $ref: ethernet-controller.yaml#

This goes just before properties (so after maintainers).

> +
> +maintainers:
> +  - Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> +  - Harini Katakam <harini.katakam@amd.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - xlnx,opb-ethernetlite-1.01.a
> +      - xlnx,opb-ethernetlite-1.01.b
> +      - xlnx,xps-ethernetlite-1.00.a
> +      - xlnx,xps-ethernetlite-2.00.a
> +      - xlnx,xps-ethernetlite-2.01.a
> +      - xlnx,xps-ethernetlite-3.00.a
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  phy-handle: true
> +
> +  local-mac-address: true
> +
> +  xlnx,tx-ping-pong:
> +    type: boolean
> +    description: hardware supports tx ping pong buffer.
> +
> +  xlnx,rx-ping-pong:
> +    type: boolean
> +    description: hardware supports rx ping pong buffer.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - phy-handle
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    axi_ethernetlite_1: ethernet@40e00000 {
> +        compatible = "xlnx,xps-ethernetlite-3.00.a";
> +        interrupt-parent = <&axi_intc_1>;
> +        interrupts = <1 0>;

Is "0" an interrupt none type? If yes, then this should be rather a
define and a proper type, not none.

> +        local-mac-address = [00 0a 35 00 00 00];

Each device should get it's own MAC address, right? I understand you
leave it for bootloader, then just fill it with 0.

> +        phy-handle = <&phy0>;
> +        reg = <0x40e00000 0x10000>;

Put the reg after compatible in DTS code.

> +        xlnx,rx-ping-pong;
> +        xlnx,tx-ping-pong;
> +    };


Best regards,
Krzysztof
Radhey Shyam Pandey May 30, 2022, 1:21 p.m. UTC | #2 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Saturday, May 21, 2022 9:14 PM
> To: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>;
> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> harini.katakam@amd.com; michal.simek@amd.com
> Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; git@amd.com
> Subject: Re: [PATCH net-next] dt-bindings: net: xilinx: document xilinx emaclite
> driver binding
> 
> 
> On 20/05/2022 09:24, Radhey Shyam Pandey wrote:
> > Add basic description for the xilinx emaclite driver DT bindings.
> >
> > Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> > ---
> > Changes since RFC:
> > - Add ethernet-controller yaml reference.
> > - 4 space indent for DTS example.
> > ---
> >  .../bindings/net/xlnx,emaclite.yaml           | 63 +++++++++++++++++++
> >  1 file changed, 63 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > new file mode 100644
> > index 000000000000..6105122ad583
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
> > @@ -0,0 +1,63 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/xlnx,emaclite.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Xilinx Emaclite Ethernet controller
> > +
> > +allOf:
> > +  - $ref: ethernet-controller.yaml#
> 
> This goes just before properties (so after maintainers).

Ok . sure will fix in v2.
> 
> > +
> > +maintainers:
> > +  - Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> > +  - Harini Katakam <harini.katakam@amd.com>
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - xlnx,opb-ethernetlite-1.01.a
> > +      - xlnx,opb-ethernetlite-1.01.b
> > +      - xlnx,xps-ethernetlite-1.00.a
> > +      - xlnx,xps-ethernetlite-2.00.a
> > +      - xlnx,xps-ethernetlite-2.01.a
> > +      - xlnx,xps-ethernetlite-3.00.a
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  phy-handle: true
> > +
> > +  local-mac-address: true
> > +
> > +  xlnx,tx-ping-pong:
> > +    type: boolean
> > +    description: hardware supports tx ping pong buffer.
> > +
> > +  xlnx,rx-ping-pong:
> > +    type: boolean
> > +    description: hardware supports rx ping pong buffer.
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - phy-handle
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    axi_ethernetlite_1: ethernet@40e00000 {
> > +        compatible = "xlnx,xps-ethernetlite-3.00.a";
> > +        interrupt-parent = <&axi_intc_1>;
> > +        interrupts = <1 0>;
> 
> Is "0" an interrupt none type? If yes, then this should be rather a
> define and a proper type, not none.

I looked at axi intc driver and it seems second cell in unused here.
For interrupt type level/edge , interrupt controller has separate
binding "xlnx,kind-of-intr".  So will remove the unused cell.
+        interrupts = <1>;

> 
> > +        local-mac-address = [00 0a 35 00 00 00];
> 
> Each device should get it's own MAC address, right? I understand you
> leave it for bootloader, then just fill it with 0.

The emaclite driver uses of_get_ethdev_address() to get mac from DT.
i.e  'local-mac-address' if present in DT it will be read and this MAC 
address is programmed in the MAC core. So I think it's ok to have a 
user defined mac-address (instead of 0s) here in DT example?
 
> 
> > +        phy-handle = <&phy0>;
> > +        reg = <0x40e00000 0x10000>;
> 
> Put the reg after compatible in DTS code.

Ok, sure will fix it in v2.
> 
> > +        xlnx,rx-ping-pong;
> > +        xlnx,tx-ping-pong;
> > +    };
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski May 30, 2022, 7:09 p.m. UTC | #3 
On 30/05/2022 15:21, Radhey Shyam Pandey wrote:
>>
>>> +        local-mac-address = [00 0a 35 00 00 00];
>>
>> Each device should get it's own MAC address, right? I understand you
>> leave it for bootloader, then just fill it with 0.
> 
> The emaclite driver uses of_get_ethdev_address() to get mac from DT.
> i.e  'local-mac-address' if present in DT it will be read and this MAC 
> address is programmed in the MAC core. So I think it's ok to have a 
> user defined mac-address (instead of 0s) here in DT example?

And you want to program the same MAC address in every device in the
world? How would that work?



Best regards,
Krzysztof
Radhey Shyam Pandey May 31, 2022, 6:19 p.m. UTC | #4 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Tuesday, May 31, 2022 12:40 AM
> To: Radhey Shyam Pandey <radheys@xilinx.com>; Radhey Shyam Pandey
> <radhey.shyam.pandey@amd.com>; davem@davemloft.net;
> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> harini.katakam@amd.com; michal.simek@amd.com
> Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; git@amd.com
> Subject: Re: [PATCH net-next] dt-bindings: net: xilinx: document xilinx emaclite
> driver binding
> 
> On 30/05/2022 15:21, Radhey Shyam Pandey wrote:
> >>
> >>> +        local-mac-address = [00 0a 35 00 00 00];
> >>
> >> Each device should get it's own MAC address, right? I understand you
> >> leave it for bootloader, then just fill it with 0.
> >
> > The emaclite driver uses of_get_ethdev_address() to get mac from DT.
> > i.e  'local-mac-address' if present in DT it will be read and this MAC
> > address is programmed in the MAC core. So I think it's ok to have a
> > user defined mac-address (instead of 0s) here in DT example?
> 
> And you want to program the same MAC address in every device in the world?
> How would that work?

I agree, for most of practical usecases mac address will be set by bootloader[1].
But just thinking for usecases where uboot can't read from non-volatile memory
user are still provided with option to set local-mac-address in DT and let linux
also configures it? Also see this in couple of other networking driver examples. 

cdns,macb.yaml:  local-mac-address: true
cdns,macb.yaml:            local-mac-address = [3a 0e 03 04 05 06];
brcm,systemport.yaml:        local-mac-address = [ 00 11 22 33 44 55 ];

In emaclite yaml - as it an example I will set it mac-address to 0 to align 
with common usecase.  local-mac-address = [00 00 00 00 00 00]

[1]: https://support.xilinx.com/s/article/53476

> 
> 
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski May 31, 2022, 8:09 p.m. UTC | #5 
On 31/05/2022 20:19, Radhey Shyam Pandey wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Tuesday, May 31, 2022 12:40 AM
>> To: Radhey Shyam Pandey <radheys@xilinx.com>; Radhey Shyam Pandey
>> <radhey.shyam.pandey@amd.com>; davem@davemloft.net;
>> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> harini.katakam@amd.com; michal.simek@amd.com
>> Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; git@amd.com
>> Subject: Re: [PATCH net-next] dt-bindings: net: xilinx: document xilinx emaclite
>> driver binding
>>
>> On 30/05/2022 15:21, Radhey Shyam Pandey wrote:
>>>>
>>>>> +        local-mac-address = [00 0a 35 00 00 00];
>>>>
>>>> Each device should get it's own MAC address, right? I understand you
>>>> leave it for bootloader, then just fill it with 0.
>>>
>>> The emaclite driver uses of_get_ethdev_address() to get mac from DT.
>>> i.e  'local-mac-address' if present in DT it will be read and this MAC
>>> address is programmed in the MAC core. So I think it's ok to have a
>>> user defined mac-address (instead of 0s) here in DT example?
>>
>> And you want to program the same MAC address in every device in the world?
>> How would that work?
> 
> I agree, for most of practical usecases mac address will be set by bootloader[1].
> But just thinking for usecases where uboot can't read from non-volatile memory
> user are still provided with option to set local-mac-address in DT and let linux
> also configures it? Also see this in couple of other networking driver examples. 

Which is not necessarily correct approach
> 
> cdns,macb.yaml:  local-mac-address: true
> cdns,macb.yaml:            local-mac-address = [3a 0e 03 04 05 06];
> brcm,systemport.yaml:        local-mac-address = [ 00 11 22 33 44 55 ];

This brcm looks like an invalid MAC, so could be just placeholder.

> 
> In emaclite yaml - as it an example I will set it mac-address to 0 to align 
> with common usecase.  local-mac-address = [00 00 00 00 00 00]
> 

Thanks.


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
new file mode 100644
index 000000000000..6105122ad583
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/xlnx,emaclite.yaml
@@ -0,0 +1,63 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/xlnx,emaclite.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Emaclite Ethernet controller
+
+allOf:
+  - $ref: ethernet-controller.yaml#
+
+maintainers:
+  - Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
+  - Harini Katakam <harini.katakam@amd.com>
+
+properties:
+  compatible:
+    enum:
+      - xlnx,opb-ethernetlite-1.01.a
+      - xlnx,opb-ethernetlite-1.01.b
+      - xlnx,xps-ethernetlite-1.00.a
+      - xlnx,xps-ethernetlite-2.00.a
+      - xlnx,xps-ethernetlite-2.01.a
+      - xlnx,xps-ethernetlite-3.00.a
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  phy-handle: true
+
+  local-mac-address: true
+
+  xlnx,tx-ping-pong:
+    type: boolean
+    description: hardware supports tx ping pong buffer.
+
+  xlnx,rx-ping-pong:
+    type: boolean
+    description: hardware supports rx ping pong buffer.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - phy-handle
+
+additionalProperties: false
+
+examples:
+  - |
+    axi_ethernetlite_1: ethernet@40e00000 {
+        compatible = "xlnx,xps-ethernetlite-3.00.a";
+        interrupt-parent = <&axi_intc_1>;
+        interrupts = <1 0>;
+        local-mac-address = [00 0a 35 00 00 00];
+        phy-handle = <&phy0>;
+        reg = <0x40e00000 0x10000>;
+        xlnx,rx-ping-pong;
+        xlnx,tx-ping-pong;
+    };