RE: [PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit() callback

[Hongxing Zhu]

> -----Original Message-----
> From: Rob Herring <robh@xxxxxxxxxx>
> Sent: 2022年4月4日 22:40
> To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> Cc: l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx; broonie@xxxxxxxxxx;
> lorenzo.pieralisi@xxxxxxx; jingoohan1@xxxxxxxxx; festevam@xxxxxxxxx;
> francesco.dolcini@xxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> kernel@xxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH v8 6/8] PCI: dwc: Add dw_pcie_host_ops.host_exit()
> callback
> 
> On Sat, Apr 02, 2022 at 03:03:00AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Rob Herring <robh@xxxxxxxxxx>
> > > Sent: 2022年4月2日 4:44
> > > To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> > > Cc: l.stach@xxxxxxxxxxxxxx; bhelgaas@xxxxxxxxxx; broonie@xxxxxxxxxx;
> > > lorenzo.pieralisi@xxxxxxx; jingoohan1@xxxxxxxxx;
> festevam@xxxxxxxxx;
> > > francesco.dolcini@xxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx;
> > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > kernel@xxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> > > Subject: Re: [PATCH v8 6/8] PCI: dwc: Add
> > > dw_pcie_host_ops.host_exit() callback
> > >
> > > On Fri, Feb 25, 2022 at 11:44:25AM +0800, Richard Zhu wrote:
> > > > When the link never comes up after ->host_init(), some drivers,
> > > > especially those that don't support hotplug, want to turn off
> > > > clocks and power supplies.
> > >
> > > Isn't supporting hotplug or not a board level decision? And hotplug
> > > doesn't have to mean physical plug/unplug. For example, you could
> > > have a soldered down PCIe device which needs regulators, resets, clocks,
> etc.
> > > for that device to be initialized before the link comes up. If that
> > > device is handled by a module loaded some time later, then the link
> > > may be down when you probe.
> > >
> > > I think the way this all needs to work is with runtime PM. If that's
> > > all in place, then either you shutdown clocks/power on timeout or
> > > via sysfs suspend. If there's a child device, then that should prevent
> suspending.
> > Hi Rob:
> > Thanks a lot for your review comments.
> > Understand what you mean.
> > i.MX PCIe doesn't support hot-plug from chip design view.
> 
> The scenario I described is not hotplug.
Hi Rob:
Sorry, I didn't get what you means before.
In the hotplug is not supported scenario.
Refer to my understand, both pci_host_probe->pci_scan_root_bus_bridge and
LTSSM training enable are one shot callback during i.MX PCIe probe procedure.
How the example you described in the previous emails can work well?
Please correct me if I'm wrong.

Best Regards
Richard Zhu

> 
> 
> > The ops.host_exit() callback is invoked only when the iMX PCIe driver
> > hooked  callback ops->start_link return an error.
> > For the platforms, that support the hot-plug feature, they can just
> > return one zero from their own ops->start_link.
> 
> You cannot have a per board start_link().
> 
> > In the current situation, i.MX PCIe does just return one zero when probe
> failed.
> > See the discussion and commit issued by Fabio below.
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.kernel.org%2Fproject%2Flinux-pci%2Fpatch%2F1641368602-20401-6-g
> i
> >
> t-send-email-hongxing.zhu%40nxp.com%2F&amp;data=04%7C01%7Chongxing
> .zhu
> > %40nxp.com%7Cb23fff6b2c264410da0a08da1648f105%7C686ea1d3bc2b4
> c6fa92cd9
> >
> 9c5c301635%7C0%7C1%7C637846799780646784%7CUnknown%7CTWFpbG
> Zsb3d8eyJWIj
> >
> oiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C30
> 00&am
> >
> p;sdata=fE9UxCq4S5K%2BUTPWfFR5h5r56%2B26g7anlPKvklo9sKw%3D&amp;
> reserve
> > d=0
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> >
> hwork.ozlabs.org%2Fproject%2Flinux-pci%2Fpatch%2F20220106103645.2790
> 80
> >
> 3-1-festevam%40gmail.com%2F&amp;data=04%7C01%7Chongxing.zhu%40nx
> p.com%
> >
> 7Cb23fff6b2c264410da0a08da1648f105%7C686ea1d3bc2b4c6fa92cd99c5c3
> 01635%
> >
> 7C0%7C1%7C637846799780646784%7CUnknown%7CTWFpbGZsb3d8eyJWIj
> oiMC4wLjAwM
> >
> DAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata
> =OG4
> >
> 5XQBOdqkH6B9KYmqU83mNs1wuHiOCb%2B3oL9ad6SQ%3D&amp;reserved=
> 0
> 
> That's a stable fix and different discussion.
> 
> > > > Add a new ->host_exit() callback in dw_pcie_host_ops so these
> > > > drivers can clean up if ->host_init() fails.
> > >
> > > I'm not really a fan of adding more ops nor the ops which aren't too
> > > specific about what they do. 'init' and 'exit' can be anything. I'd
> > > rather see more specific ops with the DWC core driver in charge of
> > > sequence of operations and the state.
> > Understand.
> 
> You don't seem to...
> 
> > i.MX PCIe can't handle the error exit properly in this case by itself.
> > So I  add one more ops.host_exit() in this series.
> >
> > Best Regards
> > Richard Zhu
> > >
> > > Rob