2022-07-01 03:44:08

by Richard Zhu

Subject: [PATCH v14 16/17] PCI: imx6: Move the imx6_pcie_ltssm_disable() earlier

Move the imx6_pcie_ltssm_disable() earlier and place it just behind the
imx6_pcie_ltssm_enable(), since it might not be only used by suspend
callback directly.
To be symmetric with imx6_pcie_ltssm_enable(), add the IMX6Q switch
case in the imx6_pcie_ltssm_disable().

Signed-off-by: Richard Zhu <[email protected]>
---
drivers/pci/controller/dwc/pci-imx6.c | 39 ++++++++++++++-------------
1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 79a05e190016..1cf8bf9035f2 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -805,6 +805,26 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
}
}

+static void imx6_pcie_ltssm_disable(struct device *dev)
+{
+ struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
+
+ switch (imx6_pcie->drvdata->variant) {
+ case IMX6Q:
+ case IMX6SX:
+ case IMX6QP:
+ regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
+ IMX6Q_GPR12_PCIE_CTL_2, 0);
+ break;
+ case IMX7D:
+ case IMX8MM:
+ reset_control_assert(imx6_pcie->apps_reset);
+ break;
+ default:
+ dev_err(dev, "ltssm_disable not supported\n");
+ }
+}
+
static int imx6_pcie_start_link(struct dw_pcie *pci)
{
struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
@@ -947,25 +967,6 @@ static const struct dw_pcie_ops dw_pcie_ops = {
};

#ifdef CONFIG_PM_SLEEP
-static void imx6_pcie_ltssm_disable(struct device *dev)
-{
- struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
-
- switch (imx6_pcie->drvdata->variant) {
- case IMX6SX:
- case IMX6QP:
- regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
- IMX6Q_GPR12_PCIE_CTL_2, 0);
- break;
- case IMX7D:
- case IMX8MM:
- reset_control_assert(imx6_pcie->apps_reset);
- break;
- default:
- dev_err(dev, "ltssm_disable not supported\n");
- }
-}
-
static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
{
struct device *dev = imx6_pcie->pci->dev;
--
2.25.1

2022-07-13 09:13:12

by Lucas Stach

Subject: Re: [PATCH v14 16/17] PCI: imx6: Move the imx6_pcie_ltssm_disable() earlier

Am Freitag, dem 01.07.2022 um 11:25 +0800 schrieb Richard Zhu:
> Move the imx6_pcie_ltssm_disable() earlier and place it just behind the
> imx6_pcie_ltssm_enable(), since it might not be only used by suspend
> callback directly.
> To be symmetric with imx6_pcie_ltssm_enable(), add the IMX6Q switch
> case in the imx6_pcie_ltssm_disable().
>
> Signed-off-by: Richard Zhu <[email protected]>
> ---
> drivers/pci/controller/dwc/pci-imx6.c | 39 ++++++++++++++-------------
> 1 file changed, 20 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 79a05e190016..1cf8bf9035f2 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -805,6 +805,26 @@ static void imx6_pcie_ltssm_enable(struct device *dev)
> }
> }
>
> +static void imx6_pcie_ltssm_disable(struct device *dev)
> +{
> + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> +
> + switch (imx6_pcie->drvdata->variant) {
> + case IMX6Q:
> + case IMX6SX:
> + case IMX6QP:
> + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> + IMX6Q_GPR12_PCIE_CTL_2, 0);
> + break;
> + case IMX7D:
> + case IMX8MM:
> + reset_control_assert(imx6_pcie->apps_reset);
> + break;
This is missing the IMX8MQ case.

> + default:
> + dev_err(dev, "ltssm_disable not supported\n");

Drop the default, we want a compile time warning if a variant isn't
covered by this switch statement.

Regards,
Lucas

> + }
> +}
> +
> static int imx6_pcie_start_link(struct dw_pcie *pci)
> {
> struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci);
> @@ -947,25 +967,6 @@ static const struct dw_pcie_ops dw_pcie_ops = {
> };
>
> #ifdef CONFIG_PM_SLEEP
> -static void imx6_pcie_ltssm_disable(struct device *dev)
> -{
> - struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> -
> - switch (imx6_pcie->drvdata->variant) {
> - case IMX6SX:
> - case IMX6QP:
> - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> - IMX6Q_GPR12_PCIE_CTL_2, 0);
> - break;
> - case IMX7D:
> - case IMX8MM:
> - reset_control_assert(imx6_pcie->apps_reset);
> - break;
> - default:
> - dev_err(dev, "ltssm_disable not supported\n");
> - }
> -}
> -
> static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie)
> {
> struct device *dev = imx6_pcie->pci->dev;

2022-07-13 11:05:24

by Richard Zhu

Subject: RE: [PATCH v14 16/17] PCI: imx6: Move the imx6_pcie_ltssm_disable() earlier

> -----Original Message-----
> From: Lucas Stach <[email protected]>
> Sent: 2022年7月13日 17:04
> To: Hongxing Zhu <[email protected]>; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]
> Cc: [email protected]; [email protected];
> [email protected]; [email protected]; dl-linux-imx
> <[email protected]>
> Subject: Re: [PATCH v14 16/17] PCI: imx6: Move the imx6_pcie_ltssm_disable()
> earlier
>
> Am Freitag, dem 01.07.2022 um 11:25 +0800 schrieb Richard Zhu:
> > Move the imx6_pcie_ltssm_disable() earlier and place it just behind
> > the imx6_pcie_ltssm_enable(), since it might not be only used by
> > suspend callback directly.
> > To be symmetric with imx6_pcie_ltssm_enable(), add the IMX6Q switch
> > case in the imx6_pcie_ltssm_disable().
> >
> > Signed-off-by: Richard Zhu <[email protected]>
> > ---
> > drivers/pci/controller/dwc/pci-imx6.c | 39
> > ++++++++++++++-------------
> > 1 file changed, 20 insertions(+), 19 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 79a05e190016..1cf8bf9035f2 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -805,6 +805,26 @@ static void imx6_pcie_ltssm_enable(struct device
> *dev)
> > }
> > }
> >
> > +static void imx6_pcie_ltssm_disable(struct device *dev) {
> > + struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > +
> > + switch (imx6_pcie->drvdata->variant) {
> > + case IMX6Q:
> > + case IMX6SX:
> > + case IMX6QP:
> > + regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > + IMX6Q_GPR12_PCIE_CTL_2, 0);
> > + break;
> > + case IMX7D:
> > + case IMX8MM:
> > + reset_control_assert(imx6_pcie->apps_reset);
> > + break;
> This is missing the IMX8MQ case.
Hi Lucas:
Good caught. I just move the function place, but forget to make a double check
on the contents accordingly.

>
> > + default:
> > + dev_err(dev, "ltssm_disable not supported\n");
>
> Drop the default, we want a compile time warning if a variant isn't covered by
> this switch statement.
>
Got that. Thanks.

Best Regards
Richard Zhu

> Regards,
> Lucas
>
> > + }
> > +}
> > +
> > static int imx6_pcie_start_link(struct dw_pcie *pci) {
> > struct imx6_pcie *imx6_pcie = to_imx6_pcie(pci); @@ -947,25 +967,6
> > @@ static const struct dw_pcie_ops dw_pcie_ops = { };
> >
> > #ifdef CONFIG_PM_SLEEP
> > -static void imx6_pcie_ltssm_disable(struct device *dev) -{
> > - struct imx6_pcie *imx6_pcie = dev_get_drvdata(dev);
> > -
> > - switch (imx6_pcie->drvdata->variant) {
> > - case IMX6SX:
> > - case IMX6QP:
> > - regmap_update_bits(imx6_pcie->iomuxc_gpr, IOMUXC_GPR12,
> > - IMX6Q_GPR12_PCIE_CTL_2, 0);
> > - break;
> > - case IMX7D:
> > - case IMX8MM:
> > - reset_control_assert(imx6_pcie->apps_reset);
> > - break;
> > - default:
> > - dev_err(dev, "ltssm_disable not supported\n");
> > - }
> > -}
> > -
> > static void imx6_pcie_pm_turnoff(struct imx6_pcie *imx6_pcie) {
> > struct device *dev = imx6_pcie->pci->dev;
>