TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
that are not supported on earlier SoCs. Add a compatible for it.
Signed-off-by: Siddharth Vadapalli <[email protected]>
---
.../mfd/ti,j721e-system-controller.yaml | 6 ++++
.../bindings/phy/ti,phy-gmii-sel.yaml | 30 ++++++++++++++++++-
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
index 1aeac43cad92..802374e7645f 100644
--- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
+++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
@@ -54,6 +54,12 @@ patternProperties:
description:
Clock provider for TI EHRPWM nodes.
+ "phy@[0-9a-f]+$":
+ type: object
+ $ref: /schemas/phy/phy-provider.yaml
+ description:
+ This is the register to set phy mode through phy-gmii-sel driver.
+
required:
- compatible
- reg
diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
index ff8a6d9eb153..0ffb97f1a77c 100644
--- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
+++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
@@ -53,12 +53,24 @@ properties:
- ti,am43xx-phy-gmii-sel
- ti,dm814-phy-gmii-sel
- ti,am654-phy-gmii-sel
+ - ti,j7200-cpsw5g-phy-gmii-sel
reg:
maxItems: 1
'#phy-cells': true
+ ti,qsgmii-main-ports:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ Required only for QSGMII mode. Array to select the port for
+ QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
+ ports automatically. Any one of the 4 CPSW5G ports can act as the
+ main port with the rest of them being the QSGMII_SUB ports.
+ items:
+ minimum: 1
+ maximum: 4
+
allOf:
- if:
properties:
@@ -73,6 +85,22 @@ allOf:
'#phy-cells':
const: 1
description: CPSW port number (starting from 1)
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - ti,j7200-cpsw5g-phy-gmii-sel
+ then:
+ properties:
+ '#phy-cells':
+ const: 1
+ description: CPSW port number (starting from 1)
+ ti,qsgmii-main-ports:
+ maxItems: 1
+ else:
+ properties:
+ ti,qsgmii-main-ports: false
- if:
properties:
compatible:
@@ -97,7 +125,7 @@ additionalProperties: false
examples:
- |
- phy_gmii_sel: phy-gmii-sel@650 {
+ phy_gmii_sel: phy@650 {
compatible = "ti,am3352-phy-gmii-sel";
reg = <0x650 0x4>;
#phy-cells = <2>;
--
2.25.1
On 01/09/2022 11:55, Siddharth Vadapalli wrote:
> TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
> that are not supported on earlier SoCs. Add a compatible for it.
>
> Signed-off-by: Siddharth Vadapalli <[email protected]>
> ---
> .../mfd/ti,j721e-system-controller.yaml | 6 ++++
> .../bindings/phy/ti,phy-gmii-sel.yaml | 30 ++++++++++++++++++-
> 2 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
> index 1aeac43cad92..802374e7645f 100644
> --- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
> @@ -54,6 +54,12 @@ patternProperties:
> description:
> Clock provider for TI EHRPWM nodes.
>
> + "phy@[0-9a-f]+$":
> + type: object
> + $ref: /schemas/phy/phy-provider.yaml
You need instead ref to specific device bindings/schema. Probably to
/schemas/phy/ti,phy-gmii-sel.yaml#
This was entirely different in v3, so your change is very confusing.
> + description:
> + This is the register to set phy mode through phy-gmii-sel driver.
I don't understand the description. Please focus on the hardware not
some drivers - what is here? Phy for something?
> +
> required:
> - compatible
> - reg
> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
> index ff8a6d9eb153..0ffb97f1a77c 100644
> --- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
> +++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
> @@ -53,12 +53,24 @@ properties:
> - ti,am43xx-phy-gmii-sel
> - ti,dm814-phy-gmii-sel
> - ti,am654-phy-gmii-sel
> + - ti,j7200-cpsw5g-phy-gmii-sel
>
> reg:
> maxItems: 1
>
> '#phy-cells': true
>
> + ti,qsgmii-main-ports:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + Required only for QSGMII mode. Array to select the port for
> + QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
> + ports automatically. Any one of the 4 CPSW5G ports can act as the
> + main port with the rest of them being the QSGMII_SUB ports.
> + items:
> + minimum: 1
> + maximum: 4
> +
> allOf:
> - if:
> properties:
> @@ -73,6 +85,22 @@ allOf:
> '#phy-cells':
> const: 1
> description: CPSW port number (starting from 1)
Blank line
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - ti,j7200-cpsw5g-phy-gmii-sel
> + then:
> + properties:
> + '#phy-cells':
> + const: 1
> + description: CPSW port number (starting from 1)
> + ti,qsgmii-main-ports:
> + maxItems: 1
It does not really make sense to limit items here, in the context of
this patch. You got a comment for it already. Your patch should make
sense on its own.
> + else:
> + properties:
> + ti,qsgmii-main-ports: false
Blank line
> - if:
> properties:
> compatible:
> @@ -97,7 +125,7 @@ additionalProperties: false
>
> examples:
> - |
> - phy_gmii_sel: phy-gmii-sel@650 {
> + phy_gmii_sel: phy@650 {
Split cleanup into separate patch.
> compatible = "ti,am3352-phy-gmii-sel";
> reg = <0x650 0x4>;
> #phy-cells = <2>;
Best regards,
Krzysztof
Hello Krzysztof,
On 01/09/22 20:51, Krzysztof Kozlowski wrote:
> On 01/09/2022 11:55, Siddharth Vadapalli wrote:
>> TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
>> that are not supported on earlier SoCs. Add a compatible for it.
>>
>> Signed-off-by: Siddharth Vadapalli <[email protected]>
>> ---
>> .../mfd/ti,j721e-system-controller.yaml | 6 ++++
>> .../bindings/phy/ti,phy-gmii-sel.yaml | 30 ++++++++++++++++++-
>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> index 1aeac43cad92..802374e7645f 100644
>> --- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>> @@ -54,6 +54,12 @@ patternProperties:
>> description:
>> Clock provider for TI EHRPWM nodes.
>>
>> + "phy@[0-9a-f]+$":
>> + type: object
>> + $ref: /schemas/phy/phy-provider.yaml
>
> You need instead ref to specific device bindings/schema. Probably to
> /schemas/phy/ti,phy-gmii-sel.yaml#
Thank you for the clarification. I will update $ref to
"/schemas/phy/ti,phy-gmii-sel.yaml#" in the v5 series.
>
> This was entirely different in v3, so your change is very confusing.
I had misunderstood Rob's comment in the v3 patch. I had initially
provided the relative path to the bindings file ti,phy-gmii-sel.yaml in
the v3 patch. When Rob commented "/schemas/phy/..", I misunderstood and
thought that I had to point $ref to a generic phy-provider schema
present in the dt-schema repo and thus, in this v4 patch, I had updated
$ref accordingly.
>
>> + description:
>> + This is the register to set phy mode through phy-gmii-sel driver.
>
> I don't understand the description. Please focus on the hardware not
> some drivers - what is here? Phy for something?
I will fix the description, updating it to the following:
"Address of the CTRLMMR_ENETx_CTRL registers which are used to configure
the phy-mode of the CPSW MAC ports."
Please let me know if the above description is fine.
>
>> +
>> required:
>> - compatible
>> - reg
>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> index ff8a6d9eb153..0ffb97f1a77c 100644
>> --- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>> @@ -53,12 +53,24 @@ properties:
>> - ti,am43xx-phy-gmii-sel
>> - ti,dm814-phy-gmii-sel
>> - ti,am654-phy-gmii-sel
>> + - ti,j7200-cpsw5g-phy-gmii-sel
>>
>> reg:
>> maxItems: 1
>>
>> '#phy-cells': true
>>
>> + ti,qsgmii-main-ports:
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + description: |
>> + Required only for QSGMII mode. Array to select the port for
>> + QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
>> + ports automatically. Any one of the 4 CPSW5G ports can act as the
>> + main port with the rest of them being the QSGMII_SUB ports.
>> + items:
>> + minimum: 1
>> + maximum: 4
>> +
>> allOf:
>> - if:
>> properties:
>> @@ -73,6 +85,22 @@ allOf:
>> '#phy-cells':
>> const: 1
>> description: CPSW port number (starting from 1)
>
> Blank line
I will fix this in the v5 series.
>
>
>> + - if:
>> + properties:
>> + compatible:
>> + contains:
>> + enum:
>> + - ti,j7200-cpsw5g-phy-gmii-sel
>> + then:
>> + properties:
>> + '#phy-cells':
>> + const: 1
>> + description: CPSW port number (starting from 1)
>> + ti,qsgmii-main-ports:
>> + maxItems: 1
>
> It does not really make sense to limit items here, in the context of
> this patch. You got a comment for it already. Your patch should make
> sense on its own.
I had defined the property as an array because there are more than one
QSGMII main ports for other devices for which I will be posting the
patches. I had planned to reuse this property, with "maxItems: 2" in the
future patches for other compatibles. However, as suggested by you, I
will change the property to a uint32 instead of uint32-array in this
series. Later, in my future patches for other devices, I will change it
back to a uint32-array when I reuse the property.
>
>> + else:
>> + properties:
>> + ti,qsgmii-main-ports: false
>
> Blank line
I will fix this in the v5 series.
>
>> - if:
>> properties:
>> compatible:
>> @@ -97,7 +125,7 @@ additionalProperties: false
>>
>> examples:
>> - |
>> - phy_gmii_sel: phy-gmii-sel@650 {
>> + phy_gmii_sel: phy@650 {
>
> Split cleanup into separate patch.
I will do so. Thank you for reviewing the patch.
Regards,
Siddharth.
On 02/09/2022 08:09, Siddharth Vadapalli wrote:
> Hello Krzysztof,
>
> On 01/09/22 20:51, Krzysztof Kozlowski wrote:
>> On 01/09/2022 11:55, Siddharth Vadapalli wrote:
>>> TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
>>> that are not supported on earlier SoCs. Add a compatible for it.
>>>
>>> Signed-off-by: Siddharth Vadapalli <[email protected]>
>>> ---
>>> .../mfd/ti,j721e-system-controller.yaml | 6 ++++
>>> .../bindings/phy/ti,phy-gmii-sel.yaml | 30 ++++++++++++++++++-
>>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>> index 1aeac43cad92..802374e7645f 100644
>>> --- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>> @@ -54,6 +54,12 @@ patternProperties:
>>> description:
>>> Clock provider for TI EHRPWM nodes.
>>>
>>> + "phy@[0-9a-f]+$":
>>> + type: object
>>> + $ref: /schemas/phy/phy-provider.yaml
>>
>> You need instead ref to specific device bindings/schema. Probably to
>> /schemas/phy/ti,phy-gmii-sel.yaml#
>
> Thank you for the clarification. I will update $ref to
> "/schemas/phy/ti,phy-gmii-sel.yaml#" in the v5 series.
>
>>
>> This was entirely different in v3, so your change is very confusing.
>
> I had misunderstood Rob's comment in the v3 patch. I had initially
> provided the relative path to the bindings file ti,phy-gmii-sel.yaml in
> the v3 patch. When Rob commented "/schemas/phy/..", I misunderstood and
> thought that I had to point $ref to a generic phy-provider schema
> present in the dt-schema repo and thus, in this v4 patch, I had updated
> $ref accordingly.
>
>>
>>> + description:
>>> + This is the register to set phy mode through phy-gmii-sel driver.
>>
>> I don't understand the description. Please focus on the hardware not
>> some drivers - what is here? Phy for something?
>
> I will fix the description, updating it to the following:
> "Address of the CTRLMMR_ENETx_CTRL registers which are used to configure
> the phy-mode of the CPSW MAC ports."
>
> Please let me know if the above description is fine.
Hm, but that's a phy node, not address of register... Isn't this a phy
node representing the phy of the CPSW MAC ports?
>
>>
>>> +
>>> required:
>>> - compatible
>>> - reg
>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>> index ff8a6d9eb153..0ffb97f1a77c 100644
>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>> @@ -53,12 +53,24 @@ properties:
>>> - ti,am43xx-phy-gmii-sel
>>> - ti,dm814-phy-gmii-sel
>>> - ti,am654-phy-gmii-sel
>>> + - ti,j7200-cpsw5g-phy-gmii-sel
>>>
>>> reg:
>>> maxItems: 1
>>>
>>> '#phy-cells': true
>>>
>>> + ti,qsgmii-main-ports:
>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>> + description: |
>>> + Required only for QSGMII mode. Array to select the port for
>>> + QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
>>> + ports automatically. Any one of the 4 CPSW5G ports can act as the
>>> + main port with the rest of them being the QSGMII_SUB ports.
>>> + items:
>>> + minimum: 1
>>> + maximum: 4
>>> +
>>> allOf:
>>> - if:
>>> properties:
>>> @@ -73,6 +85,22 @@ allOf:
>>> '#phy-cells':
>>> const: 1
>>> description: CPSW port number (starting from 1)
>>
>> Blank line
>
> I will fix this in the v5 series.
>
>>
>>
>>> + - if:
>>> + properties:
>>> + compatible:
>>> + contains:
>>> + enum:
>>> + - ti,j7200-cpsw5g-phy-gmii-sel
>>> + then:
>>> + properties:
>>> + '#phy-cells':
>>> + const: 1
>>> + description: CPSW port number (starting from 1)
>>> + ti,qsgmii-main-ports:
>>> + maxItems: 1
>>
>> It does not really make sense to limit items here, in the context of
>> this patch. You got a comment for it already. Your patch should make
>> sense on its own.
>
> I had defined the property as an array because there are more than one
> QSGMII main ports for other devices for which I will be posting the
> patches. I had planned to reuse this property, with "maxItems: 2" in the
> future patches for other compatibles. However, as suggested by you, I
> will change the property to a uint32 instead of uint32-array in this
> series. Later, in my future patches for other devices, I will change it
> back to a uint32-array when I reuse the property.
Wait, no. You should not change the property. This should be
uint32-array, because you will extend it soon, just maxItems must be
defined in top-level place.
Best regards,
Krzysztof
Hello Krzysztof,
On 05/09/22 18:39, Krzysztof Kozlowski wrote:
> On 02/09/2022 08:09, Siddharth Vadapalli wrote:
>> Hello Krzysztof,
>>
>> On 01/09/22 20:51, Krzysztof Kozlowski wrote:
>>> On 01/09/2022 11:55, Siddharth Vadapalli wrote:
>>>> TI's J7200 SoC supports additional PHY modes like QSGMII and SGMII
>>>> that are not supported on earlier SoCs. Add a compatible for it.
>>>>
>>>> Signed-off-by: Siddharth Vadapalli <[email protected]>
>>>> ---
>>>> .../mfd/ti,j721e-system-controller.yaml | 6 ++++
>>>> .../bindings/phy/ti,phy-gmii-sel.yaml | 30 ++++++++++++++++++-
>>>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>>> index 1aeac43cad92..802374e7645f 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>>> +++ b/Documentation/devicetree/bindings/mfd/ti,j721e-system-controller.yaml
>>>> @@ -54,6 +54,12 @@ patternProperties:
>>>> description:
>>>> Clock provider for TI EHRPWM nodes.
>>>>
>>>> + "phy@[0-9a-f]+$":
>>>> + type: object
>>>> + $ref: /schemas/phy/phy-provider.yaml
>>>
>>> You need instead ref to specific device bindings/schema. Probably to
>>> /schemas/phy/ti,phy-gmii-sel.yaml#
>>
>> Thank you for the clarification. I will update $ref to
>> "/schemas/phy/ti,phy-gmii-sel.yaml#" in the v5 series.
>>
>>>
>>> This was entirely different in v3, so your change is very confusing.
>>
>> I had misunderstood Rob's comment in the v3 patch. I had initially
>> provided the relative path to the bindings file ti,phy-gmii-sel.yaml in
>> the v3 patch. When Rob commented "/schemas/phy/..", I misunderstood and
>> thought that I had to point $ref to a generic phy-provider schema
>> present in the dt-schema repo and thus, in this v4 patch, I had updated
>> $ref accordingly.
>>
>>>
>>>> + description:
>>>> + This is the register to set phy mode through phy-gmii-sel driver.
>>>
>>> I don't understand the description. Please focus on the hardware not
>>> some drivers - what is here? Phy for something?
>>
>> I will fix the description, updating it to the following:
>> "Address of the CTRLMMR_ENETx_CTRL registers which are used to configure
>> the phy-mode of the CPSW MAC ports."
>>
>> Please let me know if the above description is fine.
>
> Hm, but that's a phy node, not address of register... Isn't this a phy
> node representing the phy of the CPSW MAC ports?
Despite it being a phy node, the phy-gmii-sel driver actually uses this
node to obtain the address of the CTRLMMR_ENETx_CTRL registers which
correspond to the CPSW MAC configuration and are therefore unrelated to
the PHY. Please let me know if my suggested description would be fine.
>
>>
>>>
>>>> +
>>>> required:
>>>> - compatible
>>>> - reg
>>>> diff --git a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>>> index ff8a6d9eb153..0ffb97f1a77c 100644
>>>> --- a/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>>> +++ b/Documentation/devicetree/bindings/phy/ti,phy-gmii-sel.yaml
>>>> @@ -53,12 +53,24 @@ properties:
>>>> - ti,am43xx-phy-gmii-sel
>>>> - ti,dm814-phy-gmii-sel
>>>> - ti,am654-phy-gmii-sel
>>>> + - ti,j7200-cpsw5g-phy-gmii-sel
>>>>
>>>> reg:
>>>> maxItems: 1
>>>>
>>>> '#phy-cells': true
>>>>
>>>> + ti,qsgmii-main-ports:
>>>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>>>> + description: |
>>>> + Required only for QSGMII mode. Array to select the port for
>>>> + QSGMII main mode. Rest of the ports are selected as QSGMII_SUB
>>>> + ports automatically. Any one of the 4 CPSW5G ports can act as the
>>>> + main port with the rest of them being the QSGMII_SUB ports.
>>>> + items:
>>>> + minimum: 1
>>>> + maximum: 4
>>>> +
>>>> allOf:
>>>> - if:
>>>> properties:
>>>> @@ -73,6 +85,22 @@ allOf:
>>>> '#phy-cells':
>>>> const: 1
>>>> description: CPSW port number (starting from 1)
>>>
>>> Blank line
>>
>> I will fix this in the v5 series.
>>
>>>
>>>
>>>> + - if:
>>>> + properties:
>>>> + compatible:
>>>> + contains:
>>>> + enum:
>>>> + - ti,j7200-cpsw5g-phy-gmii-sel
>>>> + then:
>>>> + properties:
>>>> + '#phy-cells':
>>>> + const: 1
>>>> + description: CPSW port number (starting from 1)
>>>> + ti,qsgmii-main-ports:
>>>> + maxItems: 1
>>>
>>> It does not really make sense to limit items here, in the context of
>>> this patch. You got a comment for it already. Your patch should make
>>> sense on its own.
>>
>> I had defined the property as an array because there are more than one
>> QSGMII main ports for other devices for which I will be posting the
>> patches. I had planned to reuse this property, with "maxItems: 2" in the
>> future patches for other compatibles. However, as suggested by you, I
>> will change the property to a uint32 instead of uint32-array in this
>> series. Later, in my future patches for other devices, I will change it
>> back to a uint32-array when I reuse the property.
>
> Wait, no. You should not change the property. This should be
> uint32-array, because you will extend it soon, just maxItems must be
> defined in top-level place.
Thank you for clarifying. I will move "maxItems: 1" to the top where
"ti,qsgmii-main-ports" property is first defined, and continue using the
property as a uint32-array.
Regards,
Siddharth.
On 06/09/2022 07:02, Siddharth Vadapalli wrote:
>>>
>>> Please let me know if the above description is fine.
>>
>> Hm, but that's a phy node, not address of register... Isn't this a phy
>> node representing the phy of the CPSW MAC ports?
>
> Despite it being a phy node, the phy-gmii-sel driver actually uses this
> node to obtain the address of the CTRLMMR_ENETx_CTRL registers which
> correspond to the CPSW MAC configuration and are therefore unrelated to
> the PHY. Please let me know if my suggested description would be fine.
Either I miss some more pieces or this is wrong design. The phy node
should not be used to pass some addresses somewhere. It is used to
define a device which will be instantiated (as parent is simple-mfd). If
you use it only to obtain some address, not to describe child device,
then this is wrong property type.
Best regards,
Krzysztof
Hello Krzysztof,
On 06/09/22 12:33, Krzysztof Kozlowski wrote:
> On 06/09/2022 07:02, Siddharth Vadapalli wrote:
>>>>
>>>> Please let me know if the above description is fine.
>>>
>>> Hm, but that's a phy node, not address of register... Isn't this a phy
>>> node representing the phy of the CPSW MAC ports?
>>
>> Despite it being a phy node, the phy-gmii-sel driver actually uses this
>> node to obtain the address of the CTRLMMR_ENETx_CTRL registers which
>> correspond to the CPSW MAC configuration and are therefore unrelated to
>> the PHY. Please let me know if my suggested description would be fine.
>
> Either I miss some more pieces or this is wrong design. The phy node
> should not be used to pass some addresses somewhere. It is used to
> define a device which will be instantiated (as parent is simple-mfd). If
> you use it only to obtain some address, not to describe child device,
> then this is wrong property type.
Sorry for describing it incompletely, and at some places incorrectly.
Yes, you were right when you initially mentioned that the phy node
corresponds to the phy of the ethernet MAC ports. I had incorrectly
understood the term "phy" there as the external Layer-1 ethernet phy.The
phy node corresponds to the phy used by the ethernet MAC, based on the
phy-mode configured. The am65-cpsw-nuss driver which is responsible for
ethernet MAC, requires the ethernet MAC's phy to be configured by the
phy-gmii-sel driver. Thus, the phy node corresponds to an actual phy
(ethernet MAC's PHY).
I plan on updating the description for the phy pattern property to the
following:
"The phy node corresponding to the ethernet MAC."
Regards,
Siddharth.