[v1] phy: qcom-qmp: more fixes and cleanups

[PATCH 04/13] phy: qcom-qmp-pcie: unify ipq registers

Posted by Johan Hovold 3 years, 4 months ago

The IPQ register array is identical to sm8250_pcie_regs_layout so drop
the former.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
index ec453f908f1d..7b3f7e42edd5 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
@@ -86,13 +86,6 @@ enum qphy_reg_layout {
 	QPHY_LAYOUT_SIZE
 };
 
-static const unsigned int ipq_pciephy_gen3_regs_layout[QPHY_LAYOUT_SIZE] = {
-	[QPHY_SW_RESET]				= 0x00,
-	[QPHY_START_CTRL]			= 0x44,
-	[QPHY_PCS_STATUS]			= 0x14,
-	[QPHY_PCS_POWER_DOWN_CONTROL]		= 0x40,
-};
-
 static const unsigned int pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
 	[QPHY_SW_RESET]			= 0x00,
 	[QPHY_START_CTRL]		= 0x08,
@@ -1492,7 +1485,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
 	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
 	.vreg_list		= NULL,
 	.num_vregs		= 0,
-	.regs			= ipq_pciephy_gen3_regs_layout,
+	.regs			= sm8250_pcie_regs_layout,
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
@@ -1523,7 +1516,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
 	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
 	.vreg_list		= NULL,
 	.num_vregs		= 0,
-	.regs			= ipq_pciephy_gen3_regs_layout,
+	.regs			= sm8250_pcie_regs_layout,
 
 	.start_ctrl		= SERDES_START | PCS_START,
 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
-- 
2.35.1

Re: [PATCH 04/13] phy: qcom-qmp-pcie: unify ipq registers

Posted by Dmitry Baryshkov 3 years, 4 months ago


On 28 September 2022 18:28:13 GMT+03:00, Johan Hovold <johan+linaro@kernel.org> wrote:
>The IPQ register array is identical to sm8250_pcie_regs_layout so drop
>the former.

I'd not do such merge. They belong to different generations. I'd suggest changing these arrays to use symbolic names defined in corresponding qmp headers.

>
>Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>---
> drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
>diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>index ec453f908f1d..7b3f7e42edd5 100644
>--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcie.c
>@@ -86,13 +86,6 @@ enum qphy_reg_layout {
> 	QPHY_LAYOUT_SIZE
> };
> 
>-static const unsigned int ipq_pciephy_gen3_regs_layout[QPHY_LAYOUT_SIZE] = {
>-	[QPHY_SW_RESET]				= 0x00,
>-	[QPHY_START_CTRL]			= 0x44,
>-	[QPHY_PCS_STATUS]			= 0x14,
>-	[QPHY_PCS_POWER_DOWN_CONTROL]		= 0x40,
>-};
>-
> static const unsigned int pciephy_regs_layout[QPHY_LAYOUT_SIZE] = {
> 	[QPHY_SW_RESET]			= 0x00,
> 	[QPHY_START_CTRL]		= 0x08,
>@@ -1492,7 +1485,7 @@ static const struct qmp_phy_cfg ipq8074_pciephy_gen3_cfg = {
> 	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
> 	.vreg_list		= NULL,
> 	.num_vregs		= 0,
>-	.regs			= ipq_pciephy_gen3_regs_layout,
>+	.regs			= sm8250_pcie_regs_layout,
> 
> 	.start_ctrl		= SERDES_START | PCS_START,
> 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,
>@@ -1523,7 +1516,7 @@ static const struct qmp_phy_cfg ipq6018_pciephy_cfg = {
> 	.num_resets		= ARRAY_SIZE(ipq8074_pciephy_reset_l),
> 	.vreg_list		= NULL,
> 	.num_vregs		= 0,
>-	.regs			= ipq_pciephy_gen3_regs_layout,
>+	.regs			= sm8250_pcie_regs_layout,
> 
> 	.start_ctrl		= SERDES_START | PCS_START,
> 	.pwrdn_ctrl		= SW_PWRDN | REFCLK_DRV_DSBL,

-- 
With best wishes
Dmitry

Re: [PATCH 04/13] phy: qcom-qmp-pcie: unify ipq registers

Posted by Johan Hovold 3 years, 4 months ago

On Wed, Sep 28, 2022 at 08:34:37PM +0300, Dmitry Baryshkov wrote:
> 
> 
> On 28 September 2022 18:28:13 GMT+03:00, Johan Hovold <johan+linaro@kernel.org> wrote:
> >The IPQ register array is identical to sm8250_pcie_regs_layout so drop
> >the former.
> 
> I'd not do such merge. They belong to different generations. I'd
> suggest changing these arrays to use symbolic names defined in
> corresponding qmp headers.

That could be done too, but this is not how these drivers are
implemented currently. Whenever the resource lists match, they end up
being reused. These values need not even change between revisions.

Johan

Re: [PATCH 04/13] phy: qcom-qmp-pcie: unify ipq registers

Posted by Neil Armstrong 3 years, 4 months ago

On 28/09/2022 17:28, Johan Hovold wrote:
> The IPQ register array is identical to sm8250_pcie_regs_layout so drop
> the former.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-pcie.c | 11 ++---------
>   1 file changed, 2 insertions(+), 9 deletions(-)
> 

<snip>

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>