diff mbox series

[v2] phy: ti: phy-j721e-wiz: fix usxgmii configuration

Message ID 20240930095745.3007057-1-s-vadapalli@ti.com
State Superseded
Headers show
Series [v2] phy: ti: phy-j721e-wiz: fix usxgmii configuration | expand

Commit Message

Siddharth Vadapalli Sept. 30, 2024, 9:57 a.m. UTC
Commit b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in
wiz driver") added support for USXGMII mode. In doing so, P0_REFCLK_SEL
was set to "pcs_mac_clk_divx1_ln_0" (0x3) and P0_STANDARD_MODE was set to
LANE_MODE_GEN1, which results in a data rate of 5.15625 Gbps. However,
since the USXGMII mode can support up to 10.3125 Gbps data rate, the
aforementioned fields should be set to "pcs_mac_clk_divx0_ln_0" (0x2) and
LANE_MODE_GEN2 respectively. Fix the configuration accordingly to truly
support USXGMII up to 10G.

Fixes: b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in wiz driver")
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

Hello,

This patch is based on commit
9852d85ec9d4 Linux 6.12-rc1
of mainline Linux.

v1:
https://lore.kernel.org/r/20240910091714.1082191-1-s-vadapalli@ti.com/
Changes since v1:
- Rebased to Linux 6.12-rc1.

Regards,
Siddharth.

 drivers/phy/ti/phy-j721e-wiz.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Roger Quadros Sept. 30, 2024, 2:06 p.m. UTC | #1
Hi Siddharth,

On 30/09/2024 12:57, Siddharth Vadapalli wrote:
> Commit b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in
> wiz driver") added support for USXGMII mode. In doing so, P0_REFCLK_SEL
> was set to "pcs_mac_clk_divx1_ln_0" (0x3) and P0_STANDARD_MODE was set to
> LANE_MODE_GEN1, which results in a data rate of 5.15625 Gbps. However,
> since the USXGMII mode can support up to 10.3125 Gbps data rate, the
> aforementioned fields should be set to "pcs_mac_clk_divx0_ln_0" (0x2) and
> LANE_MODE_GEN2 respectively. Fix the configuration accordingly to truly
> support USXGMII up to 10G.
> 
> Fixes: b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in wiz driver")
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> Hello,
> 
> This patch is based on commit
> 9852d85ec9d4 Linux 6.12-rc1
> of mainline Linux.
> 
> v1:
> https://lore.kernel.org/r/20240910091714.1082191-1-s-vadapalli@ti.com/
> Changes since v1:
> - Rebased to Linux 6.12-rc1.
> 
> Regards,
> Siddharth.
> 
>  drivers/phy/ti/phy-j721e-wiz.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> index a6c0c5607ffd..c6e846d385d2 100644
> --- a/drivers/phy/ti/phy-j721e-wiz.c
> +++ b/drivers/phy/ti/phy-j721e-wiz.c
> @@ -450,8 +450,8 @@ static int wiz_mode_select(struct wiz *wiz)
>  		} else if (wiz->lane_phy_type[i] == PHY_TYPE_USXGMII) {
>  			ret = regmap_field_write(wiz->p0_mac_src_sel[i], 0x3);
>  			ret = regmap_field_write(wiz->p0_rxfclk_sel[i], 0x3);

I'm not sure. Don't we have to change rxfclk as well?

> -			ret = regmap_field_write(wiz->p0_refclk_sel[i], 0x3);
> -			mode = LANE_MODE_GEN1;
> +			ret = regmap_field_write(wiz->p0_refclk_sel[i], 0x2);
> +			mode = LANE_MODE_GEN2;
>  		} else {
>  			continue;
>  		}
Siddharth Vadapalli Oct. 1, 2024, 3:10 a.m. UTC | #2
On Mon, Sep 30, 2024 at 05:06:35PM +0300, Roger Quadros wrote:

Hello Roger,

> Hi Siddharth,
> 
> On 30/09/2024 12:57, Siddharth Vadapalli wrote:
> > Commit b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in
> > wiz driver") added support for USXGMII mode. In doing so, P0_REFCLK_SEL
> > was set to "pcs_mac_clk_divx1_ln_0" (0x3) and P0_STANDARD_MODE was set to
> > LANE_MODE_GEN1, which results in a data rate of 5.15625 Gbps. However,
> > since the USXGMII mode can support up to 10.3125 Gbps data rate, the
> > aforementioned fields should be set to "pcs_mac_clk_divx0_ln_0" (0x2) and
> > LANE_MODE_GEN2 respectively. Fix the configuration accordingly to truly
> > support USXGMII up to 10G.
> > 
> > Fixes: b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in wiz driver")
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>

