[PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Siddharth Vadapalli]

Add compatible for J721e CPSW9G.

Add support to power on and configure SERDES PHY.

Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.

Siddharth Vadapalli (8):
  dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e
    CPSW9G
  net: ethernet: ti: am65-cpsw: Add support for SERDES configuration
  net: ethernet: ti: am65-cpsw: Add mac control function
  net: ethernet: ti: am65-cpsw: Add mac enable link function
  net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration
  net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200
    CPSW5G
  net: ethernet: ti: am65-cpsw: Add support for J721e CPSW9G
  net: ethernet: ti: am65-cpsw: Enable SGMII mode for J721e CPSW9G

 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |  23 ++-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      | 186 +++++++++++++++---
 2 files changed, 178 insertions(+), 31 deletions(-)

-- 
2.25.1

[PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Siddharth Vadapalli]

Add compatible for J721e CPSW9G.

Add support to power on and configure SERDES PHY.

Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.

Siddharth Vadapalli (8):
  dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e
    CPSW9G
  net: ethernet: ti: am65-cpsw: Add support for SERDES configuration
  net: ethernet: ti: am65-cpsw: Add mac control function
  net: ethernet: ti: am65-cpsw: Add mac enable link function
  net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration
  net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200
    CPSW5G
  net: ethernet: ti: am65-cpsw: Add support for J721e CPSW9G
  net: ethernet: ti: am65-cpsw: Enable SGMII mode for J721e CPSW9G

 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   |  23 ++-
 drivers/net/ethernet/ti/am65-cpsw-nuss.c      | 186 +++++++++++++++---
 2 files changed, 178 insertions(+), 31 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
ports) CPSW9G module and add compatible for it.

Changes made:
    - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
    - Extend pattern properties for new compatible.
    - Change maximum number of CPSW ports to 8 for new compatible.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
 1 file changed, 21 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 821974815dec..868b7fb58b06 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -57,6 +57,7 @@ properties:
       - ti,am654-cpsw-nuss
       - ti,j7200-cpswxg-nuss
       - ti,j721e-cpsw-nuss
+      - ti,j721e-cpswxg-nuss
       - ti,am642-cpsw-nuss
 
   reg:
@@ -111,7 +112,7 @@ properties:
         const: 0
 
     patternProperties:
-      "^port@[1-4]$":
+      "^port@[1-8]$":
         type: object
         description: CPSWxG NUSS external ports
 
@@ -121,7 +122,7 @@ properties:
         properties:
           reg:
             minimum: 1
-            maximum: 4
+            maximum: 8
             description: CPSW port number
 
           phys:
@@ -181,6 +182,21 @@ required:
   - '#size-cells'
 
 allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: ti,j721e-cpswxg-nuss
+    then:
+      properties:
+        ethernet-ports:
+          patternProperties:
+            "^port@[5-8]$": false
+            properties:
+              reg:
+                maximum: 4
+
   - if:
       not:
         properties:
@@ -192,6 +208,9 @@ allOf:
         ethernet-ports:
           patternProperties:
             "^port@[3-4]$": false
+            properties:
+              reg:
+                maximum: 2
 
 additionalProperties: false
 
-- 
2.25.1

[PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
ports) CPSW9G module and add compatible for it.

Changes made:
    - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
    - Extend pattern properties for new compatible.
    - Change maximum number of CPSW ports to 8 for new compatible.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
 1 file changed, 21 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 821974815dec..868b7fb58b06 100644
--- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
+++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
@@ -57,6 +57,7 @@ properties:
       - ti,am654-cpsw-nuss
       - ti,j7200-cpswxg-nuss
       - ti,j721e-cpsw-nuss
+      - ti,j721e-cpswxg-nuss
       - ti,am642-cpsw-nuss
 
   reg:
@@ -111,7 +112,7 @@ properties:
         const: 0
 
     patternProperties:
-      "^port@[1-4]$":
+      "^port@[1-8]$":
         type: object
         description: CPSWxG NUSS external ports
 
@@ -121,7 +122,7 @@ properties:
         properties:
           reg:
             minimum: 1
-            maximum: 4
+            maximum: 8
             description: CPSW port number
 
           phys:
@@ -181,6 +182,21 @@ required:
   - '#size-cells'
 
 allOf:
+  - if:
+      not:
+        properties:
+          compatible:
+            contains:
+              const: ti,j721e-cpswxg-nuss
+    then:
+      properties:
+        ethernet-ports:
+          patternProperties:
+            "^port@[5-8]$": false
+            properties:
+              reg:
+                maximum: 4
+
   - if:
       not:
         properties:
@@ -192,6 +208,9 @@ allOf:
         ethernet-ports:
           patternProperties:
             "^port@[3-4]$": false
+            properties:
+              reg:
+                maximum: 2
 
 additionalProperties: false
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Rob Herring]

On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
> ports) CPSW9G module and add compatible for it.
> 
> Changes made:
>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>     - Extend pattern properties for new compatible.
>     - Change maximum number of CPSW ports to 8 for new compatible.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>  1 file changed, 21 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 821974815dec..868b7fb58b06 100644
> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> @@ -57,6 +57,7 @@ properties:
>        - ti,am654-cpsw-nuss
>        - ti,j7200-cpswxg-nuss
>        - ti,j721e-cpsw-nuss
> +      - ti,j721e-cpswxg-nuss
>        - ti,am642-cpsw-nuss
>  
>    reg:
> @@ -111,7 +112,7 @@ properties:
>          const: 0
>  
>      patternProperties:
> -      "^port@[1-4]$":
> +      "^port@[1-8]$":
>          type: object
>          description: CPSWxG NUSS external ports
>  
> @@ -121,7 +122,7 @@ properties:
>          properties:
>            reg:
>              minimum: 1
> -            maximum: 4
> +            maximum: 8
>              description: CPSW port number
>  
>            phys:
> @@ -181,6 +182,21 @@ required:
>    - '#size-cells'
>  
>  allOf:
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: ti,j721e-cpswxg-nuss
> +    then:
> +      properties:
> +        ethernet-ports:
> +          patternProperties:
> +            "^port@[5-8]$": false
> +            properties:
> +              reg:
> +                maximum: 4

Your indentation is off. 'properties' here is under patternProperties 
making it a DT property.

> +
>    - if:
>        not:
>          properties:
> @@ -192,6 +208,9 @@ allOf:
>          ethernet-ports:
>            patternProperties:
>              "^port@[3-4]$": false
> +            properties:
> +              reg:
> +                maximum: 2

Same here.

>  
>  additionalProperties: false
>  
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Rob Herring]

On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
> ports) CPSW9G module and add compatible for it.
> 
> Changes made:
>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>     - Extend pattern properties for new compatible.
>     - Change maximum number of CPSW ports to 8 for new compatible.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>  1 file changed, 21 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 821974815dec..868b7fb58b06 100644
> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
> @@ -57,6 +57,7 @@ properties:
>        - ti,am654-cpsw-nuss
>        - ti,j7200-cpswxg-nuss
>        - ti,j721e-cpsw-nuss
> +      - ti,j721e-cpswxg-nuss
>        - ti,am642-cpsw-nuss
>  
>    reg:
> @@ -111,7 +112,7 @@ properties:
>          const: 0
>  
>      patternProperties:
> -      "^port@[1-4]$":
> +      "^port@[1-8]$":
>          type: object
>          description: CPSWxG NUSS external ports
>  
> @@ -121,7 +122,7 @@ properties:
>          properties:
>            reg:
>              minimum: 1
> -            maximum: 4
> +            maximum: 8
>              description: CPSW port number
>  
>            phys:
> @@ -181,6 +182,21 @@ required:
>    - '#size-cells'
>  
>  allOf:
> +  - if:
> +      not:
> +        properties:
> +          compatible:
> +            contains:
> +              const: ti,j721e-cpswxg-nuss
> +    then:
> +      properties:
> +        ethernet-ports:
> +          patternProperties:
> +            "^port@[5-8]$": false
> +            properties:
> +              reg:
> +                maximum: 4

Your indentation is off. 'properties' here is under patternProperties 
making it a DT property.

> +
>    - if:
>        not:
>          properties:
> @@ -192,6 +208,9 @@ allOf:
>          ethernet-ports:
>            patternProperties:
>              "^port@[3-4]$": false
> +            properties:
> +              reg:
> +                maximum: 2

Same here.

>  
>  additionalProperties: false
>  
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Hello Rob,

On 14/09/22 21:50, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>> ports) CPSW9G module and add compatible for it.
>>
>> Changes made:
>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>     - Extend pattern properties for new compatible.
>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>>  1 file changed, 21 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 821974815dec..868b7fb58b06 100644
>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> @@ -57,6 +57,7 @@ properties:
>>        - ti,am654-cpsw-nuss
>>        - ti,j7200-cpswxg-nuss
>>        - ti,j721e-cpsw-nuss
>> +      - ti,j721e-cpswxg-nuss
>>        - ti,am642-cpsw-nuss
>>  
>>    reg:
>> @@ -111,7 +112,7 @@ properties:
>>          const: 0
>>  
>>      patternProperties:
>> -      "^port@[1-4]$":
>> +      "^port@[1-8]$":
>>          type: object
>>          description: CPSWxG NUSS external ports
>>  
>> @@ -121,7 +122,7 @@ properties:
>>          properties:
>>            reg:
>>              minimum: 1
>> -            maximum: 4
>> +            maximum: 8
>>              description: CPSW port number
>>  
>>            phys:
>> @@ -181,6 +182,21 @@ required:
>>    - '#size-cells'
>>  
>>  allOf:
>> +  - if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              const: ti,j721e-cpswxg-nuss
>> +    then:
>> +      properties:
>> +        ethernet-ports:
>> +          patternProperties:
>> +            "^port@[5-8]$": false
>> +            properties:
>> +              reg:
>> +                maximum: 4
> 
> Your indentation is off. 'properties' here is under patternProperties 
> making it a DT property.
> 
>> +
>>    - if:
>>        not:
>>          properties:
>> @@ -192,6 +208,9 @@ allOf:
>>          ethernet-ports:
>>            patternProperties:
>>              "^port@[3-4]$": false
>> +            properties:
>> +              reg:
>> +                maximum: 2
> 
> Same here.

Thank you for reviewing the patch. Sorry for the indentation errors. I
will fix them in the v2 series.

Regards,
Siddharth.

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Hello Rob,

On 14/09/22 21:50, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>> ports) CPSW9G module and add compatible for it.
>>
>> Changes made:
>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>     - Extend pattern properties for new compatible.
>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>>  1 file changed, 21 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 821974815dec..868b7fb58b06 100644
>> --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml
>> @@ -57,6 +57,7 @@ properties:
>>        - ti,am654-cpsw-nuss
>>        - ti,j7200-cpswxg-nuss
>>        - ti,j721e-cpsw-nuss
>> +      - ti,j721e-cpswxg-nuss
>>        - ti,am642-cpsw-nuss
>>  
>>    reg:
>> @@ -111,7 +112,7 @@ properties:
>>          const: 0
>>  
>>      patternProperties:
>> -      "^port@[1-4]$":
>> +      "^port@[1-8]$":
>>          type: object
>>          description: CPSWxG NUSS external ports
>>  
>> @@ -121,7 +122,7 @@ properties:
>>          properties:
>>            reg:
>>              minimum: 1
>> -            maximum: 4
>> +            maximum: 8
>>              description: CPSW port number
>>  
>>            phys:
>> @@ -181,6 +182,21 @@ required:
>>    - '#size-cells'
>>  
>>  allOf:
>> +  - if:
>> +      not:
>> +        properties:
>> +          compatible:
>> +            contains:
>> +              const: ti,j721e-cpswxg-nuss
>> +    then:
>> +      properties:
>> +        ethernet-ports:
>> +          patternProperties:
>> +            "^port@[5-8]$": false
>> +            properties:
>> +              reg:
>> +                maximum: 4
> 
> Your indentation is off. 'properties' here is under patternProperties 
> making it a DT property.
> 
>> +
>>    - if:
>>        not:
>>          properties:
>> @@ -192,6 +208,9 @@ allOf:
>>          ethernet-ports:
>>            patternProperties:
>>              "^port@[3-4]$": false
>> +            properties:
>> +              reg:
>> +                maximum: 2
> 
> Same here.

Thank you for reviewing the patch. Sorry for the indentation errors. I
will fix them in the v2 series.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Rob Herring]

On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
> ports) CPSW9G module and add compatible for it.
> 
> Changes made:
>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>     - Extend pattern properties for new compatible.
>     - Change maximum number of CPSW ports to 8 for new compatible.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

What's the base for this patch? It didn't apply for me.

Run 'make dt_binding_check'. It should point out the issue I did. If 
not, let me know.

Rob

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Rob Herring]

On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
> ports) CPSW9G module and add compatible for it.
> 
> Changes made:
>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>     - Extend pattern properties for new compatible.
>     - Change maximum number of CPSW ports to 8 for new compatible.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>  1 file changed, 21 insertions(+), 2 deletions(-)

What's the base for this patch? It didn't apply for me.

Run 'make dt_binding_check'. It should point out the issue I did. If 
not, let me know.

Rob

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Hello Rob,

On 14/09/22 21:53, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>> ports) CPSW9G module and add compatible for it.
>>
>> Changes made:
>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>     - Extend pattern properties for new compatible.
>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> What's the base for this patch? It didn't apply for me.

