mbox series

[v4,00/12] Fix and improve the Rockchip endpoint driver

Message ID 20241011121408.89890-1-dlemoal@kernel.org
Headers show
Series Fix and improve the Rockchip endpoint driver | expand

Message

Damien Le Moal Oct. 11, 2024, 12:13 p.m. UTC
This patch series fix the PCI address mapping handling of the Rockchip
endpoint driver, refactor some of its code, improves link training and
adds handling of the #PERST signal.

This series is organized as follows:
 - Patch 1 fixes the rockchip ATU programming
 - Patch 2, 3 and 4 introduce small code improvments
 - Patch 5 implements the .get_mem_map() operation to make the RK3399
   endpoint controller driver fully functional with the new
   pci_epc_mem_map() function
 - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
 - Patch 10 introduces the .stop() endpoint controller operation to
   correctly disable the endpopint controller after use
 - Patch 11 improves link training
 - Patch 12 implements handling of the #PERST signal

This patch series depends on the PCI endpoint core patches from the
V5 series "Improve PCI memory mapping API". The patches were tested
using a Pine Rockpro64 board used as an endpoint with the test endpoint
function driver and a prototype nvme endpoint function driver.

Changes from v3:
 - Addressed Mani's comments (see mailing list for details).
 - Removed old patch 11 (dt-binding changes) and instead use in patch 12
   the already defined reset_gpios property.
 - Added patch 6
 - Added review tags

Changes from v2:
 - Split the patch series
 - Corrected patch 11 to add the missing "maxItem"

Changes from v1:
 - Changed pci_epc_check_func() to pci_epc_function_is_valid() in patch
   1.
 - Removed patch "PCI: endpoint: Improve pci_epc_mem_alloc_addr()"
   (former patch 2 of v1)
 - Various typos cleanups all over. Also fixed some blank space
   indentation.
 - Added review tags

Damien Le Moal (12):
  PCI: rockchip-ep: Fix address translation unit programming
  PCI: rockchip-ep: Use a macro to define EP controller .align feature
  PCI: rockchip-ep: Improve rockchip_pcie_ep_unmap_addr()
  PCI: rockchip-ep: Improve rockchip_pcie_ep_map_addr()
  PCI: rockchip-ep: Implement the pci_epc_ops::get_mem_map() operation
  PCI: rockchip-ep: Rename rockchip_pcie_parse_ep_dt()
  PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() memory allocations
  PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() MSI-X hiding
  PCI: rockchip-ep: Refactor endpoint link training enable
  PCI: rockship-ep: Implement the pci_epc_ops::stop_link() operation
  PCI: rockchip-ep: Improve link training
  PCI: rockchip-ep: Handle PERST# signal in endpoint mode

 drivers/pci/controller/pcie-rockchip-ep.c   | 408 ++++++++++++++++----
 drivers/pci/controller/pcie-rockchip-host.c |   4 +-
 drivers/pci/controller/pcie-rockchip.c      |  21 +-
 drivers/pci/controller/pcie-rockchip.h      |  24 +-
 4 files changed, 370 insertions(+), 87 deletions(-)

Comments

Anand Moon Oct. 16, 2024, 5:32 a.m. UTC | #1
Hi Damien,

On Fri, 11 Oct 2024 at 17:55, Damien Le Moal <dlemoal@kernel.org> wrote:
>
> This patch series fix the PCI address mapping handling of the Rockchip
> endpoint driver, refactor some of its code, improves link training and
> adds handling of the #PERST signal.
>
> This series is organized as follows:
>  - Patch 1 fixes the rockchip ATU programming
>  - Patch 2, 3 and 4 introduce small code improvments
>  - Patch 5 implements the .get_mem_map() operation to make the RK3399
>    endpoint controller driver fully functional with the new
>    pci_epc_mem_map() function
>  - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
>  - Patch 10 introduces the .stop() endpoint controller operation to
>    correctly disable the endpopint controller after use
>  - Patch 11 improves link training
>  - Patch 12 implements handling of the #PERST signal
>
> This patch series depends on the PCI endpoint core patches from the
> V5 series "Improve PCI memory mapping API". The patches were tested
> using a Pine Rockpro64 board used as an endpoint with the test endpoint
> function driver and a prototype nvme endpoint function driver.

Can we test this feature on Radxa Rock PI 4b hardware with an external
nvme card?

Thanks
-Anand