[...]

> > diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
> > index a6c0c5607ffd..c6e846d385d2 100644
> > --- a/drivers/phy/ti/phy-j721e-wiz.c
> > +++ b/drivers/phy/ti/phy-j721e-wiz.c
> > @@ -450,8 +450,8 @@ static int wiz_mode_select(struct wiz *wiz)
> >  		} else if (wiz->lane_phy_type[i] == PHY_TYPE_USXGMII) {
> >  			ret = regmap_field_write(wiz->p0_mac_src_sel[i], 0x3);
> >  			ret = regmap_field_write(wiz->p0_rxfclk_sel[i], 0x3);
> 
> I'm not sure. Don't we have to change rxfclk as well?

This patch was validated on a custom board with the J7200 SR 2.0 SoC.
The signal corresponding to the USXGMII lane of the SERDES was measured
to be 5 Gbps without this patch and 10 Gbps with this patch applied,
with no other changes required. Apart from measuring the signal, ping
was also validated.

Regards,
Siddharth.
Roger Quadros Oct. 1, 2024, 7:51 a.m. UTC | #3
On 01/10/2024 06:10, Siddharth Vadapalli wrote:
> On Mon, Sep 30, 2024 at 05:06:35PM +0300, Roger Quadros wrote:
> 
> Hello Roger,
> 
>> Hi Siddharth,
>>
>> On 30/09/2024 12:57, Siddharth Vadapalli wrote:
>>> Commit b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in
>>> wiz driver") added support for USXGMII mode. In doing so, P0_REFCLK_SEL
>>> was set to "pcs_mac_clk_divx1_ln_0" (0x3) and P0_STANDARD_MODE was set to
>>> LANE_MODE_GEN1, which results in a data rate of 5.15625 Gbps. However,
>>> since the USXGMII mode can support up to 10.3125 Gbps data rate, the
>>> aforementioned fields should be set to "pcs_mac_clk_divx0_ln_0" (0x2) and
>>> LANE_MODE_GEN2 respectively. Fix the configuration accordingly to truly
>>> support USXGMII up to 10G.
>>>
>>> Fixes: b64a85fb8f53 ("phy: ti: phy-j721e-wiz.c: Add usxgmii support in wiz driver")
>>> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> 
> [...]
> 
>>> diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
>>> index a6c0c5607ffd..c6e846d385d2 100644
>>> --- a/drivers/phy/ti/phy-j721e-wiz.c
>>> +++ b/drivers/phy/ti/phy-j721e-wiz.c
>>> @@ -450,8 +450,8 @@ static int wiz_mode_select(struct wiz *wiz)
>>>  		} else if (wiz->lane_phy_type[i] == PHY_TYPE_USXGMII) {
>>>  			ret = regmap_field_write(wiz->p0_mac_src_sel[i], 0x3);
>>>  			ret = regmap_field_write(wiz->p0_rxfclk_sel[i], 0x3);
>>
>> I'm not sure. Don't we have to change rxfclk as well?
> 
> This patch was validated on a custom board with the J7200 SR 2.0 SoC.
> The signal corresponding to the USXGMII lane of the SERDES was measured
> to be 5 Gbps without this patch and 10 Gbps with this patch applied,
> with no other changes required. Apart from measuring the signal, ping
> was also validated.

Thanks. This is useful piece off information that could go in the commit log.

Reviewed-by: Roger Quadros <rogerq@kernel.org>
diff mbox series

Patch

diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index a6c0c5607ffd..c6e846d385d2 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -450,8 +450,8 @@  static int wiz_mode_select(struct wiz *wiz)
 		} else if (wiz->lane_phy_type[i] == PHY_TYPE_USXGMII) {
 			ret = regmap_field_write(wiz->p0_mac_src_sel[i], 0x3);
 			ret = regmap_field_write(wiz->p0_rxfclk_sel[i], 0x3);
-			ret = regmap_field_write(wiz->p0_refclk_sel[i], 0x3);
-			mode = LANE_MODE_GEN1;
+			ret = regmap_field_write(wiz->p0_refclk_sel[i], 0x2);
+			mode = LANE_MODE_GEN2;
 		} else {
 			continue;
 		}