On 18/05/2023 16:34, Krzysztof Kozlowski wrote: > On 18/05/2023 08:17, Gaddam, Sarath Babu Naidu wrote: >> >> >>> -----Original Message----- >>> From: Gaddam, Sarath Babu Naidu >>> Sent: Wednesday, May 3, 2023 3:01 PM >>> To: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>; >>> davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; >>> pabeni@xxxxxxxxxx; robh+dt@xxxxxxxxxx; >>> krzysztof.kozlowski+dt@xxxxxxxxxx >>> Cc: michal.simek@xxxxxxxxxx; radhey.shyam.pandey@xxxxxxxxxx; >>> netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- >>> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sarangi, >>> Anirudha <anirudha.sarangi@xxxxxxx>; Katakam, Harini >>> <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx> >>> Subject: RE: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet: >>> convert bindings document to yaml >>> >>> >>> >>>> -----Original Message----- >>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>>> Sent: Tuesday, May 2, 2023 3:56 PM >>>> To: Gaddam, Sarath Babu Naidu >>>> <sarath.babu.naidu.gaddam@xxxxxxx>; davem@xxxxxxxxxxxxx; >>>> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; >>>> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx >>>> Cc: michal.simek@xxxxxxxxxx; radhey.shyam.pandey@xxxxxxxxxx; >>>> netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- >>>> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sarangi, >>>> Anirudha <anirudha.sarangi@xxxxxxx>; Katakam, Harini >>>> <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx> >>>> Subject: Re: [PATCH net-next V7] dt-bindings: net: xlnx,axi-ethernet: >>>> convert bindings document to yaml >>>> >>>> On 28/03/2023 14:52, Gaddam, Sarath Babu Naidu wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx> >>>>>> Sent: Tuesday, March 14, 2023 9:22 PM >>>>>> To: Gaddam, Sarath Babu Naidu >>>>>> <sarath.babu.naidu.gaddam@xxxxxxx>; davem@xxxxxxxxxxxxx; >>>>>> edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; >>>>>> robh+dt@xxxxxxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx >>>>>> Cc: michal.simek@xxxxxxxxxx; radhey.shyam.pandey@xxxxxxxxxx; >>>>>> netdev@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm- >>>>>> kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Sarangi, >>>>>> Anirudha <anirudha.sarangi@xxxxxxx>; Katakam, Harini >>>>>> <harini.katakam@xxxxxxx>; git (AMD-Xilinx) <git@xxxxxxx> >>>>>> 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@xxxxxxxxxx> >>>>>>> >>>>>>> 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 ? >>>> >>>> Yes, I anticipate errors. What you wrote here looks incorrect based on >>>> the schema. >>>> >>>> Also, See also my further comments (or you ignored them?). >>>> >>>> You can come many months after my review to ask about details, to be >>>> sure I will forget the topic. >>> >>> >>> Hi Krzysztof, Apologies for miscommunication. I replied to this thread on >>> March 28 and said that I would address remaining review comments in >>> the next version. >>> >>> Lore link: >>> https://lore.kernel.org/all/MW5PR12MB559880B0E220BDBD64E06D24 >>> 87889@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ >>> https://lore.kernel.org/all/MW5PR12MB5598678BB9AB6EC2FFC424F48 >>> 7889@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/ >>> >>> I planned to send next version with phy-mode and pcs-handle maxItems >>> fixed.but I wanted to close on the axistream-connected discussion before >>> doing so. >>> >>> Related to axistream-connected discussion: >>> I already ran dt binding check for schema and dts node validation. I >>> assume this should point any errors on it. > > And how do we know that you tested correct DTS with it? > >>> >> >> Hi Krzysztof, Could you please comment on this ? Please let me if I missed >> any changes or comments. > > I don't think anything improved in this patchset. The binding has > incomplete and I believe incorrect constraints for axistream-connected. > You did not provide DTS to prove me otherwise. If you are not going to > fix the issue nor provide DTS, what I can say more? Looks wrong. I think my concerns would be solved if you extend the example with a correct second possibility - device with axistream-connected. An example, which will match all your constraints and all your descriptions (we expect then not having any reg, right?). Best regards, Krzysztof
