2022-10-13 02:10:38

by Richard Zhu

[permalink] [raw]
Subject: [PATCH RESEND v12 0/4] Add the iMX8MP PCIe support

Re-base to the pci-v6.1-changes of pci/next branch.
Update the cover-letter, and re-send the v12 patch-set.
This series adds the i.MX8MP PCIe support and tested on i.MX8MP
EVK board when one PCIe NVME device is used.

- i.MX8MP PCIe has reversed initial PERST bit value refer to i.MX8MQ/i.MX8MM.
Add the PHY PERST explicitly for i.MX8MP PCIe PHY.
- Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
And share as much as possible codes with i.MX8MM PCIe PHY.
- Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
driver.

Main changes v11-->v12:
- In the local down kernel(6.0-rc7 plus local codes) PM tests, i.MX8MP
PCIe encounters link failure.
To resolve this failure, the resets of i.MX8MP PCIe PHY should be always
kept 1b'1. Merge the fix into v12 patches here.

Main changes v10-->v11:
Refer to Ahmad's comments do the following changes;
- Correct the spell mistake and refine the commit log.
- Make codes indent by the member name
- Use the dev_err_probe replace the dev_err.

Main changes v9-->v10:
- Refer to Vinod's review comments, drop the array, and use the static data
structure directly in the drvdata definition.

Main changes v8-->v9:
- Split the PHY driver changes into three patches.
- To keep the format consistent, re-define the PHY_CMN_REG75, and remove
two useless BIT definitions.
- Refine the i.MX8MM PCIe PHY driver, let it more reviewable, flexible,
and easy to expand.
- Add the i.MX8MP PCIe PHY support.
- Only PHY related patches in v9, Since the others patches had been merged
by Phillipp/Shawn/Lorenzo [1].

Main changes v7-->v8:
- Add the Reviewed-by tag, no other changes.
Only two patches in v8, Since the others patches had been merged by
Phillipp/Shawn/Lorenzo [1].

<snipped.>

[1]
Shawn's tree
d50650500064 arm64: dts: imx8mp-evk: Add PCIe support
9e65987b9584 arm64: dts: imx8mp: Add iMX8MP PCIe support
5506018d3dec soc: imx: imx8mp-blk-ctrl: handle PCIe PHY resets

Philipp's tree
051d9eb40388 reset: imx7: Fix the iMX8MP PCIe PHY PERST support

Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml | 16 ++++++++--
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 142 ++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------
2 files changed, 106 insertions(+), 52 deletions(-)