>
> Changes from v3:
>  - Addressed Mani's comments (see mailing list for details).
>  - Removed old patch 11 (dt-binding changes) and instead use in patch 12
>    the already defined reset_gpios property.
>  - Added patch 6
>  - Added review tags
>
> Changes from v2:
>  - Split the patch series
>  - Corrected patch 11 to add the missing "maxItem"
>
> Changes from v1:
>  - Changed pci_epc_check_func() to pci_epc_function_is_valid() in patch
>    1.
>  - Removed patch "PCI: endpoint: Improve pci_epc_mem_alloc_addr()"
>    (former patch 2 of v1)
>  - Various typos cleanups all over. Also fixed some blank space
>    indentation.
>  - Added review tags
>
> Damien Le Moal (12):
>   PCI: rockchip-ep: Fix address translation unit programming
>   PCI: rockchip-ep: Use a macro to define EP controller .align feature
>   PCI: rockchip-ep: Improve rockchip_pcie_ep_unmap_addr()
>   PCI: rockchip-ep: Improve rockchip_pcie_ep_map_addr()
>   PCI: rockchip-ep: Implement the pci_epc_ops::get_mem_map() operation
>   PCI: rockchip-ep: Rename rockchip_pcie_parse_ep_dt()
>   PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() memory allocations
>   PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() MSI-X hiding
>   PCI: rockchip-ep: Refactor endpoint link training enable
>   PCI: rockship-ep: Implement the pci_epc_ops::stop_link() operation
>   PCI: rockchip-ep: Improve link training
>   PCI: rockchip-ep: Handle PERST# signal in endpoint mode
>
>  drivers/pci/controller/pcie-rockchip-ep.c   | 408 ++++++++++++++++----
>  drivers/pci/controller/pcie-rockchip-host.c |   4 +-
>  drivers/pci/controller/pcie-rockchip.c      |  21 +-
>  drivers/pci/controller/pcie-rockchip.h      |  24 +-
>  4 files changed, 370 insertions(+), 87 deletions(-)
>
> --
> 2.47.0
>
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
Damien Le Moal Oct. 16, 2024, 6:15 a.m. UTC | #2
On 10/16/24 2:32 PM, Anand Moon wrote:
> Hi Damien,
> 
> On Fri, 11 Oct 2024 at 17:55, Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> This patch series fix the PCI address mapping handling of the Rockchip
>> endpoint driver, refactor some of its code, improves link training and
>> adds handling of the #PERST signal.
>>
>> This series is organized as follows:
>>  - Patch 1 fixes the rockchip ATU programming
>>  - Patch 2, 3 and 4 introduce small code improvments
>>  - Patch 5 implements the .get_mem_map() operation to make the RK3399
>>    endpoint controller driver fully functional with the new
>>    pci_epc_mem_map() function
>>  - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
>>  - Patch 10 introduces the .stop() endpoint controller operation to
>>    correctly disable the endpopint controller after use
>>  - Patch 11 improves link training
>>  - Patch 12 implements handling of the #PERST signal
>>
>> This patch series depends on the PCI endpoint core patches from the
>> V5 series "Improve PCI memory mapping API". The patches were tested
>> using a Pine Rockpro64 board used as an endpoint with the test endpoint
>> function driver and a prototype nvme endpoint function driver.
> 
> Can we test this feature on Radxa Rock PI 4b hardware with an external
> nvme card?

This patch series is to fix the PCI controller operation in endpoint (EP) mode.
If you only want to use an NVMe device connected to the board M.2 M-Key slot,
these patches are not needed. If that board PCI controller does not work as a
PCI host (RC mode), then these patches will not help.
Anand Moon Oct. 16, 2024, 7:22 a.m. UTC | #3
Hi Damien,

On Wed, 16 Oct 2024 at 11:45, Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 10/16/24 2:32 PM, Anand Moon wrote:
> > Hi Damien,
> >
> > On Fri, 11 Oct 2024 at 17:55, Damien Le Moal <dlemoal@kernel.org> wrote:
> >>
> >> This patch series fix the PCI address mapping handling of the Rockchip
> >> endpoint driver, refactor some of its code, improves link training and
> >> adds handling of the #PERST signal.
> >>
> >> This series is organized as follows:
> >>  - Patch 1 fixes the rockchip ATU programming
> >>  - Patch 2, 3 and 4 introduce small code improvments
> >>  - Patch 5 implements the .get_mem_map() operation to make the RK3399
> >>    endpoint controller driver fully functional with the new
> >>    pci_epc_mem_map() function
> >>  - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
> >>  - Patch 10 introduces the .stop() endpoint controller operation to
> >>    correctly disable the endpopint controller after use
> >>  - Patch 11 improves link training
> >>  - Patch 12 implements handling of the #PERST signal
> >>
> >> This patch series depends on the PCI endpoint core patches from the
> >> V5 series "Improve PCI memory mapping API". The patches were tested
> >> using a Pine Rockpro64 board used as an endpoint with the test endpoint
> >> function driver and a prototype nvme endpoint function driver.
> >
> > Can we test this feature on Radxa Rock PI 4b hardware with an external
> > nvme card?
>
> This patch series is to fix the PCI controller operation in endpoint (EP) mode.
> If you only want to use an NVMe device connected to the board M.2 M-Key slot,
> these patches are not needed. If that board PCI controller does not work as a
> PCI host (RC mode), then these patches will not help.
>

