[PATCH 0/4] phy: rockchip: snps-pcie3: Fix bifurcation and spelling

[PATCH 0/4] phy: rockchip: snps-pcie3: Fix bifurcation and spelling

[Sebastian Kropatsch]

Several fixes for RK3588's PCIe PHY from upstream Linux [1][2], which
uses a very similar driver.
Tested on a FriendlyElec CM3588 NAS which uses PCIe bifurcation in
1x1x1x1 mode to enable four M.2 NVMe sockets. With these fixes applied,
NVMe SSDs get properly recognized in U-Boot.

While at it, correct a few instances where "rockchip" was written as
"rochchip" in two Rockchip PHY files.

Cheers,
Sebastian

Links:
[1] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c?id=f8020dfb311d2b6cf657668792aaa5fa8863a7dd
[2] https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c?id=55491a5fa163bf15158f34f3650b3985f25622b9

---
Sebastian Kropatsch (4):
  phy: rockchip: naneng-combphy: Fix "rockchip" spelling
  phy: rockchip: snps-pcie3: Fix "rockchip" spelling
  phy: rockchip: snps-pcie3: Fix bifurcation for RK3588
  phy: rockchip: snps-pcie3: Fix clearing PHP_GRF_PCIESEL_CON bits

 .../rockchip/phy-rockchip-naneng-combphy.c    |  4 +-
 .../phy/rockchip/phy-rockchip-snps-pcie3.c    | 42 ++++++++-----------
 2 files changed, 20 insertions(+), 26 deletions(-)

-- 
2.43.0

[PATCH 1/4] phy: rockchip: naneng-combphy: Fix "rockchip" spelling

[Sebastian Kropatsch]

Replace "rochchip" by "rockchip" in two instances.

Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
---
 drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index 3ad339bccc..1b85cbcce8 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -225,7 +225,7 @@ static int rockchip_combphy_xlate(struct phy *phy, struct ofnode_phandle_args *a
 	return 0;
 }
 