[RESEND v12 1/4] dt-binding: phy: Add i.MX8MP PCIe PHY binding
[RESEND v12 2/4] phy: freescale: imx8m-pcie: Refine register
[RESEND v12 3/4] phy: freescale: imx8m-pcie: Refine i.MX8MM PCIe PHY
[RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY


2022-10-13 02:10:39

by Richard Zhu

[permalink] [raw]
Subject: [RESEND v12 1/4] dt-binding: phy: Add i.MX8MP PCIe PHY binding

Add i.MX8MP PCIe PHY binding.
On i.MX8MM, the initialized default value of PERST bit(BIT3) of
SRC_PCIEPHY_RCR is 1b'1.
But i.MX8MP has one inversed default value 1b'0 of PERST bit.

And the PERST bit should be kept 1b'1 after power and clocks are stable.
So add one more PERST explicitly for i.MX8MP PCIe PHY.

Signed-off-by: Richard Zhu <[email protected]>
Tested-by: Marek Vasut <[email protected]>
Tested-by: Richard Leitner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
Reviewed-by: Rob Herring <[email protected]>
---
.../bindings/phy/fsl,imx8-pcie-phy.yaml | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
index b6421eedece3..692783c7fd69 100644
--- a/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/fsl,imx8-pcie-phy.yaml
@@ -16,6 +16,7 @@ properties:
compatible:
enum:
- fsl,imx8mm-pcie-phy
+ - fsl,imx8mp-pcie-phy

reg:
maxItems: 1
@@ -28,11 +29,16 @@ properties:
- const: ref

resets:
- maxItems: 1
+ minItems: 1
+ maxItems: 2

reset-names:
- items:
- - const: pciephy
+ oneOf:
+ - items: # for iMX8MM
+ - const: pciephy
+ - items: # for IMX8MP
+ - const: pciephy
+ - const: perst

fsl,refclk-pad-mode:
description: |
@@ -60,6 +66,10 @@ properties:
description: A boolean property indicating the CLKREQ# signal is
not supported in the board design (optional)

+ power-domains:
+ description: PCIe PHY power domain (optional).
+ maxItems: 1
+
required:
- "#phy-cells"
- compatible
--
2.25.1

2022-10-13 02:10:57

by Richard Zhu

[permalink] [raw]
Subject: [RESEND v12 2/4] phy: freescale: imx8m-pcie: Refine register definitions

No function changes, refine PHY register definitions.
- Keep align with other CMN PHY registers, refine the definitions of
PHY_CMN_REG75.
- Remove two BIT definitions that are not used at all.

Signed-off-by: Richard Zhu <[email protected]>
Signed-off-by: Lucas Stach <[email protected]>
Tested-by: Marek Vasut <[email protected]>
Tested-by: Richard Leitner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 11 ++++-------
1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index c93286483b42..3c8c255499cd 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -31,12 +31,10 @@
#define IMX8MM_PCIE_PHY_CMN_REG065 0x194
#define ANA_AUX_RX_TERM (BIT(7) | BIT(4))
#define ANA_AUX_TX_LVL GENMASK(3, 0)
-#define IMX8MM_PCIE_PHY_CMN_REG75 0x1D4
-#define PCIE_PHY_CMN_REG75_PLL_DONE 0x3
+#define IMX8MM_PCIE_PHY_CMN_REG075 0x1D4
+#define ANA_PLL_DONE 0x3
#define PCIE_PHY_TRSV_REG5 0x414
-#define PCIE_PHY_TRSV_REG5_GEN1_DEEMP 0x2D
#define PCIE_PHY_TRSV_REG6 0x418
-#define PCIE_PHY_TRSV_REG6_GEN2_DEEMP 0xF

#define IMX8MM_GPR_PCIE_REF_CLK_SEL GENMASK(25, 24)
#define IMX8MM_GPR_PCIE_REF_CLK_PLL FIELD_PREP(IMX8MM_GPR_PCIE_REF_CLK_SEL, 0x3)
@@ -131,9 +129,8 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
reset_control_deassert(imx8_phy->reset);

/* Polling to check the phy is ready or not. */
- ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG75,
- val, val == PCIE_PHY_CMN_REG75_PLL_DONE,
- 10, 20000);
+ ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
+ val, val == ANA_PLL_DONE, 10, 20000);
return ret;
}

--
2.25.1

2022-10-13 02:11:04

by Richard Zhu

[permalink] [raw]
Subject: [RESEND v12 3/4] phy: freescale: imx8m-pcie: Refine i.MX8MM PCIe PHY driver

To make it more flexible and easy to expand. Refine i.MX8MM PCIe PHY
driver.
- Use gpr compatible string to avoid the codes duplications when add
another platform PCIe PHY support.
- Re-arrange the codes to let it more flexible and easy to expand.
No functional change. Re-arrange the TX tuning, since internal registers
can be wrote through APB interface before assertion of CMN_RST.

Signed-off-by: Richard Zhu <[email protected]>
Signed-off-by: Lucas Stach <[email protected]>
Tested-by: Marek Vasut <[email protected]>
Tested-by: Richard Leitner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
Reviewed-by: Ahmad Fatoum <[email protected]>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 106 +++++++++++++--------
1 file changed, 66 insertions(+), 40 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 3c8c255499cd..3e494612db3c 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -11,6 +11,7 @@
#include <linux/mfd/syscon.h>
#include <linux/mfd/syscon/imx7-iomuxc-gpr.h>
#include <linux/module.h>
+#include <linux/of_device.h>
#include <linux/phy/phy.h>
#include <linux/platform_device.h>
#include <linux/regmap.h>
@@ -45,6 +46,15 @@
#define IMX8MM_GPR_PCIE_SSC_EN BIT(16)
#define IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE BIT(9)

