diff mbox series

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

Message ID 20230908142919.14849-7-Parthiban.Veerasooran@microchip.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series Add support for OPEN Alliance 10BASE-T1x MACPHY Serial Interface | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers warning 1 maintainers not CCed: parthiban.veerasooran@microchip.com
netdev/build_clang success Errors and warnings before: 9 this patch: 9
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 61 lines checked
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Parthiban Veerasooran Sept. 8, 2023, 2:29 p.m. UTC
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

Comments

Krzysztof Kozlowski Sept. 10, 2023, 10:55 a.m. UTC | #1
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
Parthiban Veerasooran Sept. 12, 2023, 12:15 p.m. UTC | #2
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
>
Krzysztof Kozlowski Sept. 12, 2023, 1:17 p.m. UTC | #3
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
Andrew Lunn Sept. 14, 2023, 2:07 a.m. UTC | #4
> +  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
Parthiban Veerasooran Sept. 19, 2023, 10:40 a.m. UTC | #5
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
Parthiban Veerasooran Sept. 19, 2023, 10:51 a.m. UTC | #6
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 mbox series

Patch

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