I was working with linux-next tagged: next-20220913.

> 
> Run 'make dt_binding_check'. It should point out the issue I did. If 
> not, let me know.

Sure. Thank you.

Regards,
Siddharth.

Re: [PATCH 1/8] dt-bindings: net: ti: k3-am654-cpsw-nuss: Update bindings for J721e CPSW9G

[Siddharth Vadapalli]

Hello Rob,

On 14/09/22 21:53, Rob Herring wrote:
> On Wed, Sep 14, 2022 at 03:20:46PM +0530, Siddharth Vadapalli wrote:
>> Update bindings for TI K3 J721e SoC which contains 9 ports (8 external
>> ports) CPSW9G module and add compatible for it.
>>
>> Changes made:
>>     - Add new compatible ti,j721e-cpswxg-nuss for CPSW9G.
>>     - Extend pattern properties for new compatible.
>>     - Change maximum number of CPSW ports to 8 for new compatible.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  .../bindings/net/ti,k3-am654-cpsw-nuss.yaml   | 23 +++++++++++++++++--
>>  1 file changed, 21 insertions(+), 2 deletions(-)
> 
> What's the base for this patch? It didn't apply for me.

I was working with linux-next tagged: next-20220913.

> 
> Run 'make dt_binding_check'. It should point out the issue I did. If 
> not, let me know.

Sure. Thank you.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Siddharth Vadapalli]

Use PHY framework APIs to initialize the SERDES connected to CPSW.

Define the functions am65_cpsw_init_phy(), am65_cpsw_enable_phy() and
am65_cpsw_disable_phy() and invoke in am65_cpsw_nuss_init_slave_ports(),
am65_cpsw_mac_link_up() and am65_cpsw_mac_link_down() respectively.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 7ef5d8208a4e..4e06def3b0de 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1404,6 +1404,50 @@ static const struct net_device_ops am65_cpsw_nuss_netdev_ops = {
 	.ndo_get_devlink_port   = am65_cpsw_ndo_get_devlink_port,
 };
 
+static void am65_cpsw_disable_phy(struct phy *phy)
+{
+	phy_power_off(phy);
+	phy_exit(phy);
+}
+
+static int am65_cpsw_enable_phy(struct phy *phy)
+{
+	int ret;
+
+	ret = phy_init(phy);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_power_on(phy);
+	if (ret < 0) {
+		phy_exit(phy);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int am65_cpsw_init_phy(struct device *dev, struct device_node *port_np)
+{
+	const char *name = "serdes-phy";
+	struct phy *phy;
+	int ret;
+
+	phy = devm_of_phy_get(dev, port_np, name);
+	if (PTR_ERR(phy) == -ENODEV)
+		return 0;
+
+	ret =  am65_cpsw_enable_phy(phy);
+	if (ret < 0)
+		goto err_phy;
+
+	return 0;
+
+err_phy:
+	devm_phy_put(dev, phy);
+	return ret;
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
 	struct net_device *ndev = port->ndev;
 	int tmo;
 
+	/* disable phy */
+	am65_cpsw_disable_phy(port->slave.ifphy);
+
 	/* disable forwarding */
 	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_DISABLE);
 
@@ -1472,6 +1519,9 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 
 	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
 
+	/* enable phy */
+	am65_cpsw_enable_phy(port->slave.ifphy);
+
 	/* enable forwarding */
 	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
 
@@ -1881,6 +1931,11 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
 			goto of_node_put;
 		}
 
+		/* Initialize the phy for the port */
+		ret = am65_cpsw_init_phy(dev, port_np);
+		if (ret)
+			return ret;
+
 		port->slave.mac_only =
 				of_property_read_bool(port_np, "ti,mac-only");
 
-- 
2.25.1

[PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Siddharth Vadapalli]

Use PHY framework APIs to initialize the SERDES connected to CPSW.

Define the functions am65_cpsw_init_phy(), am65_cpsw_enable_phy() and
am65_cpsw_disable_phy() and invoke in am65_cpsw_nuss_init_slave_ports(),
am65_cpsw_mac_link_up() and am65_cpsw_mac_link_down() respectively.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 55 ++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 7ef5d8208a4e..4e06def3b0de 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1404,6 +1404,50 @@ static const struct net_device_ops am65_cpsw_nuss_netdev_ops = {
 	.ndo_get_devlink_port   = am65_cpsw_ndo_get_devlink_port,
 };
 
+static void am65_cpsw_disable_phy(struct phy *phy)
+{
+	phy_power_off(phy);
+	phy_exit(phy);
+}
+
+static int am65_cpsw_enable_phy(struct phy *phy)
+{
+	int ret;
+
+	ret = phy_init(phy);
+	if (ret < 0)
+		return ret;
+
+	ret = phy_power_on(phy);
+	if (ret < 0) {
+		phy_exit(phy);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int am65_cpsw_init_phy(struct device *dev, struct device_node *port_np)
+{
+	const char *name = "serdes-phy";
+	struct phy *phy;
+	int ret;
+
+	phy = devm_of_phy_get(dev, port_np, name);
+	if (PTR_ERR(phy) == -ENODEV)
+		return 0;
+
+	ret =  am65_cpsw_enable_phy(phy);
+	if (ret < 0)
+		goto err_phy;
+
+	return 0;
+
+err_phy:
+	devm_phy_put(dev, phy);
+	return ret;
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
 	struct net_device *ndev = port->ndev;
 	int tmo;
 
+	/* disable phy */
+	am65_cpsw_disable_phy(port->slave.ifphy);
+
 	/* disable forwarding */
 	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_DISABLE);
 
@@ -1472,6 +1519,9 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 
 	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
 
+	/* enable phy */
+	am65_cpsw_enable_phy(port->slave.ifphy);
+
 	/* enable forwarding */
 	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
 
@@ -1881,6 +1931,11 @@ static int am65_cpsw_nuss_init_slave_ports(struct am65_cpsw_common *common)
 			goto of_node_put;
 		}
 
+		/* Initialize the phy for the port */
+		ret = am65_cpsw_init_phy(dev, port_np);
+		if (ret)
+			return ret;
+
 		port->slave.mac_only =
 				of_property_read_bool(port_np, "ti,mac-only");
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:47PM +0530, Siddharth Vadapalli wrote:
> @@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
>  	struct net_device *ndev = port->ndev;
>  	int tmo;
>  
> +	/* disable phy */
> +	am65_cpsw_disable_phy(port->slave.ifphy);
> +

This seems really strange. If you have a serdes interface which
presumably supports SGMII, 1000base-X etc, then link status is sent
across the serdes interface. If you power down the serdes, then you
can't receive the link status, and so mac_link_up() won't be called.

Are you really sure you want to be enabling and disabling the PHY
in mac_link_down()/mac_link_up() ?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:47PM +0530, Siddharth Vadapalli wrote:
> @@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
>  	struct net_device *ndev = port->ndev;
>  	int tmo;
>  
> +	/* disable phy */
> +	am65_cpsw_disable_phy(port->slave.ifphy);
> +

This seems really strange. If you have a serdes interface which
presumably supports SGMII, 1000base-X etc, then link status is sent
across the serdes interface. If you power down the serdes, then you
can't receive the link status, and so mac_link_up() won't be called.

Are you really sure you want to be enabling and disabling the PHY
in mac_link_down()/mac_link_up() ?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:07, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:47PM +0530, Siddharth Vadapalli wrote:
>> @@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
>>  	struct net_device *ndev = port->ndev;
>>  	int tmo;
>>  
>> +	/* disable phy */
>> +	am65_cpsw_disable_phy(port->slave.ifphy);
>> +
> 
> This seems really strange. If you have a serdes interface which
> presumably supports SGMII, 1000base-X etc, then link status is sent
> across the serdes interface. If you power down the serdes, then you
> can't receive the link status, and so mac_link_up() won't be called.
> 
> Are you really sure you want to be enabling and disabling the PHY
> in mac_link_down()/mac_link_up() ?

Thank you for reviewing the patch. The PHY passed to the
"am65_cpsw_disable_phy()" and "am65_cpsw_disable_phy()" functions within
the "am65_cpsw_nuss_mac_link_down()" and "am65_cpsw_nuss_mac_link_up()"
functions respectively, is the CPSW ethernet MAC's PHY and not the
SERDES PHY. The SERDES PHY is powered on through the function call to
the "am65_cpsw_init_phy()" function.

The calls to the functions "am65_cpsw_enable_phy()" and
"am65_cpsw_disable_phy()" within the "am65_cpsw_nuss_mac_link_up()" and
"am65_cpsw_nuss_mac_link_down()" functions respectively, try to power on
and power off the CPSW ethernet MAC's phy. Looking at it again,they do
nothing, since the driver corresponding to the ethernet MAC's PHY which
happens to be drivers/phy/ti/phy-gmii-sel.c, does not provide any
methods to power on and power off the ethernet MAC's PHY. I have just
realized that this is stale code and will remove it in the v2 series.

Also, I realize now that I did not invoke "am65_cpsw_disable_phy()" on
the SERDES PHY in the driver's remove function. I will fix this in the
v2 series.

Regards,
Siddharth.

Re: [PATCH 2/8] net: ethernet: ti: am65-cpsw: Add support for SERDES configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:07, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:47PM +0530, Siddharth Vadapalli wrote:
>> @@ -1427,6 +1471,9 @@ static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned
>>  	struct net_device *ndev = port->ndev;
>>  	int tmo;
>>  
>> +	/* disable phy */
>> +	am65_cpsw_disable_phy(port->slave.ifphy);
>> +
> 
> This seems really strange. If you have a serdes interface which
> presumably supports SGMII, 1000base-X etc, then link status is sent
> across the serdes interface. If you power down the serdes, then you
> can't receive the link status, and so mac_link_up() won't be called.
> 
> Are you really sure you want to be enabling and disabling the PHY
> in mac_link_down()/mac_link_up() ?

Thank you for reviewing the patch. The PHY passed to the
"am65_cpsw_disable_phy()" and "am65_cpsw_disable_phy()" functions within
the "am65_cpsw_nuss_mac_link_down()" and "am65_cpsw_nuss_mac_link_up()"
functions respectively, is the CPSW ethernet MAC's PHY and not the
SERDES PHY. The SERDES PHY is powered on through the function call to
the "am65_cpsw_init_phy()" function.

The calls to the functions "am65_cpsw_enable_phy()" and
"am65_cpsw_disable_phy()" within the "am65_cpsw_nuss_mac_link_up()" and
"am65_cpsw_nuss_mac_link_down()" functions respectively, try to power on
and power off the CPSW ethernet MAC's phy. Looking at it again,they do
nothing, since the driver corresponding to the ethernet MAC's PHY which
happens to be drivers/phy/ti/phy-gmii-sel.c, does not provide any
methods to power on and power off the ethernet MAC's PHY. I have just
realized that this is stale code and will remove it in the v2 series.

Also, I realize now that I did not invoke "am65_cpsw_disable_phy()" on
the SERDES PHY in the driver's remove function. I will fix this in the
v2 series.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/8] net: ethernet: ti: am65-cpsw: Add mac control function

[Siddharth Vadapalli]

Add function am65_cpsw_nuss_mac_control() corresponding to the mac
control register writes that are performed in the
am65_cpsw_nuss_mac_link_up() function and use it in the
am65_cpsw_nuss_mac_link_up() function. The newly added function will be
used in am65_cpsw_nuss_mac_config() function in a future patch, thereby
making it necessary to define a new function for the redundant mac control
operations.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 4e06def3b0de..c7e6ad374e1a 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1448,6 +1448,31 @@ static int am65_cpsw_init_phy(struct device *dev, struct device_node *port_np)
 	return ret;
 }
 
+static void am65_cpsw_nuss_mac_control(struct am65_cpsw_port *port, phy_interface_t interface,
+				       int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+	u32 mac_control = CPSW_SL_CTL_GMII_EN;
+
+	if (speed == SPEED_1000)
+		mac_control |= CPSW_SL_CTL_GIG;
+	if (speed == SPEED_10 && interface == PHY_INTERFACE_MODE_RGMII)
+		/* Can be used with in band mode only */
+		mac_control |= CPSW_SL_CTL_EXT_EN;
+	if (speed == SPEED_100 && interface == PHY_INTERFACE_MODE_RMII)
+		mac_control |= CPSW_SL_CTL_IFCTL_A;
+	if (duplex)
+		mac_control |= CPSW_SL_CTL_FULLDUPLEX;
+
+	/* rx_pause/tx_pause */
+	if (rx_pause)
+		mac_control |= CPSW_SL_CTL_RX_FLOW_EN;
+
+	if (tx_pause)
+		mac_control |= CPSW_SL_CTL_TX_FLOW_EN;
+
+	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1497,27 +1522,9 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
 	struct am65_cpsw_common *common = port->common;
-	u32 mac_control = CPSW_SL_CTL_GMII_EN;
 	struct net_device *ndev = port->ndev;
 
-	if (speed == SPEED_1000)
-		mac_control |= CPSW_SL_CTL_GIG;
-	if (speed == SPEED_10 && interface == PHY_INTERFACE_MODE_RGMII)
-		/* Can be used with in band mode only */
-		mac_control |= CPSW_SL_CTL_EXT_EN;
-	if (speed == SPEED_100 && interface == PHY_INTERFACE_MODE_RMII)
-		mac_control |= CPSW_SL_CTL_IFCTL_A;
-	if (duplex)
-		mac_control |= CPSW_SL_CTL_FULLDUPLEX;
-
-	/* rx_pause/tx_pause */
-	if (rx_pause)
-		mac_control |= CPSW_SL_CTL_RX_FLOW_EN;
-
-	if (tx_pause)
-		mac_control |= CPSW_SL_CTL_TX_FLOW_EN;
-
-	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
+	am65_cpsw_nuss_mac_control(port, interface, speed, duplex, tx_pause, rx_pause);
 
 	/* enable phy */
 	am65_cpsw_enable_phy(port->slave.ifphy);
-- 
2.25.1

[PATCH 3/8] net: ethernet: ti: am65-cpsw: Add mac control function

[Siddharth Vadapalli]

Add function am65_cpsw_nuss_mac_control() corresponding to the mac
control register writes that are performed in the
am65_cpsw_nuss_mac_link_up() function and use it in the
am65_cpsw_nuss_mac_link_up() function. The newly added function will be
used in am65_cpsw_nuss_mac_config() function in a future patch, thereby
making it necessary to define a new function for the redundant mac control
operations.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 4e06def3b0de..c7e6ad374e1a 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1448,6 +1448,31 @@ static int am65_cpsw_init_phy(struct device *dev, struct device_node *port_np)
 	return ret;
 }
 