+enum imx8_pcie_phy_type {
+ IMX8MM,
+};
+
+struct imx8_pcie_phy_drvdata {
+ const char *gpr;
+ enum imx8_pcie_phy_type variant;
+};
+
struct imx8_pcie_phy {
void __iomem *base;
struct clk *clk;
@@ -55,6 +65,7 @@ struct imx8_pcie_phy {
u32 tx_deemph_gen1;
u32 tx_deemph_gen2;
bool clkreq_unused;
+ const struct imx8_pcie_phy_drvdata *drvdata;
};

static int imx8_pcie_phy_power_on(struct phy *phy)
@@ -66,31 +77,17 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
reset_control_assert(imx8_phy->reset);

pad_mode = imx8_phy->refclk_pad_mode;
- /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE,
- imx8_phy->clkreq_unused ?
- 0 : IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_AUX_EN,
- IMX8MM_GPR_PCIE_AUX_EN);
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_POWER_OFF, 0);
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_SSC_EN, 0);
-
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_REF_CLK_SEL,
- pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ?
- IMX8MM_GPR_PCIE_REF_CLK_EXT :
- IMX8MM_GPR_PCIE_REF_CLK_PLL);
- usleep_range(100, 200);
-
- /* Do the PHY common block reset */
- regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
- IMX8MM_GPR_PCIE_CMN_RST,
- IMX8MM_GPR_PCIE_CMN_RST);
- usleep_range(200, 500);
+ switch (imx8_phy->drvdata->variant) {
+ case IMX8MM:
+ /* Tune PHY de-emphasis setting to pass PCIe compliance. */
+ if (imx8_phy->tx_deemph_gen1)
+ writel(imx8_phy->tx_deemph_gen1,
+ imx8_phy->base + PCIE_PHY_TRSV_REG5);
+ if (imx8_phy->tx_deemph_gen2)
+ writel(imx8_phy->tx_deemph_gen2,
+ imx8_phy->base + PCIE_PHY_TRSV_REG6);
+ break;
+ }

if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ||
pad_mode == IMX8_PCIE_REFCLK_PAD_UNUSED) {
@@ -118,15 +115,37 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG065);
}

- /* Tune PHY de-emphasis setting to pass PCIe compliance. */
- if (imx8_phy->tx_deemph_gen1)
- writel(imx8_phy->tx_deemph_gen1,
- imx8_phy->base + PCIE_PHY_TRSV_REG5);
- if (imx8_phy->tx_deemph_gen2)
- writel(imx8_phy->tx_deemph_gen2,
- imx8_phy->base + PCIE_PHY_TRSV_REG6);
+ /* Set AUX_EN_OVERRIDE 1'b0, when the CLKREQ# isn't hooked */
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE,
+ imx8_phy->clkreq_unused ?
+ 0 : IMX8MM_GPR_PCIE_AUX_EN_OVERRIDE);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_AUX_EN,
+ IMX8MM_GPR_PCIE_AUX_EN);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_POWER_OFF, 0);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_SSC_EN, 0);

- reset_control_deassert(imx8_phy->reset);
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_REF_CLK_SEL,
+ pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ?
+ IMX8MM_GPR_PCIE_REF_CLK_EXT :
+ IMX8MM_GPR_PCIE_REF_CLK_PLL);
+ usleep_range(100, 200);
+
+ /* Do the PHY common block reset */
+ regmap_update_bits(imx8_phy->iomuxc_gpr, IOMUXC_GPR14,
+ IMX8MM_GPR_PCIE_CMN_RST,
+ IMX8MM_GPR_PCIE_CMN_RST);
+
+ switch (imx8_phy->drvdata->variant) {
+ case IMX8MM:
+ reset_control_deassert(imx8_phy->reset);
+ usleep_range(200, 500);
+ break;
+ }

