diff mbox series

[RESEND,v2,3/5] PCI: imx6: Fix the regulator dump when link never came up

Message ID 1634277941-6672-4-git-send-email-hongxing.zhu@nxp.com (mailing list archive)
State New, archived
Headers show
Series PCI: imx6: refine codes and add compliance tests mode support | expand

Commit Message

Richard Zhu Oct. 15, 2021, 6:05 a.m. UTC 
When PCIe PHY link never came up and vpcie regulator is present, there
would be following dump when try to put the regulator.
Disable this regulator to fix this dump when link never came up.

  imx6q-pcie 33800000.pcie: Phy link never came up
  imx6q-pcie: probe of 33800000.pcie failed with error -110
  ------------[ cut here ]------------
  WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256 _regulator_put.part.0+0x14c/0x158
  Modules linked in:
  CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
  Hardware name: FSL i.MX8MM EVK board (DT)
  Workqueue: events_unbound async_run_entry_fn
  pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
  pc : _regulator_put.part.0+0x14c/0x158
  lr : regulator_put+0x34/0x48
  sp : ffff8000122ebb30
  x29: ffff8000122ebb30 x28: ffff800011be7000 x27: 0000000000000000
  x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000025f2bc
  x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
  x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
  x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000108
  x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
  x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
  x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
  x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
  x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
  Call trace:
   _regulator_put.part.0+0x14c/0x158
   regulator_put+0x34/0x48
   devm_regulator_release+0x10/0x18
   release_nodes+0x38/0x60
   devres_release_all+0x88/0xd0
   really_probe+0xd0/0x2e8
   __driver_probe_device+0x74/0xd8
   driver_probe_device+0x7c/0x108
   __device_attach_driver+0x8c/0xd0
   bus_for_each_drv+0x74/0xc0
   __device_attach_async_helper+0xb4/0xd8
   async_run_entry_fn+0x30/0x100
   process_one_work+0x19c/0x320
   worker_thread+0x48/0x418
   kthread+0x14c/0x158
   ret_from_fork+0x10/0x18
  ---[ end trace 3664ca4a50ce849b ]---

Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Lucas Stach Oct. 15, 2021, 6:22 p.m. UTC | #1 
Am Freitag, dem 15.10.2021 um 14:05 +0800 schrieb Richard Zhu:
> When PCIe PHY link never came up and vpcie regulator is present, there
> would be following dump when try to put the regulator.
> Disable this regulator to fix this dump when link never came up.
> 
>   imx6q-pcie 33800000.pcie: Phy link never came up
>   imx6q-pcie: probe of 33800000.pcie failed with error -110
>   ------------[ cut here ]------------
>   WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256 _regulator_put.part.0+0x14c/0x158
>   Modules linked in:
>   CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
>   Hardware name: FSL i.MX8MM EVK board (DT)
>   Workqueue: events_unbound async_run_entry_fn
>   pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
>   pc : _regulator_put.part.0+0x14c/0x158
>   lr : regulator_put+0x34/0x48
>   sp : ffff8000122ebb30
>   x29: ffff8000122ebb30 x28: ffff800011be7000 x27: 0000000000000000
>   x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000025f2bc
>   x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
>   x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
>   x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000108
>   x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
>   x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
>   x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
>   x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
>   x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
>   Call trace:
>    _regulator_put.part.0+0x14c/0x158
>    regulator_put+0x34/0x48
>    devm_regulator_release+0x10/0x18
>    release_nodes+0x38/0x60
>    devres_release_all+0x88/0xd0
>    really_probe+0xd0/0x2e8
>    __driver_probe_device+0x74/0xd8
>    driver_probe_device+0x7c/0x108
>    __device_attach_driver+0x8c/0xd0
>    bus_for_each_drv+0x74/0xc0
>    __device_attach_async_helper+0xb4/0xd8
>    async_run_entry_fn+0x30/0x100
>    process_one_work+0x19c/0x320
>    worker_thread+0x48/0x418
>    kthread+0x14c/0x158
>    ret_from_fork+0x10/0x18
>   ---[ end trace 3664ca4a50ce849b ]---
> 
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> ---
>  drivers/pci/controller/dwc/pci-imx6.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 3372775834a2..cc837f8bf6d4 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -853,6 +853,8 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
>  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
>  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
>  	imx6_pcie_reset_phy(imx6_pcie);
> +	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0)
> +		regulator_disable(imx6_pcie->vpcie);

