arm64: defconfig: enable PCIe controller on TI platforms

[PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Achal Verma 2 years, 9 months ago

Enable PCIe controller and serdes drivers to enable PCIe functionality.
* Enable Cadence serdes phy and wrapper driver.
* Enable Cadence PCIe controller driver.

Signed-off-by: Achal Verma <a-verma1@ti.com>
---
 arch/arm64/configs/defconfig | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index a24609e14d50..ff187dd585c2 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -230,9 +230,16 @@ CONFIG_PCIE_HISI_STB=y
 CONFIG_PCIE_TEGRA194_HOST=m
 CONFIG_PCIE_VISCONTI_HOST=y
 CONFIG_PCIE_LAYERSCAPE_GEN4=y
+CONFIG_PCIE_CADENCE=y
+CONFIG_PCIE_CADENCE_HOST=y
+CONFIG_PCIE_CADENCE_EP=y
+CONFIG_PCI_J721E=y
+CONFIG_PCI_J721E_HOST=y
+CONFIG_PCI_J721E_EP=y
 CONFIG_PCI_ENDPOINT=y
 CONFIG_PCI_ENDPOINT_CONFIGFS=y
 CONFIG_PCI_EPF_TEST=m
+CONFIG_PCI_EPF_NTB=m
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
 CONFIG_FW_LOADER_USER_HELPER=y
@@ -1330,8 +1337,8 @@ CONFIG_RESET_TI_SCI=y
 CONFIG_PHY_XGENE=y
 CONFIG_PHY_CAN_TRANSCEIVER=m
 CONFIG_PHY_SUN4I_USB=y
-CONFIG_PHY_CADENCE_TORRENT=m
-CONFIG_PHY_CADENCE_SIERRA=m
+CONFIG_PHY_CADENCE_TORRENT=y
+CONFIG_PHY_CADENCE_SIERRA=y
 CONFIG_PHY_MIXEL_MIPI_DPHY=m
 CONFIG_PHY_FSL_IMX8M_PCIE=y
 CONFIG_PHY_HI6220_USB=y
@@ -1363,8 +1370,8 @@ CONFIG_PHY_SAMSUNG_UFS=y
 CONFIG_PHY_UNIPHIER_USB2=y
 CONFIG_PHY_UNIPHIER_USB3=y
 CONFIG_PHY_TEGRA_XUSB=y
-CONFIG_PHY_AM654_SERDES=m
-CONFIG_PHY_J721E_WIZ=m
+CONFIG_PHY_AM654_SERDES=y
+CONFIG_PHY_J721E_WIZ=y
 CONFIG_ARM_CCI_PMU=m
 CONFIG_ARM_CCN=m
 CONFIG_ARM_CMN=m
-- 
2.25.1

Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Krzysztof Kozlowski 2 years, 9 months ago

On 09/05/2023 08:34, Achal Verma wrote:
> Enable PCIe controller and serdes drivers to enable PCIe functionality.
> * Enable Cadence serdes phy and wrapper driver.
> * Enable Cadence PCIe controller driver.

Why? IOW, which boards needs it?
> 
> Signed-off-by: Achal Verma <a-verma1@ti.com>
> ---
>  arch/arm64/configs/defconfig | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index a24609e14d50..ff187dd585c2 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -230,9 +230,16 @@ CONFIG_PCIE_HISI_STB=y
>  CONFIG_PCIE_TEGRA194_HOST=m
>  CONFIG_PCIE_VISCONTI_HOST=y
>  CONFIG_PCIE_LAYERSCAPE_GEN4=y
> +CONFIG_PCIE_CADENCE=y

=m

> +CONFIG_PCIE_CADENCE_HOST=y
> +CONFIG_PCIE_CADENCE_EP=y
> +CONFIG_PCI_J721E=y

=m

> +CONFIG_PCI_J721E_HOST=y
> +CONFIG_PCI_J721E_EP=y
>  CONFIG_PCI_ENDPOINT=y
>  CONFIG_PCI_ENDPOINT_CONFIGFS=y
>  CONFIG_PCI_EPF_TEST=m
> +CONFIG_PCI_EPF_NTB=m
>  CONFIG_DEVTMPFS=y
>  CONFIG_DEVTMPFS_MOUNT=y
>  CONFIG_FW_LOADER_USER_HELPER=y
> @@ -1330,8 +1337,8 @@ CONFIG_RESET_TI_SCI=y
>  CONFIG_PHY_XGENE=y
>  CONFIG_PHY_CAN_TRANSCEIVER=m
>  CONFIG_PHY_SUN4I_USB=y
> -CONFIG_PHY_CADENCE_TORRENT=m
> -CONFIG_PHY_CADENCE_SIERRA=m
> +CONFIG_PHY_CADENCE_TORRENT=y
> +CONFIG_PHY_CADENCE_SIERRA=y

Why? Commit msg does not explain it.

>  CONFIG_PHY_MIXEL_MIPI_DPHY=m
>  CONFIG_PHY_FSL_IMX8M_PCIE=y
>  CONFIG_PHY_HI6220_USB=y
> @@ -1363,8 +1370,8 @@ CONFIG_PHY_SAMSUNG_UFS=y
>  CONFIG_PHY_UNIPHIER_USB2=y
>  CONFIG_PHY_UNIPHIER_USB3=y
>  CONFIG_PHY_TEGRA_XUSB=y
> -CONFIG_PHY_AM654_SERDES=m
> -CONFIG_PHY_J721E_WIZ=m
> +CONFIG_PHY_AM654_SERDES=y
> +CONFIG_PHY_J721E_WIZ=y

Why?

>  CONFIG_ARM_CCI_PMU=m
>  CONFIG_ARM_CCN=m
>  CONFIG_ARM_CMN=m

Best regards,
Krzysztof

Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Vignesh Raghavendra 2 years, 9 months ago

Hi Achal,

On 09/05/23 12:09, Krzysztof Kozlowski wrote:
> On 09/05/2023 08:34, Achal Verma wrote:
>> Enable PCIe controller and serdes drivers to enable PCIe functionality.
>> * Enable Cadence serdes phy and wrapper driver.
>> * Enable Cadence PCIe controller driver.
> 
> Why? IOW, which boards needs it?
>>
>> Signed-off-by: Achal Verma <a-verma1@ti.com>
>> ---
>>  arch/arm64/configs/defconfig | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
>> index a24609e14d50..ff187dd585c2 100644
>> --- a/arch/arm64/configs/defconfig
>> +++ b/arch/arm64/configs/defconfig
>> @@ -230,9 +230,16 @@ CONFIG_PCIE_HISI_STB=y
>>  CONFIG_PCIE_TEGRA194_HOST=m
>>  CONFIG_PCIE_VISCONTI_HOST=y
>>  CONFIG_PCIE_LAYERSCAPE_GEN4=y
>> +CONFIG_PCIE_CADENCE=y
> 
> =m
> 
>> +CONFIG_PCIE_CADENCE_HOST=y
>> +CONFIG_PCIE_CADENCE_EP=y
>> +CONFIG_PCI_J721E=y
> 
> =m

Also, see [0] for history. We really want these to be 
modules unless its necessary for bootup.

You may want to revive [1] and get it to mainline

[0] https://lore.kernel.org/linux-arm-kernel/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/
[1] https://lore.kernel.org/linux-arm-kernel/20230110153805.GA1505901@bhelgaas/


[...]

-- 
Regards
Vignesh

Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Arnd Bergmann 2 years, 9 months ago

On Tue, May 9, 2023, at 10:08, Vignesh Raghavendra wrote:

> Also, see [0] for history. We really want these to be 
> modules unless its necessary for bootup.
>
> You may want to revive [1] and get it to mainline
>
> [0] 
> https://lore.kernel.org/linux-arm-kernel/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/
> [1] 
> https://lore.kernel.org/linux-arm-kernel/20230110153805.GA1505901@bhelgaas/

Agreed, that seems simple enough. Ideally these should even
be removable modules, not just single-load but unremovable.

Doing that may require changes to the cadence PCIe host
code if that does not support unloading yet (I have not
checked), but should not require any changes to the core
PCIe host code that supports loadable/removable modules.

     Arnd

Re: [EXTERNAL] Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Verma, Achal 2 years, 9 months ago

Hello,
On 5/9/2023 1:58 PM, Arnd Bergmann wrote:
> On Tue, May 9, 2023, at 10:08, Vignesh Raghavendra wrote:
> 
>> Also, see [0] for history. We really want these to be
>> modules unless its necessary for bootup.
>>
>> You may want to revive [1] and get it to mainline
>>
>> [0]
>> https://lore.kernel.org/linux-arm-kernel/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/
>> [1]
>> https://lore.kernel.org/linux-arm-kernel/20230110153805.GA1505901@bhelgaas/
> 
> Agreed, that seems simple enough. Ideally these should even
> be removable modules, not just single-load but unremovable.
> 
> Doing that may require changes to the cadence PCIe host
> code if that does not support unloading yet (I have not
> checked), but should not require any changes to the core
> PCIe host code that supports loadable/removable modules.
> 
>       Arnd
So, my understanding is that following change is expected
+CONFIG_PCIE_CADENCE=m
+CONFIG_PCIE_CADENCE_HOST=m
+CONFIG_PCIE_CADENCE_EP=m
+CONFIG_PCI_J721E=m
+CONFIG_PCI_J721E_HOST=m
+CONFIG_PCI_J721E_EP=m
+CONFIG_PCI_EPF_NTB=m

I also want to inform that pci_j721e.c is a single file with both host 
and EP functionality, last attempt to build it as modules depending on 
host or EP selected failed because of symbols dependency.
Refactoring of pci_j721e.c into common, host and EP specific files could 
work similar to the Cadence driver, So I will follow this way and push 
the changes.

Please let me know if there are concerns.


Thanks,
Achal Verma

Re: [EXTERNAL] Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Krzysztof Kozlowski 2 years, 9 months ago

On 09/05/2023 11:19, Verma, Achal wrote:
> 
> Hello,
> On 5/9/2023 1:58 PM, Arnd Bergmann wrote:
>> On Tue, May 9, 2023, at 10:08, Vignesh Raghavendra wrote:
>>
>>> Also, see [0] for history. We really want these to be
>>> modules unless its necessary for bootup.
>>>
>>> You may want to revive [1] and get it to mainline
>>>
>>> [0]
>>> https://lore.kernel.org/linux-arm-kernel/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/
>>> [1]
>>> https://lore.kernel.org/linux-arm-kernel/20230110153805.GA1505901@bhelgaas/
>>
>> Agreed, that seems simple enough. Ideally these should even
>> be removable modules, not just single-load but unremovable.
>>
>> Doing that may require changes to the cadence PCIe host
>> code if that does not support unloading yet (I have not
>> checked), but should not require any changes to the core
>> PCIe host code that supports loadable/removable modules.
>>
>>       Arnd
> So, my understanding is that following change is expected
> +CONFIG_PCIE_CADENCE=m
> +CONFIG_PCIE_CADENCE_HOST=m
> +CONFIG_PCIE_CADENCE_EP=m
> +CONFIG_PCI_J721E=m
> +CONFIG_PCI_J721E_HOST=m
> +CONFIG_PCI_J721E_EP=m
> +CONFIG_PCI_EPF_NTB=m
> 
> I also want to inform that pci_j721e.c is a single file with both host 
> and EP functionality, last attempt to build it as modules depending on 
> host or EP selected failed because of symbols dependency.
> Refactoring of pci_j721e.c into common, host and EP specific files could 
> work similar to the Cadence driver, So I will follow this way and push 
> the changes.
> 
> Please let me know if there are concerns.

Aren't HOST and EP just customizing main module?

Best regards,
Krzysztof

Re: [EXTERNAL] Re: [PATCH] arm64: defconfig: enable PCIe controller on TI platforms

Posted by Verma, Achal 2 years, 9 months ago


On 5/9/2023 3:01 PM, Krzysztof Kozlowski wrote:
> On 09/05/2023 11:19, Verma, Achal wrote:
>>
>> Hello,
>> On 5/9/2023 1:58 PM, Arnd Bergmann wrote:
>>> On Tue, May 9, 2023, at 10:08, Vignesh Raghavendra wrote:
>>>
>>>> Also, see [0] for history. We really want these to be
>>>> modules unless its necessary for bootup.
>>>>
>>>> You may want to revive [1] and get it to mainline
>>>>
>>>> [0]
>>>> https://lore.kernel.org/linux-arm-kernel/CAK8P3a2VSBvOn1o+q1PYZaQ6LS9U4cz+DZGuDbisHkwNs2dAAw@mail.gmail.com/
>>>> [1]
>>>> https://lore.kernel.org/linux-arm-kernel/20230110153805.GA1505901@bhelgaas/
>>>
>>> Agreed, that seems simple enough. Ideally these should even
>>> be removable modules, not just single-load but unremovable.
>>>
>>> Doing that may require changes to the cadence PCIe host
>>> code if that does not support unloading yet (I have not
>>> checked), but should not require any changes to the core
>>> PCIe host code that supports loadable/removable modules.
>>>
>>>        Arnd
>> So, my understanding is that following change is expected
>> +CONFIG_PCIE_CADENCE=m
>> +CONFIG_PCIE_CADENCE_HOST=m
>> +CONFIG_PCIE_CADENCE_EP=m
>> +CONFIG_PCI_J721E=m
>> +CONFIG_PCI_J721E_HOST=m
>> +CONFIG_PCI_J721E_EP=m
>> +CONFIG_PCI_EPF_NTB=m
>>
>> I also want to inform that pci_j721e.c is a single file with both host
>> and EP functionality, last attempt to build it as modules depending on
>> host or EP selected failed because of symbols dependency.
>> Refactoring of pci_j721e.c into common, host and EP specific files could
>> work similar to the Cadence driver, So I will follow this way and push
>> the changes.
>>
>> Please let me know if there are concerns.
> 
> Aren't HOST and EP just customizing main module?
Current pci-j721e driver is composed of host specific and endpoint 
specific code encapsulated in switch case and shared required common 
code, last attempt of putting endpoint and host specific code inside EP 
and HOST CONFIGS respectively failed (also reported by kernel test bot) 
when tried with different combinations of Y/M/N along with CADENCE 
driver on which pci-j721e depends, So only way is to break main module 
into common, host and endpoint files.
> 
> Best regards,
> Krzysztof
>