All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Stuebner <heiko@sntech.de>
To: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Sebastian Reichel <sre@kernel.org>
Cc: Shawn Lin <shawn.lin@rock-chips.com>,
	Michal Tomek <mtdev79b@gmail.com>,
	linux-phy@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: Re: [PATCH 3/3] phy: rockchip: naneng-combphy: Fix mux on rk3588
Date: Thu, 04 Apr 2024 21:30:52 +0200	[thread overview]
Message-ID: <2643155.ElGaqSPkdT@phil> (raw)
In-Reply-To: <20240404-rk3588-pcie-bifurcation-fixes-v1-3-9907136eeafd@kernel.org>

Am Donnerstag, 4. April 2024, 19:11:28 CEST schrieb Sebastian Reichel:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> The pcie1l0_sel and pcie1l1_sel bits in PCIESEL_CON configure the
> mux for PCIe1L0 and PCIe1L1 to either the PIPE Combo PHYs or the
> PCIe3 PHY. Thus this configuration interfers with the data-lanes
> configuration done by the PCIe3 PHY.
> 
> RK3588 has three Combo PHYs. The first one has a dedicated PCIe
> controller and is not affected by this. For the other two Combo
> PHYs, there is one mux for each of them.
> 
> pcie1l0_sel selects if PCIe 1L0 is muxed to Combo PHY 1 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> pcie1l1_sel selects if PCIe 1L1 is muxed to Combo PHY 2 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> Currently the code always muxes 1L0 and 1L1 to the Combi PHYs
> once one of them is being used in PCIe mode. This is obviously
> wrong when at least one of the ports should be muxed to the
> PCIe3 PHY.
> 
> Fix this by introducing Combo PHY identification and then only
> setting up the required bit.
> 
> Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
> Reported-by: Michal Tomek <mtdev79b@gmail.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 36 ++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 76b9cf417591..bf74e429ff46 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -125,12 +125,15 @@ struct rockchip_combphy_grfcfg {
>  };
>  
>  struct rockchip_combphy_cfg {
> +	unsigned int num_phys;
> +	unsigned int phy_ids[3];
>  	const struct rockchip_combphy_grfcfg *grfcfg;
>  	int (*combphy_cfg)(struct rockchip_combphy_priv *priv);
>  };
>  
>  struct rockchip_combphy_priv {
>  	u8 type;
> +	int id;
>  	void __iomem *mmio;
>  	int num_clks;
>  	struct clk_bulk_data *clks;
> @@ -320,7 +323,7 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  	struct rockchip_combphy_priv *priv;
>  	const struct rockchip_combphy_cfg *phy_cfg;
>  	struct resource *res;
> -	int ret;
> +	int ret, id;
>  
>  	phy_cfg = of_device_get_match_data(dev);
>  	if (!phy_cfg) {
> @@ -338,6 +341,15 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* find the phy-id from the io address */
> +	priv->id = -ENODEV;
> +	for (id = 0; id < phy_cfg->num_phys; id++) {
> +		if (res->start == phy_cfg->phy_ids[id]) {
> +			priv->id = id;
> +			break;
> +		}
> +	}
> +
>  	priv->dev = dev;
>  	priv->type = PHY_NONE;
>  	priv->cfg = phy_cfg;
> @@ -562,6 +574,12 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3568_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfe820000,
> +		0xfe830000,
> +		0xfe840000,
> +	},
>  	.grfcfg		= &rk3568_combphy_grfcfgs,
>  	.combphy_cfg	= rk3568_combphy_cfg,
>  };
> @@ -578,8 +596,14 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +		switch (priv->id) {
> +		case 1:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> +			break;
> +		case 2:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +			break;
> +		}
>  		break;
>  	case PHY_TYPE_USB3:
>  		/* Set SSC downward spread spectrum */
> @@ -736,6 +760,12 @@ static const struct rockchip_combphy_grfcfg rk3588_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfee00000,
> +		0xfee10000,
> +		0xfee20000,
> +	},
>  	.grfcfg		= &rk3588_combphy_grfcfgs,
>  	.combphy_cfg	= rk3588_combphy_cfg,
>  };
> 
> 





