* [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-08 5:18 ` Siddharth Vadapalli
0 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-08 5:18 UTC (permalink / raw
To: davem, edumazet, kuba, linux, pabeni, robh+dt,
krzysztof.kozlowski, krzysztof.kozlowski+dt, nsekhar, rogerq
Cc: netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Update bindings to include Serdes PHY as an optional PHY, in addition to
the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
Serdes PHY is optional. The Serdes PHY handle has to be provided only
when the Serdes is being configured in a Single-Link protocol. Using the
name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
driver can obtain the Serdes PHY and request the Serdes to be
configured.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
Hello,
This patch corresponds to the Serdes PHY bindings that were missed out in
the series at:
https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
This was pointed out at:
https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
Changes from v1:
1. Describe phys property with minItems, items and description.
2. Use minItems and items in phy-names.
3. Remove the description in phy-names.
v1:
https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
.../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index 900063411a20..0fb48bb6a041 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -126,8 +126,18 @@ properties:
description: CPSW port number
phys:
- maxItems: 1
- description: phandle on phy-gmii-sel PHY
+ minItems: 1
+ items:
+ - description: CPSW MAC's PHY.
+ - description: Serdes PHY. Serdes PHY is required only if
+ the Serdes has to be configured in the
+ Single-Link configuration.
+
+ phy-names:
+ minItems: 1
+ items:
+ - const: mac-phy
+ - const: serdes-phy
label:
description: label associated with this port
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-08 5:18 ` Siddharth Vadapalli
0 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-08 5:18 UTC (permalink / raw
To: davem, edumazet, kuba, linux, pabeni, robh+dt,
krzysztof.kozlowski, krzysztof.kozlowski+dt, nsekhar, rogerq
Cc: netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Update bindings to include Serdes PHY as an optional PHY, in addition to
the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
Serdes PHY is optional. The Serdes PHY handle has to be provided only
when the Serdes is being configured in a Single-Link protocol. Using the
name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
driver can obtain the Serdes PHY and request the Serdes to be
configured.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
Hello,
This patch corresponds to the Serdes PHY bindings that were missed out in
the series at:
https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
This was pointed out at:
https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
Changes from v1:
1. Describe phys property with minItems, items and description.
2. Use minItems and items in phy-names.
3. Remove the description in phy-names.
v1:
https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
.../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
index 900063411a20..0fb48bb6a041 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -126,8 +126,18 @@ properties:
description: CPSW port number
phys:
- maxItems: 1
- description: phandle on phy-gmii-sel PHY
+ minItems: 1
+ items:
+ - description: CPSW MAC's PHY.
+ - description: Serdes PHY. Serdes PHY is required only if
+ the Serdes has to be configured in the
+ Single-Link configuration.
+
+ phy-names:
+ minItems: 1
+ items:
+ - const: mac-phy
+ - const: serdes-phy
label:
description: label associated with this port
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-08 5:18 ` Siddharth Vadapalli
@ 2023-03-08 8:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-08 8:34 UTC (permalink / raw
To: Siddharth Vadapalli, davem, edumazet, kuba, linux, pabeni,
robh+dt, krzysztof.kozlowski+dt, nsekhar, rogerq
Cc: netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 08/03/2023 06:18, Siddharth Vadapalli wrote:
> Update bindings to include Serdes PHY as an optional PHY, in addition to
> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
> Serdes PHY is optional. The Serdes PHY handle has to be provided only
> when the Serdes is being configured in a Single-Link protocol. Using the
> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
> driver can obtain the Serdes PHY and request the Serdes to be
> configured.
>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>
> Hello,
>
> This patch corresponds to the Serdes PHY bindings that were missed out in
> the series at:
> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
> This was pointed out at:
> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>
> Changes from v1:
> 1. Describe phys property with minItems, items and description.
> 2. Use minItems and items in phy-names.
> 3. Remove the description in phy-names.
>
> v1:
> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>
> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> index 900063411a20..0fb48bb6a041 100644
> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> @@ -126,8 +126,18 @@ properties:
> description: CPSW port number
>
> phys:
> - maxItems: 1
> - description: phandle on phy-gmii-sel PHY
> + minItems: 1
> + items:
> + - description: CPSW MAC's PHY.
> + - description: Serdes PHY. Serdes PHY is required only if
> + the Serdes has to be configured in the
> + Single-Link configuration.
> +
> + phy-names:
> + minItems: 1
> + items:
> + - const: mac-phy
> + - const: serdes-phy
Drop "phy" suffixes.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-08 8:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-08 8:34 UTC (permalink / raw
To: Siddharth Vadapalli, davem, edumazet, kuba, linux, pabeni,
robh+dt, krzysztof.kozlowski+dt, nsekhar, rogerq
Cc: netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 08/03/2023 06:18, Siddharth Vadapalli wrote:
> Update bindings to include Serdes PHY as an optional PHY, in addition to
> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
> Serdes PHY is optional. The Serdes PHY handle has to be provided only
> when the Serdes is being configured in a Single-Link protocol. Using the
> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
> driver can obtain the Serdes PHY and request the Serdes to be
> configured.
>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>
> Hello,
>
> This patch corresponds to the Serdes PHY bindings that were missed out in
> the series at:
> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
> This was pointed out at:
> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>
> Changes from v1:
> 1. Describe phys property with minItems, items and description.
> 2. Use minItems and items in phy-names.
> 3. Remove the description in phy-names.
>
> v1:
> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>
> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> index 900063411a20..0fb48bb6a041 100644
> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> @@ -126,8 +126,18 @@ properties:
> description: CPSW port number
>
> phys:
> - maxItems: 1
> - description: phandle on phy-gmii-sel PHY
> + minItems: 1
> + items:
> + - description: CPSW MAC's PHY.
> + - description: Serdes PHY. Serdes PHY is required only if
> + the Serdes has to be configured in the
> + Single-Link configuration.
> +
> + phy-names:
> + minItems: 1
> + items:
> + - const: mac-phy
> + - const: serdes-phy
Drop "phy" suffixes.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-08 8:34 ` Krzysztof Kozlowski
@ 2023-03-08 8:38 ` Siddharth Vadapalli
-1 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-08 8:38 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 08/03/23 14:04, Krzysztof Kozlowski wrote:
> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>> when the Serdes is being configured in a Single-Link protocol. Using the
>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>> driver can obtain the Serdes PHY and request the Serdes to be
>> configured.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>
>> Hello,
>>
>> This patch corresponds to the Serdes PHY bindings that were missed out in
>> the series at:
>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>> This was pointed out at:
>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>
>> Changes from v1:
>> 1. Describe phys property with minItems, items and description.
>> 2. Use minItems and items in phy-names.
>> 3. Remove the description in phy-names.
>>
>> v1:
>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>
>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> index 900063411a20..0fb48bb6a041 100644
>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> @@ -126,8 +126,18 @@ properties:
>> description: CPSW port number
>>
>> phys:
>> - maxItems: 1
>> - description: phandle on phy-gmii-sel PHY
>> + minItems: 1
>> + items:
>> + - description: CPSW MAC's PHY.
>> + - description: Serdes PHY. Serdes PHY is required only if
>> + the Serdes has to be configured in the
>> + Single-Link configuration.
>> +
>> + phy-names:
>> + minItems: 1
>> + items:
>> + - const: mac-phy
>> + - const: serdes-phy
>
> Drop "phy" suffixes.
The am65-cpsw driver fetches the Serdes PHY by looking for the string
"serdes-phy". Therefore, modifying the string will require changing the driver's
code as well. Please let me know if it is absolutely necessary to drop the phy
suffix. If so, I will post a new series with the changes involving dt-bindings
changes and the driver changes.
Regards,
Siddharth.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-08 8:38 ` Siddharth Vadapalli
0 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-08 8:38 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 08/03/23 14:04, Krzysztof Kozlowski wrote:
> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>> when the Serdes is being configured in a Single-Link protocol. Using the
>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>> driver can obtain the Serdes PHY and request the Serdes to be
>> configured.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>
>> Hello,
>>
>> This patch corresponds to the Serdes PHY bindings that were missed out in
>> the series at:
>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>> This was pointed out at:
>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>
>> Changes from v1:
>> 1. Describe phys property with minItems, items and description.
>> 2. Use minItems and items in phy-names.
>> 3. Remove the description in phy-names.
>>
>> v1:
>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>
>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> index 900063411a20..0fb48bb6a041 100644
>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> @@ -126,8 +126,18 @@ properties:
>> description: CPSW port number
>>
>> phys:
>> - maxItems: 1
>> - description: phandle on phy-gmii-sel PHY
>> + minItems: 1
>> + items:
>> + - description: CPSW MAC's PHY.
>> + - description: Serdes PHY. Serdes PHY is required only if
>> + the Serdes has to be configured in the
>> + Single-Link configuration.
>> +
>> + phy-names:
>> + minItems: 1
>> + items:
>> + - const: mac-phy
>> + - const: serdes-phy
>
> Drop "phy" suffixes.
The am65-cpsw driver fetches the Serdes PHY by looking for the string
"serdes-phy". Therefore, modifying the string will require changing the driver's
code as well. Please let me know if it is absolutely necessary to drop the phy
suffix. If so, I will post a new series with the changes involving dt-bindings
changes and the driver changes.
Regards,
Siddharth.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-08 8:38 ` Siddharth Vadapalli
@ 2023-03-08 12:34 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-08 12:34 UTC (permalink / raw
To: Siddharth Vadapalli, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 08/03/2023 09:38, Siddharth Vadapalli wrote:
> Hello Krzysztof,
>
> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>> driver can obtain the Serdes PHY and request the Serdes to be
>>> configured.
>>>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>>
>>> Hello,
>>>
>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>> the series at:
>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>> This was pointed out at:
>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>
>>> Changes from v1:
>>> 1. Describe phys property with minItems, items and description.
>>> 2. Use minItems and items in phy-names.
>>> 3. Remove the description in phy-names.
>>>
>>> v1:
>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>
>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> index 900063411a20..0fb48bb6a041 100644
>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> @@ -126,8 +126,18 @@ properties:
>>> description: CPSW port number
>>>
>>> phys:
>>> - maxItems: 1
>>> - description: phandle on phy-gmii-sel PHY
>>> + minItems: 1
>>> + items:
>>> + - description: CPSW MAC's PHY.
>>> + - description: Serdes PHY. Serdes PHY is required only if
>>> + the Serdes has to be configured in the
>>> + Single-Link configuration.
>>> +
>>> + phy-names:
>>> + minItems: 1
>>> + items:
>>> + - const: mac-phy
>>> + - const: serdes-phy
>>
>> Drop "phy" suffixes.
>
> The am65-cpsw driver fetches the Serdes PHY by looking for the string
> "serdes-phy". Therefore, modifying the string will require changing the driver's
> code as well. Please let me know if it is absolutely necessary to drop the phy
> suffix. If so, I will post a new series with the changes involving dt-bindings
> changes and the driver changes.
Why the driver uses some properties before adding them to the binding?
And is it correct method of adding ABI? You add incorrect properties
without documentation and then use this as an argument "driver already
does it"?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-08 12:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-08 12:34 UTC (permalink / raw
To: Siddharth Vadapalli, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 08/03/2023 09:38, Siddharth Vadapalli wrote:
> Hello Krzysztof,
>
> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>> driver can obtain the Serdes PHY and request the Serdes to be
>>> configured.
>>>
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>> ---
>>>
>>> Hello,
>>>
>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>> the series at:
>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>> This was pointed out at:
>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>
>>> Changes from v1:
>>> 1. Describe phys property with minItems, items and description.
>>> 2. Use minItems and items in phy-names.
>>> 3. Remove the description in phy-names.
>>>
>>> v1:
>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>
>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> index 900063411a20..0fb48bb6a041 100644
>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>> @@ -126,8 +126,18 @@ properties:
>>> description: CPSW port number
>>>
>>> phys:
>>> - maxItems: 1
>>> - description: phandle on phy-gmii-sel PHY
>>> + minItems: 1
>>> + items:
>>> + - description: CPSW MAC's PHY.
>>> + - description: Serdes PHY. Serdes PHY is required only if
>>> + the Serdes has to be configured in the
>>> + Single-Link configuration.
>>> +
>>> + phy-names:
>>> + minItems: 1
>>> + items:
>>> + - const: mac-phy
>>> + - const: serdes-phy
>>
>> Drop "phy" suffixes.
>
> The am65-cpsw driver fetches the Serdes PHY by looking for the string
> "serdes-phy". Therefore, modifying the string will require changing the driver's
> code as well. Please let me know if it is absolutely necessary to drop the phy
> suffix. If so, I will post a new series with the changes involving dt-bindings
> changes and the driver changes.
Why the driver uses some properties before adding them to the binding?
And is it correct method of adding ABI? You add incorrect properties
without documentation and then use this as an argument "driver already
does it"?
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-08 12:34 ` Krzysztof Kozlowski
@ 2023-03-09 4:18 ` Siddharth Vadapalli
-1 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-09 4:18 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 08/03/23 18:04, Krzysztof Kozlowski wrote:
> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>> Hello Krzysztof,
>>
>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>> configured.
>>>>
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>> ---
>>>>
>>>> Hello,
>>>>
>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>> the series at:
>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>> This was pointed out at:
>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>
>>>> Changes from v1:
>>>> 1. Describe phys property with minItems, items and description.
>>>> 2. Use minItems and items in phy-names.
>>>> 3. Remove the description in phy-names.
>>>>
>>>> v1:
>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>
>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> index 900063411a20..0fb48bb6a041 100644
>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> @@ -126,8 +126,18 @@ properties:
>>>> description: CPSW port number
>>>>
>>>> phys:
>>>> - maxItems: 1
>>>> - description: phandle on phy-gmii-sel PHY
>>>> + minItems: 1
>>>> + items:
>>>> + - description: CPSW MAC's PHY.
>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>> + the Serdes has to be configured in the
>>>> + Single-Link configuration.
>>>> +
>>>> + phy-names:
>>>> + minItems: 1
>>>> + items:
>>>> + - const: mac-phy
>>>> + - const: serdes-phy
>>>
>>> Drop "phy" suffixes.
>>
>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>> code as well. Please let me know if it is absolutely necessary to drop the phy
>> suffix. If so, I will post a new series with the changes involving dt-bindings
>> changes and the driver changes.
>
> Why the driver uses some properties before adding them to the binding?
I missed adding the bindings for the Serdes PHY as a part of the series
mentioned in the section below the tearline of the patch. With this patch, I am
attempting to fix it.
>
> And is it correct method of adding ABI? You add incorrect properties
> without documentation and then use this as an argument "driver already
> does it"?
I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
based on the driver already using it. I did not mean it in that sense. I simply
meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
context, I felt that if it was a suggestion, I would prefer retaining the names
with the "phy" suffixes, since the driver is already using it. Additionally, I
also mentioned in my earlier comment that if it is necessary to drop the "phy"
suffix, then I will do so and add another patch to change the string the driver
looks for as well.
I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
will post the v3 series making this change, along with the patch to update the
string the driver looks for.
Regards,
Siddharth.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-09 4:18 ` Siddharth Vadapalli
0 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-09 4:18 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 08/03/23 18:04, Krzysztof Kozlowski wrote:
> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>> Hello Krzysztof,
>>
>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>> configured.
>>>>
>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>> ---
>>>>
>>>> Hello,
>>>>
>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>> the series at:
>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>> This was pointed out at:
>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>
>>>> Changes from v1:
>>>> 1. Describe phys property with minItems, items and description.
>>>> 2. Use minItems and items in phy-names.
>>>> 3. Remove the description in phy-names.
>>>>
>>>> v1:
>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>
>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> index 900063411a20..0fb48bb6a041 100644
>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>> @@ -126,8 +126,18 @@ properties:
>>>> description: CPSW port number
>>>>
>>>> phys:
>>>> - maxItems: 1
>>>> - description: phandle on phy-gmii-sel PHY
>>>> + minItems: 1
>>>> + items:
>>>> + - description: CPSW MAC's PHY.
>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>> + the Serdes has to be configured in the
>>>> + Single-Link configuration.
>>>> +
>>>> + phy-names:
>>>> + minItems: 1
>>>> + items:
>>>> + - const: mac-phy
>>>> + - const: serdes-phy
>>>
>>> Drop "phy" suffixes.
>>
>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>> code as well. Please let me know if it is absolutely necessary to drop the phy
>> suffix. If so, I will post a new series with the changes involving dt-bindings
>> changes and the driver changes.
>
> Why the driver uses some properties before adding them to the binding?
I missed adding the bindings for the Serdes PHY as a part of the series
mentioned in the section below the tearline of the patch. With this patch, I am
attempting to fix it.
>
> And is it correct method of adding ABI? You add incorrect properties
> without documentation and then use this as an argument "driver already
> does it"?
I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
based on the driver already using it. I did not mean it in that sense. I simply
meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
context, I felt that if it was a suggestion, I would prefer retaining the names
with the "phy" suffixes, since the driver is already using it. Additionally, I
also mentioned in my earlier comment that if it is necessary to drop the "phy"
suffix, then I will do so and add another patch to change the string the driver
looks for as well.
I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
will post the v3 series making this change, along with the patch to update the
string the driver looks for.
Regards,
Siddharth.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-09 4:18 ` Siddharth Vadapalli
@ 2023-03-09 6:21 ` Krzysztof Kozlowski
-1 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-09 6:21 UTC (permalink / raw
To: Siddharth Vadapalli, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 09/03/2023 05:18, Siddharth Vadapalli wrote:
> Hello Krzysztof,
>
> On 08/03/23 18:04, Krzysztof Kozlowski wrote:
>> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>>> Hello Krzysztof,
>>>
>>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>>> configured.
>>>>>
>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>> ---
>>>>>
>>>>> Hello,
>>>>>
>>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>>> the series at:
>>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>>> This was pointed out at:
>>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>>
>>>>> Changes from v1:
>>>>> 1. Describe phys property with minItems, items and description.
>>>>> 2. Use minItems and items in phy-names.
>>>>> 3. Remove the description in phy-names.
>>>>>
>>>>> v1:
>>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>>
>>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> index 900063411a20..0fb48bb6a041 100644
>>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> @@ -126,8 +126,18 @@ properties:
>>>>> description: CPSW port number
>>>>>
>>>>> phys:
>>>>> - maxItems: 1
>>>>> - description: phandle on phy-gmii-sel PHY
>>>>> + minItems: 1
>>>>> + items:
>>>>> + - description: CPSW MAC's PHY.
>>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>>> + the Serdes has to be configured in the
>>>>> + Single-Link configuration.
>>>>> +
>>>>> + phy-names:
>>>>> + minItems: 1
>>>>> + items:
>>>>> + - const: mac-phy
>>>>> + - const: serdes-phy
>>>>
>>>> Drop "phy" suffixes.
>>>
>>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>>> code as well. Please let me know if it is absolutely necessary to drop the phy
>>> suffix. If so, I will post a new series with the changes involving dt-bindings
>>> changes and the driver changes.
>>
>> Why the driver uses some properties before adding them to the binding?
>
> I missed adding the bindings for the Serdes PHY as a part of the series
> mentioned in the section below the tearline of the patch. With this patch, I am
> attempting to fix it.
>
>>
>> And is it correct method of adding ABI? You add incorrect properties
>> without documentation and then use this as an argument "driver already
>> does it"?
>
> I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
> based on the driver already using it. I did not mean it in that sense. I simply
> meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
> context, I felt that if it was a suggestion, I would prefer retaining the names
> with the "phy" suffixes, since the driver is already using it. Additionally, I
> also mentioned in my earlier comment that if it is necessary to drop the "phy"
> suffix, then I will do so and add another patch to change the string the driver
> looks for as well.
>
> I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
> will post the v3 series making this change, along with the patch to update the
> string the driver looks for.
Drop the "phy" suffix.
It's a new binding. "phy" as suffix for "phy" is useless and for new
bindings it should be dropped. If you submitted driver changes without
bindings, which document the ABI, it's not good, but also not a reason
for me for exceptions.
Best regards,
Krzysztof
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-09 6:21 ` Krzysztof Kozlowski
0 siblings, 0 replies; 14+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-09 6:21 UTC (permalink / raw
To: Siddharth Vadapalli, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk
On 09/03/2023 05:18, Siddharth Vadapalli wrote:
> Hello Krzysztof,
>
> On 08/03/23 18:04, Krzysztof Kozlowski wrote:
>> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>>> Hello Krzysztof,
>>>
>>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>>> configured.
>>>>>
>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>> ---
>>>>>
>>>>> Hello,
>>>>>
>>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>>> the series at:
>>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>>> This was pointed out at:
>>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>>
>>>>> Changes from v1:
>>>>> 1. Describe phys property with minItems, items and description.
>>>>> 2. Use minItems and items in phy-names.
>>>>> 3. Remove the description in phy-names.
>>>>>
>>>>> v1:
>>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>>
>>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> index 900063411a20..0fb48bb6a041 100644
>>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>> @@ -126,8 +126,18 @@ properties:
>>>>> description: CPSW port number
>>>>>
>>>>> phys:
>>>>> - maxItems: 1
>>>>> - description: phandle on phy-gmii-sel PHY
>>>>> + minItems: 1
>>>>> + items:
>>>>> + - description: CPSW MAC's PHY.
>>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>>> + the Serdes has to be configured in the
>>>>> + Single-Link configuration.
>>>>> +
>>>>> + phy-names:
>>>>> + minItems: 1
>>>>> + items:
>>>>> + - const: mac-phy
>>>>> + - const: serdes-phy
>>>>
>>>> Drop "phy" suffixes.
>>>
>>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>>> code as well. Please let me know if it is absolutely necessary to drop the phy
>>> suffix. If so, I will post a new series with the changes involving dt-bindings
>>> changes and the driver changes.
>>
>> Why the driver uses some properties before adding them to the binding?
>
> I missed adding the bindings for the Serdes PHY as a part of the series
> mentioned in the section below the tearline of the patch. With this patch, I am
> attempting to fix it.
>
>>
>> And is it correct method of adding ABI? You add incorrect properties
>> without documentation and then use this as an argument "driver already
>> does it"?
>
> I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
> based on the driver already using it. I did not mean it in that sense. I simply
> meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
> context, I felt that if it was a suggestion, I would prefer retaining the names
> with the "phy" suffixes, since the driver is already using it. Additionally, I
> also mentioned in my earlier comment that if it is necessary to drop the "phy"
> suffix, then I will do so and add another patch to change the string the driver
> looks for as well.
>
> I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
> will post the v3 series making this change, along with the patch to update the
> string the driver looks for.
Drop the "phy" suffix.
It's a new binding. "phy" as suffix for "phy" is useless and for new
bindings it should be dropped. If you submitted driver changes without
bindings, which document the ABI, it's not good, but also not a reason
for me for exceptions.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
2023-03-09 6:21 ` Krzysztof Kozlowski
@ 2023-03-09 7:24 ` Siddharth Vadapalli
-1 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-09 7:24 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 09/03/23 11:51, Krzysztof Kozlowski wrote:
> On 09/03/2023 05:18, Siddharth Vadapalli wrote:
>> Hello Krzysztof,
>>
>> On 08/03/23 18:04, Krzysztof Kozlowski wrote:
>>> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>>>> Hello Krzysztof,
>>>>
>>>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>>>> configured.
>>>>>>
>>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>>> ---
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>>>> the series at:
>>>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>>>> This was pointed out at:
>>>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>>>
>>>>>> Changes from v1:
>>>>>> 1. Describe phys property with minItems, items and description.
>>>>>> 2. Use minItems and items in phy-names.
>>>>>> 3. Remove the description in phy-names.
>>>>>>
>>>>>> v1:
>>>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>>>
>>>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> index 900063411a20..0fb48bb6a041 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> @@ -126,8 +126,18 @@ properties:
>>>>>> description: CPSW port number
>>>>>>
>>>>>> phys:
>>>>>> - maxItems: 1
>>>>>> - description: phandle on phy-gmii-sel PHY
>>>>>> + minItems: 1
>>>>>> + items:
>>>>>> + - description: CPSW MAC's PHY.
>>>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>>>> + the Serdes has to be configured in the
>>>>>> + Single-Link configuration.
>>>>>> +
>>>>>> + phy-names:
>>>>>> + minItems: 1
>>>>>> + items:
>>>>>> + - const: mac-phy
>>>>>> + - const: serdes-phy
>>>>>
>>>>> Drop "phy" suffixes.
>>>>
>>>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>>>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>>>> code as well. Please let me know if it is absolutely necessary to drop the phy
>>>> suffix. If so, I will post a new series with the changes involving dt-bindings
>>>> changes and the driver changes.
>>>
>>> Why the driver uses some properties before adding them to the binding?
>>
>> I missed adding the bindings for the Serdes PHY as a part of the series
>> mentioned in the section below the tearline of the patch. With this patch, I am
>> attempting to fix it.
>>
>>>
>>> And is it correct method of adding ABI? You add incorrect properties
>>> without documentation and then use this as an argument "driver already
>>> does it"?
>>
>> I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
>> based on the driver already using it. I did not mean it in that sense. I simply
>> meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
>> context, I felt that if it was a suggestion, I would prefer retaining the names
>> with the "phy" suffixes, since the driver is already using it. Additionally, I
>> also mentioned in my earlier comment that if it is necessary to drop the "phy"
>> suffix, then I will do so and add another patch to change the string the driver
>> looks for as well.
>>
>> I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
>> will post the v3 series making this change, along with the patch to update the
>> string the driver looks for.
>
> Drop the "phy" suffix.
>
> It's a new binding. "phy" as suffix for "phy" is useless and for new
> bindings it should be dropped. If you submitted driver changes without
> bindings, which document the ABI, it's not good, but also not a reason
> for me for exceptions.
Thank you for clarifying. I will post the v3 series dropping the "phy" suffix
and also include the patch to change the name used in the driver to refer to the
Serdes PHY.
Regards,
Siddharth.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY
@ 2023-03-09 7:24 ` Siddharth Vadapalli
0 siblings, 0 replies; 14+ messages in thread
From: Siddharth Vadapalli @ 2023-03-09 7:24 UTC (permalink / raw
To: Krzysztof Kozlowski, krzysztof.kozlowski+dt
Cc: davem, edumazet, kuba, linux, pabeni, robh+dt, nsekhar, rogerq,
netdev, devicetree, linux-kernel, linux-arm-kernel, srk,
s-vadapalli
Hello Krzysztof,
On 09/03/23 11:51, Krzysztof Kozlowski wrote:
> On 09/03/2023 05:18, Siddharth Vadapalli wrote:
>> Hello Krzysztof,
>>
>> On 08/03/23 18:04, Krzysztof Kozlowski wrote:
>>> On 08/03/2023 09:38, Siddharth Vadapalli wrote:
>>>> Hello Krzysztof,
>>>>
>>>> On 08/03/23 14:04, Krzysztof Kozlowski wrote:
>>>>> On 08/03/2023 06:18, Siddharth Vadapalli wrote:
>>>>>> Update bindings to include Serdes PHY as an optional PHY, in addition to
>>>>>> the existing CPSW MAC's PHY. The CPSW MAC's PHY is required while the
>>>>>> Serdes PHY is optional. The Serdes PHY handle has to be provided only
>>>>>> when the Serdes is being configured in a Single-Link protocol. Using the
>>>>>> name "serdes-phy" to represent the Serdes PHY handle, the am65-cpsw-nuss
>>>>>> driver can obtain the Serdes PHY and request the Serdes to be
>>>>>> configured.
>>>>>>
>>>>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>>>>>> ---
>>>>>>
>>>>>> Hello,
>>>>>>
>>>>>> This patch corresponds to the Serdes PHY bindings that were missed out in
>>>>>> the series at:
>>>>>> https://lore.kernel.org/r/20230104103432.1126403-1-s-vadapalli@ti.com/
>>>>>> This was pointed out at:
>>>>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@mail.gmail.com/
>>>>>>
>>>>>> Changes from v1:
>>>>>> 1. Describe phys property with minItems, items and description.
>>>>>> 2. Use minItems and items in phy-names.
>>>>>> 3. Remove the description in phy-names.
>>>>>>
>>>>>> v1:
>>>>>> https://lore.kernel.org/r/20230306094750.159657-1-s-vadapalli@ti.com/
>>>>>>
>>>>>> .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 14 ++++++++++++--
>>>>>> 1 file changed, 12 insertions(+), 2 deletions(-)
>>>>>>
>>>>>> diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> index 900063411a20..0fb48bb6a041 100644
>>>>>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>>>>>> @@ -126,8 +126,18 @@ properties:
>>>>>> description: CPSW port number
>>>>>>
>>>>>> phys:
>>>>>> - maxItems: 1
>>>>>> - description: phandle on phy-gmii-sel PHY
>>>>>> + minItems: 1
>>>>>> + items:
>>>>>> + - description: CPSW MAC's PHY.
>>>>>> + - description: Serdes PHY. Serdes PHY is required only if
>>>>>> + the Serdes has to be configured in the
>>>>>> + Single-Link configuration.
>>>>>> +
>>>>>> + phy-names:
>>>>>> + minItems: 1
>>>>>> + items:
>>>>>> + - const: mac-phy
>>>>>> + - const: serdes-phy
>>>>>
>>>>> Drop "phy" suffixes.
>>>>
>>>> The am65-cpsw driver fetches the Serdes PHY by looking for the string
>>>> "serdes-phy". Therefore, modifying the string will require changing the driver's
>>>> code as well. Please let me know if it is absolutely necessary to drop the phy
>>>> suffix. If so, I will post a new series with the changes involving dt-bindings
>>>> changes and the driver changes.
>>>
>>> Why the driver uses some properties before adding them to the binding?
>>
>> I missed adding the bindings for the Serdes PHY as a part of the series
>> mentioned in the section below the tearline of the patch. With this patch, I am
>> attempting to fix it.
>>
>>>
>>> And is it correct method of adding ABI? You add incorrect properties
>>> without documentation and then use this as an argument "driver already
>>> does it"?
>>
>> I apologize if my earlier comment appeared to justify the usage of "serdes-phy"
>> based on the driver already using it. I did not mean it in that sense. I simply
>> meant to ask if dropping "phy" suffixes was a suggestion or a rule. In that
>> context, I felt that if it was a suggestion, I would prefer retaining the names
>> with the "phy" suffixes, since the driver is already using it. Additionally, I
>> also mentioned in my earlier comment that if it is necessary to drop the "phy"
>> suffix, then I will do so and add another patch to change the string the driver
>> looks for as well.
>>
>> I shall take it that dropping "phy" suffixes is a rule/necessity. With this, I
>> will post the v3 series making this change, along with the patch to update the
>> string the driver looks for.
>
> Drop the "phy" suffix.
>
> It's a new binding. "phy" as suffix for "phy" is useless and for new
> bindings it should be dropped. If you submitted driver changes without
> bindings, which document the ABI, it's not good, but also not a reason
> for me for exceptions.
Thank you for clarifying. I will post the v3 series dropping the "phy" suffix
and also include the patch to change the name used in the driver to refer to the
Serdes PHY.
Regards,
Siddharth.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2023-03-09 7:25 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-03-08 5:18 [PATCH net-next v2] dt-bindings: net: ti: k3-am654-cpsw-nuss: Document Serdes PHY Siddharth Vadapalli
2023-03-08 5:18 ` Siddharth Vadapalli
2023-03-08 8:34 ` Krzysztof Kozlowski
2023-03-08 8:34 ` Krzysztof Kozlowski
2023-03-08 8:38 ` Siddharth Vadapalli
2023-03-08 8:38 ` Siddharth Vadapalli
2023-03-08 12:34 ` Krzysztof Kozlowski
2023-03-08 12:34 ` Krzysztof Kozlowski
2023-03-09 4:18 ` Siddharth Vadapalli
2023-03-09 4:18 ` Siddharth Vadapalli
2023-03-09 6:21 ` Krzysztof Kozlowski
2023-03-09 6:21 ` Krzysztof Kozlowski
2023-03-09 7:24 ` Siddharth Vadapalli
2023-03-09 7:24 ` Siddharth Vadapalli
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.