| Message ID | 20230908142919.14849-7-Parthiban.Veerasooran@microchip.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
On 08/09/2023 16:29, Parthiban Veerasooran wrote: > Add device-tree support for Microchip's LAN865X MACPHY for configuring > the OPEN Alliance 10BASE-T1x MACPHY Serial Interface parameters. Please use subject prefixes matching the subsystem. You can get them for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching. > > Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> > --- > .../bindings/net/microchip,lan865x.yaml | 54 +++++++++++++++++++ > MAINTAINERS | 1 + > 2 files changed, 55 insertions(+) > create mode 100644 Documentation/devicetree/bindings/net/microchip,lan865x.yaml > > diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml > new file mode 100644 > index 000000000000..3465b2c97690 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml > @@ -0,0 +1,54 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/microchip,lan865x.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip LAN8650/1 10BASE-T1S MACPHY Ethernet Controllers > + > +maintainers: > + - Parthiban Veerasooran <parthiban.veerasooran@microchip.com> > + > +description: | > + Device tree properties for LAN8650/1 10BASE-T1S MACPHY Ethernet Drop "Device tree properties for" and instead describe the hardware. > + controller. > + > +allOf: > + - $ref: ethernet-controller.yaml# > + > +properties: > + compatible: > + items: No need for items. Just enum. > + - enum: > + - microchip,lan865x No wildcards in compatibles. Missing blank line. > + reg: > + maxItems: 1 > + > + local-mac-address: true > + oa-chunk-size: true > + oa-tx-cut-through: true > + oa-rx-cut-through: true > + oa-protected: true What are all these? Where are they defined that you skip description, type and vendor prefix? > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + ethernet@1{ Missing space > + compatible = "microchip,lan865x"; > + reg = <1>; /* CE0 */ CE0? chip-select? What does this comment mean in this context? > + local-mac-address = [04 05 06 01 02 03]; > + oa-chunk-size = <64>; > + oa-tx-cut-through; > + oa-rx-cut-through; > + oa-protected; Best regards, Krzysztof
Hi Krzysztof, Thank you for reviewing the patch. On 10/09/23 4:25 pm, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 08/09/2023 16:29, Parthiban Veerasooran wrote: >> Add device-tree support for Microchip's LAN865X MACPHY for configuring >> the OPEN Alliance 10BASE-T1x MACPHY Serial Interface parameters. > > Please use subject prefixes matching the subsystem. You can get them for > example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory > your patch is touching. Ok sure, so it will become like, dt-bindings: net: add device-tree support for Microchip's LAN865X MACPHY I will correct it in the next revision. > >> >> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> >> --- >> .../bindings/net/microchip,lan865x.yaml | 54 +++++++++++++++++++ >> MAINTAINERS | 1 + >> 2 files changed, 55 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/net/microchip,lan865x.yaml >> >> diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >> new file mode 100644 >> index 000000000000..3465b2c97690 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >> @@ -0,0 +1,54 @@ >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/net/microchip,lan865x.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Microchip LAN8650/1 10BASE-T1S MACPHY Ethernet Controllers >> + >> +maintainers: >> + - Parthiban Veerasooran <parthiban.veerasooran@microchip.com> >> + >> +description: | >> + Device tree properties for LAN8650/1 10BASE-T1S MACPHY Ethernet > > Drop "Device tree properties for" and instead describe the hardware. sure, will do it. > >> + controller. >> + >> +allOf: >> + - $ref: ethernet-controller.yaml# >> + >> +properties: >> + compatible: >> + items: > > No need for items. Just enum. Ok noted. > > >> + - enum: >> + - microchip,lan865x > > No wildcards in compatibles. Yes then we don't need enum also isn't it? > > Missing blank line. Ok will add it. > > > >> + reg: >> + maxItems: 1 >> + >> + local-mac-address: true >> + oa-chunk-size: true >> + oa-tx-cut-through: true >> + oa-rx-cut-through: true >> + oa-protected: true > > What are all these? Where are they defined that you skip description, > type and vendor prefix? Ok missed it. Will do it in the next revision. > >> + >> +required: >> + - compatible >> + - reg >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + spi { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + ethernet@1{ > > Missing space Ok will add it. > >> + compatible = "microchip,lan865x"; >> + reg = <1>; /* CE0 */ > > CE0? chip-select? What does this comment mean in this context? Yes it is chip-select. Will add proper comment. Best Regards, Parthiban V > >> + local-mac-address = [04 05 06 01 02 03]; >> + oa-chunk-size = <64>; >> + oa-tx-cut-through; >> + oa-rx-cut-through; >> + oa-protected; > > > > Best regards, > Krzysztof >
On 12/09/2023 14:15, Parthiban.Veerasooran@microchip.com wrote: > Hi Krzysztof, > > Thank you for reviewing the patch. > > On 10/09/23 4:25 pm, Krzysztof Kozlowski wrote: >> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >> >> On 08/09/2023 16:29, Parthiban Veerasooran wrote: >>> Add device-tree support for Microchip's LAN865X MACPHY for configuring >>> the OPEN Alliance 10BASE-T1x MACPHY Serial Interface parameters. >> >> Please use subject prefixes matching the subsystem. You can get them for >> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >> your patch is touching. > Ok sure, so it will become like, > > dt-bindings: net: add device-tree support for Microchip's LAN865X MACPHY > > I will correct it in the next revision. "device-tree support for " is redundant, drop >> >>> >>> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> >>> --- >>> .../bindings/net/microchip,lan865x.yaml | 54 +++++++++++++++++++ >>> MAINTAINERS | 1 + >>> 2 files changed, 55 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>> >>> diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>> new file mode 100644 >>> index 000000000000..3465b2c97690 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>> @@ -0,0 +1,54 @@ >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>> +%YAML 1.2 >>> +--- >>> +$id: http://devicetree.org/schemas/net/microchip,lan865x.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: Microchip LAN8650/1 10BASE-T1S MACPHY Ethernet Controllers >>> + >>> +maintainers: >>> + - Parthiban Veerasooran <parthiban.veerasooran@microchip.com> >>> + >>> +description: | >>> + Device tree properties for LAN8650/1 10BASE-T1S MACPHY Ethernet >> >> Drop "Device tree properties for" and instead describe the hardware. > sure, will do it. >> >>> + controller. >>> + >>> +allOf: >>> + - $ref: ethernet-controller.yaml# >>> + >>> +properties: >>> + compatible: >>> + items: >> >> No need for items. Just enum. > Ok noted. >> >> >>> + - enum: >>> + - microchip,lan865x >> >> No wildcards in compatibles. > Yes then we don't need enum also isn't it? I don't see correlation between these two. Please read the writing bindings guidelines. >> >> Missing blank line. > Ok will add it. >> >> >> >>> + reg: >>> + maxItems: 1 >>> + >>> + local-mac-address: true >>> + oa-chunk-size: true >>> + oa-tx-cut-through: true >>> + oa-rx-cut-through: true >>> + oa-protected: true >> >> What are all these? Where are they defined that you skip description, >> type and vendor prefix? > Ok missed it. Will do it in the next revision. No, drop them or explain why they are hardware properties. >> >>> + >>> +required: >>> + - compatible >>> + - reg >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + spi { >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + ethernet@1{ >> >> Missing space > Ok will add it. >> >>> + compatible = "microchip,lan865x"; >>> + reg = <1>; /* CE0 */ >> >> CE0? chip-select? What does this comment mean in this context? > Yes it is chip-select. Will add proper comment. Why? isn't reg obvious? Best regards, Krzysztof
> + oa-chunk-size: true > + oa-tx-cut-through: true > + oa-rx-cut-through: true > + oa-protected: true Please split this up into properties all OA TC6 devices are expected to use, and those specific to the LAN865x. Put the generic properties into a .yaml file, which you then inherit into the device specific yaml file. Also, LAN865x specific properties should have a vendor prefix. Andrew
Hi Andrew, On 14/09/23 7:37 am, Andrew Lunn wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > >> + oa-chunk-size: true >> + oa-tx-cut-through: true >> + oa-rx-cut-through: true >> + oa-protected: true > > Please split this up into properties all OA TC6 devices are expected > to use, and those specific to the LAN865x. Put the generic properties > into a .yaml file, which you then inherit into the device specific > yaml file. > > Also, LAN865x specific properties should have a vendor prefix. Sure, will do both. Best Regards, Parthiban V > > Andrew
Hi Krzysztof, On 12/09/23 6:47 pm, Krzysztof Kozlowski wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On 12/09/2023 14:15, Parthiban.Veerasooran@microchip.com wrote: >> Hi Krzysztof, >> >> Thank you for reviewing the patch. >> >> On 10/09/23 4:25 pm, Krzysztof Kozlowski wrote: >>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe >>> >>> On 08/09/2023 16:29, Parthiban Veerasooran wrote: >>>> Add device-tree support for Microchip's LAN865X MACPHY for configuring >>>> the OPEN Alliance 10BASE-T1x MACPHY Serial Interface parameters. >>> >>> Please use subject prefixes matching the subsystem. You can get them for >>> example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory >>> your patch is touching. >> Ok sure, so it will become like, >> >> dt-bindings: net: add device-tree support for Microchip's LAN865X MACPHY >> >> I will correct it in the next revision. > > "device-tree support for " is redundant, drop Ah ok will do that. > >>> >>>> >>>> Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> >>>> --- >>>> .../bindings/net/microchip,lan865x.yaml | 54 +++++++++++++++++++ >>>> MAINTAINERS | 1 + >>>> 2 files changed, 55 insertions(+) >>>> create mode 100644 Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>>> new file mode 100644 >>>> index 000000000000..3465b2c97690 >>>> --- /dev/null >>>> +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml >>>> @@ -0,0 +1,54 @@ >>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) >>>> +%YAML 1.2 >>>> +--- >>>> +$id: http://devicetree.org/schemas/net/microchip,lan865x.yaml# >>>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>>> + >>>> +title: Microchip LAN8650/1 10BASE-T1S MACPHY Ethernet Controllers >>>> + >>>> +maintainers: >>>> + - Parthiban Veerasooran <parthiban.veerasooran@microchip.com> >>>> + >>>> +description: | >>>> + Device tree properties for LAN8650/1 10BASE-T1S MACPHY Ethernet >>> >>> Drop "Device tree properties for" and instead describe the hardware. >> sure, will do it. >>> >>>> + controller. >>>> + >>>> +allOf: >>>> + - $ref: ethernet-controller.yaml# >>>> + >>>> +properties: >>>> + compatible: >>>> + items: >>> >>> No need for items. Just enum. >> Ok noted. >>> >>> >>>> + - enum: >>>> + - microchip,lan865x >>> >>> No wildcards in compatibles. >> Yes then we don't need enum also isn't it? > > I don't see correlation between these two. Please read the writing > bindings guidelines. Ok, will check it out. > > >>> >>> Missing blank line. >> Ok will add it. >>> >>> >>> >>>> + reg: >>>> + maxItems: 1 >>>> + >>>> + local-mac-address: true >>>> + oa-chunk-size: true >>>> + oa-tx-cut-through: true >>>> + oa-rx-cut-through: true >>>> + oa-protected: true >>> >>> What are all these? Where are they defined that you skip description, >>> type and vendor prefix? >> Ok missed it. Will do it in the next revision. > > No, drop them or explain why they are hardware properties. Will separate hardware specific and OA specific properties. > >>> >>>> + >>>> +required: >>>> + - compatible >>>> + - reg >>>> + >>>> +additionalProperties: false >>>> + >>>> +examples: >>>> + - | >>>> + spi { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + ethernet@1{ >>> >>> Missing space >> Ok will add it. >>> >>>> + compatible = "microchip,lan865x"; >>>> + reg = <1>; /* CE0 */ >>> >>> CE0? chip-select? What does this comment mean in this context? >> Yes it is chip-select. Will add proper comment. > > Why? isn't reg obvious? Sorry, yes it is reg. The comment is wrong. Will remove it. Best Regards, Parthiban V > > Best regards, > Krzysztof > >
diff --git a/Documentation/devicetree/bindings/net/microchip,lan865x.yaml b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml new file mode 100644 index 000000000000..3465b2c97690 --- /dev/null +++ b/Documentation/devicetree/bindings/net/microchip,lan865x.yaml @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/microchip,lan865x.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip LAN8650/1 10BASE-T1S MACPHY Ethernet Controllers + +maintainers: + - Parthiban Veerasooran <parthiban.veerasooran@microchip.com> + +description: | + Device tree properties for LAN8650/1 10BASE-T1S MACPHY Ethernet + controller. + +allOf: + - $ref: ethernet-controller.yaml# + +properties: + compatible: + items: + - enum: + - microchip,lan865x + reg: + maxItems: 1 + + local-mac-address: true + oa-chunk-size: true + oa-tx-cut-through: true + oa-rx-cut-through: true + oa-protected: true + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + spi { + #address-cells = <1>; + #size-cells = <0>; + + ethernet@1{ + compatible = "microchip,lan865x"; + reg = <1>; /* CE0 */ + local-mac-address = [04 05 06 01 02 03]; + oa-chunk-size = <64>; + oa-tx-cut-through; + oa-rx-cut-through; + oa-protected; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 666c042a15b2..2bbb7f17d74e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -13883,6 +13883,7 @@ MICROCHIP LAN8650/1 10BASE-T1S MACPHY ETHERNET DRIVER M: Parthiban Veerasooran <parthiban.veerasooran@microchip.com> L: netdev@vger.kernel.org S: Maintained +F: Documentation/devicetree/bindings/net/microchip,lan865x.yaml F: drivers/net/ethernet/microchip/lan865x.c MICROCHIP LAN87xx/LAN937x T1 PHY DRIVER
Add device-tree support for Microchip's LAN865X MACPHY for configuring the OPEN Alliance 10BASE-T1x MACPHY Serial Interface parameters. Signed-off-by: Parthiban Veerasooran <Parthiban.Veerasooran@microchip.com> --- .../bindings/net/microchip,lan865x.yaml | 54 +++++++++++++++++++ MAINTAINERS | 1 + 2 files changed, 55 insertions(+) create mode 100644 Documentation/devicetree/bindings/net/microchip,lan865x.yaml