-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Sebastian Reichel <sre@kernel.org>
Cc: Shawn Lin <shawn.lin@rock-chips.com>,
	Michal Tomek <mtdev79b@gmail.com>,
	linux-phy@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: Re: [PATCH 3/3] phy: rockchip: naneng-combphy: Fix mux on rk3588
Date: Thu, 04 Apr 2024 21:30:52 +0200	[thread overview]
Message-ID: <2643155.ElGaqSPkdT@phil> (raw)
In-Reply-To: <20240404-rk3588-pcie-bifurcation-fixes-v1-3-9907136eeafd@kernel.org>

Am Donnerstag, 4. April 2024, 19:11:28 CEST schrieb Sebastian Reichel:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> The pcie1l0_sel and pcie1l1_sel bits in PCIESEL_CON configure the
> mux for PCIe1L0 and PCIe1L1 to either the PIPE Combo PHYs or the
> PCIe3 PHY. Thus this configuration interfers with the data-lanes
> configuration done by the PCIe3 PHY.
> 
> RK3588 has three Combo PHYs. The first one has a dedicated PCIe
> controller and is not affected by this. For the other two Combo
> PHYs, there is one mux for each of them.
> 
> pcie1l0_sel selects if PCIe 1L0 is muxed to Combo PHY 1 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> pcie1l1_sel selects if PCIe 1L1 is muxed to Combo PHY 2 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> Currently the code always muxes 1L0 and 1L1 to the Combi PHYs
> once one of them is being used in PCIe mode. This is obviously
> wrong when at least one of the ports should be muxed to the
> PCIe3 PHY.
> 
> Fix this by introducing Combo PHY identification and then only
> setting up the required bit.
> 
> Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
> Reported-by: Michal Tomek <mtdev79b@gmail.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 36 ++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 76b9cf417591..bf74e429ff46 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -125,12 +125,15 @@ struct rockchip_combphy_grfcfg {
>  };
>  
>  struct rockchip_combphy_cfg {
> +	unsigned int num_phys;
> +	unsigned int phy_ids[3];
>  	const struct rockchip_combphy_grfcfg *grfcfg;
>  	int (*combphy_cfg)(struct rockchip_combphy_priv *priv);
>  };
>  
>  struct rockchip_combphy_priv {
>  	u8 type;
> +	int id;
>  	void __iomem *mmio;
>  	int num_clks;
>  	struct clk_bulk_data *clks;
> @@ -320,7 +323,7 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  	struct rockchip_combphy_priv *priv;
>  	const struct rockchip_combphy_cfg *phy_cfg;
>  	struct resource *res;
> -	int ret;
> +	int ret, id;
>  
>  	phy_cfg = of_device_get_match_data(dev);
>  	if (!phy_cfg) {
> @@ -338,6 +341,15 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* find the phy-id from the io address */
> +	priv->id = -ENODEV;
> +	for (id = 0; id < phy_cfg->num_phys; id++) {
> +		if (res->start == phy_cfg->phy_ids[id]) {
> +			priv->id = id;
> +			break;
> +		}
> +	}
> +
>  	priv->dev = dev;
>  	priv->type = PHY_NONE;
>  	priv->cfg = phy_cfg;
> @@ -562,6 +574,12 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3568_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfe820000,
> +		0xfe830000,
> +		0xfe840000,
> +	},
>  	.grfcfg		= &rk3568_combphy_grfcfgs,
>  	.combphy_cfg	= rk3568_combphy_cfg,
>  };
> @@ -578,8 +596,14 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +		switch (priv->id) {
> +		case 1:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> +			break;
> +		case 2:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +			break;
> +		}
>  		break;
>  	case PHY_TYPE_USB3:
>  		/* Set SSC downward spread spectrum */
> @@ -736,6 +760,12 @@ static const struct rockchip_combphy_grfcfg rk3588_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfee00000,
> +		0xfee10000,
> +		0xfee20000,
> +	},
>  	.grfcfg		= &rk3588_combphy_grfcfgs,
>  	.combphy_cfg	= rk3588_combphy_cfg,
>  };
> 
> 





