Re: [PATCH v2] PCI: kirin: Fix section mismatch warning

[Nathan Chancellor]

On Mon, Oct 01, 2018 at 03:53:41PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 20, 2018 at 11:58:28AM +0100, Lorenzo Pieralisi wrote:
> > [+Paul]
> > 
> > On Wed, Sep 19, 2018 at 02:08:08PM -0700, Nick Desaulniers wrote:
> > > On Wed, Sep 19, 2018 at 11:55 AM Nathan Chancellor
> > > <natechancellor@xxxxxxxxx> wrote:
> > > >
> > > > WARNING: vmlinux.o(.text+0x4758cc): Section mismatch in reference from
> > > > the function kirin_pcie_probe() to the function
> > > > .init.text:kirin_add_pcie_port()
> > > > The function kirin_pcie_probe() references
> > > > the function __init kirin_add_pcie_port().
> > > > This is often because kirin_pcie_probe lacks a __init
> > > > annotation or the annotation of kirin_add_pcie_port is wrong.
> > > >
> > > > Add the __init annotation to both kirin_pcie_probe and
> > > > kirin_pcie_add_msi then use builtin_platform_driver_probe
> > > > instead of builtin_platform_driver + .probe to avoid a section
> > > > mismatch warning with kirin_pcie_driver.
> > > >
> > > > Fixes: fc5165db245a ("PCI: kirin: Add HiSilicon Kirin SoC PCIe controller driver")
> > > > Reported-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > > > Suggested-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > > > Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> > > > ---
> > > >
> > > > v1 -> v2:
> > > >
> > > > Instead of removing the annotation on kirin_add_pcie_port, add it to
> > > > kirin_pcie_add_msi and kirin_pcie_probe. To avoid a warning with this
> > > > configuration, use builtin_platform_driver_probe.
> > > >
> > > >  drivers/pci/controller/dwc/pcie-kirin.c | 9 ++++-----
> > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-kirin.c b/drivers/pci/controller/dwc/pcie-kirin.c
> > > > index 5352e0c3be82..f64fed12de51 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-kirin.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-kirin.c
> > > > @@ -448,8 +448,8 @@ static const struct dw_pcie_host_ops kirin_pcie_host_ops = {
> > > >         .host_init = kirin_pcie_host_init,
> > > >  };
> > > >
> > > > -static int kirin_pcie_add_msi(struct dw_pcie *pci,
> > > > -                               struct platform_device *pdev)
> > > > +static int __init kirin_pcie_add_msi(struct dw_pcie *pci,
> > > > +                                    struct platform_device *pdev)
> > > >  {
> > > >         int irq;
> > > >
> > > > @@ -481,7 +481,7 @@ static int __init kirin_add_pcie_port(struct dw_pcie *pci,
> > > >         return dw_pcie_host_init(&pci->pp);
> > > >  }
> > > >
> > > > -static int kirin_pcie_probe(struct platform_device *pdev)
> > > > +static int __init kirin_pcie_probe(struct platform_device *pdev)
> > > >  {
> > > >         struct device *dev = &pdev->dev;
> > > >         struct kirin_pcie *kirin_pcie;
> > > > @@ -533,11 +533,10 @@ static const struct of_device_id kirin_pcie_match[] = {
> > > >  };
> > > >
> > > >  static struct platform_driver kirin_pcie_driver = {
> > > > -       .probe                  = kirin_pcie_probe,
> > > >         .driver                 = {
> > > >                 .name                   = "kirin-pcie",
> > > >                 .of_match_table = kirin_pcie_match,
> > > >                 .suppress_bind_attrs = true,
> > > >         },
> > > >  };
> > > > -builtin_platform_driver(kirin_pcie_driver);
> > > > +builtin_platform_driver_probe(kirin_pcie_driver, kirin_pcie_probe);
> > > 
> > > It would be good to get additional review from someone who knows more
> > > about driver callback lifecycles, but I think this is the correct fix.
> > > Thanks Nathan.
> > > Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> > 
> > AFAICS we can't use builtin_platform_driver_probe() if the probe()
> > itself may need deferral (and I *reckon* it may need that, see eg
> > clk_get()) so I suspect we have to go back to v1 for the patch,
> > apologies if so.
> 
> I think that the best course of action consists in merging v1 since
> we have not reached a definitive conclusion on v2, please let me
> know.
> 
> Thanks,
> Lorenzo

Hi Lorenzo,

I am perfectly fine with v1 being merged as either patch solves the
warning. I agree with Nick that if this version works it would be more
proper but it also carries more risk with regards to deferral like you
stated. Please let me know if I need to resend it.

Thanks,
Nathan