Re: [PATCH 0/4] Loadable Module support for PCIe Cadence and J721E

[manivannan.sadhasivam@xxxxxxxxxx]

On Wed, Mar 19, 2025 at 05:31:01PM +0800, Peter Chen wrote:
> On 25-03-19 14:25:34, Siddharth Vadapalli wrote:
> > > >
> > > > Hello,
> > > >
> > > > This series enables support to build the PCIe Cadence Controller drivers
> > > > and the PCI J721E Application/Wrapper/Glue driver as Loadable Kernel
> > > > Modules. The motivation for this series is that PCIe is not a necessity
> > > > for booting the SoC, due to which it doesn't have to be a built-in
> > > > module. Additionally, the defconfig doesn't enable the PCIe Cadence
> > > > Controller drivers and the PCI J721E driver, due to which PCIe is not
> > > > supported by default. Enabling the configs as of now (i.e. without this
> > > > series) will result in built-in drivers i.e. a bloated Linux Image for
> > > > everyone who doesn't have the PCIe Controller.
> > >
> > > If the user doesn't enable PCIe controller device through DTS/ACPI,
> > > that's doesn't matter.
> > 
> > The Linux Image for arm64 systems built using:
> > arch/arm64/configs/defconfig
> > will not have support for the Cadence PCIe Controller and the PCIe J721e
> > driver, because these configs aren't enabled.
> > 
> > >
> > > > @@ -209,6 +209,12 @@ CONFIG_NFC=m
> > > >  CONFIG_NFC_NCI=m
> > > >  CONFIG_NFC_S3FWRN5_I2C=m
> > > >  CONFIG_PCI=y
> > > > +CONFIG_PCI_J721E=m
> > > > +CONFIG_PCI_J721E_HOST=m
> > > > +CONFIG_PCI_J721E_EP=m
> > > > +CONFIG_PCIE_CADENCE=m
> > > > +CONFIG_PCIE_CADENCE_HOST=m
> > > > +CONFIG_PCIE_CADENCE_EP=m
> > >
> > > The common Cadence configuration will be select if the glue layer's
> > > configuration is select according to Kconfig.
> > >
> > > Please do not set common configuration as module, some user may need
> > > it as build-in like dw's. Considering the situation, the rootfs is at
> > > NVMe.
> > 
> > The common configuration at the moment is "DISABLED" i.e. no support for
> > the Cadence Controller at all. Which "user" are you referring to? This
> > series was introduced since having the drivers built-in was pushed back at:
> 
> We are using Cadence controller, and prepare upstream radxa-o6 board
> whose rootfs is at PCIe NVMe.
> 

It doesn't matter. Only criteria AFAIK to build the driver as built-in in
defconfig is that it should be a depedency for console. For some time, storage
was also a dependency, but for sure PCIe is not.

Moreover, CONFIG_BLK_DEV_NVME is built as a module in ARM64 defconfig. So it
doesn't matter if you build PCIe controller driver as a built-in or not. You
need to load the NVMe driver somehow.

So please use initramfs.

> You could build driver as module for TI glue layer, but don't force
> other vendors using module as well, see dwc as an example please.
> 

DWC is a bad example here. Only reason the DWC drivers are not loadable is due
to the in-built MSI controller implementation as irqchip. People tend to build
the irqchip controllers as always built-in for some known issues. Even then some
driver developers prefer to built them as loadable module but suppress unbind to
avoid rmmoding the module.

- Mani

-- 
மணிவண்ணன் சதாசிவம்