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@xxxxxx> >>>> --- >>>> >>>> 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@xxxxxx/ >>>> This was pointed out at: >>>> https://lore.kernel.org/r/CAMuHMdW5atq-FuLEL3htuE3t2uO86anLL3zeY7n1RqqMP_rH1g@xxxxxxxxxxxxxx/ >>>> >>>> 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@xxxxxx/ >>>> >>>> .../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.