/* Polling to check the phy is ready or not. */
ret = readl_poll_timeout(imx8_phy->base + IMX8MM_PCIE_PHY_CMN_REG075,
@@ -157,6 +176,17 @@ static const struct phy_ops imx8_pcie_phy_ops = {
.owner = THIS_MODULE,
};

+static const struct imx8_pcie_phy_drvdata imx8mm_drvdata = {
+ .gpr = "fsl,imx8mm-iomuxc-gpr",
+ .variant = IMX8MM,
+};
+
+static const struct of_device_id imx8_pcie_phy_of_match[] = {
+ {.compatible = "fsl,imx8mm-pcie-phy", .data = &imx8mm_drvdata, },
+ { },
+};
+MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
+
static int imx8_pcie_phy_probe(struct platform_device *pdev)
{
struct phy_provider *phy_provider;
@@ -169,6 +199,8 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
if (!imx8_phy)
return -ENOMEM;

+ imx8_phy->drvdata = of_device_get_match_data(dev);
+
/* get PHY refclk pad mode */
of_property_read_u32(np, "fsl,refclk-pad-mode",
&imx8_phy->refclk_pad_mode);
@@ -194,7 +226,7 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)

/* Grab GPR config register range */
imx8_phy->iomuxc_gpr =
- syscon_regmap_lookup_by_compatible("fsl,imx6q-iomuxc-gpr");
+ syscon_regmap_lookup_by_compatible(imx8_phy->drvdata->gpr);
if (IS_ERR(imx8_phy->iomuxc_gpr)) {
dev_err(dev, "unable to find iomuxc registers\n");
return PTR_ERR(imx8_phy->iomuxc_gpr);
@@ -222,12 +254,6 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
}

-static const struct of_device_id imx8_pcie_phy_of_match[] = {
- {.compatible = "fsl,imx8mm-pcie-phy",},
- { },
-};
-MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
-
static struct platform_driver imx8_pcie_phy_driver = {
.probe = imx8_pcie_phy_probe,
.driver = {
--
2.25.1

2022-10-13 02:29:15

by Richard Zhu

[permalink] [raw]
Subject: [RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY support

Add i.MX8MP PCIe PHY support.

Signed-off-by: Richard Zhu <[email protected]>
Signed-off-by: Lucas Stach <[email protected]>
Tested-by: Marek Vasut <[email protected]>
Tested-by: Richard Leitner <[email protected]>
Tested-by: Alexander Stein <[email protected]>
Reviewed-by: Lucas Stach <[email protected]>
Reviewed-by: Ahmad Fatoum <[email protected]>
---
drivers/phy/freescale/phy-fsl-imx8m-pcie.c | 25 ++++++++++++++++++++--
1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
index 3e494612db3c..7585e8080b77 100644
--- a/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
+++ b/drivers/phy/freescale/phy-fsl-imx8m-pcie.c
@@ -48,6 +48,7 @@

enum imx8_pcie_phy_type {
IMX8MM,
+ IMX8MP,
};

struct imx8_pcie_phy_drvdata {
@@ -60,6 +61,7 @@ struct imx8_pcie_phy {
struct clk *clk;
struct phy *phy;
struct regmap *iomuxc_gpr;
+ struct reset_control *perst;
struct reset_control *reset;
u32 refclk_pad_mode;
u32 tx_deemph_gen1;
@@ -74,11 +76,11 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
u32 val, pad_mode;
struct imx8_pcie_phy *imx8_phy = phy_get_drvdata(phy);

- reset_control_assert(imx8_phy->reset);
-
pad_mode = imx8_phy->refclk_pad_mode;
switch (imx8_phy->drvdata->variant) {
case IMX8MM:
+ reset_control_assert(imx8_phy->reset);
+
/* Tune PHY de-emphasis setting to pass PCIe compliance. */
if (imx8_phy->tx_deemph_gen1)
writel(imx8_phy->tx_deemph_gen1,
@@ -87,6 +89,8 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
writel(imx8_phy->tx_deemph_gen2,
imx8_phy->base + PCIE_PHY_TRSV_REG6);
break;
+ case IMX8MP: /* Do nothing. */
+ break;
}

if (pad_mode == IMX8_PCIE_REFCLK_PAD_INPUT ||
@@ -141,6 +145,9 @@ static int imx8_pcie_phy_power_on(struct phy *phy)
IMX8MM_GPR_PCIE_CMN_RST);

switch (imx8_phy->drvdata->variant) {
+ case IMX8MP:
+ reset_control_deassert(imx8_phy->perst);
+ fallthrough;
case IMX8MM:
reset_control_deassert(imx8_phy->reset);
usleep_range(200, 500);
@@ -181,8 +188,14 @@ static const struct imx8_pcie_phy_drvdata imx8mm_drvdata = {
.variant = IMX8MM,
};

+static const struct imx8_pcie_phy_drvdata imx8mp_drvdata = {
+ .gpr = "fsl,imx8mp-iomuxc-gpr",
+ .variant = IMX8MP,
+};
+
static const struct of_device_id imx8_pcie_phy_of_match[] = {
{.compatible = "fsl,imx8mm-pcie-phy", .data = &imx8mm_drvdata, },
+ {.compatible = "fsl,imx8mp-pcie-phy", .data = &imx8mp_drvdata, },
{ },
};
MODULE_DEVICE_TABLE(of, imx8_pcie_phy_of_match);
@@ -238,6 +251,14 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
return PTR_ERR(imx8_phy->reset);
}

+ if (imx8_phy->drvdata->variant == IMX8MP) {
+ imx8_phy->perst =
+ devm_reset_control_get_exclusive(dev, "perst");
+ if (IS_ERR(imx8_phy->perst))
+ dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
+ "Failed to get PCIE PHY PERST control\n");
+ }
+
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
imx8_phy->base = devm_ioremap_resource(dev, res);
if (IS_ERR(imx8_phy->base))
--
2.25.1

2022-10-17 06:08:48

by Vinod Koul

[permalink] [raw]
Subject: Re: [PATCH RESEND v12 0/4] Add the iMX8MP PCIe support

On 13-10-22, 09:46, Richard Zhu wrote:
> Re-base to the pci-v6.1-changes of pci/next branch.
> Update the cover-letter, and re-send the v12 patch-set.
> This series adds the i.MX8MP PCIe support and tested on i.MX8MP
> EVK board when one PCIe NVME device is used.
>
> - i.MX8MP PCIe has reversed initial PERST bit value refer to i.MX8MQ/i.MX8MM.
> Add the PHY PERST explicitly for i.MX8MP PCIe PHY.
> - Add the i.MX8MP PCIe PHY support in the i.MX8M PCIe PHY driver.
> And share as much as possible codes with i.MX8MM PCIe PHY.
> - Add the i.MX8MP PCIe support in binding document, DTS files, and PCIe
> driver.

Applied, thanks

--
~Vinod

2022-12-17 20:27:54

by Marek Vasut

[permalink] [raw]
Subject: Re: [RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY support

On 10/13/22 03:47, Richard Zhu wrote:

Hi,

[...]

> @@ -238,6 +251,14 @@ static int imx8_pcie_phy_probe(struct platform_device *pdev)
> return PTR_ERR(imx8_phy->reset);
> }
>
> + if (imx8_phy->drvdata->variant == IMX8MP) {
> + imx8_phy->perst =
> + devm_reset_control_get_exclusive(dev, "perst");
> + if (IS_ERR(imx8_phy->perst))
> + dev_err_probe(dev, PTR_ERR(imx8_phy->perst),

I just notice this , are we missing return here ?

That is ... return dev_err_probe(...) ?

> + "Failed to get PCIE PHY PERST control\n");
> + }

[...]

2022-12-19 04:02:13

by Richard Zhu

[permalink] [raw]
Subject: RE: [RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY support

> -----Original Message-----
> From: Marek Vasut <[email protected]>
> Sent: 2022年12月18日 3:47
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe
> PHY support
>
> On 10/13/22 03:47, Richard Zhu wrote:
>
> Hi,
>
> [...]
>
> > @@ -238,6 +251,14 @@ static int imx8_pcie_phy_probe(struct
> platform_device *pdev)
> > return PTR_ERR(imx8_phy->reset);
> > }
> >
> > + if (imx8_phy->drvdata->variant == IMX8MP) {
> > + imx8_phy->perst =
> > + devm_reset_control_get_exclusive(dev, "perst");
> > + if (IS_ERR(imx8_phy->perst))
> > + dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
>
> I just notice this , are we missing return here ?
>
> That is ... return dev_err_probe(...) ?

Ooh, it's my fault. Thanks for your reminder.
One return should be here.
- dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
+ return dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
Hi Vinod @[email protected]:
Should I re-summit one fix patch?

Best Regards
Richard Zhu

>
> > + "Failed to get PCIE PHY PERST control\n");
> > + }
>
> [...]

2022-12-19 06:41:27

by Marek Vasut

[permalink] [raw]
Subject: Re: [RESEND v12 4/4] phy: freescale: imx8m-pcie: Add i.MX8MP PCIe PHY support

On 12/19/22 04:25, Hongxing Zhu wrote:

Hi,

>>> @@ -238,6 +251,14 @@ static int imx8_pcie_phy_probe(struct
>> platform_device *pdev)
>>> return PTR_ERR(imx8_phy->reset);
>>> }
>>>
>>> + if (imx8_phy->drvdata->variant == IMX8MP) {
>>> + imx8_phy->perst =
>>> + devm_reset_control_get_exclusive(dev, "perst");
>>> + if (IS_ERR(imx8_phy->perst))
>>> + dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
>>
>> I just notice this , are we missing return here ?
>>
>> That is ... return dev_err_probe(...) ?
>
> Ooh, it's my fault. Thanks for your reminder.
> One return should be here.
> - dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
> + return dev_err_probe(dev, PTR_ERR(imx8_phy->perst),
> Hi Vinod @[email protected]:
> Should I re-summit one fix patch?

I think just send a follow up patch with Fixes: tag .