+static void am65_cpsw_nuss_mac_control(struct am65_cpsw_port *port, phy_interface_t interface,
+				       int speed, int duplex, bool tx_pause, bool rx_pause)
+{
+	u32 mac_control = CPSW_SL_CTL_GMII_EN;
+
+	if (speed == SPEED_1000)
+		mac_control |= CPSW_SL_CTL_GIG;
+	if (speed == SPEED_10 && interface == PHY_INTERFACE_MODE_RGMII)
+		/* Can be used with in band mode only */
+		mac_control |= CPSW_SL_CTL_EXT_EN;
+	if (speed == SPEED_100 && interface == PHY_INTERFACE_MODE_RMII)
+		mac_control |= CPSW_SL_CTL_IFCTL_A;
+	if (duplex)
+		mac_control |= CPSW_SL_CTL_FULLDUPLEX;
+
+	/* rx_pause/tx_pause */
+	if (rx_pause)
+		mac_control |= CPSW_SL_CTL_RX_FLOW_EN;
+
+	if (tx_pause)
+		mac_control |= CPSW_SL_CTL_TX_FLOW_EN;
+
+	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1497,27 +1522,9 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
 	struct am65_cpsw_common *common = port->common;
-	u32 mac_control = CPSW_SL_CTL_GMII_EN;
 	struct net_device *ndev = port->ndev;
 
-	if (speed == SPEED_1000)
-		mac_control |= CPSW_SL_CTL_GIG;
-	if (speed == SPEED_10 && interface == PHY_INTERFACE_MODE_RGMII)
-		/* Can be used with in band mode only */
-		mac_control |= CPSW_SL_CTL_EXT_EN;
-	if (speed == SPEED_100 && interface == PHY_INTERFACE_MODE_RMII)
-		mac_control |= CPSW_SL_CTL_IFCTL_A;
-	if (duplex)
-		mac_control |= CPSW_SL_CTL_FULLDUPLEX;
-
-	/* rx_pause/tx_pause */
-	if (rx_pause)
-		mac_control |= CPSW_SL_CTL_RX_FLOW_EN;
-
-	if (tx_pause)
-		mac_control |= CPSW_SL_CTL_TX_FLOW_EN;
-
-	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
+	am65_cpsw_nuss_mac_control(port, interface, speed, duplex, tx_pause, rx_pause);
 
 	/* enable phy */
 	am65_cpsw_enable_phy(port->slave.ifphy);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/8] net: ethernet: ti: am65-cpsw: Add mac control function

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:48PM +0530, Siddharth Vadapalli wrote:
> Add function am65_cpsw_nuss_mac_control() corresponding to the mac
> control register writes that are performed in the
> am65_cpsw_nuss_mac_link_up() function and use it in the
> am65_cpsw_nuss_mac_link_up() function. The newly added function will be
> used in am65_cpsw_nuss_mac_config() function in a future patch, thereby
> making it necessary to define a new function for the redundant mac control
> operations.

I think if you debug why you don't see mac_link_up called when in
fixed-link mode, you won't need this change.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 3/8] net: ethernet: ti: am65-cpsw: Add mac control function

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:48PM +0530, Siddharth Vadapalli wrote:
> Add function am65_cpsw_nuss_mac_control() corresponding to the mac
> control register writes that are performed in the
> am65_cpsw_nuss_mac_link_up() function and use it in the
> am65_cpsw_nuss_mac_link_up() function. The newly added function will be
> used in am65_cpsw_nuss_mac_config() function in a future patch, thereby
> making it necessary to define a new function for the redundant mac control
> operations.

I think if you debug why you don't see mac_link_up called when in
fixed-link mode, you won't need this change.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 4/8] net: ethernet: ti: am65-cpsw: Add mac enable link function

[Siddharth Vadapalli]

Add function am65_cpsw_nuss_mac_enable_link() to invoke
am65_cpsw_enable_phy(), cpsw_ale_control_set(), am65_cpsw_qos_link_up()
and netif_tx_wake_all_queues() to prevent duplicate code. The above
set of function calls are currently invoked by the
am65_cpsw_nuss_mac_link_up() function. In a later patch in this series
meant for adding fixed-link support, even the am65_cpsw_nuss_mac_config()
function will invoke the same set of functions.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 25 ++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index c7e6ad374e1a..72b1df12f320 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1473,6 +1473,20 @@ static void am65_cpsw_nuss_mac_control(struct am65_cpsw_port *port, phy_interfac
 	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
 }
 
+static void am65_cpsw_nuss_mac_enable_link(struct am65_cpsw_port *port, int speed, int duplex)
+{
+	struct am65_cpsw_common *common = port->common;
+	struct net_device *ndev = port->ndev;
+	/* enable phy */
+	am65_cpsw_enable_phy(port->slave.ifphy);
+
+	/* enable forwarding */
+	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
+
+	am65_cpsw_qos_link_up(ndev, speed);
+	netif_tx_wake_all_queues(ndev);
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1521,19 +1535,10 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
-	struct am65_cpsw_common *common = port->common;
-	struct net_device *ndev = port->ndev;
 
 	am65_cpsw_nuss_mac_control(port, interface, speed, duplex, tx_pause, rx_pause);
 
-	/* enable phy */
-	am65_cpsw_enable_phy(port->slave.ifphy);
-
-	/* enable forwarding */
-	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
-
-	am65_cpsw_qos_link_up(ndev, speed);
-	netif_tx_wake_all_queues(ndev);
+	am65_cpsw_nuss_mac_enable_link(port, speed, duplex);
 }
 
 static const struct phylink_mac_ops am65_cpsw_phylink_mac_ops = {
-- 
2.25.1

[PATCH 4/8] net: ethernet: ti: am65-cpsw: Add mac enable link function

[Siddharth Vadapalli]

Add function am65_cpsw_nuss_mac_enable_link() to invoke
am65_cpsw_enable_phy(), cpsw_ale_control_set(), am65_cpsw_qos_link_up()
and netif_tx_wake_all_queues() to prevent duplicate code. The above
set of function calls are currently invoked by the
am65_cpsw_nuss_mac_link_up() function. In a later patch in this series
meant for adding fixed-link support, even the am65_cpsw_nuss_mac_config()
function will invoke the same set of functions.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 25 ++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index c7e6ad374e1a..72b1df12f320 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1473,6 +1473,20 @@ static void am65_cpsw_nuss_mac_control(struct am65_cpsw_port *port, phy_interfac
 	cpsw_sl_ctl_set(port->slave.mac_sl, mac_control);
 }
 
+static void am65_cpsw_nuss_mac_enable_link(struct am65_cpsw_port *port, int speed, int duplex)
+{
+	struct am65_cpsw_common *common = port->common;
+	struct net_device *ndev = port->ndev;
+	/* enable phy */
+	am65_cpsw_enable_phy(port->slave.ifphy);
+
+	/* enable forwarding */
+	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
+
+	am65_cpsw_qos_link_up(ndev, speed);
+	netif_tx_wake_all_queues(ndev);
+}
+
 static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned int mode,
 				      const struct phylink_link_state *state)
 {
@@ -1521,19 +1535,10 @@ static void am65_cpsw_nuss_mac_link_up(struct phylink_config *config, struct phy
 	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
-	struct am65_cpsw_common *common = port->common;
-	struct net_device *ndev = port->ndev;
 
 	am65_cpsw_nuss_mac_control(port, interface, speed, duplex, tx_pause, rx_pause);
 
-	/* enable phy */
-	am65_cpsw_enable_phy(port->slave.ifphy);
-
-	/* enable forwarding */
-	cpsw_ale_control_set(common->ale, port->port_id, ALE_PORT_STATE, ALE_PORT_STATE_FORWARD);
-
-	am65_cpsw_qos_link_up(ndev, speed);
-	netif_tx_wake_all_queues(ndev);
+	am65_cpsw_nuss_mac_enable_link(port, speed, duplex);
 }
 
 static const struct phylink_mac_ops am65_cpsw_phylink_mac_ops = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/8] net: ethernet: ti: am65-cpsw: Add mac enable link function

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:49PM +0530, Siddharth Vadapalli wrote:
> Add function am65_cpsw_nuss_mac_enable_link() to invoke
> am65_cpsw_enable_phy(), cpsw_ale_control_set(), am65_cpsw_qos_link_up()
> and netif_tx_wake_all_queues() to prevent duplicate code. The above
> set of function calls are currently invoked by the
> am65_cpsw_nuss_mac_link_up() function. In a later patch in this series
> meant for adding fixed-link support, even the am65_cpsw_nuss_mac_config()
> function will invoke the same set of functions.

I don't think you will need this patch once you've debugged why
mac_link_up() doesn't get called for a fixed link (it should be
called.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 4/8] net: ethernet: ti: am65-cpsw: Add mac enable link function

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:49PM +0530, Siddharth Vadapalli wrote:
> Add function am65_cpsw_nuss_mac_enable_link() to invoke
> am65_cpsw_enable_phy(), cpsw_ale_control_set(), am65_cpsw_qos_link_up()
> and netif_tx_wake_all_queues() to prevent duplicate code. The above
> set of function calls are currently invoked by the
> am65_cpsw_nuss_mac_link_up() function. In a later patch in this series
> meant for adding fixed-link support, even the am65_cpsw_nuss_mac_config()
> function will invoke the same set of functions.

I don't think you will need this patch once you've debugged why
mac_link_up() doesn't get called for a fixed link (it should be
called.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
the relevant operations in am65_cpsw_nuss_mac_config() itself.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 40 ++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 72b1df12f320..1739c389af20 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
 	struct am65_cpsw_common *common = port->common;
+	struct fwnode_handle *fwnode;
+	bool fixed_link = false;
 
 	if (common->pdata.extra_modes & BIT(state->interface))
 		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
 		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
