RE: [PATCH v10 4/6] PCI: hisi: Add PCIe host support for HiSilicon SoC Hip05

[Gabriele Paoloni]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@xxxxxxxx]
> Sent: Tuesday, October 13, 2015 12:19 PM
> To: Gabriele Paoloni
> Cc: Wangzhou (B); Bjorn Helgaas; Bjorn Helgaas; jingoohan1@xxxxxxxxx;
> pratyush.anand@xxxxxxxxx; linux@xxxxxxxxxxxxxxxx;
> thomas.petazzoni@xxxxxxxxxxxxxxxxxx; lorenzo.pieralisi@xxxxxxx;
> james.morse@xxxxxxx; Liviu.Dudau@xxxxxxx; jason@xxxxxxxxxxxxxx;
> robh@xxxxxxxxxx; gabriel.fernandez@xxxxxxxxxx;
> Minghuan.Lian@xxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; zhangjukuo; qiuzhenfa; liudongdong (C);
> qiujiang; xuwei (O); Liguozhu (Kenneth); Wangkefeng (Kevin); Rob
> Herring
> Subject: Re: [PATCH v10 4/6] PCI: hisi: Add PCIe host support for
> HiSilicon SoC Hip05
> 
> On Tuesday 13 October 2015 06:58:42 Gabriele Paoloni wrote:
> >
> > > >> +
> > > >> +static int __init hisi_pcie_init(void)
> > > >> +{
> > > >> +	return platform_driver_probe(&hisi_pcie_driver,
> hisi_pcie_probe);
> > > >> +}
> > > >> +subsys_initcall(hisi_pcie_init);
> > > >
> > > > Can you use module_platform_driver() or
> module_platform_driver_probe()
> > > > here instead of the subsys_initcall()?  No, I don't really know
> what
> > > > the difference between module_platform_driver() and
> > > > module_platform_driver_probe() is, sorry :)
> 
> module_platform_driver_probe() will only call the probe function once
> (and fail in case of -EPROBE_DEFER), while module_platform_driver()
> installs the platform driver in a way that the device can be bound
> and unbound at any point.
> 
> > > In fact, I used module_platform_driver_probe in previous version,
> but
> > > A PCIe VGA card of HiSilicon will use Hip05 PCIe host, so we
> modified
> > > module_platform_driver_probe to subsys_initcall which will be
> called
> > > before module_platform_driver_probe.
> > >
> > > We will upstream the driver of above PCIe VGA card soon.
> 
> I don't see a reason why a VGA driver would need the PCI host to be
> probed this early, unless it is the only usable console in the system.
> 
> > Hi Bjorn, firstly many thanks for looking at this.
> >
> > About this last bit the reason why we use subsys_initcall() is that
> > our host bridge is embedded in the SoC and as such is not hot-
> pluggable
> > for instance see:
> > http://lxr.free-electrons.com/source/Documentation/driver-
> model/platform.txt#L59
> 
> We should still be able to build the driver as a loadable module,
> even if you don't do that on your own kernels.

Hi Arnd, I don't see the point of having loadable KOs for platform
devices that are integrated into SoCs (like PCIe Host Controllers...)

> 
> This doesn't mean that it has to be module_platform_driver,
> subsys_initcall
> will also work in a loadable module, it just won't be as early. However,
> we should try to come up with a consistent approach for all PCI host
> drivers,
> I don't see any reason for hisi to be different from the others here.

To me it sounds more appropriate to adopt subsys_initcall() for all the
PCI Host Bridge controllers rather than having them as loadable modules...

What is your view?

Thanks

Gab

> 
> > Our approach is also used by Exynos:
> > http://lxr.free-electrons.com/source/drivers/pci/host/pci-
> exynos.c#L644
> 
> Maybe we should change that too.
> 
> 	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/