Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization

[Vidya Sagar]

On 11/29/2019 8:10 PM, Vidya Sagar wrote:

Hi Christoph,
Could you please let me know what am I missing here?

Thanks,
Vidya Sagar

> On 11/27/2019 3:18 PM, Christoph Hellwig wrote:
> > On Wed, Nov 13, 2019 at 02:38:48PM +0530, Vidya Sagar wrote:
> > > +ÂÂÂ if (ep->ops->get_features) {
> > > +ÂÂÂÂÂÂÂ epc_features = ep->ops->get_features(ep);
> > > +ÂÂÂÂÂÂÂ if (epc_features->skip_core_init)
> > > +ÂÂÂÂÂÂÂÂÂÂÂ return 0;
> > > ÂÂÂÂÂ }
> > > +ÂÂÂ return dw_pcie_ep_init_complete(ep);
> >
> > This calling convention is strange. Just split the early part of
> > dw_pcie_ep_init into an dw_pcie_ep_early and either add a tiny
> > wrapper like:
> >
> > int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > {
> > ÂÂÂÂint error;
> >
> > ÂÂÂÂerror = dw_pcie_ep_init_early(ep);
> > ÂÂÂÂif (error)
> > ÂÂÂÂÂÂÂ return error;
> > ÂÂÂÂreturn dw_pcie_ep_init_late(ep);
> > }
> >
> > or just open code that in the few callers. That keeps the calling
> > conventions much simpler and avoids relying on a callback and flag.
> I'm not sure if I got this right. I think in any case, code that is going to be
> part of dw_pcie_ep_init_late() needs to depend on callback and flag right?
> I mean, unless it is confirmed (by calling the get_features() callback and
> checking whether or not the core is available for programming) dw_pcie_ep_init_late()
> can't be called right?
> Please let me know if I'm missing something here.
>
> - Vidya Sagar
> >