Thanks for your inputs, I don't think my board supports this feature.

> --
> Damien Le Moal
> Western Digital Research

Thanks
-Anand
Damien Le Moal Oct. 16, 2024, 8:08 a.m. UTC | #4
On 10/16/24 4:22 PM, Anand Moon wrote:
> Hi Damien,
> 
> On Wed, 16 Oct 2024 at 11:45, Damien Le Moal <dlemoal@kernel.org> wrote:
>>
>> On 10/16/24 2:32 PM, Anand Moon wrote:
>>> Hi Damien,
>>>
>>> On Fri, 11 Oct 2024 at 17:55, Damien Le Moal <dlemoal@kernel.org> wrote:
>>>>
>>>> This patch series fix the PCI address mapping handling of the Rockchip
>>>> endpoint driver, refactor some of its code, improves link training and
>>>> adds handling of the #PERST signal.
>>>>
>>>> This series is organized as follows:
>>>>  - Patch 1 fixes the rockchip ATU programming
>>>>  - Patch 2, 3 and 4 introduce small code improvments
>>>>  - Patch 5 implements the .get_mem_map() operation to make the RK3399
>>>>    endpoint controller driver fully functional with the new
>>>>    pci_epc_mem_map() function
>>>>  - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
>>>>  - Patch 10 introduces the .stop() endpoint controller operation to
>>>>    correctly disable the endpopint controller after use
>>>>  - Patch 11 improves link training
>>>>  - Patch 12 implements handling of the #PERST signal
>>>>
>>>> This patch series depends on the PCI endpoint core patches from the
>>>> V5 series "Improve PCI memory mapping API". The patches were tested
>>>> using a Pine Rockpro64 board used as an endpoint with the test endpoint
>>>> function driver and a prototype nvme endpoint function driver.
>>>
>>> Can we test this feature on Radxa Rock PI 4b hardware with an external
>>> nvme card?
>>
>> This patch series is to fix the PCI controller operation in endpoint (EP) mode.
>> If you only want to use an NVMe device connected to the board M.2 M-Key slot,
>> these patches are not needed. If that board PCI controller does not work as a
>> PCI host (RC mode), then these patches will not help.
>>
> 
> Thanks for your inputs, I don't think my board supports this feature.

The Rock 4B board uses a RK3399 SoC. So the PCIe port should work as long as
you have the right device tree for the board. The mainline kernel currently has
this DT:

rk3399-rock-pi-4b.dts

Which uses

rk3399-rock-pi-4.dtsi

which has:

&pcie0 {
        ep-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_HIGH>;
        num-lanes = <4>;
        pinctrl-0 = <&pcie_clkreqnb_cpm>;
        pinctrl-names = "default";
        vpcie0v9-supply = <&vcc_0v9>;
        vpcie1v8-supply = <&vcc_1v8>;
        vpcie3v3-supply = <&vcc3v3_pcie>;
        status = "okay";
};

So it looks to me like the PCIe port is supported just fine. FOr the PCIe port
node definition look at rk3399.dtsi and rk3399-base.dtsi.
Anand Moon Oct. 19, 2024, 6:24 a.m. UTC | #5
Hi Damien,

