[PATCH] phy: rockchip: naneng-combphy: fix RK3588 SATA pipe_con1 register offset

[PATCH] phy: rockchip: naneng-combphy: fix RK3588 SATA pipe_con1 register offset

[Ricardo Pardini via B4 Relay]

From: Ricardo Pardini <ricardo@pardini.net>

On RK3588, the sata2 PHY speed is selected by sata2_phy_spdmode in
PHP_GRF_PHP_CON1[1:0] (offset 0x0004), with sata2_phy_rx_err in bit 2.
rk3588_combphy_grfcfgs has pipe_con1_for_sata at offset 0x0000 though,
so the value 0x2 (6.0 Gb/s, rx_err cleared) ends up in PHP_GRF_PHP_CON0
and CON1 is never written.

This leaves sata2 at its 1.5 Gb/s reset value, so the link only trains
at 1.5 Gb/s, even with a 6 Gb/s capable disk and no SControl speed
limit. sata0 and sata1 are not affected: their speed fields are in
PHP_GRF_PHP_CON0 and are set by pipe_con0_for_sata.

The register layout is described in the RK3588 TRM V1.0 Part1, section
6.17 "PHP_GRF Register Description". Rockchip fixed this in their BSP
kernel in 2022 by moving pipe_con1_for_sata to offset 0x0004.

Tested with an mSATA SSD on sata2 (combphy2_psu). Before:

  ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
  Timing O_DIRECT disk reads: 374 MB in 3.01 seconds = 124.45 MB/sec

After:

  ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
  Timing O_DIRECT disk reads: 1266 MB in 3.00 seconds = 421.78 MB/sec

Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
Cc: stable@vger.kernel.org
Link: https://github.com/rockchip-linux/kernel/commit/2f5fe2c0bbd4fa95102aadd60da1100989103aa0
Cc: Yifeng Zhao <yifeng.zhao@rock-chips.com>
Signed-off-by: Ricardo Pardini <ricardo@pardini.net>
---
This fixes the pipe_con1 GRF offset for SATA in the RK3588 combphy
table, so sata2 links train at 6 Gb/s instead of being stuck at
1.5 Gb/s.

I found it when an mSATA SSD on sata2 would only link at 1.5 Gb/s,
and traced it to Rockchip's BSP kernel, which fixed the same offset in
2022 in a commit titled "config sata2 rate to 6G for rk3588" (linked
in the patch). The RK3588 TRM V1.0 Part1 (section 6.17) agrees:
sata2_phy_spdmode is in PHP_GRF_PHP_CON1, while the sata0/sata1 speed
fields are in PHP_GRF_PHP_CON0 and already set correctly.

The misplaced write lands in PHP_GRF_PHP_CON0[2:0], which are the PCIe
link reset grant bits, and turns on the one for pcie3x2 whenever a
combphy is put in SATA mode. I don't know of any effect from that, but
this patch stops it too.

Only sata2 was tested. U-Boot has the same table and the same problem;
a fix is being sent there separately.
---
 drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 3102a99128f6..c469222dba8b 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -1395,7 +1395,7 @@ static const struct rockchip_combphy_grfcfg rk3588_combphy_grfcfgs = {
 	.con3_for_sata		= { 0x000c, 15, 0, 0x00, 0x0407 },
 	/* pipe-grf */
 	.pipe_con0_for_sata	= { 0x0000, 11, 5, 0x00, 0x22 },
-	.pipe_con1_for_sata	= { 0x0000, 2, 0, 0x00, 0x2 },
+	.pipe_con1_for_sata	= { 0x0004, 2, 0, 0x00, 0x2 },
 	.pipe_pcie1l0_sel	= { 0x0100, 0, 0, 0x01, 0x0 },
 	.pipe_pcie1l1_sel	= { 0x0100, 1, 1, 0x01, 0x0 },
 };

---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261024-rk3588-combphy-sata2-6g-03bb30151552

Best regards,
--  
Ricardo Pardini <ricardo@pardini.net>

Re: [PATCH] phy: rockchip: naneng-combphy: fix RK3588 SATA pipe_con1 register offset

[Sebastian Reichel]