This doesn't seem like the right place to add this. I guess it would be
better to have proper error handling after dw_pcie_host_init to roll
back things like the clock and regulator enable.

Regards,
Lucas

>  	return ret;
>  }
>
Fabio Estevam Oct. 15, 2021, 6:34 p.m. UTC | #2 
Hi Richard,

On Fri, Oct 15, 2021 at 3:32 AM Richard Zhu <hongxing.zhu@nxp.com> wrote:
>
> When PCIe PHY link never came up and vpcie regulator is present, there
> would be following dump when try to put the regulator.
> Disable this regulator to fix this dump when link never came up.
>
>   imx6q-pcie 33800000.pcie: Phy link never came up
>   imx6q-pcie: probe of 33800000.pcie failed with error -110
>   ------------[ cut here ]------------
>   WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256 _regulator_put.part.0+0x14c/0x158
>   Modules linked in:
>   CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
>   Hardware name: FSL i.MX8MM EVK board (DT)
>   Workqueue: events_unbound async_run_entry_fn
>   pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
>   pc : _regulator_put.part.0+0x14c/0x158
>   lr : regulator_put+0x34/0x48
>   sp : ffff8000122ebb30
>   x29: ffff8000122ebb30 x28: ffff800011be7000 x27: 0000000000000000
>   x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000025f2bc
>   x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
>   x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
>   x17: 000000040044ffff x16: 00400032b5503510 x15: 0000000000000108
>   x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
>   x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
>   x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
>   x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
>   x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
>   Call trace:
>    _regulator_put.part.0+0x14c/0x158
>    regulator_put+0x34/0x48
>    devm_regulator_release+0x10/0x18
>    release_nodes+0x38/0x60
>    devres_release_all+0x88/0xd0
>    really_probe+0xd0/0x2e8
>    __driver_probe_device+0x74/0xd8
>    driver_probe_device+0x7c/0x108
>    __device_attach_driver+0x8c/0xd0
>    bus_for_each_drv+0x74/0xc0
>    __device_attach_async_helper+0xb4/0xd8
>    async_run_entry_fn+0x30/0x100
>    process_one_work+0x19c/0x320
>    worker_thread+0x48/0x418
>    kthread+0x14c/0x158
>    ret_from_fork+0x10/0x18
>   ---[ end trace 3664ca4a50ce849b ]---
>
> Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>