On Wed, 16 Oct 2024 at 13:38, Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 10/16/24 4:22 PM, Anand Moon wrote:
> > Hi Damien,
> >
> > On Wed, 16 Oct 2024 at 11:45, Damien Le Moal <dlemoal@kernel.org> wrote:
> >>
> >> On 10/16/24 2:32 PM, Anand Moon wrote:
> >>> Hi Damien,
> >>>
> >>> On Fri, 11 Oct 2024 at 17:55, Damien Le Moal <dlemoal@kernel.org> wrote:
> >>>>
> >>>> This patch series fix the PCI address mapping handling of the Rockchip
> >>>> endpoint driver, refactor some of its code, improves link training and
> >>>> adds handling of the #PERST signal.
> >>>>
> >>>> This series is organized as follows:
> >>>>  - Patch 1 fixes the rockchip ATU programming
> >>>>  - Patch 2, 3 and 4 introduce small code improvments
> >>>>  - Patch 5 implements the .get_mem_map() operation to make the RK3399
> >>>>    endpoint controller driver fully functional with the new
> >>>>    pci_epc_mem_map() function
> >>>>  - Patch 6, 7, 8 and 9 refactor the driver code to make it more readable
> >>>>  - Patch 10 introduces the .stop() endpoint controller operation to
> >>>>    correctly disable the endpopint controller after use
> >>>>  - Patch 11 improves link training
> >>>>  - Patch 12 implements handling of the #PERST signal
> >>>>
> >>>> This patch series depends on the PCI endpoint core patches from the
> >>>> V5 series "Improve PCI memory mapping API". The patches were tested
> >>>> using a Pine Rockpro64 board used as an endpoint with the test endpoint
> >>>> function driver and a prototype nvme endpoint function driver.
> >>>
> >>> Can we test this feature on Radxa Rock PI 4b hardware with an external
> >>> nvme card?
> >>
> >> This patch series is to fix the PCI controller operation in endpoint (EP) mode.
> >> If you only want to use an NVMe device connected to the board M.2 M-Key slot,
> >> these patches are not needed. If that board PCI controller does not work as a
> >> PCI host (RC mode), then these patches will not help.
> >>
> >
> > Thanks for your inputs, I don't think my board supports this feature.
>
> The Rock 4B board uses a RK3399 SoC. So the PCIe port should work as long as
> you have the right device tree for the board. The mainline kernel currently has
> this DT:
>
> rk3399-rock-pi-4b.dts
>
> Which uses
>
> rk3399-rock-pi-4.dtsi
>
> which has:
>
> &pcie0 {
>         ep-gpios = <&gpio4 RK_PD3 GPIO_ACTIVE_HIGH>;
>         num-lanes = <4>;
>         pinctrl-0 = <&pcie_clkreqnb_cpm>;
>         pinctrl-names = "default";
>         vpcie0v9-supply = <&vcc_0v9>;
>         vpcie1v8-supply = <&vcc_1v8>;
>         vpcie3v3-supply = <&vcc3v3_pcie>;
>         status = "okay";
> };
>
> So it looks to me like the PCIe port is supported just fine. FOr the PCIe port
> node definition look at rk3399.dtsi and rk3399-base.dtsi.
>
I have a question can new test external low power GPU with external cables
which supports PCI host (RC mode) with external power supply for GPU card.

Which mode is suitable for the PCIe endpoint controller or PCIe host controller?

> --
> Damien Le Moal
> Western Digital Research

Thanks
-Anand
Damien Le Moal Oct. 20, 2024, 1:06 a.m. UTC | #6
On 10/19/24 15:24, Anand Moon wrote:
> I have a question can new test external low power GPU with external cables
> which supports PCI host (RC mode) with external power supply for GPU card.

I do not understand this sentence.

> Which mode is suitable for the PCIe endpoint controller or PCIe host controller?

If you do not know/understand what PCI endpoint is, you probably do not need it
at all. If you are using your board to simply connect and use regular PCI
devices, you do not need the PCI endpoint framework/drivers.
Anand Moon Oct. 20, 2024, 3:18 a.m. UTC | #7
Hi Damien

On Sun, 20 Oct 2024 at 06:36, Damien Le Moal <dlemoal@kernel.org> wrote:
>
> On 10/19/24 15:24, Anand Moon wrote:
> > I have a question can new test external low power GPU with external cables
> > which supports PCI host (RC mode) with external power supply for GPU card.
>
> I do not understand this sentence.
Sorry for my poor English,

I wanted to check the feasibility of testing external GPU on the
Rockchip platform,
Just like Jeff Gearing

[1] https://www.jeffgeerling.com/blog/2024/use-external-gpu-on-raspberry-pi-5-4k-gaming

>
> > Which mode is suitable for the PCIe endpoint controller or PCIe host controller?
>
> If you do not know/understand what PCI endpoint is, you probably do not need it
> at all. If you are using your board to simply connect and use regular PCI
> devices, you do not need the PCI endpoint framework/drivers.
>
Ok.
> --
> Damien Le Moal
> Western Digital Research

Thanks
-Anand