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@xxxxxxxxxxxxx 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@xxxxxxxxxxxxx> >>>> --- >>>> .../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@xxxxxxxxxxxxx> >>>> + >>>> +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 > >