I am seeing this on imx6 too. When you send a v2, after addressing
Lucas' comments, please add a Fixes tag/
Richard Zhu Oct. 19, 2021, 7:39 a.m. UTC | #3 
> -----Original Message-----
> From: Lucas Stach <l.stach@pengutronix.de>
> Sent: Saturday, October 16, 2021 2:23 AM
> To: Richard Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com
> Cc: linux-pci@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de
> Subject: Re: [RESEND v2 3/5] PCI: imx6: Fix the regulator dump when link
> never came up
> 
> Am Freitag, dem 15.10.2021 um 14:05 +0800 schrieb Richard Zhu:
> > When PCIe PHY link never came up and vpcie regulator is present, there
> > would be following dump when try to put the regulator.
> > Disable this regulator to fix this dump when link never came up.
> >
> >   imx6q-pcie 33800000.pcie: Phy link never came up
> >   imx6q-pcie: probe of 33800000.pcie failed with error -110
> >   ------------[ cut here ]------------
> >   WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256
> _regulator_put.part.0+0x14c/0x158
> >   Modules linked in:
> >   CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted
> 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
> >   Hardware name: FSL i.MX8MM EVK board (DT)
> >   Workqueue: events_unbound async_run_entry_fn
> >   pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> >   pc : _regulator_put.part.0+0x14c/0x158
> >   lr : regulator_put+0x34/0x48
> >   sp : ffff8000122ebb30
> >   x29: ffff8000122ebb30 x28: ffff800011be7000 x27: 0000000000000000
> >   x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000025f2bc
> >   x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
> >   x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
> >   x17: 000000040044ffff x16: 00400032b5503510 x15:
> 0000000000000108
> >   x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
> >   x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
> >   x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
> >   x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
> >   x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
> >   Call trace:
> >    _regulator_put.part.0+0x14c/0x158
> >    regulator_put+0x34/0x48
> >    devm_regulator_release+0x10/0x18
> >    release_nodes+0x38/0x60
> >    devres_release_all+0x88/0xd0
> >    really_probe+0xd0/0x2e8
> >    __driver_probe_device+0x74/0xd8
> >    driver_probe_device+0x7c/0x108
> >    __device_attach_driver+0x8c/0xd0
> >    bus_for_each_drv+0x74/0xc0
> >    __device_attach_async_helper+0xb4/0xd8
> >    async_run_entry_fn+0x30/0x100
> >    process_one_work+0x19c/0x320
> >    worker_thread+0x48/0x418
> >    kthread+0x14c/0x158
> >    ret_from_fork+0x10/0x18
> >   ---[ end trace 3664ca4a50ce849b ]---
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > ---
> >  drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > b/drivers/pci/controller/dwc/pci-imx6.c
> > index 3372775834a2..cc837f8bf6d4 100644
> > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > @@ -853,6 +853,8 @@ static int imx6_pcie_start_link(struct dw_pcie *pci)
> >  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
> >  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
> >  	imx6_pcie_reset_phy(imx6_pcie);
> > +	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0)
> > +		regulator_disable(imx6_pcie->vpcie);
> 
> This doesn't seem like the right place to add this. I guess it would be better to
> have proper error handling after dw_pcie_host_init to roll back things like the
> clock and regulator enable.
[Richard Zhu] This is good suggestion. It's make sense that move the regulator disable to the
dw_pcie_host_init error handling since the regulator have the enabled check, before to disable it.
But it seems that there are some potential problems when do the similar operations to the clocks disable here.
Because that the dw_pcie_host_init might have some error returns when the host_init is not invoked at all.
How to handle this situation although this is one corner case?

