* [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd
@ 2019-01-21 22:50 Leonard Crestez
2019-01-21 22:50 ` [PATCH 1/2] PCI: imx: Fix probe failure without power domain Leonard Crestez
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Leonard Crestez @ 2019-01-21 22:50 UTC (permalink / raw)
To: Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach
Cc: Ulf Hansson, Richard Zhu, Shawn Guo, linux-pci, linux-arm-kernel,
kernel, dl-linux-imx
This fixes pci probing on imx6qp as reported by Lukas Hartmann.
The second patch fixes a check that shouldn't actually fail.
Leonard Crestez (2):
PCI: imx: Fix probe failure without power domain
PCI: imx: Fix checking pd_pcie_phy device_link
drivers/pci/controller/dwc/pci-imx6.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 8+ messages in thread* [PATCH 1/2] PCI: imx: Fix probe failure without power domain 2019-01-21 22:50 [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Leonard Crestez @ 2019-01-21 22:50 ` Leonard Crestez 2019-01-22 7:12 ` Uwe Kleine-König 2019-01-21 22:50 ` [PATCH 2/2] PCI: imx: Fix checking pd_pcie_phy device_link Leonard Crestez 2019-01-25 17:59 ` [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Lorenzo Pieralisi 2 siblings, 1 reply; 8+ messages in thread From: Leonard Crestez @ 2019-01-21 22:50 UTC (permalink / raw) To: Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach Cc: Ulf Hansson, Richard Zhu, Shawn Guo, linux-pci, linux-arm-kernel, kernel, dl-linux-imx On chips without a separate power domain for PCI (such as 6q/6qp) the imx6_pcie_attach_pd function incorrectly returns an error. Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find anything. Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") Reported-by: Lukas F.Hartmann <lukas@mntmn.com> Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index 52e47dac028f..ac5f6ae0b254 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev) return 0; imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); if (IS_ERR(imx6_pcie->pd_pcie)) return PTR_ERR(imx6_pcie->pd_pcie); + /* Do nothing when power domain missing */ + if (!imx6_pcie->pd_pcie) + return 0; link = device_link_add(dev, imx6_pcie->pd_pcie, DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); if (!link) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: imx: Fix probe failure without power domain 2019-01-21 22:50 ` [PATCH 1/2] PCI: imx: Fix probe failure without power domain Leonard Crestez @ 2019-01-22 7:12 ` Uwe Kleine-König 2019-01-22 9:17 ` Leonard Crestez 0 siblings, 1 reply; 8+ messages in thread From: Uwe Kleine-König @ 2019-01-22 7:12 UTC (permalink / raw) To: Leonard Crestez Cc: Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach, Ulf Hansson, Richard Zhu, linux-pci, dl-linux-imx, kernel, Shawn Guo, linux-arm-kernel On Mon, Jan 21, 2019 at 10:50:04PM +0000, Leonard Crestez wrote: > On chips without a separate power domain for PCI (such as 6q/6qp) the > imx6_pcie_attach_pd function incorrectly returns an error. > > Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find > anything. > > Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") > Reported-by: Lukas F.Hartmann <lukas@mntmn.com> > Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> > --- > drivers/pci/controller/dwc/pci-imx6.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > index 52e47dac028f..ac5f6ae0b254 100644 > --- a/drivers/pci/controller/dwc/pci-imx6.c > +++ b/drivers/pci/controller/dwc/pci-imx6.c > @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev) > return 0; > > imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); > if (IS_ERR(imx6_pcie->pd_pcie)) > return PTR_ERR(imx6_pcie->pd_pcie); > + /* Do nothing when power domain missing */ > + if (!imx6_pcie->pd_pcie) > + return 0; As I said in the mail that proposed this patch for testing: I think it would be better to change dev_pm_domain_attach_by_name to not return an error indication by returning NULL or an ERR_PTR value. (Or change device_link_add to accept NULL if NULL is a dummy value.) Just repeating it here to have it near the actual patch. Best regards Uwe > link = device_link_add(dev, imx6_pcie->pd_pcie, > DL_FLAG_STATELESS | > DL_FLAG_PM_RUNTIME | > DL_FLAG_RPM_ACTIVE); > if (!link) { > -- > 2.17.1 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: imx: Fix probe failure without power domain 2019-01-22 7:12 ` Uwe Kleine-König @ 2019-01-22 9:17 ` Leonard Crestez 2019-01-22 9:44 ` Uwe Kleine-König 0 siblings, 1 reply; 8+ messages in thread From: Leonard Crestez @ 2019-01-22 9:17 UTC (permalink / raw) To: Uwe Kleine-König, Ulf Hansson Cc: Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach, Richard Zhu, linux-pci, dl-linux-imx, kernel, Shawn Guo, linux-arm-kernel, Rafael J. Wysocki, Kevin Hilman, linux-pm, Viresh Kumar, Jon Hunter On 1/22/2019 9:12 AM, Uwe Kleine-König wrote: > On Mon, Jan 21, 2019 at 10:50:04PM +0000, Leonard Crestez wrote: >> On chips without a separate power domain for PCI (such as 6q/6qp) the >> imx6_pcie_attach_pd function incorrectly returns an error. >> >> Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find >> anything. >> >> Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") >> >> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c >> @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev) >> return 0; >> >> imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); >> if (IS_ERR(imx6_pcie->pd_pcie)) >> return PTR_ERR(imx6_pcie->pd_pcie); >> + /* Do nothing when power domain missing */ >> + if (!imx6_pcie->pd_pcie) >> + return 0; > > As I said in the mail that proposed this patch for testing: I think it > would be better to change dev_pm_domain_attach_by_name to not return an > error indication by returning NULL or an ERR_PTR value. (Or change > device_link_add to accept NULL if NULL is a dummy value.) > > Just repeating it here to have it near the actual patch. Link to that previous discussion: http://lists.infradead.org/pipermail/linux-arm-kernel/2019-January/626859.html I'm not sure what you mean. Should dev_pm_domain_attach_by_name return ERR_PTR(-ENOENT) for name not found? It would still require special handling for callers. The device_link_add function already returns NULL if the consumer or supplier is NULL but that's also the only way it signals failure. Maybe that function should be adjusted to return ERR_PTR instead? Changing core API for a driver bug seems inappropriate. The current imx6_pcie_attach_pd code is complicated because it tries to distinguish between "multi pd" and "no pd / single pd". I wish there was a way for these PM_RUNTIME device_links to be setup automatically in core and have multi-PD behave the same as a single PD. There aren't many multi-PD users but commit 6494a9ad86de ("usb: xhci: tegra: Add genpd support") seems to do pretty much the same thing. -- Regards, Leonard ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: imx: Fix probe failure without power domain 2019-01-22 9:17 ` Leonard Crestez @ 2019-01-22 9:44 ` Uwe Kleine-König 2019-01-22 9:52 ` Leonard Crestez 0 siblings, 1 reply; 8+ messages in thread From: Uwe Kleine-König @ 2019-01-22 9:44 UTC (permalink / raw) To: Leonard Crestez Cc: Ulf Hansson, Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach, Richard Zhu, linux-pci, dl-linux-imx, kernel, Shawn Guo, linux-arm-kernel, Rafael J. Wysocki, Kevin Hilman, linux-pm, Viresh Kumar, Jon Hunter On Tue, Jan 22, 2019 at 09:17:22AM +0000, Leonard Crestez wrote: > On 1/22/2019 9:12 AM, Uwe Kleine-König wrote: > > On Mon, Jan 21, 2019 at 10:50:04PM +0000, Leonard Crestez wrote: > >> On chips without a separate power domain for PCI (such as 6q/6qp) the > >> imx6_pcie_attach_pd function incorrectly returns an error. > >> > >> Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find > >> anything. > >> > >> Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") > >> > >> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c > >> @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev) > >> return 0; > >> > >> imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); > >> if (IS_ERR(imx6_pcie->pd_pcie)) > >> return PTR_ERR(imx6_pcie->pd_pcie); > >> + /* Do nothing when power domain missing */ > >> + if (!imx6_pcie->pd_pcie) > >> + return 0; > > > > As I said in the mail that proposed this patch for testing: I think it > > would be better to change dev_pm_domain_attach_by_name to not return an > > error indication by returning NULL or an ERR_PTR value. (Or change > > device_link_add to accept NULL if NULL is a dummy value.) > > > > Just repeating it here to have it near the actual patch. > > Link to that previous discussion: > http://lists.infradead.org/pipermail/linux-arm-kernel/2019-January/626859.html > > I'm not sure what you mean. Should dev_pm_domain_attach_by_name return > ERR_PTR(-ENOENT) for name not found? It would still require special > handling for callers. Yes, callers would still need to handle an error, but they only need to check for IS_ERR(retval) instead of IS_ERR(retval) && retval != NULL. > The device_link_add function already returns NULL if the consumer or > supplier is NULL but that's also the only way it signals failure. Maybe > that function should be adjusted to return ERR_PTR instead? > > Changing core API for a driver bug seems inappropriate. Well, if the driver bug is there because the API is used in a wrong way and the right way is strange, fixing the API and checking other users for similar mistakes is the right thing to do. > The current imx6_pcie_attach_pd code is complicated because it tries to > distinguish between "multi pd" and "no pd / single pd". I wish there was > a way for these PM_RUNTIME device_links to be setup automatically in > core and have multi-PD behave the same as a single PD. > > There aren't many multi-PD users but commit 6494a9ad86de ("usb: xhci: > tegra: Add genpd support") seems to do pretty much the same thing. I don't know enough about power domains to follow this argumentation in the time I have available. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ | ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] PCI: imx: Fix probe failure without power domain 2019-01-22 9:44 ` Uwe Kleine-König @ 2019-01-22 9:52 ` Leonard Crestez 0 siblings, 0 replies; 8+ messages in thread From: Leonard Crestez @ 2019-01-22 9:52 UTC (permalink / raw) To: Uwe Kleine-König Cc: Ulf Hansson, Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach, Richard Zhu, linux-pci, dl-linux-imx, kernel, Shawn Guo, linux-arm-kernel, Rafael J. Wysocki, Kevin Hilman, linux-pm, Viresh Kumar, Jon Hunter On 1/22/19 11:44 AM, Uwe Kleine-König wrote: > On Tue, Jan 22, 2019 at 09:17:22AM +0000, Leonard Crestez wrote: >> On 1/22/2019 9:12 AM, Uwe Kleine-König wrote: >>> On Mon, Jan 21, 2019 at 10:50:04PM +0000, Leonard Crestez wrote: >>>> On chips without a separate power domain for PCI (such as 6q/6qp) the >>>> imx6_pcie_attach_pd function incorrectly returns an error. >>>> >>>> Fix by returning 0 if dev_pm_domain_attach_by_name doesn't find >>>> anything. >>>> >>>> Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") >>>> >>>> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c >>>> @@ -308,10 +308,13 @@ static int imx6_pcie_attach_pd(struct device *dev) >>>> return 0; >>>> >>>> imx6_pcie->pd_pcie = dev_pm_domain_attach_by_name(dev, "pcie"); >>>> if (IS_ERR(imx6_pcie->pd_pcie)) >>>> return PTR_ERR(imx6_pcie->pd_pcie); >>>> + /* Do nothing when power domain missing */ >>>> + if (!imx6_pcie->pd_pcie) >>>> + return 0; >>> >>> As I said in the mail that proposed this patch for testing: I think it >>> would be better to change dev_pm_domain_attach_by_name to not return an >>> error indication by returning NULL or an ERR_PTR value. (Or change >>> device_link_add to accept NULL if NULL is a dummy value.) >> >> I'm not sure what you mean. Should dev_pm_domain_attach_by_name return >> ERR_PTR(-ENOENT) for name not found? It would still require special >> handling for callers. > > Yes, callers would still need to handle an error, but they only need to > check for IS_ERR(retval) instead of IS_ERR(retval) && retval != NULL. But "not found" is not treated as an error here, it's what happens when no PD is assigned. The fix makes imx6_pcie_attach_pd return success in this case. Making dev_pm_domain_attach_by_name return ENOENT would still require a special case for ENOENT here. -- Regards, Leonard ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] PCI: imx: Fix checking pd_pcie_phy device_link 2019-01-21 22:50 [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Leonard Crestez 2019-01-21 22:50 ` [PATCH 1/2] PCI: imx: Fix probe failure without power domain Leonard Crestez @ 2019-01-21 22:50 ` Leonard Crestez 2019-01-25 17:59 ` [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Lorenzo Pieralisi 2 siblings, 0 replies; 8+ messages in thread From: Leonard Crestez @ 2019-01-21 22:50 UTC (permalink / raw) To: Lorenzo Pieralisi, Lukas F . Hartmann, Lucas Stach Cc: Ulf Hansson, Richard Zhu, Shawn Guo, linux-pci, linux-arm-kernel, kernel, dl-linux-imx This check was completely wrong. It doesn't break anything specific because adding the second device_link can't fail. Fixes: 3f7cceeab895 ("PCI: imx: Add multi-pd support") Signed-off-by: Leonard Crestez <leonard.crestez@nxp.com> --- drivers/pci/controller/dwc/pci-imx6.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c index ac5f6ae0b254..80f843030e36 100644 --- a/drivers/pci/controller/dwc/pci-imx6.c +++ b/drivers/pci/controller/dwc/pci-imx6.c @@ -324,17 +324,17 @@ static int imx6_pcie_attach_pd(struct device *dev) imx6_pcie->pd_pcie_phy = dev_pm_domain_attach_by_name(dev, "pcie_phy"); if (IS_ERR(imx6_pcie->pd_pcie_phy)) return PTR_ERR(imx6_pcie->pd_pcie_phy); - device_link_add(dev, imx6_pcie->pd_pcie_phy, + link = device_link_add(dev, imx6_pcie->pd_pcie_phy, DL_FLAG_STATELESS | DL_FLAG_PM_RUNTIME | DL_FLAG_RPM_ACTIVE); - if (IS_ERR(link)) { - dev_err(dev, "Failed to add device_link to pcie_phy pd: %ld\n", PTR_ERR(link)); - return PTR_ERR(link); + if (!link) { + dev_err(dev, "Failed to add device_link to pcie_phy pd.\n"); + return -EINVAL; } return 0; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd 2019-01-21 22:50 [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Leonard Crestez 2019-01-21 22:50 ` [PATCH 1/2] PCI: imx: Fix probe failure without power domain Leonard Crestez 2019-01-21 22:50 ` [PATCH 2/2] PCI: imx: Fix checking pd_pcie_phy device_link Leonard Crestez @ 2019-01-25 17:59 ` Lorenzo Pieralisi 2 siblings, 0 replies; 8+ messages in thread From: Lorenzo Pieralisi @ 2019-01-25 17:59 UTC (permalink / raw) To: Leonard Crestez Cc: Lukas F . Hartmann, Lucas Stach, Ulf Hansson, Richard Zhu, Shawn Guo, linux-pci, linux-arm-kernel, kernel, dl-linux-imx On Mon, Jan 21, 2019 at 10:50:03PM +0000, Leonard Crestez wrote: > This fixes pci probing on imx6qp as reported by Lukas Hartmann. > > The second patch fixes a check that shouldn't actually fail. > > Leonard Crestez (2): > PCI: imx: Fix probe failure without power domain > PCI: imx: Fix checking pd_pcie_phy device_link > > drivers/pci/controller/dwc/pci-imx6.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) I have applied them to pci/controller-fixes and Bjorn already queued them for one of the upcoming -rc*. Thanks, Lorenzo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-01-25 17:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-21 22:50 [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Leonard Crestez 2019-01-21 22:50 ` [PATCH 1/2] PCI: imx: Fix probe failure without power domain Leonard Crestez 2019-01-22 7:12 ` Uwe Kleine-König 2019-01-22 9:17 ` Leonard Crestez 2019-01-22 9:44 ` Uwe Kleine-König 2019-01-22 9:52 ` Leonard Crestez 2019-01-21 22:50 ` [PATCH 2/2] PCI: imx: Fix checking pd_pcie_phy device_link Leonard Crestez 2019-01-25 17:59 ` [PATCH 0/2] PCI: imx: imx6_pcie_attach_pd Lorenzo Pieralisi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).