_______________________________________________
Linux-rockchip mailing list
Linux-rockchip@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Stuebner <heiko@sntech.de>
To: Vinod Koul <vkoul@kernel.org>,
	Kishon Vijay Abraham I <kishon@kernel.org>,
	Sebastian Reichel <sre@kernel.org>
Cc: Shawn Lin <shawn.lin@rock-chips.com>,
	Michal Tomek <mtdev79b@gmail.com>,
	linux-phy@lists.infradead.org,
	linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
	Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: Re: [PATCH 3/3] phy: rockchip: naneng-combphy: Fix mux on rk3588
Date: Thu, 04 Apr 2024 21:30:52 +0200	[thread overview]
Message-ID: <2643155.ElGaqSPkdT@phil> (raw)
In-Reply-To: <20240404-rk3588-pcie-bifurcation-fixes-v1-3-9907136eeafd@kernel.org>

Am Donnerstag, 4. April 2024, 19:11:28 CEST schrieb Sebastian Reichel:
> From: Sebastian Reichel <sebastian.reichel@collabora.com>
> 
> The pcie1l0_sel and pcie1l1_sel bits in PCIESEL_CON configure the
> mux for PCIe1L0 and PCIe1L1 to either the PIPE Combo PHYs or the
> PCIe3 PHY. Thus this configuration interfers with the data-lanes
> configuration done by the PCIe3 PHY.
> 
> RK3588 has three Combo PHYs. The first one has a dedicated PCIe
> controller and is not affected by this. For the other two Combo
> PHYs, there is one mux for each of them.
> 
> pcie1l0_sel selects if PCIe 1L0 is muxed to Combo PHY 1 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> pcie1l1_sel selects if PCIe 1L1 is muxed to Combo PHY 2 when
> bit is set to 0 or to the PCIe3 PHY when bit is set to 1.
> 
> Currently the code always muxes 1L0 and 1L1 to the Combi PHYs
> once one of them is being used in PCIe mode. This is obviously
> wrong when at least one of the ports should be muxed to the
> PCIe3 PHY.
> 
> Fix this by introducing Combo PHY identification and then only
> setting up the required bit.
> 
> Fixes: a03c44277253 ("phy: rockchip: Add naneng combo phy support for RK3588")
> Reported-by: Michal Tomek <mtdev79b@gmail.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Reviewed-by: Heiko Stuebner <heiko@sntech.de>