Best Regards
Richard Zhu
> 
> Regards,
> Lucas
> 
> >  	return ret;
> >  }
> >
>
Richard Zhu Oct. 19, 2021, 7:44 a.m. UTC | #4 
> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: Saturday, October 16, 2021 2:35 AM
> To: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>; Bjorn Helgaas
> <bhelgaas@google.com>; Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>;
> linux-pci@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>; moderated
> list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> <linux-arm-kernel@lists.infradead.org>; linux-kernel
> <linux-kernel@vger.kernel.org>; Sascha Hauer <kernel@pengutronix.de>
> Subject: Re: [RESEND v2 3/5] PCI: imx6: Fix the regulator dump when link
> never came up
> 
> Hi Richard,
> 
> On Fri, Oct 15, 2021 at 3:32 AM Richard Zhu <hongxing.zhu@nxp.com> wrote:
> >
> > When PCIe PHY link never came up and vpcie regulator is present, there
> > would be following dump when try to put the regulator.
> > Disable this regulator to fix this dump when link never came up.
> >
> >   imx6q-pcie 33800000.pcie: Phy link never came up
> >   imx6q-pcie: probe of 33800000.pcie failed with error -110
> >   ------------[ cut here ]------------
> >   WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256
> _regulator_put.part.0+0x14c/0x158
> >   Modules linked in:
> >   CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted
> 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
> >   Hardware name: FSL i.MX8MM EVK board (DT)
> >   Workqueue: events_unbound async_run_entry_fn
> >   pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> >   pc : _regulator_put.part.0+0x14c/0x158
> >   lr : regulator_put+0x34/0x48
> >   sp : ffff8000122ebb30
> >   x29: ffff8000122ebb30 x28: ffff800011be7000 x27: 0000000000000000
> >   x26: 0000000000000000 x25: 0000000000000000 x24: ffff00000025f2bc
> >   x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
> >   x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
> >   x17: 000000040044ffff x16: 00400032b5503510 x15:
> 0000000000000108
> >   x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
> >   x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
> >   x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
> >   x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
> >   x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
> >   Call trace:
> >    _regulator_put.part.0+0x14c/0x158
> >    regulator_put+0x34/0x48
> >    devm_regulator_release+0x10/0x18
> >    release_nodes+0x38/0x60
> >    devres_release_all+0x88/0xd0
> >    really_probe+0xd0/0x2e8
> >    __driver_probe_device+0x74/0xd8
> >    driver_probe_device+0x7c/0x108
> >    __device_attach_driver+0x8c/0xd0
> >    bus_for_each_drv+0x74/0xc0
> >    __device_attach_async_helper+0xb4/0xd8
> >    async_run_entry_fn+0x30/0x100
> >    process_one_work+0x19c/0x320
> >    worker_thread+0x48/0x418
> >    kthread+0x14c/0x158
> >    ret_from_fork+0x10/0x18
> >   ---[ end trace 3664ca4a50ce849b ]---
> >
> > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> 
> I am seeing this on imx6 too. When you send a v2, after addressing Lucas'
> comments, please add a Fixes tag/
[Richard Zhu] Okay, no problem. Thanks.
BR
Richard
Richard Zhu Oct. 20, 2021, 3:22 a.m. UTC | #5 
> -----Original Message-----
> From: Richard Zhu
> Sent: Tuesday, October 19, 2021 3:40 PM
> To: Lucas Stach <l.stach@pengutronix.de>; bhelgaas@google.com;
> lorenzo.pieralisi@arm.com
> Cc: linux-pci@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> kernel@pengutronix.de
> Subject: RE: [RESEND v2 3/5] PCI: imx6: Fix the regulator dump when link
> never came up
> 
> > -----Original Message-----
> > From: Lucas Stach <l.stach@pengutronix.de>
> > Sent: Saturday, October 16, 2021 2:23 AM
> > To: Richard Zhu <hongxing.zhu@nxp.com>; bhelgaas@google.com;
> > lorenzo.pieralisi@arm.com
> > Cc: linux-pci@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > kernel@pengutronix.de
> > Subject: Re: [RESEND v2 3/5] PCI: imx6: Fix the regulator dump when
> > link never came up
> >
> > Am Freitag, dem 15.10.2021 um 14:05 +0800 schrieb Richard Zhu:
> > > When PCIe PHY link never came up and vpcie regulator is present,
> > > there would be following dump when try to put the regulator.
> > > Disable this regulator to fix this dump when link never came up.
> > >
> > >   imx6q-pcie 33800000.pcie: Phy link never came up
> > >   imx6q-pcie: probe of 33800000.pcie failed with error -110
> > >   ------------[ cut here ]------------
> > >   WARNING: CPU: 3 PID: 119 at drivers/regulator/core.c:2256
> > _regulator_put.part.0+0x14c/0x158
> > >   Modules linked in:
> > >   CPU: 3 PID: 119 Comm: kworker/u8:2 Not tainted
> > 5.13.0-rc7-next-20210625-94710-ge4e92b2588a3 #10
> > >   Hardware name: FSL i.MX8MM EVK board (DT)
> > >   Workqueue: events_unbound async_run_entry_fn
> > >   pstate: 80000005 (Nzcv daif -PAN -UAO -TCO BTYPE=--)
> > >   pc : _regulator_put.part.0+0x14c/0x158
> > >   lr : regulator_put+0x34/0x48
> > >   sp : ffff8000122ebb30
> > >   x29: ffff8000122ebb30 x28: ffff800011be7000 x27:
> 0000000000000000
> > >   x26: 0000000000000000 x25: 0000000000000000 x24:
> ffff00000025f2bc
> > >   x23: ffff00000025f2c0 x22: ffff00000025f010 x21: ffff8000122ebc18
> > >   x20: ffff800011e3fa60 x19: ffff00000375fd80 x18: 0000000000000010
> > >   x17: 000000040044ffff x16: 00400032b5503510 x15:
> > 0000000000000108
> > >   x14: ffff0000003cc938 x13: 00000000ffffffea x12: 0000000000000000
> > >   x11: 0000000000000000 x10: ffff80001076ba88 x9 : ffff80001076a540
> > >   x8 : ffff00000025f2c0 x7 : ffff0000001f4450 x6 : ffff000000176cd8
> > >   x5 : ffff000003857880 x4 : 0000000000000000 x3 : ffff800011e3fe30
> > >   x2 : ffff0000003cc4c0 x1 : 0000000000000000 x0 : 0000000000000001
> > >   Call trace:
> > >    _regulator_put.part.0+0x14c/0x158
> > >    regulator_put+0x34/0x48
> > >    devm_regulator_release+0x10/0x18
> > >    release_nodes+0x38/0x60
> > >    devres_release_all+0x88/0xd0
> > >    really_probe+0xd0/0x2e8
> > >    __driver_probe_device+0x74/0xd8
> > >    driver_probe_device+0x7c/0x108
> > >    __device_attach_driver+0x8c/0xd0
> > >    bus_for_each_drv+0x74/0xc0
> > >    __device_attach_async_helper+0xb4/0xd8
> > >    async_run_entry_fn+0x30/0x100
> > >    process_one_work+0x19c/0x320
> > >    worker_thread+0x48/0x418
> > >    kthread+0x14c/0x158
> > >    ret_from_fork+0x10/0x18
> > >   ---[ end trace 3664ca4a50ce849b ]---
> > >
> > > Signed-off-by: Richard Zhu <hongxing.zhu@nxp.com>
> > > ---
> > >  drivers/pci/controller/dwc/pci-imx6.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > index 3372775834a2..cc837f8bf6d4 100644
> > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > @@ -853,6 +853,8 @@ static int imx6_pcie_start_link(struct dw_pcie
> *pci)
> > >  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
> > >  		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
> > >  	imx6_pcie_reset_phy(imx6_pcie);
> > > +	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) >
> 0)
> > > +		regulator_disable(imx6_pcie->vpcie);
> >
> > This doesn't seem like the right place to add this. I guess it would
> > be better to have proper error handling after dw_pcie_host_init to
> > roll back things like the clock and regulator enable.
> [Richard Zhu] This is good suggestion. It's make sense that move the regulator
> disable to the dw_pcie_host_init error handling since the regulator have the
> enabled check, before to disable it.
> But it seems that there are some potential problems when do the similar
> operations to the clocks disable here.
> Because that the dw_pcie_host_init might have some error returns when the
> host_init is not invoked at all.
> How to handle this situation although this is one corner case?
> 
[Richard Zhu] I have an idea to handle this case.
Add one more callback(e.x: host_exit() ) into dw_pcie_host_ops struct, then do the host_exit() when there is a error return after host_init() is invoked.
How about this solution?
I would send the v3 patch-set later if you don't say no to this method.

BR
Richard  

> Best Regards
> Richard Zhu
> >
> > Regards,
> > Lucas
> >
> > >  	return ret;
> > >  }
> > >
> >
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 3372775834a2..cc837f8bf6d4 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -853,6 +853,8 @@  static int imx6_pcie_start_link(struct dw_pcie *pci)
 		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG0),
 		dw_pcie_readl_dbi(pci, PCIE_PORT_DEBUG1));
 	imx6_pcie_reset_phy(imx6_pcie);
+	if (imx6_pcie->vpcie && regulator_is_enabled(imx6_pcie->vpcie) > 0)
+		regulator_disable(imx6_pcie->vpcie);
 	return ret;
 }