+
+	/* Detecting fixed-link */
+	fwnode = of_node_to_fwnode(port->slave.phy_node);
+	if (fwnode)
+		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
+
+	if (fixed_link) {
+		/* In fixed-link mode, mac_link_up is not invoked.
+		 * Therefore, the relevant mac_link_up operations
+		 * have to be moved to mac_config.
+		 */
+		am65_cpsw_nuss_mac_control(port, state->interface, state->speed,
+					   state->duplex, state->pause & MLO_PAUSE_TX,
+					   state->pause & MLO_PAUSE_RX);
+
+		if (state->speed == SPEED_1000)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_1G;
+		if (state->speed == SPEED_100)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_100M;
+		if (state->duplex)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX;
+
+		if (state->interface == PHY_INTERFACE_MODE_SGMII &&
+		    (common->pdata.extra_modes & BIT(state->interface))) {
+			writel(mr_adv_ability,
+			       port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+			writel((AM65_CPSW_SGMII_CONTROL_MASTER_MODE |
+			       AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE),
+			       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
+		}
+
+		am65_cpsw_nuss_mac_enable_link(port, state->speed, state->duplex);
+	} else {
+		if (state->interface == PHY_INTERFACE_MODE_SGMII &&
+		    (common->pdata.extra_modes & BIT(state->interface)))
+			writel(MAC2PHY_MR_ADV_ABILITY,
+			       port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+	}
 }
 
 static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned int mode,
-- 
2.25.1

[PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
the relevant operations in am65_cpsw_nuss_mac_config() itself.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 40 ++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 72b1df12f320..1739c389af20 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
 	struct am65_cpsw_common *common = port->common;
+	struct fwnode_handle *fwnode;
+	bool fixed_link = false;
 
 	if (common->pdata.extra_modes & BIT(state->interface))
 		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
 		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
+
+	/* Detecting fixed-link */
+	fwnode = of_node_to_fwnode(port->slave.phy_node);
+	if (fwnode)
+		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
+
+	if (fixed_link) {
+		/* In fixed-link mode, mac_link_up is not invoked.
+		 * Therefore, the relevant mac_link_up operations
+		 * have to be moved to mac_config.
+		 */
+		am65_cpsw_nuss_mac_control(port, state->interface, state->speed,
+					   state->duplex, state->pause & MLO_PAUSE_TX,
+					   state->pause & MLO_PAUSE_RX);
+
+		if (state->speed == SPEED_1000)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_1G;
+		if (state->speed == SPEED_100)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_100M;
+		if (state->duplex)
+			mr_adv_ability |= MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX;
+
+		if (state->interface == PHY_INTERFACE_MODE_SGMII &&
+		    (common->pdata.extra_modes & BIT(state->interface))) {
+			writel(mr_adv_ability,
+			       port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+			writel((AM65_CPSW_SGMII_CONTROL_MASTER_MODE |
+			       AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE),
+			       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
+		}
+
+		am65_cpsw_nuss_mac_enable_link(port, state->speed, state->duplex);
+	} else {
+		if (state->interface == PHY_INTERFACE_MODE_SGMII &&
+		    (common->pdata.extra_modes & BIT(state->interface)))
+			writel(MAC2PHY_MR_ADV_ABILITY,
+			       port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+	}
 }
 
 static void am65_cpsw_nuss_mac_link_down(struct phylink_config *config, unsigned int mode,
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 72b1df12f320..1739c389af20 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>  							  phylink_config);
>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>  	struct am65_cpsw_common *common = port->common;
> +	struct fwnode_handle *fwnode;
> +	bool fixed_link = false;
>  
>  	if (common->pdata.extra_modes & BIT(state->interface))
>  		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
>  		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
> +
> +	/* Detecting fixed-link */
> +	fwnode = of_node_to_fwnode(port->slave.phy_node);
> +	if (fwnode)
> +		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
> +
> +	if (fixed_link) {
> +		/* In fixed-link mode, mac_link_up is not invoked.
> +		 * Therefore, the relevant mac_link_up operations
> +		 * have to be moved to mac_config.
> +		 */

This seems very wrong. Why is mac_link_up() not invoked? Have you
debugged this? It works for other people.

Please debug rather than adding hacks to drivers when you find
things that don't seem to work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 72b1df12f320..1739c389af20 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>  							  phylink_config);
>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>  	struct am65_cpsw_common *common = port->common;
> +	struct fwnode_handle *fwnode;
> +	bool fixed_link = false;
>  
>  	if (common->pdata.extra_modes & BIT(state->interface))
>  		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
>  		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
> +
> +	/* Detecting fixed-link */
> +	fwnode = of_node_to_fwnode(port->slave.phy_node);
> +	if (fwnode)
> +		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
> +
> +	if (fixed_link) {
> +		/* In fixed-link mode, mac_link_up is not invoked.
> +		 * Therefore, the relevant mac_link_up operations
> +		 * have to be moved to mac_config.
> +		 */

This seems very wrong. Why is mac_link_up() not invoked? Have you
debugged this? It works for other people.

Please debug rather than adding hacks to drivers when you find
things that don't seem to work.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:11, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> index 72b1df12f320..1739c389af20 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> @@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>>  							  phylink_config);
>>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>>  	struct am65_cpsw_common *common = port->common;
>> +	struct fwnode_handle *fwnode;
>> +	bool fixed_link = false;
>>  
>>  	if (common->pdata.extra_modes & BIT(state->interface))
>>  		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
>>  		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
>> +
>> +	/* Detecting fixed-link */
>> +	fwnode = of_node_to_fwnode(port->slave.phy_node);
>> +	if (fwnode)
>> +		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
>> +
>> +	if (fixed_link) {
>> +		/* In fixed-link mode, mac_link_up is not invoked.
>> +		 * Therefore, the relevant mac_link_up operations
>> +		 * have to be moved to mac_config.
>> +		 */
> 
> This seems very wrong. Why is mac_link_up() not invoked? Have you
> debugged this? It works for other people.
> 
> Please debug rather than adding hacks to drivers when you find
> things that don't seem to work.

I will debug and find out. I had assumed that mac_link_up() is not
invoked in fixed-link mode. Thank you for clarifying.

Regards,
Siddharth.

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:11, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> index 72b1df12f320..1739c389af20 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> @@ -1494,10 +1494,50 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>>  							  phylink_config);
>>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>>  	struct am65_cpsw_common *common = port->common;
>> +	struct fwnode_handle *fwnode;
>> +	bool fixed_link = false;
>>  
>>  	if (common->pdata.extra_modes & BIT(state->interface))
>>  		writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
>>  		       port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
>> +
>> +	/* Detecting fixed-link */
>> +	fwnode = of_node_to_fwnode(port->slave.phy_node);
>> +	if (fwnode)
>> +		fixed_link = !!fwnode_get_named_child_node(fwnode, "fixed-link");
>> +
>> +	if (fixed_link) {
>> +		/* In fixed-link mode, mac_link_up is not invoked.
>> +		 * Therefore, the relevant mac_link_up operations
>> +		 * have to be moved to mac_config.
>> +		 */
> 
> This seems very wrong. Why is mac_link_up() not invoked? Have you
> debugged this? It works for other people.
> 
> Please debug rather than adding hacks to drivers when you find
> things that don't seem to work.

I will debug and find out. I had assumed that mac_link_up() is not
invoked in fixed-link mode. Thank you for clarifying.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> the relevant operations in am65_cpsw_nuss_mac_config() itself.

Further to my other comments, you also fail to explain that, when in
fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
since you are sending the duplex setting and speed settings via the
SGMII control word. Also, as SGMII was invented for a PHY to be able
to communicate the media negotiation resolution to the MAC, SGMII
defines that the PHY fills in the speed and duplex information in
the control word to pass it to the MAC, and the MAC acknowledges this
information. There is no need (and SGMII doesn't permit) the MAC to
advertise what it's doing.

Maybe this needs to be explained in the commit message?

This doesn't have any bearing on the other comments I've made.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> the relevant operations in am65_cpsw_nuss_mac_config() itself.

Further to my other comments, you also fail to explain that, when in
fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
since you are sending the duplex setting and speed settings via the
SGMII control word. Also, as SGMII was invented for a PHY to be able
to communicate the media negotiation resolution to the MAC, SGMII
defines that the PHY fills in the speed and duplex information in
the control word to pass it to the MAC, and the MAC acknowledges this
information. There is no need (and SGMII doesn't permit) the MAC to
advertise what it's doing.

Maybe this needs to be explained in the commit message?

This doesn't have any bearing on the other comments I've made.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:39, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> 
> Further to my other comments, you also fail to explain that, when in
> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> since you are sending the duplex setting and speed settings via the
> SGMII control word. Also, as SGMII was invented for a PHY to be able
> to communicate the media negotiation resolution to the MAC, SGMII
> defines that the PHY fills in the speed and duplex information in
> the control word to pass it to the MAC, and the MAC acknowledges this
> information. There is no need (and SGMII doesn't permit) the MAC to
> advertise what it's doing.
> 
> Maybe this needs to be explained in the commit message?

I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
PHY. Based on your clarification in the previous mails that there is an
issue with the fixed-link mode which I need to debug, I assume that what
you are referring to here also happens to be a consequence of that.
Please let me know if I have misunderstood what you meant to convey.

Regards,
Siddharth.

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:39, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> 
> Further to my other comments, you also fail to explain that, when in
> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> since you are sending the duplex setting and speed settings via the
> SGMII control word. Also, as SGMII was invented for a PHY to be able
> to communicate the media negotiation resolution to the MAC, SGMII
> defines that the PHY fills in the speed and duplex information in
> the control word to pass it to the MAC, and the MAC acknowledges this
> information. There is no need (and SGMII doesn't permit) the MAC to
> advertise what it's doing.
> 
> Maybe this needs to be explained in the commit message?

I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
PHY. Based on your clarification in the previous mails that there is an
issue with the fixed-link mode which I need to debug, I assume that what
you are referring to here also happens to be a consequence of that.
Please let me know if I have misunderstood what you meant to convey.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

Hi,

On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> Hello Russell,
> 
> On 14/09/22 21:39, Russell King (Oracle) wrote:
> > On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> > 
> > Further to my other comments, you also fail to explain that, when in
> > fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> > since you are sending the duplex setting and speed settings via the
> > SGMII control word. Also, as SGMII was invented for a PHY to be able
> > to communicate the media negotiation resolution to the MAC, SGMII
> > defines that the PHY fills in the speed and duplex information in
> > the control word to pass it to the MAC, and the MAC acknowledges this
> > information. There is no need (and SGMII doesn't permit) the MAC to
> > advertise what it's doing.
> > 
> > Maybe this needs to be explained in the commit message?
> 
> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> PHY. Based on your clarification in the previous mails that there is an
> issue with the fixed-link mode which I need to debug, I assume that what
> you are referring to here also happens to be a consequence of that.
> Please let me know if I have misunderstood what you meant to convey.

I think what you're saying is that you have this setup:

  ethernet MAC <--SGMII link--> ethernet PHY <---> media

which you are operating in fixed link mode?

From the SGMII specification: "This is achieved by using the Auto-
Negotiation functionality defined in Clause 37 of the IEEE
Specification 802.3z. Instead of the ability advertisement, the PHY
sends the control information via its tx_config_Reg[15:0] as specified
in Table 1 whenever the control information changes. Upon receiving
control information, the MAC acknowledges the update of the control
information by asserting bit 14 of its tx_config_reg{15:0] as specified
in Table 1."

For the control word sent from the MAC to the PHY, table 1 specifies a
value of 0x4001. All the zero bits in that word which are zero are
marked as "Reserved for future use." There are no fields for speed and
duplex in this acknowledgement word to the PHY.

I hope this clears up my point.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

Hi,

On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> Hello Russell,
> 
> On 14/09/22 21:39, Russell King (Oracle) wrote:
> > On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> > 
> > Further to my other comments, you also fail to explain that, when in
> > fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> > since you are sending the duplex setting and speed settings via the
> > SGMII control word. Also, as SGMII was invented for a PHY to be able
> > to communicate the media negotiation resolution to the MAC, SGMII
> > defines that the PHY fills in the speed and duplex information in
> > the control word to pass it to the MAC, and the MAC acknowledges this
> > information. There is no need (and SGMII doesn't permit) the MAC to
> > advertise what it's doing.
> > 
> > Maybe this needs to be explained in the commit message?
> 
> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> PHY. Based on your clarification in the previous mails that there is an
> issue with the fixed-link mode which I need to debug, I assume that what
> you are referring to here also happens to be a consequence of that.
> Please let me know if I have misunderstood what you meant to convey.

I think what you're saying is that you have this setup:

  ethernet MAC <--SGMII link--> ethernet PHY <---> media

which you are operating in fixed link mode?

From the SGMII specification: "This is achieved by using the Auto-
Negotiation functionality defined in Clause 37 of the IEEE
Specification 802.3z. Instead of the ability advertisement, the PHY
sends the control information via its tx_config_Reg[15:0] as specified
in Table 1 whenever the control information changes. Upon receiving
control information, the MAC acknowledges the update of the control
information by asserting bit 14 of its tx_config_reg{15:0] as specified
in Table 1."

For the control word sent from the MAC to the PHY, table 1 specifies a
value of 0x4001. All the zero bits in that word which are zero are
marked as "Reserved for future use." There are no fields for speed and
duplex in this acknowledgement word to the PHY.

I hope this clears up my point.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 15/09/22 15:37, Russell King (Oracle) wrote:
> Hi,
> 
> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
>> Hello Russell,
>>
>> On 14/09/22 21:39, Russell King (Oracle) wrote:
>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
>>>
>>> Further to my other comments, you also fail to explain that, when in
>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
>>> since you are sending the duplex setting and speed settings via the
>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
>>> to communicate the media negotiation resolution to the MAC, SGMII
>>> defines that the PHY fills in the speed and duplex information in
>>> the control word to pass it to the MAC, and the MAC acknowledges this
>>> information. There is no need (and SGMII doesn't permit) the MAC to
>>> advertise what it's doing.
>>>
>>> Maybe this needs to be explained in the commit message?
>>
>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
>> PHY. Based on your clarification in the previous mails that there is an
>> issue with the fixed-link mode which I need to debug, I assume that what
>> you are referring to here also happens to be a consequence of that.
>> Please let me know if I have misunderstood what you meant to convey.
> 
> I think what you're saying is that you have this setup:
> 
>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> 
> which you are operating in fixed link mode?

Yes, and the other end is connected to my PC's ethernet port.

> 
> From the SGMII specification: "This is achieved by using the Auto-
> Negotiation functionality defined in Clause 37 of the IEEE
> Specification 802.3z. Instead of the ability advertisement, the PHY
> sends the control information via its tx_config_Reg[15:0] as specified
> in Table 1 whenever the control information changes. Upon receiving
> control information, the MAC acknowledges the update of the control
> information by asserting bit 14 of its tx_config_reg{15:0] as specified
> in Table 1."
> 
> For the control word sent from the MAC to the PHY, table 1 specifies a
> value of 0x4001. All the zero bits in that word which are zero are
> marked as "Reserved for future use." There are no fields for speed and
> duplex in this acknowledgement word to the PHY.
> 
> I hope this clears up my point.

Thank you for the detailed explanation. After reading the above, my
understanding is that even in the fixed-link mode, the ethernet MAC is
not supposed to advertise the speed and duplex settings. The ethernet
MACs present on both ends of the connection are supposed to be set to
the same speed and duplex settings via the devicetree node. Thus, only
for my setup which happens to be a special case of fixed-link mode where
the ethernet PHY is present, I am having to send the control word due to
the presence of a PHY in between. And, I am supposed to mention this in
the commit message, which I haven't done. Please let me know if this is
what I was supposed to understand.

I am planning to change to a proper fixed-link setup without any
ethernet PHY between the MACs, for debugging the driver's fixed-link
mode where the "mac_link_up()" is not invoked. Additionally, with this
new setup, my MAC will not have to emulate being an ethernet PHY,
thereby making my patch cleaner in the process. The v2 series will be
based on the new setup. I hope that this is fine.

Regards,
Siddharth.

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 15/09/22 15:37, Russell King (Oracle) wrote:
> Hi,
> 
> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
>> Hello Russell,
>>
>> On 14/09/22 21:39, Russell King (Oracle) wrote:
>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
>>>
>>> Further to my other comments, you also fail to explain that, when in
>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
>>> since you are sending the duplex setting and speed settings via the
>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
>>> to communicate the media negotiation resolution to the MAC, SGMII
>>> defines that the PHY fills in the speed and duplex information in
>>> the control word to pass it to the MAC, and the MAC acknowledges this
>>> information. There is no need (and SGMII doesn't permit) the MAC to
>>> advertise what it's doing.
>>>
>>> Maybe this needs to be explained in the commit message?
>>
>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
>> PHY. Based on your clarification in the previous mails that there is an
>> issue with the fixed-link mode which I need to debug, I assume that what
>> you are referring to here also happens to be a consequence of that.
>> Please let me know if I have misunderstood what you meant to convey.
> 
> I think what you're saying is that you have this setup:
> 
>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> 
> which you are operating in fixed link mode?

Yes, and the other end is connected to my PC's ethernet port.

> 
> From the SGMII specification: "This is achieved by using the Auto-
> Negotiation functionality defined in Clause 37 of the IEEE
> Specification 802.3z. Instead of the ability advertisement, the PHY
> sends the control information via its tx_config_Reg[15:0] as specified
> in Table 1 whenever the control information changes. Upon receiving
> control information, the MAC acknowledges the update of the control
> information by asserting bit 14 of its tx_config_reg{15:0] as specified
> in Table 1."
> 
> For the control word sent from the MAC to the PHY, table 1 specifies a
> value of 0x4001. All the zero bits in that word which are zero are
> marked as "Reserved for future use." There are no fields for speed and
> duplex in this acknowledgement word to the PHY.
> 
> I hope this clears up my point.

Thank you for the detailed explanation. After reading the above, my
understanding is that even in the fixed-link mode, the ethernet MAC is
not supposed to advertise the speed and duplex settings. The ethernet
MACs present on both ends of the connection are supposed to be set to
the same speed and duplex settings via the devicetree node. Thus, only
for my setup which happens to be a special case of fixed-link mode where
the ethernet PHY is present, I am having to send the control word due to
the presence of a PHY in between. And, I am supposed to mention this in
the commit message, which I haven't done. Please let me know if this is
what I was supposed to understand.

I am planning to change to a proper fixed-link setup without any
ethernet PHY between the MACs, for debugging the driver's fixed-link
mode where the "mac_link_up()" is not invoked. Additionally, with this
new setup, my MAC will not have to emulate being an ethernet PHY,
thereby making my patch cleaner in the process. The v2 series will be
based on the new setup. I hope that this is fine.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
> On 15/09/22 15:37, Russell King (Oracle) wrote:
> > Hi,
> > 
> > On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> >> Hello Russell,
> >>
> >> On 14/09/22 21:39, Russell King (Oracle) wrote:
> >>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> >>>
> >>> Further to my other comments, you also fail to explain that, when in
> >>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> >>> since you are sending the duplex setting and speed settings via the
> >>> SGMII control word. Also, as SGMII was invented for a PHY to be able
> >>> to communicate the media negotiation resolution to the MAC, SGMII
> >>> defines that the PHY fills in the speed and duplex information in
> >>> the control word to pass it to the MAC, and the MAC acknowledges this
> >>> information. There is no need (and SGMII doesn't permit) the MAC to
> >>> advertise what it's doing.
> >>>
> >>> Maybe this needs to be explained in the commit message?
> >>
> >> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> >> PHY. Based on your clarification in the previous mails that there is an
> >> issue with the fixed-link mode which I need to debug, I assume that what
> >> you are referring to here also happens to be a consequence of that.
> >> Please let me know if I have misunderstood what you meant to convey.
> > 
> > I think what you're saying is that you have this setup:
> > 
> >   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> > 
> > which you are operating in fixed link mode?
> 
> Yes, and the other end is connected to my PC's ethernet port.
> 
> > 
> > From the SGMII specification: "This is achieved by using the Auto-
> > Negotiation functionality defined in Clause 37 of the IEEE
> > Specification 802.3z. Instead of the ability advertisement, the PHY
> > sends the control information via its tx_config_Reg[15:0] as specified
> > in Table 1 whenever the control information changes. Upon receiving
> > control information, the MAC acknowledges the update of the control
> > information by asserting bit 14 of its tx_config_reg{15:0] as specified
> > in Table 1."
> > 
> > For the control word sent from the MAC to the PHY, table 1 specifies a
> > value of 0x4001. All the zero bits in that word which are zero are
> > marked as "Reserved for future use." There are no fields for speed and
> > duplex in this acknowledgement word to the PHY.
> > 
> > I hope this clears up my point.
> 
> Thank you for the detailed explanation. After reading the above, my
> understanding is that even in the fixed-link mode, the ethernet MAC is
> not supposed to advertise the speed and duplex settings. The ethernet
> MACs present on both ends of the connection are supposed to be set to
> the same speed and duplex settings via the devicetree node. Thus, only
> for my setup which happens to be a special case of fixed-link mode where
> the ethernet PHY is present, I am having to send the control word due to
> the presence of a PHY in between.

In SGMII, the control word is only passed between the ethernet MAC and
the ethernet PHY. It is not conveyed across the media.

> And, I am supposed to mention this in
> the commit message, which I haven't done. Please let me know if this is
> what I was supposed to understand.

If you implement this conventionally, then you don't need to mention it
in the commit message, because you're following the standard.

> I am planning to change to a proper fixed-link setup without any
> ethernet PHY between the MACs, for debugging the driver's fixed-link
> mode where the "mac_link_up()" is not invoked.

SGMII is designed for the setup in the diagram I provided in my previous
email. It is not designed for two MACs to talk direct to each other
without any ethernet PHY because of the asymmetric nature of the control
word.

The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
reception of that, the MAC responds with 0x4001. Finally, the PHY
responds with 0xd801 to acknowledge the receipt of the MAC response.

If both ends of the link are SGMII, both ends will be waiting for
the control word from a PHY which is not present, and the link will
not come up.

1000base-X is a symmetric protocol where both ends of the link
advertise their capabilities, acknowledge each others abilities and
resolve the duplex and pause settings.

SGMII is a Cisco proprietary modification of 1000base-X designed for
communicating the results of media autonegotiation between an
ethernet PHY and ethernet MAC.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
> On 15/09/22 15:37, Russell King (Oracle) wrote:
> > Hi,
> > 
> > On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> >> Hello Russell,
> >>
> >> On 14/09/22 21:39, Russell King (Oracle) wrote:
> >>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> >>>
> >>> Further to my other comments, you also fail to explain that, when in
> >>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> >>> since you are sending the duplex setting and speed settings via the
> >>> SGMII control word. Also, as SGMII was invented for a PHY to be able
> >>> to communicate the media negotiation resolution to the MAC, SGMII
> >>> defines that the PHY fills in the speed and duplex information in
> >>> the control word to pass it to the MAC, and the MAC acknowledges this
> >>> information. There is no need (and SGMII doesn't permit) the MAC to
> >>> advertise what it's doing.
> >>>
> >>> Maybe this needs to be explained in the commit message?
> >>
> >> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> >> PHY. Based on your clarification in the previous mails that there is an
> >> issue with the fixed-link mode which I need to debug, I assume that what
> >> you are referring to here also happens to be a consequence of that.
> >> Please let me know if I have misunderstood what you meant to convey.
> > 
> > I think what you're saying is that you have this setup:
> > 
> >   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> > 
> > which you are operating in fixed link mode?
> 
> Yes, and the other end is connected to my PC's ethernet port.
> 
> > 
> > From the SGMII specification: "This is achieved by using the Auto-
> > Negotiation functionality defined in Clause 37 of the IEEE
> > Specification 802.3z. Instead of the ability advertisement, the PHY
> > sends the control information via its tx_config_Reg[15:0] as specified
> > in Table 1 whenever the control information changes. Upon receiving
> > control information, the MAC acknowledges the update of the control
> > information by asserting bit 14 of its tx_config_reg{15:0] as specified
> > in Table 1."
> > 
> > For the control word sent from the MAC to the PHY, table 1 specifies a
> > value of 0x4001. All the zero bits in that word which are zero are
> > marked as "Reserved for future use." There are no fields for speed and
> > duplex in this acknowledgement word to the PHY.
> > 
> > I hope this clears up my point.
> 
> Thank you for the detailed explanation. After reading the above, my
> understanding is that even in the fixed-link mode, the ethernet MAC is
> not supposed to advertise the speed and duplex settings. The ethernet
> MACs present on both ends of the connection are supposed to be set to
> the same speed and duplex settings via the devicetree node. Thus, only
> for my setup which happens to be a special case of fixed-link mode where
> the ethernet PHY is present, I am having to send the control word due to
> the presence of a PHY in between.

In SGMII, the control word is only passed between the ethernet MAC and
the ethernet PHY. It is not conveyed across the media.

> And, I am supposed to mention this in
> the commit message, which I haven't done. Please let me know if this is
> what I was supposed to understand.

If you implement this conventionally, then you don't need to mention it
in the commit message, because you're following the standard.

> I am planning to change to a proper fixed-link setup without any
> ethernet PHY between the MACs, for debugging the driver's fixed-link
> mode where the "mac_link_up()" is not invoked.

SGMII is designed for the setup in the diagram I provided in my previous
email. It is not designed for two MACs to talk direct to each other
without any ethernet PHY because of the asymmetric nature of the control
word.

The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
reception of that, the MAC responds with 0x4001. Finally, the PHY
responds with 0xd801 to acknowledge the receipt of the MAC response.

If both ends of the link are SGMII, both ends will be waiting for
the control word from a PHY which is not present, and the link will
not come up.

1000base-X is a symmetric protocol where both ends of the link
advertise their capabilities, acknowledge each others abilities and
resolve the duplex and pause settings.

SGMII is a Cisco proprietary modification of 1000base-X designed for
communicating the results of media autonegotiation between an
ethernet PHY and ethernet MAC.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 16/09/22 12:50, Russell King (Oracle) wrote:
> On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
>> On 15/09/22 15:37, Russell King (Oracle) wrote:
>>> Hi,
>>>
>>> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
>>>> Hello Russell,
>>>>
>>>> On 14/09/22 21:39, Russell King (Oracle) wrote:
>>>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>>>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>>>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>>>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>>>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
>>>>>
>>>>> Further to my other comments, you also fail to explain that, when in
>>>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
>>>>> since you are sending the duplex setting and speed settings via the
>>>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
>>>>> to communicate the media negotiation resolution to the MAC, SGMII
>>>>> defines that the PHY fills in the speed and duplex information in
>>>>> the control word to pass it to the MAC, and the MAC acknowledges this
>>>>> information. There is no need (and SGMII doesn't permit) the MAC to
>>>>> advertise what it's doing.
>>>>>
>>>>> Maybe this needs to be explained in the commit message?
>>>>
>>>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
>>>> PHY. Based on your clarification in the previous mails that there is an
>>>> issue with the fixed-link mode which I need to debug, I assume that what
>>>> you are referring to here also happens to be a consequence of that.
>>>> Please let me know if I have misunderstood what you meant to convey.
>>>
>>> I think what you're saying is that you have this setup:
>>>
>>>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
>>>
>>> which you are operating in fixed link mode?
>>
>> Yes, and the other end is connected to my PC's ethernet port.
>>
>>>
>>> From the SGMII specification: "This is achieved by using the Auto-
>>> Negotiation functionality defined in Clause 37 of the IEEE
>>> Specification 802.3z. Instead of the ability advertisement, the PHY
>>> sends the control information via its tx_config_Reg[15:0] as specified
>>> in Table 1 whenever the control information changes. Upon receiving
>>> control information, the MAC acknowledges the update of the control
>>> information by asserting bit 14 of its tx_config_reg{15:0] as specified
>>> in Table 1."
>>>
>>> For the control word sent from the MAC to the PHY, table 1 specifies a
>>> value of 0x4001. All the zero bits in that word which are zero are
>>> marked as "Reserved for future use." There are no fields for speed and
>>> duplex in this acknowledgement word to the PHY.
>>>
>>> I hope this clears up my point.
>>
>> Thank you for the detailed explanation. After reading the above, my
>> understanding is that even in the fixed-link mode, the ethernet MAC is
>> not supposed to advertise the speed and duplex settings. The ethernet
>> MACs present on both ends of the connection are supposed to be set to
>> the same speed and duplex settings via the devicetree node. Thus, only
>> for my setup which happens to be a special case of fixed-link mode where
>> the ethernet PHY is present, I am having to send the control word due to
>> the presence of a PHY in between.
> 
> In SGMII, the control word is only passed between the ethernet MAC and
> the ethernet PHY. It is not conveyed across the media.
> 
>> And, I am supposed to mention this in
>> the commit message, which I haven't done. Please let me know if this is
>> what I was supposed to understand.
> 
> If you implement this conventionally, then you don't need to mention it
> in the commit message, because you're following the standard.
> 
>> I am planning to change to a proper fixed-link setup without any
>> ethernet PHY between the MACs, for debugging the driver's fixed-link
>> mode where the "mac_link_up()" is not invoked.
> 
> SGMII is designed for the setup in the diagram I provided in my previous
> email. It is not designed for two MACs to talk direct to each other
> without any ethernet PHY because of the asymmetric nature of the control
> word.
> 
> The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
> reception of that, the MAC responds with 0x4001. Finally, the PHY
> responds with 0xd801 to acknowledge the receipt of the MAC response.
> 
> If both ends of the link are SGMII, both ends will be waiting for
> the control word from a PHY which is not present, and the link will
> not come up.
> 
> 1000base-X is a symmetric protocol where both ends of the link
> advertise their capabilities, acknowledge each others abilities and
> resolve the duplex and pause settings.
> 
> SGMII is a Cisco proprietary modification of 1000base-X designed for
> communicating the results of media autonegotiation between an
> ethernet PHY and ethernet MAC.


I will try to implement and test SGMII mode in the conventional way with
both the MAC and the PHY present. If I am unable to do so, I will revert
to the current set of patches for the special case where the MAC
emulates a PHY, and mention this setup in the commit message of the v2
series. I hope this approach would be fine to proceed with. Please let
me know in case of any suggestions.

Regards,
Siddharth.

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 16/09/22 12:50, Russell King (Oracle) wrote:
> On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
>> On 15/09/22 15:37, Russell King (Oracle) wrote:
>>> Hi,
>>>
>>> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
>>>> Hello Russell,
>>>>
>>>> On 14/09/22 21:39, Russell King (Oracle) wrote:
>>>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
>>>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
>>>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
>>>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
>>>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
>>>>>
>>>>> Further to my other comments, you also fail to explain that, when in
>>>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
>>>>> since you are sending the duplex setting and speed settings via the
>>>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
>>>>> to communicate the media negotiation resolution to the MAC, SGMII
>>>>> defines that the PHY fills in the speed and duplex information in
>>>>> the control word to pass it to the MAC, and the MAC acknowledges this
>>>>> information. There is no need (and SGMII doesn't permit) the MAC to
>>>>> advertise what it's doing.
>>>>>
>>>>> Maybe this needs to be explained in the commit message?
>>>>
>>>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
>>>> PHY. Based on your clarification in the previous mails that there is an
>>>> issue with the fixed-link mode which I need to debug, I assume that what
>>>> you are referring to here also happens to be a consequence of that.
>>>> Please let me know if I have misunderstood what you meant to convey.
>>>
>>> I think what you're saying is that you have this setup:
>>>
>>>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
>>>
>>> which you are operating in fixed link mode?
>>
>> Yes, and the other end is connected to my PC's ethernet port.
>>
>>>
>>> From the SGMII specification: "This is achieved by using the Auto-
>>> Negotiation functionality defined in Clause 37 of the IEEE
>>> Specification 802.3z. Instead of the ability advertisement, the PHY
>>> sends the control information via its tx_config_Reg[15:0] as specified
>>> in Table 1 whenever the control information changes. Upon receiving
>>> control information, the MAC acknowledges the update of the control
>>> information by asserting bit 14 of its tx_config_reg{15:0] as specified
>>> in Table 1."
>>>
>>> For the control word sent from the MAC to the PHY, table 1 specifies a
>>> value of 0x4001. All the zero bits in that word which are zero are
>>> marked as "Reserved for future use." There are no fields for speed and
>>> duplex in this acknowledgement word to the PHY.
>>>
>>> I hope this clears up my point.
>>
>> Thank you for the detailed explanation. After reading the above, my
>> understanding is that even in the fixed-link mode, the ethernet MAC is
>> not supposed to advertise the speed and duplex settings. The ethernet
>> MACs present on both ends of the connection are supposed to be set to
>> the same speed and duplex settings via the devicetree node. Thus, only
>> for my setup which happens to be a special case of fixed-link mode where
>> the ethernet PHY is present, I am having to send the control word due to
>> the presence of a PHY in between.
> 
> In SGMII, the control word is only passed between the ethernet MAC and
> the ethernet PHY. It is not conveyed across the media.
> 
>> And, I am supposed to mention this in
>> the commit message, which I haven't done. Please let me know if this is
>> what I was supposed to understand.
> 
> If you implement this conventionally, then you don't need to mention it
> in the commit message, because you're following the standard.
> 
>> I am planning to change to a proper fixed-link setup without any
>> ethernet PHY between the MACs, for debugging the driver's fixed-link
>> mode where the "mac_link_up()" is not invoked.
> 
> SGMII is designed for the setup in the diagram I provided in my previous
> email. It is not designed for two MACs to talk direct to each other
> without any ethernet PHY because of the asymmetric nature of the control
> word.
> 
> The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
> reception of that, the MAC responds with 0x4001. Finally, the PHY
> responds with 0xd801 to acknowledge the receipt of the MAC response.
> 
> If both ends of the link are SGMII, both ends will be waiting for
> the control word from a PHY which is not present, and the link will
> not come up.
> 
> 1000base-X is a symmetric protocol where both ends of the link
> advertise their capabilities, acknowledge each others abilities and
> resolve the duplex and pause settings.
> 
> SGMII is a Cisco proprietary modification of 1000base-X designed for
> communicating the results of media autonegotiation between an
> ethernet PHY and ethernet MAC.


I will try to implement and test SGMII mode in the conventional way with
both the MAC and the PHY present. If I am unable to do so, I will revert
to the current set of patches for the special case where the MAC
emulates a PHY, and mention this setup in the commit message of the v2
series. I hope this approach would be fine to proceed with. Please let
me know in case of any suggestions.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Fri, Sep 16, 2022 at 02:33:23PM +0530, Siddharth Vadapalli wrote:
> Hello Russell,
> 
> On 16/09/22 12:50, Russell King (Oracle) wrote:
> > On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
> >> On 15/09/22 15:37, Russell King (Oracle) wrote:
> >>> Hi,
> >>>
> >>> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> >>>> Hello Russell,
> >>>>
> >>>> On 14/09/22 21:39, Russell King (Oracle) wrote:
> >>>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >>>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >>>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >>>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >>>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> >>>>>
> >>>>> Further to my other comments, you also fail to explain that, when in
> >>>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> >>>>> since you are sending the duplex setting and speed settings via the
> >>>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
> >>>>> to communicate the media negotiation resolution to the MAC, SGMII
> >>>>> defines that the PHY fills in the speed and duplex information in
> >>>>> the control word to pass it to the MAC, and the MAC acknowledges this
> >>>>> information. There is no need (and SGMII doesn't permit) the MAC to
> >>>>> advertise what it's doing.
> >>>>>
> >>>>> Maybe this needs to be explained in the commit message?
> >>>>
> >>>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> >>>> PHY. Based on your clarification in the previous mails that there is an
> >>>> issue with the fixed-link mode which I need to debug, I assume that what
> >>>> you are referring to here also happens to be a consequence of that.
> >>>> Please let me know if I have misunderstood what you meant to convey.
> >>>
> >>> I think what you're saying is that you have this setup:
> >>>
> >>>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> >>>
> >>> which you are operating in fixed link mode?
> >>
> >> Yes, and the other end is connected to my PC's ethernet port.
> >>
> >>>
> >>> From the SGMII specification: "This is achieved by using the Auto-
> >>> Negotiation functionality defined in Clause 37 of the IEEE
> >>> Specification 802.3z. Instead of the ability advertisement, the PHY
> >>> sends the control information via its tx_config_Reg[15:0] as specified
> >>> in Table 1 whenever the control information changes. Upon receiving
> >>> control information, the MAC acknowledges the update of the control
> >>> information by asserting bit 14 of its tx_config_reg{15:0] as specified
> >>> in Table 1."
> >>>
> >>> For the control word sent from the MAC to the PHY, table 1 specifies a
> >>> value of 0x4001. All the zero bits in that word which are zero are
> >>> marked as "Reserved for future use." There are no fields for speed and
> >>> duplex in this acknowledgement word to the PHY.
> >>>
> >>> I hope this clears up my point.
> >>
> >> Thank you for the detailed explanation. After reading the above, my
> >> understanding is that even in the fixed-link mode, the ethernet MAC is
> >> not supposed to advertise the speed and duplex settings. The ethernet
> >> MACs present on both ends of the connection are supposed to be set to
> >> the same speed and duplex settings via the devicetree node. Thus, only
> >> for my setup which happens to be a special case of fixed-link mode where
> >> the ethernet PHY is present, I am having to send the control word due to
> >> the presence of a PHY in between.
> > 
> > In SGMII, the control word is only passed between the ethernet MAC and
> > the ethernet PHY. It is not conveyed across the media.
> > 
> >> And, I am supposed to mention this in
> >> the commit message, which I haven't done. Please let me know if this is
> >> what I was supposed to understand.
> > 
> > If you implement this conventionally, then you don't need to mention it
> > in the commit message, because you're following the standard.
> > 
> >> I am planning to change to a proper fixed-link setup without any
> >> ethernet PHY between the MACs, for debugging the driver's fixed-link
> >> mode where the "mac_link_up()" is not invoked.
> > 
> > SGMII is designed for the setup in the diagram I provided in my previous
> > email. It is not designed for two MACs to talk direct to each other
> > without any ethernet PHY because of the asymmetric nature of the control
> > word.
> > 
> > The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
> > reception of that, the MAC responds with 0x4001. Finally, the PHY
> > responds with 0xd801 to acknowledge the receipt of the MAC response.
> > 
> > If both ends of the link are SGMII, both ends will be waiting for
> > the control word from a PHY which is not present, and the link will
> > not come up.
> > 
> > 1000base-X is a symmetric protocol where both ends of the link
> > advertise their capabilities, acknowledge each others abilities and
> > resolve the duplex and pause settings.
> > 
> > SGMII is a Cisco proprietary modification of 1000base-X designed for
> > communicating the results of media autonegotiation between an
> > ethernet PHY and ethernet MAC.
> 
> 
> I will try to implement and test SGMII mode in the conventional way with
> both the MAC and the PHY present. If I am unable to do so, I will revert
> to the current set of patches for the special case where the MAC
> emulates a PHY, and mention this setup in the commit message of the v2
> series. I hope this approach would be fine to proceed with. Please let
> me know in case of any suggestions.

What exact setups are you trying to support with this patch set?

If you're looking to only add support for SGMII, then all you need
to do is to make sure it works with a PHY. Fixed-link in SGMII only
makes sense if you're directly connected to something like a network
switch, but even then, network switches tend to use 1000base-X in a
fixed-link mode rather than SGMII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Russell King (Oracle)]

On Fri, Sep 16, 2022 at 02:33:23PM +0530, Siddharth Vadapalli wrote:
> Hello Russell,
> 
> On 16/09/22 12:50, Russell King (Oracle) wrote:
> > On Fri, Sep 16, 2022 at 10:24:48AM +0530, Siddharth Vadapalli wrote:
> >> On 15/09/22 15:37, Russell King (Oracle) wrote:
> >>> Hi,
> >>>
> >>> On Thu, Sep 15, 2022 at 02:58:52PM +0530, Siddharth Vadapalli wrote:
> >>>> Hello Russell,
> >>>>
> >>>> On 14/09/22 21:39, Russell King (Oracle) wrote:
> >>>>> On Wed, Sep 14, 2022 at 03:20:50PM +0530, Siddharth Vadapalli wrote:
> >>>>>> Check for fixed-link in am65_cpsw_nuss_mac_config() using struct
> >>>>>> am65_cpsw_slave_data's phy_node property to obtain fwnode. Since
> >>>>>> am65_cpsw_nuss_mac_link_up() is not invoked in fixed-link mode, perform
> >>>>>> the relevant operations in am65_cpsw_nuss_mac_config() itself.
> >>>>>
> >>>>> Further to my other comments, you also fail to explain that, when in
> >>>>> fixed-link SGMII mode, you _emulate_ being a PHY - which I deduce
> >>>>> since you are sending the duplex setting and speed settings via the
> >>>>> SGMII control word. Also, as SGMII was invented for a PHY to be able
> >>>>> to communicate the media negotiation resolution to the MAC, SGMII
> >>>>> defines that the PHY fills in the speed and duplex information in
> >>>>> the control word to pass it to the MAC, and the MAC acknowledges this
> >>>>> information. There is no need (and SGMII doesn't permit) the MAC to
> >>>>> advertise what it's doing.
> >>>>>
> >>>>> Maybe this needs to be explained in the commit message?
> >>>>
> >>>> I had tested SGMII fixed-link mode using a bootstrapped ethernet layer-1
> >>>> PHY. Based on your clarification in the previous mails that there is an
> >>>> issue with the fixed-link mode which I need to debug, I assume that what
> >>>> you are referring to here also happens to be a consequence of that.
> >>>> Please let me know if I have misunderstood what you meant to convey.
> >>>
> >>> I think what you're saying is that you have this setup:
> >>>
> >>>   ethernet MAC <--SGMII link--> ethernet PHY <---> media
> >>>
> >>> which you are operating in fixed link mode?
> >>
> >> Yes, and the other end is connected to my PC's ethernet port.
> >>
> >>>
> >>> From the SGMII specification: "This is achieved by using the Auto-
> >>> Negotiation functionality defined in Clause 37 of the IEEE
> >>> Specification 802.3z. Instead of the ability advertisement, the PHY
> >>> sends the control information via its tx_config_Reg[15:0] as specified
> >>> in Table 1 whenever the control information changes. Upon receiving
> >>> control information, the MAC acknowledges the update of the control
> >>> information by asserting bit 14 of its tx_config_reg{15:0] as specified
> >>> in Table 1."
> >>>
> >>> For the control word sent from the MAC to the PHY, table 1 specifies a
> >>> value of 0x4001. All the zero bits in that word which are zero are
> >>> marked as "Reserved for future use." There are no fields for speed and
> >>> duplex in this acknowledgement word to the PHY.
> >>>
> >>> I hope this clears up my point.
> >>
> >> Thank you for the detailed explanation. After reading the above, my
> >> understanding is that even in the fixed-link mode, the ethernet MAC is
> >> not supposed to advertise the speed and duplex settings. The ethernet
> >> MACs present on both ends of the connection are supposed to be set to
> >> the same speed and duplex settings via the devicetree node. Thus, only
> >> for my setup which happens to be a special case of fixed-link mode where
> >> the ethernet PHY is present, I am having to send the control word due to
> >> the presence of a PHY in between.
> > 
> > In SGMII, the control word is only passed between the ethernet MAC and
> > the ethernet PHY. It is not conveyed across the media.
> > 
> >> And, I am supposed to mention this in
> >> the commit message, which I haven't done. Please let me know if this is
> >> what I was supposed to understand.
> > 
> > If you implement this conventionally, then you don't need to mention it
> > in the commit message, because you're following the standard.
> > 
> >> I am planning to change to a proper fixed-link setup without any
> >> ethernet PHY between the MACs, for debugging the driver's fixed-link
> >> mode where the "mac_link_up()" is not invoked.
> > 
> > SGMII is designed for the setup in the diagram I provided in my previous
> > email. It is not designed for two MACs to talk direct to each other
> > without any ethernet PHY because of the asymmetric nature of the control
> > word.
> > 
> > The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
> > reception of that, the MAC responds with 0x4001. Finally, the PHY
> > responds with 0xd801 to acknowledge the receipt of the MAC response.
> > 
> > If both ends of the link are SGMII, both ends will be waiting for
> > the control word from a PHY which is not present, and the link will
> > not come up.
> > 
> > 1000base-X is a symmetric protocol where both ends of the link
> > advertise their capabilities, acknowledge each others abilities and
> > resolve the duplex and pause settings.
> > 
> > SGMII is a Cisco proprietary modification of 1000base-X designed for
> > communicating the results of media autonegotiation between an
> > ethernet PHY and ethernet MAC.
> 
> 
> I will try to implement and test SGMII mode in the conventional way with
> both the MAC and the PHY present. If I am unable to do so, I will revert
> to the current set of patches for the special case where the MAC
> emulates a PHY, and mention this setup in the commit message of the v2
> series. I hope this approach would be fine to proceed with. Please let
> me know in case of any suggestions.

What exact setups are you trying to support with this patch set?

If you're looking to only add support for SGMII, then all you need
to do is to make sure it works with a PHY. Fixed-link in SGMII only
makes sense if you're directly connected to something like a network
switch, but even then, network switches tend to use 1000base-X in a
fixed-link mode rather than SGMII.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 16/09/22 14:44, Russell King (Oracle) wrote:
> On Fri, Sep 16, 2022 at 02:33:23PM +0530, Siddharth Vadapalli wrote:
>> Hello Russell,
>>
>> On 16/09/22 12:50, Russell King (Oracle) wrote:
>>> SGMII is designed for the setup in the diagram I provided in my previous
>>> email. It is not designed for two MACs to talk direct to each other
>>> without any ethernet PHY because of the asymmetric nature of the control
>>> word.
>>>
>>> The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
>>> reception of that, the MAC responds with 0x4001. Finally, the PHY
>>> responds with 0xd801 to acknowledge the receipt of the MAC response.
>>>
>>> If both ends of the link are SGMII, both ends will be waiting for
>>> the control word from a PHY which is not present, and the link will
>>> not come up.
>>>
>>> 1000base-X is a symmetric protocol where both ends of the link
>>> advertise their capabilities, acknowledge each others abilities and
>>> resolve the duplex and pause settings.
>>>
>>> SGMII is a Cisco proprietary modification of 1000base-X designed for
>>> communicating the results of media autonegotiation between an
>>> ethernet PHY and ethernet MAC.
>>
>> I will try to implement and test SGMII mode in the conventional way with
>> both the MAC and the PHY present. If I am unable to do so, I will revert
>> to the current set of patches for the special case where the MAC
>> emulates a PHY, and mention this setup in the commit message of the v2
>> series. I hope this approach would be fine to proceed with. Please let
>> me know in case of any suggestions.
> 
> What exact setups are you trying to support with this patch set?
> 
> If you're looking to only add support for SGMII, then all you need
> to do is to make sure it works with a PHY. Fixed-link in SGMII only
> makes sense if you're directly connected to something like a network
> switch, but even then, network switches tend to use 1000base-X in a
> fixed-link mode rather than SGMII.

I plan to support the standard MAC2PHY based SGMII mode. However, the
SGMII ethernet PHY that I have with me has issues which is why I had
tried the unconventional fixed-link SGMII mode with the MAC emulating
the ethernet PHY. I will try to obtain a functional SGMII ethernet PHY
and test the standard SGMII mode.

Thank you for patiently answering my questions and helping me understand
the SGMII mode better :)

Regards,
Siddharth.

Re: [PATCH 5/8] net: ethernet: ti: am65-cpsw: Add support for fixed-link configuration

[Siddharth Vadapalli]

Hello Russell,

On 16/09/22 14:44, Russell King (Oracle) wrote:
> On Fri, Sep 16, 2022 at 02:33:23PM +0530, Siddharth Vadapalli wrote:
>> Hello Russell,
>>
>> On 16/09/22 12:50, Russell King (Oracle) wrote:
>>> SGMII is designed for the setup in the diagram I provided in my previous
>>> email. It is not designed for two MACs to talk direct to each other
>>> without any ethernet PHY because of the asymmetric nature of the control
>>> word.
>>>
>>> The PHY sends e.g. a control word of 0x9801 for 1G full duplex. On
>>> reception of that, the MAC responds with 0x4001. Finally, the PHY
>>> responds with 0xd801 to acknowledge the receipt of the MAC response.
>>>
>>> If both ends of the link are SGMII, both ends will be waiting for
>>> the control word from a PHY which is not present, and the link will
>>> not come up.
>>>
>>> 1000base-X is a symmetric protocol where both ends of the link
>>> advertise their capabilities, acknowledge each others abilities and
>>> resolve the duplex and pause settings.
>>>
>>> SGMII is a Cisco proprietary modification of 1000base-X designed for
>>> communicating the results of media autonegotiation between an
>>> ethernet PHY and ethernet MAC.
>>
>> I will try to implement and test SGMII mode in the conventional way with
>> both the MAC and the PHY present. If I am unable to do so, I will revert
>> to the current set of patches for the special case where the MAC
>> emulates a PHY, and mention this setup in the commit message of the v2
>> series. I hope this approach would be fine to proceed with. Please let
>> me know in case of any suggestions.
> 
> What exact setups are you trying to support with this patch set?
> 
> If you're looking to only add support for SGMII, then all you need
> to do is to make sure it works with a PHY. Fixed-link in SGMII only
> makes sense if you're directly connected to something like a network
> switch, but even then, network switches tend to use 1000base-X in a
> fixed-link mode rather than SGMII.

I plan to support the standard MAC2PHY based SGMII mode. However, the
SGMII ethernet PHY that I have with me has issues which is why I had
tried the unconventional fixed-link SGMII mode with the MAC emulating
the ethernet PHY. I will try to obtain a functional SGMII ethernet PHY
and test the standard SGMII mode.

Thank you for patiently answering my questions and helping me understand
the SGMII mode better :)

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Add support for SGMII mode in both fixed-link MAC2MAC master mode and
MAC2PHY modes for CPSW5G ports.

Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.

The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
their device.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 1739c389af20..3f40178436ff 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -75,7 +75,15 @@
 #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
 
 #define AM65_CPSW_SGMII_CONTROL_REG		0x010
+#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018
 #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
+#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)
+
+#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
+#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
+#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
+#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
+#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
 
 #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
 #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
@@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
 	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
+	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;
 	struct am65_cpsw_common *common = port->common;
 	struct fwnode_handle *fwnode;
 	bool fixed_link = false;
@@ -2105,8 +2114,12 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
 		__set_bit(PHY_INTERFACE_MODE_RMII,
 			  port->slave.phylink_config.supported_interfaces);
 	} else if (common->pdata.extra_modes & BIT(port->slave.phy_if)) {
-		__set_bit(PHY_INTERFACE_MODE_QSGMII,
-			  port->slave.phylink_config.supported_interfaces);
+		if (port->slave.phy_if == PHY_INTERFACE_MODE_QSGMII)
+			__set_bit(PHY_INTERFACE_MODE_QSGMII,
+				  port->slave.phylink_config.supported_interfaces);
+		else
+			__set_bit(PHY_INTERFACE_MODE_SGMII,
+				  port->slave.phylink_config.supported_interfaces);
 	} else {
 		dev_err(dev, "selected phy-mode is not supported\n");
 		return -EOPNOTSUPP;
@@ -2744,7 +2757,7 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
 	.quirks = 0,
 	.ale_dev_id = "am64-cpswxg",
 	.fdqring_mode = K3_RINGACC_RING_MODE_RING,
-	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
-- 
2.25.1

[PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Add support for SGMII mode in both fixed-link MAC2MAC master mode and
MAC2PHY modes for CPSW5G ports.

Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.

The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
their device.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 1739c389af20..3f40178436ff 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -75,7 +75,15 @@
 #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
 
 #define AM65_CPSW_SGMII_CONTROL_REG		0x010
+#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018
 #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
+#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)
+
+#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
+#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
+#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
+#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
+#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
 
 #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
 #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
@@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
 	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
 							  phylink_config);
 	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
+	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;
 	struct am65_cpsw_common *common = port->common;
 	struct fwnode_handle *fwnode;
 	bool fixed_link = false;
@@ -2105,8 +2114,12 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
 		__set_bit(PHY_INTERFACE_MODE_RMII,
 			  port->slave.phylink_config.supported_interfaces);
 	} else if (common->pdata.extra_modes & BIT(port->slave.phy_if)) {
-		__set_bit(PHY_INTERFACE_MODE_QSGMII,
-			  port->slave.phylink_config.supported_interfaces);
+		if (port->slave.phy_if == PHY_INTERFACE_MODE_QSGMII)
+			__set_bit(PHY_INTERFACE_MODE_QSGMII,
+				  port->slave.phylink_config.supported_interfaces);
+		else
+			__set_bit(PHY_INTERFACE_MODE_SGMII,
+				  port->slave.phylink_config.supported_interfaces);
 	} else {
 		dev_err(dev, "selected phy-mode is not supported\n");
 		return -EOPNOTSUPP;
@@ -2744,7 +2757,7 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
 	.quirks = 0,
 	.ale_dev_id = "am64-cpswxg",
 	.fdqring_mode = K3_RINGACC_RING_MODE_RING,
-	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
> Add support for SGMII mode in both fixed-link MAC2MAC master mode and
> MAC2PHY modes for CPSW5G ports.
> 
> Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.
> 
> The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
> PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
> their device.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 1739c389af20..3f40178436ff 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -75,7 +75,15 @@
>  #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
>  
>  #define AM65_CPSW_SGMII_CONTROL_REG		0x010
> +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018

This doesn't seem to be used in this patch, should it be part of some
other patch in the series?

>  #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
> +#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)

Ditto.

> +
> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)

Most of the above don't seem to be used, and the only one that seems to
be used is used in a variable declaration where the variable isn't used,
and thus us also unused.

>  
>  #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
>  #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
> @@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>  	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
>  							  phylink_config);
>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
> +	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;

This doesn't seem to be used; should it be part of a different patch?

I get the impression that most of this patch should be elsewhere in this
series.

>  	struct am65_cpsw_common *common = port->common;
>  	struct fwnode_handle *fwnode;
>  	bool fixed_link = false;
> @@ -2105,8 +2114,12 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
>  		__set_bit(PHY_INTERFACE_MODE_RMII,
>  			  port->slave.phylink_config.supported_interfaces);
>  	} else if (common->pdata.extra_modes & BIT(port->slave.phy_if)) {
> -		__set_bit(PHY_INTERFACE_MODE_QSGMII,
> -			  port->slave.phylink_config.supported_interfaces);
> +		if (port->slave.phy_if == PHY_INTERFACE_MODE_QSGMII)
> +			__set_bit(PHY_INTERFACE_MODE_QSGMII,
> +				  port->slave.phylink_config.supported_interfaces);
> +		else
> +			__set_bit(PHY_INTERFACE_MODE_SGMII,
> +				  port->slave.phylink_config.supported_interfaces);
>  	} else {
>  		dev_err(dev, "selected phy-mode is not supported\n");
>  		return -EOPNOTSUPP;
> @@ -2744,7 +2757,7 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
>  	.quirks = 0,
>  	.ale_dev_id = "am64-cpswxg",
>  	.fdqring_mode = K3_RINGACC_RING_MODE_RING,
> -	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
> +	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
>  };
>  
>  static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
> -- 
> 2.25.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
> Add support for SGMII mode in both fixed-link MAC2MAC master mode and
> MAC2PHY modes for CPSW5G ports.
> 
> Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.
> 
> The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
> PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
> their device.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 1739c389af20..3f40178436ff 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -75,7 +75,15 @@
>  #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
>  
>  #define AM65_CPSW_SGMII_CONTROL_REG		0x010
> +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018

This doesn't seem to be used in this patch, should it be part of some
other patch in the series?

>  #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
> +#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)

Ditto.

> +
> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)

Most of the above don't seem to be used, and the only one that seems to
be used is used in a variable declaration where the variable isn't used,
and thus us also unused.

>  
>  #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
>  #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
> @@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>  	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
>  							  phylink_config);
>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
> +	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;

This doesn't seem to be used; should it be part of a different patch?

I get the impression that most of this patch should be elsewhere in this
series.

>  	struct am65_cpsw_common *common = port->common;
>  	struct fwnode_handle *fwnode;
>  	bool fixed_link = false;
> @@ -2105,8 +2114,12 @@ am65_cpsw_nuss_init_port_ndev(struct am65_cpsw_common *common, u32 port_idx)
>  		__set_bit(PHY_INTERFACE_MODE_RMII,
>  			  port->slave.phylink_config.supported_interfaces);
>  	} else if (common->pdata.extra_modes & BIT(port->slave.phy_if)) {
> -		__set_bit(PHY_INTERFACE_MODE_QSGMII,
> -			  port->slave.phylink_config.supported_interfaces);
> +		if (port->slave.phy_if == PHY_INTERFACE_MODE_QSGMII)
> +			__set_bit(PHY_INTERFACE_MODE_QSGMII,
> +				  port->slave.phylink_config.supported_interfaces);
> +		else
> +			__set_bit(PHY_INTERFACE_MODE_SGMII,
> +				  port->slave.phylink_config.supported_interfaces);
>  	} else {
>  		dev_err(dev, "selected phy-mode is not supported\n");
>  		return -EOPNOTSUPP;
> @@ -2744,7 +2757,7 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
>  	.quirks = 0,
>  	.ale_dev_id = "am64-cpswxg",
>  	.fdqring_mode = K3_RINGACC_RING_MODE_RING,
> -	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
> +	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
>  };
>  
>  static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
> -- 
> 2.25.1
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:14, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
>> Add support for SGMII mode in both fixed-link MAC2MAC master mode and
>> MAC2PHY modes for CPSW5G ports.
>>
>> Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.
>>
>> The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
>> PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
>> their device.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
>>  1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> index 1739c389af20..3f40178436ff 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> @@ -75,7 +75,15 @@
>>  #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
>>  
>>  #define AM65_CPSW_SGMII_CONTROL_REG		0x010
>> +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018
> 
> This doesn't seem to be used in this patch, should it be part of some
> other patch in the series?
> 
>>  #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
>> +#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)
> 
> Ditto.
> 
>> +
>> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
>> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
>> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
>> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
>> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
> 
> Most of the above don't seem to be used, and the only one that seems to
> be used is used in a variable declaration where the variable isn't used,
> and thus us also unused.
> 
>>  
>>  #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
>>  #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
>> @@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>>  	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
>>  							  phylink_config);
>>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>> +	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;
> 
> This doesn't seem to be used; should it be part of a different patch?
> 
> I get the impression that most of this patch should be elsewhere in this
> series.

Thank you for pointing it out. These should have been a part of the
previous patch [PATCH 5/8]. Sorry for the confusion. I will fix this in
the v2 series.

Regards,
Siddharth.

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:14, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
>> Add support for SGMII mode in both fixed-link MAC2MAC master mode and
>> MAC2PHY modes for CPSW5G ports.
>>
>> Add SGMII mode to the list of extra_modes in j7200_cpswxg_pdata.
>>
>> The MAC2PHY mode has been tested in fixed-link mode using a bootstrapped
>> PHY. The MAC2MAC mode has been tested by a customer with J7200 SoC on
>> their device.
>>
>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
>> ---
>>  drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++++++++++++++++---
>>  1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> index 1739c389af20..3f40178436ff 100644
>> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>> @@ -75,7 +75,15 @@
>>  #define AM65_CPSW_PORTN_REG_TS_CTL_LTYPE2       0x31C
>>  
>>  #define AM65_CPSW_SGMII_CONTROL_REG		0x010
>> +#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG	0x018
> 
> This doesn't seem to be used in this patch, should it be part of some
> other patch in the series?
> 
>>  #define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE	BIT(0)
>> +#define AM65_CPSW_SGMII_CONTROL_MASTER_MODE	BIT(5)
> 
> Ditto.
> 
>> +
>> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
>> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
>> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
>> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
>> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
> 
> Most of the above don't seem to be used, and the only one that seems to
> be used is used in a variable declaration where the variable isn't used,
> and thus us also unused.
> 
>>  
>>  #define AM65_CPSW_CTL_VLAN_AWARE		BIT(1)
>>  #define AM65_CPSW_CTL_P0_ENABLE			BIT(2)
>> @@ -1493,6 +1501,7 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
>>  	struct am65_cpsw_slave_data *slave = container_of(config, struct am65_cpsw_slave_data,
>>  							  phylink_config);
>>  	struct am65_cpsw_port *port = container_of(slave, struct am65_cpsw_port, slave);
>> +	u32 mr_adv_ability = MAC2MAC_MR_ADV_ABILITY_BASE;
> 
> This doesn't seem to be used; should it be part of a different patch?
> 
> I get the impression that most of this patch should be elsewhere in this
> series.

Thank you for pointing it out. These should have been a part of the
previous patch [PATCH 5/8]. Sorry for the confusion. I will fix this in
the v2 series.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)

In addition to my other comments, this looks like a reimplementation of
the LPA_SGMII* constants found in include/uapi/linux/mii.h

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Russell King (Oracle)]

On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)

In addition to my other comments, this looks like a reimplementation of
the LPA_SGMII* constants found in include/uapi/linux/mii.h

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:34, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
>> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
>> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
>> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
>> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
>> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
> 
> In addition to my other comments, this looks like a reimplementation of
> the LPA_SGMII* constants found in include/uapi/linux/mii.h

I was not aware of this. Thank you for letting me know. I will use the
existing constants in the v2 series.

Regards,
Siddharth.

Re: [PATCH 6/8] net: ethernet: ti: am65-cpsw: Add support for SGMII mode for J7200 CPSW5G

[Siddharth Vadapalli]

Hello Russell,

On 14/09/22 21:34, Russell King (Oracle) wrote:
> On Wed, Sep 14, 2022 at 03:20:51PM +0530, Siddharth Vadapalli wrote:
>> +#define MAC2MAC_MR_ADV_ABILITY_BASE		(BIT(15) | BIT(0))
>> +#define MAC2MAC_MR_ADV_ABILITY_FULLDUPLEX	BIT(12)
>> +#define MAC2MAC_MR_ADV_ABILITY_1G		BIT(11)
>> +#define MAC2MAC_MR_ADV_ABILITY_100M		BIT(10)
>> +#define MAC2PHY_MR_ADV_ABILITY			BIT(0)
> 
> In addition to my other comments, this looks like a reimplementation of
> the LPA_SGMII* constants found in include/uapi/linux/mii.h

I was not aware of this. Thank you for letting me know. I will use the
existing constants in the v2 series.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 7/8] net: ethernet: ti: am65-cpsw: Add support for J721e CPSW9G

[Siddharth Vadapalli]

CPSW9G in J721e supports additional modes like QSGMII and SGMII.
Add new compatible for J721e in am65-cpsw driver.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 3f40178436ff..65114f233550 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2760,11 +2760,19 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
 	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
+static const struct am65_cpsw_pdata j721e_cpswxg_pdata = {
+	.quirks = 0,
+	.ale_dev_id = "am64-cpswxg",
+	.fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+};
+
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
 	{ .compatible = "ti,am654-cpsw-nuss", .data = &am65x_sr1_0},
 	{ .compatible = "ti,j721e-cpsw-nuss", .data = &j721e_pdata},
 	{ .compatible = "ti,am642-cpsw-nuss", .data = &am64x_cpswxg_pdata},
 	{ .compatible = "ti,j7200-cpswxg-nuss", .data = &j7200_cpswxg_pdata},
+	{ .compatible = "ti,j721e-cpswxg-nuss", .data = &j721e_cpswxg_pdata},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, am65_cpsw_nuss_of_mtable);
-- 
2.25.1

[PATCH 7/8] net: ethernet: ti: am65-cpsw: Add support for J721e CPSW9G

[Siddharth Vadapalli]

CPSW9G in J721e supports additional modes like QSGMII and SGMII.
Add new compatible for J721e in am65-cpsw driver.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 3f40178436ff..65114f233550 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2760,11 +2760,19 @@ static const struct am65_cpsw_pdata j7200_cpswxg_pdata = {
 	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
+static const struct am65_cpsw_pdata j721e_cpswxg_pdata = {
+	.quirks = 0,
+	.ale_dev_id = "am64-cpswxg",
+	.fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+};
+
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
 	{ .compatible = "ti,am654-cpsw-nuss", .data = &am65x_sr1_0},
 	{ .compatible = "ti,j721e-cpsw-nuss", .data = &j721e_pdata},
 	{ .compatible = "ti,am642-cpsw-nuss", .data = &am64x_cpswxg_pdata},
 	{ .compatible = "ti,j7200-cpswxg-nuss", .data = &j7200_cpswxg_pdata},
+	{ .compatible = "ti,j721e-cpswxg-nuss", .data = &j721e_cpswxg_pdata},
 	{ /* sentinel */ },
 };
 MODULE_DEVICE_TABLE(of, am65_cpsw_nuss_of_mtable);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 8/8] net: ethernet: ti: am65-cpsw: Enable SGMII mode for J721e CPSW9G

[Siddharth Vadapalli]

Add SGMII mode to the list of extra_modes in j721e_cpswxg_pdata.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 65114f233550..442e87055cf3 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2764,7 +2764,7 @@ static const struct am65_cpsw_pdata j721e_cpswxg_pdata = {
 	.quirks = 0,
 	.ale_dev_id = "am64-cpswxg",
 	.fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
-	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
-- 
2.25.1

[PATCH 8/8] net: ethernet: ti: am65-cpsw: Enable SGMII mode for J721e CPSW9G

[Siddharth Vadapalli]

Add SGMII mode to the list of extra_modes in j721e_cpswxg_pdata.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 65114f233550..442e87055cf3 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2764,7 +2764,7 @@ static const struct am65_cpsw_pdata j721e_cpswxg_pdata = {
 	.quirks = 0,
 	.ale_dev_id = "am64-cpswxg",
 	.fdqring_mode = K3_RINGACC_RING_MODE_MESSAGE,
-	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII),
+	.extra_modes = BIT(PHY_INTERFACE_MODE_QSGMII) | BIT(PHY_INTERFACE_MODE_SGMII),
 };
 
 static const struct of_device_id am65_cpsw_nuss_of_mtable[] = {
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Krzysztof Kozlowski]

On 14/09/2022 10:50, Siddharth Vadapalli wrote:
> Add compatible for J721e CPSW9G.
> 
> Add support to power on and configure SERDES PHY.
> 
> Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.

I got two same patchsets from you... version your patches instead. See
submitting-patches doc.

Best regards,
Krzysztof

Re: [PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Krzysztof Kozlowski]

On 14/09/2022 10:50, Siddharth Vadapalli wrote:
> Add compatible for J721e CPSW9G.
> 
> Add support to power on and configure SERDES PHY.
> 
> Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.

I got two same patchsets from you... version your patches instead. See
submitting-patches doc.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Siddharth Vadapalli]

Hello Krzysztof,

On 16/09/22 15:27, Krzysztof Kozlowski wrote:
> On 14/09/2022 10:50, Siddharth Vadapalli wrote:
>> Add compatible for J721e CPSW9G.
>>
>> Add support to power on and configure SERDES PHY.
>>
>> Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.
> 
> I got two same patchsets from you... version your patches instead. See
> submitting-patches doc.

I had posted two series, both v1, with the series corresponding to
patches meant for the PHY and the NET subsystems. I did not realize
while posting, that the cover letters for both of them had the same
subject, making them appear as the same series, but of different
versions. I apologize for the confusion.

Regards,
Siddharth.

Re: [PATCH 0/8] Add support for J721e CPSW9G and SGMII mode

[Siddharth Vadapalli]

Hello Krzysztof,

On 16/09/22 15:27, Krzysztof Kozlowski wrote:
> On 14/09/2022 10:50, Siddharth Vadapalli wrote:
>> Add compatible for J721e CPSW9G.
>>
>> Add support to power on and configure SERDES PHY.
>>
>> Add support for SGMII mode for J7200 CPSW5G and J721e CPSW9G.
> 
> I got two same patchsets from you... version your patches instead. See
> submitting-patches doc.

I had posted two series, both v1, with the series corresponding to
patches meant for the PHY and the NET subsystems. I did not realize
while posting, that the cover letters for both of them had the same
subject, making them appear as the same series, but of different
versions. I apologize for the confusion.

Regards,
Siddharth.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel