netdev - Re: [RFC PATCH net-next 6/6] microchip: lan865x: add device-tree support for Microchip's LAN865X MACPHY

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f429ea93-9cb2-8869-a98d-fb55161cf880@microchip.com>
Date: Tue, 12 Sep 2023 12:15:06 +0000
From: <Parthiban.Veerasooran@...rochip.com>
To: <krzysztof.kozlowski@...aro.org>
CC: <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
	<Horatiu.Vultur@...rochip.com>, <Woojung.Huh@...rochip.com>,
	<Nicolas.Ferre@...rochip.com>, <UNGLinuxDriver@...rochip.com>,
	<Thorsten.Kummermehr@...rochip.com>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<robh+dt@...nel.org>, <krzysztof.kozlowski+dt@...aro.org>,
	<conor+dt@...nel.org>, <corbet@....net>, <Steen.Hegelund@...rochip.com>,
	<rdunlap@...radead.org>, <horms@...nel.org>, <casper.casan@...il.com>,
	<andrew@...n.ch>
Subject: Re: [RFC PATCH net-next 6/6] microchip: lan865x: add device-tree
 support for Microchip's LAN865X MACPHY

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@...rochip.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@...rochip.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
>