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@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? > > 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 >