[-- Attachment #1: Type: text/plain, Size: 4588 bytes --]

Hi,

On Mon, Nov 09, 2026 at 02:12:47AM +0100, Ricardo Pardini via B4 Relay wrote:
> From: Ricardo Pardini <ricardo@pardini.net>
> 
> On RK3588, the sata2 PHY speed is selected by sata2_phy_spdmode in
> PHP_GRF_PHP_CON1[1:0] (offset 0x0004), with sata2_phy_rx_err in bit 2.
> rk3588_combphy_grfcfgs has pipe_con1_for_sata at offset 0x0000 though,
> so the value 0x2 (6.0 Gb/s, rx_err cleared) ends up in PHP_GRF_PHP_CON0
> and CON1 is never written.
> 
> This leaves sata2 at its 1.5 Gb/s reset value, so the link only trains
> at 1.5 Gb/s, even with a 6 Gb/s capable disk and no SControl speed
> limit. sata0 and sata1 are not affected: their speed fields are in
> PHP_GRF_PHP_CON0 and are set by pipe_con0_for_sata.
> 
> The register layout is described in the RK3588 TRM V1.0 Part1, section
> 6.17 "PHP_GRF Register Description". Rockchip fixed this in their BSP
> kernel in 2022 by moving pipe_con1_for_sata to offset 0x0004.

Drop the last sentence. It's not relevant for somebody checking the
git log in a year or two.

> Tested with an mSATA SSD on sata2 (combphy2_psu). Before:
> 
>   ata2: SATA link up 1.5 Gbps (SStatus 113 SControl 300)
>   Timing O_DIRECT disk reads: 374 MB in 3.01 seconds = 124.45 MB/sec
> 
> After:
> 
>   ata2: SATA link up 6.0 Gbps (SStatus 133 SControl 300)
>   Timing O_DIRECT disk reads: 1266 MB in 3.00 seconds = 421.78 MB/sec
> 
> Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
> Cc: stable@vger.kernel.org
> Link: https://github.com/rockchip-linux/kernel/commit/2f5fe2c0bbd4fa95102aadd60da1100989103aa0
> Cc: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> Signed-off-by: Ricardo Pardini <ricardo@pardini.net>
> ---

The patch is 1:1 from Yifeng Zhao with a better commit message.
You should keep him as the patch author with his Signed-off-by
followed by your Signed-off-by. Feel free to also add my R-b tag.
E.g. like this:

> ... (the commit message)
> 
> Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yifeng Zhao <yifeng.zhao@rock-chips.com>
> [add proper commit message]
> Signed-off-by: Ricardo Pardini <ricardo@pardini.net>


Otherwise:

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>

> This fixes the pipe_con1 GRF offset for SATA in the RK3588 combphy
> table, so sata2 links train at 6 Gb/s instead of being stuck at
> 1.5 Gb/s.
> 
> I found it when an mSATA SSD on sata2 would only link at 1.5 Gb/s,
> and traced it to Rockchip's BSP kernel, which fixed the same offset in
> 2022 in a commit titled "config sata2 rate to 6G for rk3588" (linked
> in the patch). The RK3588 TRM V1.0 Part1 (section 6.17) agrees:
> sata2_phy_spdmode is in PHP_GRF_PHP_CON1, while the sata0/sata1 speed
> fields are in PHP_GRF_PHP_CON0 and already set correctly.
>
> The misplaced write lands in PHP_GRF_PHP_CON0[2:0], which are the PCIe
> link reset grant bits, and turns on the one for pcie3x2 whenever a
> combphy is put in SATA mode. I don't know of any effect from that, but
> this patch stops it too.

You can add the info about this writing the PCIe link reset grant
bits for pcie3x2 to the commit message and drop the previous three
paragraphs from below the fold that are basically repeating the info
from the commit message.

Greetings,

-- Sebastian

> Only sata2 was tested. U-Boot has the same table and the same problem;
> a fix is being sent there separately.
> ---
>  drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 3102a99128f6..c469222dba8b 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -1395,7 +1395,7 @@ static const struct rockchip_combphy_grfcfg rk3588_combphy_grfcfgs = {
>  	.con3_for_sata		= { 0x000c, 15, 0, 0x00, 0x0407 },
>  	/* pipe-grf */
>  	.pipe_con0_for_sata	= { 0x0000, 11, 5, 0x00, 0x22 },
> -	.pipe_con1_for_sata	= { 0x0000, 2, 0, 0x00, 0x2 },
> +	.pipe_con1_for_sata	= { 0x0004, 2, 0, 0x00, 0x2 },
>  	.pipe_pcie1l0_sel	= { 0x0100, 0, 0, 0x01, 0x0 },
>  	.pipe_pcie1l1_sel	= { 0x0100, 1, 1, 0x01, 0x0 },
>  };
> 
> ---
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
> change-id: 20261024-rk3588-combphy-sata2-6g-03bb30151552
> 
> Best regards,
> --  
> Ricardo Pardini <ricardo@pardini.net>
> 
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]