> ---
>  drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 36 ++++++++++++++++++++--
>  1 file changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index 76b9cf417591..bf74e429ff46 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -125,12 +125,15 @@ struct rockchip_combphy_grfcfg {
>  };
>  
>  struct rockchip_combphy_cfg {
> +	unsigned int num_phys;
> +	unsigned int phy_ids[3];
>  	const struct rockchip_combphy_grfcfg *grfcfg;
>  	int (*combphy_cfg)(struct rockchip_combphy_priv *priv);
>  };
>  
>  struct rockchip_combphy_priv {
>  	u8 type;
> +	int id;
>  	void __iomem *mmio;
>  	int num_clks;
>  	struct clk_bulk_data *clks;
> @@ -320,7 +323,7 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  	struct rockchip_combphy_priv *priv;
>  	const struct rockchip_combphy_cfg *phy_cfg;
>  	struct resource *res;
> -	int ret;
> +	int ret, id;
>  
>  	phy_cfg = of_device_get_match_data(dev);
>  	if (!phy_cfg) {
> @@ -338,6 +341,15 @@ static int rockchip_combphy_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* find the phy-id from the io address */
> +	priv->id = -ENODEV;
> +	for (id = 0; id < phy_cfg->num_phys; id++) {
> +		if (res->start == phy_cfg->phy_ids[id]) {
> +			priv->id = id;
> +			break;
> +		}
> +	}
> +
>  	priv->dev = dev;
>  	priv->type = PHY_NONE;
>  	priv->cfg = phy_cfg;
> @@ -562,6 +574,12 @@ static const struct rockchip_combphy_grfcfg rk3568_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3568_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfe820000,
> +		0xfe830000,
> +		0xfe840000,
> +	},
>  	.grfcfg		= &rk3568_combphy_grfcfgs,
>  	.combphy_cfg	= rk3568_combphy_cfg,
>  };
> @@ -578,8 +596,14 @@ static int rk3588_combphy_cfg(struct rockchip_combphy_priv *priv)
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con1_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con2_for_pcie, true);
>  		rockchip_combphy_param_write(priv->phy_grf, &cfg->con3_for_pcie, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> -		rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +		switch (priv->id) {
> +		case 1:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l0_sel, true);
> +			break;
> +		case 2:
> +			rockchip_combphy_param_write(priv->pipe_grf, &cfg->pipe_pcie1l1_sel, true);
> +			break;
> +		}
>  		break;
>  	case PHY_TYPE_USB3:
>  		/* Set SSC downward spread spectrum */
> @@ -736,6 +760,12 @@ static const struct rockchip_combphy_grfcfg rk3588_combphy_grfcfgs = {
>  };
>  
>  static const struct rockchip_combphy_cfg rk3588_combphy_cfgs = {
> +	.num_phys = 3,
> +	.phy_ids = {
> +		0xfee00000,
> +		0xfee10000,
> +		0xfee20000,
> +	},
>  	.grfcfg		= &rk3588_combphy_grfcfgs,
>  	.combphy_cfg	= rk3588_combphy_cfg,
>  };
> 
> 





  reply	other threads:[~2024-04-04 19:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-04 17:11 [PATCH 0/3] RK3588 PCIe bifurcation fixes Sebastian Reichel
2024-04-04 17:11 ` Sebastian Reichel
2024-04-04 17:11 ` Sebastian Reichel
2024-04-04 17:11 ` [PATCH 1/3] phy: rockchip-snps-pcie3: fix bifurcation on rk3588 Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 19:38   ` Heiko Stuebner
2024-04-04 19:38     ` Heiko Stuebner
2024-04-04 19:38     ` Heiko Stuebner
2024-04-04 17:11 ` [PATCH 2/3] phy: rockchip-snps-pcie3: fix clearing PHP_GRF_PCIESEL_CON bits Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 19:26   ` Heiko Stuebner
2024-04-04 19:26     ` Heiko Stuebner
2024-04-04 19:26     ` Heiko Stuebner
2024-04-04 17:11 ` [PATCH 3/3] phy: rockchip: naneng-combphy: Fix mux on rk3588 Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 17:11   ` Sebastian Reichel
2024-04-04 19:30   ` Heiko Stuebner [this message]
2024-04-04 19:30     ` Heiko Stuebner
2024-04-04 19:30     ` Heiko Stuebner
2024-04-06  9:18 ` [PATCH 0/3] RK3588 PCIe bifurcation fixes Vinod Koul
2024-04-06  9:18   ` Vinod Koul
2024-04-06  9:18   ` Vinod Koul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2643155.ElGaqSPkdT@phil \
    --to=heiko@sntech.de \
    --cc=kishon@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=linux-rockchip@lists.infradead.org \
    --cc=mtdev79b@gmail.com \
    --cc=sebastian.reichel@collabora.com \
    --cc=shawn.lin@rock-chips.com \
    --cc=sre@kernel.org \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.