-static const struct phy_ops rochchip_combphy_ops = {
+static const struct phy_ops rockchip_combphy_ops = {
 	.init = rockchip_combphy_init,
 	.exit = rockchip_combphy_exit,
 	.of_xlate = rockchip_combphy_xlate,
@@ -535,7 +535,7 @@ U_BOOT_DRIVER(rockchip_naneng_combphy) = {
 	.name		= "naneng-combphy",
 	.id		= UCLASS_PHY,
 	.of_match	= rockchip_combphy_ids,
-	.ops		= &rochchip_combphy_ops,
+	.ops		= &rockchip_combphy_ops,
 	.probe		= rockchip_combphy_probe,
 	.priv_auto	= sizeof(struct rockchip_combphy_priv),
 };
-- 
2.43.0

Re: [PATCH 1/4] phy: rockchip: naneng-combphy: Fix "rockchip" spelling

[Kever Yang]

On 2024/7/15 05:23, Sebastian Kropatsch wrote:
> Replace "rochchip" by "rockchip" in two instances.
>
> Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 3ad339bccc..1b85cbcce8 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -225,7 +225,7 @@ static int rockchip_combphy_xlate(struct phy *phy, struct ofnode_phandle_args *a
>   	return 0;
>   }
>   
> -static const struct phy_ops rochchip_combphy_ops = {
> +static const struct phy_ops rockchip_combphy_ops = {
>   	.init = rockchip_combphy_init,
>   	.exit = rockchip_combphy_exit,
>   	.of_xlate = rockchip_combphy_xlate,
> @@ -535,7 +535,7 @@ U_BOOT_DRIVER(rockchip_naneng_combphy) = {
>   	.name		= "naneng-combphy",
>   	.id		= UCLASS_PHY,
>   	.of_match	= rockchip_combphy_ids,
> -	.ops		= &rochchip_combphy_ops,
> +	.ops		= &rockchip_combphy_ops,
>   	.probe		= rockchip_combphy_probe,
>   	.priv_auto	= sizeof(struct rockchip_combphy_priv),
>   };

[PATCH 2/4] phy: rockchip: snps-pcie3: Fix "rockchip" spelling

[Sebastian Kropatsch]

Several identifiers use "rochchip" instead of "rockchip".
Fix this by replacing every instance of "rochchip" with "rockchip".

Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
---
 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 2737bd81dd..1c94875aaa 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -164,7 +164,7 @@ static const struct rockchip_p3phy_ops rk3588_ops = {
 	.phy_init = rockchip_p3phy_rk3588_init,
 };
 
-static int rochchip_p3phy_init(struct phy *phy)
+static int rockchip_p3phy_init(struct phy *phy)
 {
 	struct rockchip_p3phy_ops *ops =
 		(struct rockchip_p3phy_ops *)dev_get_driver_data(phy->dev);
@@ -185,7 +185,7 @@ static int rochchip_p3phy_init(struct phy *phy)
 	return ret;
 }
 
-static int rochchip_p3phy_exit(struct phy *phy)
+static int rockchip_p3phy_exit(struct phy *phy)
 {
 	struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev);
 
@@ -251,9 +251,9 @@ static int rockchip_p3phy_probe(struct udevice *dev)
 	return 0;
 }
 
-static struct phy_ops rochchip_p3phy_ops = {
-	.init = rochchip_p3phy_init,
-	.exit = rochchip_p3phy_exit,
+static struct phy_ops rockchip_p3phy_ops = {
+	.init = rockchip_p3phy_init,
+	.exit = rockchip_p3phy_exit,
 };
 
 static const struct udevice_id rockchip_p3phy_of_match[] = {
@@ -272,7 +272,7 @@ U_BOOT_DRIVER(rockchip_pcie3phy) = {
 	.name		= "rockchip_pcie3phy",
 	.id		= UCLASS_PHY,
 	.of_match	= rockchip_p3phy_of_match,
-	.ops		= &rochchip_p3phy_ops,
+	.ops		= &rockchip_p3phy_ops,
 	.probe		= rockchip_p3phy_probe,
 	.priv_auto	= sizeof(struct rockchip_p3phy_priv),
 };
-- 
2.43.0

Re: [PATCH 2/4] phy: rockchip: snps-pcie3: Fix "rockchip" spelling

[Kever Yang]

On 2024/7/15 05:23, Sebastian Kropatsch wrote:
> Several identifiers use "rochchip" instead of "rockchip".
> Fix this by replacing every instance of "rochchip" with "rockchip".
>
> Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index 2737bd81dd..1c94875aaa 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -164,7 +164,7 @@ static const struct rockchip_p3phy_ops rk3588_ops = {
>   	.phy_init = rockchip_p3phy_rk3588_init,
>   };
>   
> -static int rochchip_p3phy_init(struct phy *phy)
> +static int rockchip_p3phy_init(struct phy *phy)
>   {
>   	struct rockchip_p3phy_ops *ops =
>   		(struct rockchip_p3phy_ops *)dev_get_driver_data(phy->dev);
> @@ -185,7 +185,7 @@ static int rochchip_p3phy_init(struct phy *phy)
>   	return ret;
>   }
>   
> -static int rochchip_p3phy_exit(struct phy *phy)
> +static int rockchip_p3phy_exit(struct phy *phy)
>   {
>   	struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev);
>   
> @@ -251,9 +251,9 @@ static int rockchip_p3phy_probe(struct udevice *dev)
>   	return 0;
>   }
>   
> -static struct phy_ops rochchip_p3phy_ops = {
> -	.init = rochchip_p3phy_init,
> -	.exit = rochchip_p3phy_exit,
> +static struct phy_ops rockchip_p3phy_ops = {
> +	.init = rockchip_p3phy_init,
> +	.exit = rockchip_p3phy_exit,
>   };
>   
>   static const struct udevice_id rockchip_p3phy_of_match[] = {
> @@ -272,7 +272,7 @@ U_BOOT_DRIVER(rockchip_pcie3phy) = {
>   	.name		= "rockchip_pcie3phy",
>   	.id		= UCLASS_PHY,
>   	.of_match	= rockchip_p3phy_of_match,
> -	.ops		= &rochchip_p3phy_ops,
> +	.ops		= &rockchip_p3phy_ops,
>   	.probe		= rockchip_p3phy_probe,
>   	.priv_auto	= sizeof(struct rockchip_p3phy_priv),
>   };

[PATCH 3/4] phy: rockchip: snps-pcie3: Fix bifurcation for RK3588

[Sebastian Kropatsch]

Misconfigured `PHP_GRF_PCIESEL` values are causing bifurcation issues,
for example on the FriendlyElec CM3588 NAS board which uses bifurcation
on both PCIe PCIe ports (all four lanes) to enable four M.2 NVMe
sockets. Without this fix, NVMe devices do not get recognized.

Correct the `PHP_GRF_PCIESEL` register configuration and simplify the
bifurcation logic, enabling proper PCIe bifurcation based on the
data-lanes property.

This fix is adapted from the upstream Linux commit by Michal Tomek:
f8020dfb311d ("phy: rockchip-snps-pcie3: fix bifurcation on rk3588")

Fixes: 50e54e80679b ("phy: rockchip: snps-pcie3: Add support for RK3588")
Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
---
 .../phy/rockchip/phy-rockchip-snps-pcie3.c    | 24 +++++++------------
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index 1c94875aaa..fadb77c25c 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -108,7 +108,7 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
 {
 	struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev);
 	u32 reg = 0;
-	u8 mode = 0;
+	u8 mode = RK3588_LANE_AGGREGATION; /* Lane aggregation by default */
 	int ret;
 
 	/* Deassert PCIe PMA output clamp mode */
@@ -117,28 +117,20 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
 
 	/* Set bifurcation if needed */
 	for (int i = 0; i < priv->num_lanes; i++) {
-		if (!priv->lanes[i])
-			mode |= (BIT(i) << 3);
-
 		if (priv->lanes[i] > 1)
-			mode |= (BIT(i) >> 1);
-	}
-
-	if (!mode) {
-		reg = RK3588_LANE_AGGREGATION;
-	} else {
-		if (mode & (BIT(0) | BIT(1)))
-			reg |= RK3588_BIFURCATION_LANE_0_1;
-
-		if (mode & (BIT(2) | BIT(3)))
-			reg |= RK3588_BIFURCATION_LANE_2_3;
+			mode &= ~RK3588_LANE_AGGREGATION;
+		if (priv->lanes[i] == 3)
+			mode |= RK3588_BIFURCATION_LANE_0_1;
+		if (priv->lanes[i] == 4)
+			mode |= RK3588_BIFURCATION_LANE_2_3;
 	}
 
+	reg = mode;
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
 		     (0x7 << 16) | reg);
 
 	/* Set pcie1ln_sel in PHP_GRF_PCIESEL_CON */
-	reg = (mode & (BIT(6) | BIT(7))) >> 6;
+	reg = mode & 3;
 	if (reg)
 		regmap_write(priv->pipe_grf, PHP_GRF_PCIESEL_CON,
 			     (reg << 16) | reg);
-- 
2.43.0

Re: [PATCH 3/4] phy: rockchip: snps-pcie3: Fix bifurcation for RK3588

[Kever Yang]

On 2024/7/15 05:23, Sebastian Kropatsch wrote:
> Misconfigured `PHP_GRF_PCIESEL` values are causing bifurcation issues,
> for example on the FriendlyElec CM3588 NAS board which uses bifurcation
> on both PCIe PCIe ports (all four lanes) to enable four M.2 NVMe
> sockets. Without this fix, NVMe devices do not get recognized.
>
> Correct the `PHP_GRF_PCIESEL` register configuration and simplify the
> bifurcation logic, enabling proper PCIe bifurcation based on the
> data-lanes property.
>
> This fix is adapted from the upstream Linux commit by Michal Tomek:
> f8020dfb311d ("phy: rockchip-snps-pcie3: fix bifurcation on rk3588")
>
> Fixes: 50e54e80679b ("phy: rockchip: snps-pcie3: Add support for RK3588")
> Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   .../phy/rockchip/phy-rockchip-snps-pcie3.c    | 24 +++++++------------
>   1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index 1c94875aaa..fadb77c25c 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -108,7 +108,7 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
>   {
>   	struct rockchip_p3phy_priv *priv = dev_get_priv(phy->dev);
>   	u32 reg = 0;
> -	u8 mode = 0;
> +	u8 mode = RK3588_LANE_AGGREGATION; /* Lane aggregation by default */
>   	int ret;
>   
>   	/* Deassert PCIe PMA output clamp mode */
> @@ -117,28 +117,20 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
>   
>   	/* Set bifurcation if needed */
>   	for (int i = 0; i < priv->num_lanes; i++) {
> -		if (!priv->lanes[i])
> -			mode |= (BIT(i) << 3);
> -
>   		if (priv->lanes[i] > 1)
> -			mode |= (BIT(i) >> 1);
> -	}
> -
> -	if (!mode) {
> -		reg = RK3588_LANE_AGGREGATION;
> -	} else {
> -		if (mode & (BIT(0) | BIT(1)))
> -			reg |= RK3588_BIFURCATION_LANE_0_1;
> -
> -		if (mode & (BIT(2) | BIT(3)))
> -			reg |= RK3588_BIFURCATION_LANE_2_3;
> +			mode &= ~RK3588_LANE_AGGREGATION;
> +		if (priv->lanes[i] == 3)
> +			mode |= RK3588_BIFURCATION_LANE_0_1;
> +		if (priv->lanes[i] == 4)
> +			mode |= RK3588_BIFURCATION_LANE_2_3;
>   	}
>   
> +	reg = mode;
>   	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
>   		     (0x7 << 16) | reg);
>   
>   	/* Set pcie1ln_sel in PHP_GRF_PCIESEL_CON */
> -	reg = (mode & (BIT(6) | BIT(7))) >> 6;
> +	reg = mode & 3;
>   	if (reg)
>   		regmap_write(priv->pipe_grf, PHP_GRF_PCIESEL_CON,
>   			     (reg << 16) | reg);

[PATCH 4/4] phy: rockchip: snps-pcie3: Fix clearing PHP_GRF_PCIESEL_CON bits

[Sebastian Kropatsch]

The pcie1ln_sel bits for the RK3588 are getting set but not cleared due
to an incorrect write mask.
Use a newly introduced constant for the write mask to fix this.
Also introduce a GENMASK-based constant for PCIE30_PHY_MODE.

This fix is adapted from the upstream Linux commit by Sebastian Reichel:
55491a5fa163 ("phy: rockchip-snps-pcie3: fix clearing PHP_GRF_PCIESEL_CON bits")

Fixes: 50e54e80679b ("phy: rockchip: snps-pcie3: Add support for RK3588")
Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
---
 drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
index fadb77c25c..62b42d1805 100644
--- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
+++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
@@ -36,6 +36,8 @@
 #define RK3588_BIFURCATION_LANE_0_1		BIT(0)
 #define RK3588_BIFURCATION_LANE_2_3		BIT(1)
 #define RK3588_LANE_AGGREGATION			BIT(2)
+#define RK3588_PCIE1LN_SEL_EN			(GENMASK(1, 0) << 16)
+#define RK3588_PCIE30_PHY_MODE_EN		(GENMASK(2, 0) << 16)
 
 /**
  * struct rockchip_p3phy_priv - RK DW PCIe PHY state
@@ -127,13 +129,13 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
 
 	reg = mode;
 	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
-		     (0x7 << 16) | reg);
+		     RK3588_PCIE30_PHY_MODE_EN | reg);
 
 	/* Set pcie1ln_sel in PHP_GRF_PCIESEL_CON */
-	reg = mode & 3;
+	reg = mode & (RK3588_BIFURCATION_LANE_0_1 | RK3588_BIFURCATION_LANE_2_3);
 	if (reg)
 		regmap_write(priv->pipe_grf, PHP_GRF_PCIESEL_CON,
-			     (reg << 16) | reg);
+			     RK3588_PCIE1LN_SEL_EN | reg);
 
 	reset_deassert(&priv->p30phy);
 	udelay(1);
-- 
2.43.0

Re: [PATCH 4/4] phy: rockchip: snps-pcie3: Fix clearing PHP_GRF_PCIESEL_CON bits

[Kever Yang]

On 2024/7/15 05:23, Sebastian Kropatsch wrote:
> The pcie1ln_sel bits for the RK3588 are getting set but not cleared due
> to an incorrect write mask.
> Use a newly introduced constant for the write mask to fix this.
> Also introduce a GENMASK-based constant for PCIE30_PHY_MODE.
>
> This fix is adapted from the upstream Linux commit by Sebastian Reichel:
> 55491a5fa163 ("phy: rockchip-snps-pcie3: fix clearing PHP_GRF_PCIESEL_CON bits")
>
> Fixes: 50e54e80679b ("phy: rockchip: snps-pcie3: Add support for RK3588")
> Signed-off-by: Sebastian Kropatsch <seb-dev@mail.de>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>   drivers/phy/rockchip/phy-rockchip-snps-pcie3.c | 8 +++++---
>   1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> index fadb77c25c..62b42d1805 100644
> --- a/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> +++ b/drivers/phy/rockchip/phy-rockchip-snps-pcie3.c
> @@ -36,6 +36,8 @@
>   #define RK3588_BIFURCATION_LANE_0_1		BIT(0)
>   #define RK3588_BIFURCATION_LANE_2_3		BIT(1)
>   #define RK3588_LANE_AGGREGATION			BIT(2)
> +#define RK3588_PCIE1LN_SEL_EN			(GENMASK(1, 0) << 16)
> +#define RK3588_PCIE30_PHY_MODE_EN		(GENMASK(2, 0) << 16)
>   
>   /**
>    * struct rockchip_p3phy_priv - RK DW PCIe PHY state
> @@ -127,13 +129,13 @@ static int rockchip_p3phy_rk3588_init(struct phy *phy)
>   
>   	reg = mode;
>   	regmap_write(priv->phy_grf, RK3588_PCIE3PHY_GRF_CMN_CON0,
> -		     (0x7 << 16) | reg);
> +		     RK3588_PCIE30_PHY_MODE_EN | reg);
>   
>   	/* Set pcie1ln_sel in PHP_GRF_PCIESEL_CON */
> -	reg = mode & 3;
> +	reg = mode & (RK3588_BIFURCATION_LANE_0_1 | RK3588_BIFURCATION_LANE_2_3);
>   	if (reg)
>   		regmap_write(priv->pipe_grf, PHP_GRF_PCIESEL_CON,
> -			     (reg << 16) | reg);
> +			     RK3588_PCIE1LN_SEL_EN | reg);
>   
>   	reset_deassert(&priv->p30phy);
>   	udelay(1);