Re: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet: convert bindings document to yaml

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 02/05/2023 12:09, Gaddam, Sarath Babu Naidu wrote:
> 
> 
>> -----Original Message-----
>> From: Gaddam, Sarath Babu Naidu
>> <sarath.babu.naidu.gaddam@amd.com>
>> Sent: Tuesday, March 28, 2023 9:31 PM
>> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>> pabeni@redhat.com; robh+dt@kernel.org;
>> krzysztof.kozlowski+dt@linaro.org
>> Cc: michal.simek@xilinx.com; radhey.shyam.pandey@xilinx.com;
>> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Sarangi,
>> Anirudha <anirudha.sarangi@amd.com>; Katakam, Harini
>> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
>> Subject: RE: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet:
>> convert bindings document to yaml
>>
>>
>>
>>> -----Original Message-----
>>> From: Gaddam, Sarath Babu Naidu
>>> <sarath.babu.naidu.gaddam@amd.com>
>>> Sent: Tuesday, March 28, 2023 6:22 PM
>>> To: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>;
>>> davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
>>> pabeni@redhat.com; robh+dt@kernel.org;
>>> krzysztof.kozlowski+dt@linaro.org
>>> Cc: michal.simek@xilinx.com; radhey.shyam.pandey@xilinx.com;
>>> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Sarangi,
>>> Anirudha <anirudha.sarangi@amd.com>; Katakam, Harini
>>> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
>>> Subject: RE: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet:
>>> convert bindings document to yaml
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>>>> Sent: Tuesday, March 14, 2023 9:22 PM
>>>> To: Gaddam, Sarath Babu Naidu
>>>> <sarath.babu.naidu.gaddam@amd.com>; davem@davemloft.net;
>>>> edumazet@google.com; kuba@kernel.org; pabeni@redhat.com;
>>>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org
>>>> Cc: michal.simek@xilinx.com; radhey.shyam.pandey@xilinx.com;
>>>> netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-arm-
>>>> kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Sarangi,
>>>> Anirudha <anirudha.sarangi@amd.com>; Katakam, Harini
>>>> <harini.katakam@amd.com>; git (AMD-Xilinx) <git@amd.com>
>>>> Subject: Re: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet:
>>>> convert bindings document to yaml
>>>>
>>>> On 08/03/2023 07:12, Sarath Babu Naidu Gaddam wrote:
>>>>> From: Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>
>>>>>
>>>>> Convert the bindings document for Xilinx AXI Ethernet Subsystem
>>> from
>>>>> txt to yaml. No changes to existing binding description.
>>>>>
>>>>
>>>> (...)
>>>>
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - xlnx,axi-ethernet-1.00.a
>>>>> +      - xlnx,axi-ethernet-1.01.a
>>>>> +      - xlnx,axi-ethernet-2.01.a
>>>>> +
>>>>> +  reg:
>>>>> +    description:
>>>>> +      Address and length of the IO space, as well as the address
>>>>> +      and length of the AXI DMA controller IO space, unless
>>>>> +      axistream-connected is specified, in which case the reg
>>>>> +      attribute of the node referenced by it is used.
>>>>
>>>> Did you test it with axistream-connected? The schema and description
>>>> feel contradictory and tests would point the issue.
>>>
>>> Thanks for review comments. We tested with axistream-connected and
>> did
>>> not observe any errors. Do you anticipate any issues/errors ?
>>
>> Just to add more details, we have tested it using below dt node
>>
>> 	axienet@0 {
>> 	        axistream-connected = <&dma>;
>>                         reg = <0x00 0x80000000 0x00 0x40000>;
>>                         compatible = "xlnx,axi-ethernet-2.01.a";
>>                         clock-names = "s_axi_lite_clk\0axis_clk\0ref_clk";
>>                         clocks = <0x03 0x47 0x03 0x47 0x18>;
>>                         phy-mode = "sgmii";
>>                         xlnx,rxcsum = <0x02>;
>>                         xlnx,rxmem = <0x1000>;
>>                         xlnx,txcsum = <0x02>;
>>                         pcs-handle = <0x19>;
>>                         phy-handle = <0x78>;
>>                         dmas = <0x17 0x00 0x17 0x01>;
>>                         dma-names = "tx_chan0\0rx_chan0";
>>                         mac-address = [ff ff ff ff ff ff];
>>                         managed = "in-band-status";
>>                         phandle = <0x79>;
>> 		mdio {
>>                                 #address-cells = <0x01>;
>>                                 #size-cells = <0x00>;
>>
>>                                 phy@0 {
>>                                         compatible = "ethernet-phy-ieee802.3-c22";
>>                                         reg = <0x00>;
>>                                         phandle = <0x78>;
>>                                 };
>>
>>                                 ethernet-phy@2 {
>>                                         device_type = "ethernet-phy";
>>                                         reg = <0x02>;
>>                                         phandle = <0x19>;
>>                                 };
>>                         };
>> 	};
>> This DT node works with our board. "&dma" is the dma DT node  and to
>> test the second case where dma  address and length  included  in the
>> axienet reg's property as below "reg = <0x00 0x80000000 0x00 0x40000
>> 0x0 0x80040000 0x0 0x1000>;"
>>
>> I did not observe any issue with above two cases. Used below command
>> to validate the yaml using above DT node.
>> make dtbs_check
>> DT_SCHEMA_FILES=Documentation/devicetree/bindings/net/xlnx,axi-
>> ethernet.yaml
>>
> 
> Hi Krzysztof,  Can you please comment If above explanation is acceptable ?
> I will address remaining review comments and send the next version.

The DTS you pointed obviously cannot work with the binding - it has
obvious mistakes. Starting with phy-mode. So whatever you did, was not
correct testing. Since nothing from your code is upstream, I cannot
verify it.

Upstream your DTS first.

Best regards,
Krzysztof