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

["hans.zhang" <hans.zhang@cixtech.com>]

On 2025/3/20 10:14, hans.zhang wrote:
> EXTERNAL EMAIL
> 
> On 2025/3/19 17:55, manivannan.sadhasivam@linaro.org wrote:
>> EXTERNAL EMAIL
>>
>> 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.
> Hi Mani,
> 
> I think the MSI RTL module provided by Synopsys PCIe controller IP is
> not a standard operation. The reason for this MSI module is probably to
> be used by some cpus that do not have ITS(LPI interrupt) designed. Or
> RISC-V SOC, etc. MSI is defined as an MSI/MSIX interrupt that starts
> with a direct write memory access.
> 
> There are also SOC vendors that do not use the built-in MSI RTL module.
> Instead, MSI/MSIX interrupts are transmitted directly to the GIC's ITS
> module via the GIC V3/V4 interface. For example, RK3588, they do not use
> the PCIe controller built-in MSI module. Some Qualcomm platforms also
> modify the PCIe controller's built-in MSI modules to connect each of
> them to 32 SPI interrupts to the GIC. I was under the impression that
> the SDM845 was designed that way. The only explanation is that SPI
> interrupts are faster than LPI interrupts without having to look up some
> tables.
> 
> So the dwc driver can also compile to ko?

Additional reason:

Often in SOC design, the RTL designer does not understand the dwc-driver 
behavior and causes some RTL bugs, and the software needs to workaround. 
  As a result, the dwc driver of build-in cannot be used, and the dwc 
driver needs to be modified, which will make it easier to compile the 
dwc driver to module.

Best regards,
Hans