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