Re: Linux Kernel Regression in Commit 3f7cceeab895fcc17ac8db0d9a5e8ca2954b4661 on i.MX6QP

Re: Linux Kernel Regression in Commit 3f7cceeab895fcc17ac8db0d9a5e8ca2954b4661 on i.MX6QP

[Leonard Crestez]

On 1/21/2019 10:34 PM, Lukas F.Hartmann wrote:
> Removing the return(ret) line after the imx6_pcie_attach_pd(dev) call
> fixes the problems for me. I am under the impression that the
> imx6_add_pcie_port still should be called even if there was an error
> attaching the power domain?

The intent was that imx6_pcie_attach_pd does nothing if no PD is defined 
in DT but it returns an error instead and causes a probe failure.

Can you please try the following:

--- drivers/pci/controller/dwc/pci-imx6.c
+++ drivers/pci/controller/dwc/pci-imx6.c
@@ -310,6 +310,9 @@ static int imx6_pcie_attach_pd(struct device *dev)
         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 |

This fix is for PCI, it's not clear why it would also affect USB in your 
case.

Thanks for reporting!

--
Regards,
Leonard

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Linux Kernel Regression in Commit 3f7cceeab895fcc17ac8db0d9a5e8ca2954b4661 on i.MX6QP

[Lukas Hartmann]

Alright, thank you for the quick response. I will try your fix in the morning. Another thing: the second device_link_add() does not assign to link, but link is then checked again for an error. Is that intentional?

Best,
Lukas

> On 21. Jan 2019, at 22:12, Leonard Crestez <leonard.crestez@nxp.com> wrote:
> 
>> On 1/21/2019 10:34 PM, Lukas F.Hartmann wrote:
>> Removing the return(ret) line after the imx6_pcie_attach_pd(dev) call
>> fixes the problems for me. I am under the impression that the
>> imx6_add_pcie_port still should be called even if there was an error
>> attaching the power domain?
> 
> The intent was that imx6_pcie_attach_pd does nothing if no PD is defined 
> in DT but it returns an error instead and causes a probe failure.
> 
> Can you please try the following:
> 
> --- drivers/pci/controller/dwc/pci-imx6.c
> +++ drivers/pci/controller/dwc/pci-imx6.c
> @@ -310,6 +310,9 @@ static int imx6_pcie_attach_pd(struct device *dev)
>         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 |
> 
> This fix is for PCI, it's not clear why it would also affect USB in your 
> case.
> 
> Thanks for reporting!
> 
> --
> Regards,
> Leonard


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: Linux Kernel Regression in Commit 3f7cceeab895fcc17ac8db0d9a5e8ca2954b4661 on i.MX6QP

[Uwe Kleine-König]

[Cc += generice power domain maintainers and linux-pm@vger.k.o]

Hello,

On Mon, Jan 21, 2019 at 09:12:45PM +0000, Leonard Crestez wrote:
> On 1/21/2019 10:34 PM, Lukas F.Hartmann wrote:
> > Removing the return(ret) line after the imx6_pcie_attach_pd(dev) call
> > fixes the problems for me. I am under the impression that the
> > imx6_add_pcie_port still should be called even if there was an error
> > attaching the power domain?
> 
> The intent was that imx6_pcie_attach_pd does nothing if no PD is defined 
> in DT but it returns an error instead and causes a probe failure.
> 
> Can you please try the following:
> 
> --- drivers/pci/controller/dwc/pci-imx6.c
> +++ drivers/pci/controller/dwc/pci-imx6.c
> @@ -310,6 +310,9 @@ static int imx6_pcie_attach_pd(struct device *dev)
>          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;

I wonder if it would make more sense to change
dev_pm_domain_attach_by_name et al. to not return NULL and error
pointers. Such functions tend to be used wrongly as can be seen here.
(Unless NULL is a dummy value that in the following shouldn't result in
problems that need catching, but it seems that is not the case.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: Linux Kernel Regression in Commit 3f7cceeab895fcc17ac8db0d9a5e8ca2954b4661 on i.MX6QP

[Uwe Kleine-König]

[Cc += generice power domain maintainers and linux-pm@vger.k.o]

Hello,

On Mon, Jan 21, 2019 at 09:12:45PM +0000, Leonard Crestez wrote:
> On 1/21/2019 10:34 PM, Lukas F.Hartmann wrote:
> > Removing the return(ret) line after the imx6_pcie_attach_pd(dev) call
> > fixes the problems for me. I am under the impression that the
> > imx6_add_pcie_port still should be called even if there was an error
> > attaching the power domain?
> 
> The intent was that imx6_pcie_attach_pd does nothing if no PD is defined 
> in DT but it returns an error instead and causes a probe failure.
> 
> Can you please try the following:
> 
> --- drivers/pci/controller/dwc/pci-imx6.c
> +++ drivers/pci/controller/dwc/pci-imx6.c
> @@ -310,6 +310,9 @@ static int imx6_pcie_attach_pd(struct device *dev)
>          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;

I wonder if it would make more sense to change
dev_pm_domain_attach_by_name et al. to not return NULL and error
pointers. Such functions tend to be used wrongly as can be seen here.
(Unless NULL is a dummy value that in the following shouldn't result in
problems that need catching, but it seems that is not the case.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel