| Message ID | 1656645935-1370-17-git-send-email-hongxing.zhu@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | PCI: imx6: refine codes and add the error propagation | expand |
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 <hongxing.zhu@nxp.com> > --- > 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;
> -----Original Message----- > From: Lucas Stach <l.stach@pengutronix.de> > Sent: 2022年7月13日 17:04 > To: Hongxing Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com; > robh+dt@kernel.org; broonie@kernel.org; lorenzo.pieralisi@arm.com; > festevam@gmail.com; francesco.dolcini@toradex.com > Cc: linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; kernel@pengutronix.de; dl-linux-imx > <linux-imx@nxp.com> > 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 <hongxing.zhu@nxp.com> > > --- > > 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; >
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;
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 <hongxing.zhu@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 39 ++++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-)