mbox series

[v5,00/14] Fix and improve the Rockchip endpoint driver

Message ID 20241017015849.190271-1-dlemoal@kernel.org (mailing list archive)
Headers show
Series Fix and improve the Rockchip endpoint driver | expand

Message

Damien Le Moal Oct. 17, 2024, 1:58 a.m. UTC
This patch series fix the PCI address mapping handling of the Rockchip
PCI 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 .align_addr() operation to make the RK3399
   endpoint controller driver fully functional with the new
   pci_epc_mem_map() function
 - Patch 6 uses the new align_addr operation function to fix the ATU
   programming for MSI IRQ data mapping
 - Patch 7, 8, 9 and 10 refactor the driver code to make it more
   readable
 - Patch 11 introduces the .stop() endpoint controller operation to
   correctly disable the endpopint controller after use
 - Patch 12 improves link training
 - Patch 13 implements handling of the #PERST signal
 - Patch 14 adds a DT overlay file to enable EP mode and define the
   PERST# GPIO (reset-gpios) property.

These 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 v4:
 - Added patch 6
 - Added comments to patch 12 and 13 to clarify link training handling
   and PERST# GPIO use.
 - Added patch 14

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 (14):
  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::align_addr() operation
  PCI: rockchip-ep: Fix MSI IRQ data mapping
  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
  arm64: dts: rockchip: Add rockpro64 overlay for PCIe endpoint mode

 arch/arm64/boot/dts/rockchip/Makefile         |   1 +
 .../rockchip/rk3399-rockpro64-pcie-ep.dtso    |  20 +
 drivers/pci/controller/pcie-rockchip-ep.c     | 432 ++++++++++++++----
 drivers/pci/controller/pcie-rockchip-host.c   |   4 +-
 drivers/pci/controller/pcie-rockchip.c        |  21 +-
 drivers/pci/controller/pcie-rockchip.h        |  24 +-
 6 files changed, 406 insertions(+), 96 deletions(-)
 create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rockpro64-pcie-ep.dtso

Comments

Damien Le Moal Oct. 29, 2024, 10:35 a.m. UTC | #1
On 10/17/24 10:58, Damien Le Moal wrote:
> This patch series fix the PCI address mapping handling of the Rockchip
> PCI 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 .align_addr() operation to make the RK3399
>    endpoint controller driver fully functional with the new
>    pci_epc_mem_map() function
>  - Patch 6 uses the new align_addr operation function to fix the ATU
>    programming for MSI IRQ data mapping
>  - Patch 7, 8, 9 and 10 refactor the driver code to make it more
>    readable
>  - Patch 11 introduces the .stop() endpoint controller operation to
>    correctly disable the endpopint controller after use
>  - Patch 12 improves link training
>  - Patch 13 implements handling of the #PERST signal
>  - Patch 14 adds a DT overlay file to enable EP mode and define the
>    PERST# GPIO (reset-gpios) property.
> 
> These 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.

Ping ? If there are no issues, can we get this queued up ?

> 
> Changes from v4:
>  - Added patch 6
>  - Added comments to patch 12 and 13 to clarify link training handling
>    and PERST# GPIO use.
>  - Added patch 14
> 
> 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 (14):
>   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::align_addr() operation
>   PCI: rockchip-ep: Fix MSI IRQ data mapping
>   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
>   arm64: dts: rockchip: Add rockpro64 overlay for PCIe endpoint mode
> 
>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>  .../rockchip/rk3399-rockpro64-pcie-ep.dtso    |  20 +
>  drivers/pci/controller/pcie-rockchip-ep.c     | 432 ++++++++++++++----
>  drivers/pci/controller/pcie-rockchip-host.c   |   4 +-
>  drivers/pci/controller/pcie-rockchip.c        |  21 +-
>  drivers/pci/controller/pcie-rockchip.h        |  24 +-
>  6 files changed, 406 insertions(+), 96 deletions(-)
>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rockpro64-pcie-ep.dtso
>
Damien Le Moal Nov. 13, 2024, 2:29 p.m. UTC | #2
On 10/29/24 19:35, Damien Le Moal wrote:
> On 10/17/24 10:58, Damien Le Moal wrote:
>> This patch series fix the PCI address mapping handling of the Rockchip
>> PCI 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 .align_addr() operation to make the RK3399
>>    endpoint controller driver fully functional with the new
>>    pci_epc_mem_map() function
>>  - Patch 6 uses the new align_addr operation function to fix the ATU
>>    programming for MSI IRQ data mapping
>>  - Patch 7, 8, 9 and 10 refactor the driver code to make it more
>>    readable
>>  - Patch 11 introduces the .stop() endpoint controller operation to
>>    correctly disable the endpopint controller after use
>>  - Patch 12 improves link training
>>  - Patch 13 implements handling of the #PERST signal
>>  - Patch 14 adds a DT overlay file to enable EP mode and define the
>>    PERST# GPIO (reset-gpios) property.
>>
>> These 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.
> 
> Ping ? If there are no issues, can we get this queued up ?

Mani,

Ping AGAIN !!!!

I do not see anything queued in pci/next. What is the blocker ?
These patches have been sitting on the list for nearly a month now, PLEASE DO
SOMETHING. Comment or apply, but please reply something.

> 
>>
>> Changes from v4:
>>  - Added patch 6
>>  - Added comments to patch 12 and 13 to clarify link training handling
>>    and PERST# GPIO use.
>>  - Added patch 14
>>
>> 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 (14):
>>   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::align_addr() operation
>>   PCI: rockchip-ep: Fix MSI IRQ data mapping
>>   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
>>   arm64: dts: rockchip: Add rockpro64 overlay for PCIe endpoint mode
>>
>>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>>  .../rockchip/rk3399-rockpro64-pcie-ep.dtso    |  20 +
>>  drivers/pci/controller/pcie-rockchip-ep.c     | 432 ++++++++++++++----
>>  drivers/pci/controller/pcie-rockchip-host.c   |   4 +-
>>  drivers/pci/controller/pcie-rockchip.c        |  21 +-
>>  drivers/pci/controller/pcie-rockchip.h        |  24 +-
>>  6 files changed, 406 insertions(+), 96 deletions(-)
>>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rockpro64-pcie-ep.dtso
>>
> 
>
Manivannan Sadhasivam Nov. 13, 2024, 5:52 p.m. UTC | #3
On Wed, Nov 13, 2024 at 11:29:48PM +0900, Damien Le Moal wrote:
> On 10/29/24 19:35, Damien Le Moal wrote:
> > On 10/17/24 10:58, Damien Le Moal wrote:
> >> This patch series fix the PCI address mapping handling of the Rockchip
> >> PCI 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 .align_addr() operation to make the RK3399
> >>    endpoint controller driver fully functional with the new
> >>    pci_epc_mem_map() function
> >>  - Patch 6 uses the new align_addr operation function to fix the ATU
> >>    programming for MSI IRQ data mapping
> >>  - Patch 7, 8, 9 and 10 refactor the driver code to make it more
> >>    readable
> >>  - Patch 11 introduces the .stop() endpoint controller operation to
> >>    correctly disable the endpopint controller after use
> >>  - Patch 12 improves link training
> >>  - Patch 13 implements handling of the #PERST signal
> >>  - Patch 14 adds a DT overlay file to enable EP mode and define the
> >>    PERST# GPIO (reset-gpios) property.
> >>
> >> These 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.
> > 
> > Ping ? If there are no issues, can we get this queued up ?
> 
> Mani,
> 
> Ping AGAIN !!!!
> 
> I do not see anything queued in pci/next. What is the blocker ?
> These patches have been sitting on the list for nearly a month now, PLEASE DO
> SOMETHING. Comment or apply, but please reply something.
> 

Damien,

Sorry for the late reply. Things got a bit hectic due to company onsite meeting.
I'm going through my queue now.

But FYI, I don't merge patches outside drivers/pci/endpoint/

- Mani

> > 
> >>
> >> Changes from v4:
> >>  - Added patch 6
> >>  - Added comments to patch 12 and 13 to clarify link training handling
> >>    and PERST# GPIO use.
> >>  - Added patch 14
> >>
> >> 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 (14):
> >>   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::align_addr() operation
> >>   PCI: rockchip-ep: Fix MSI IRQ data mapping
> >>   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
> >>   arm64: dts: rockchip: Add rockpro64 overlay for PCIe endpoint mode
> >>
> >>  arch/arm64/boot/dts/rockchip/Makefile         |   1 +
> >>  .../rockchip/rk3399-rockpro64-pcie-ep.dtso    |  20 +
> >>  drivers/pci/controller/pcie-rockchip-ep.c     | 432 ++++++++++++++----
> >>  drivers/pci/controller/pcie-rockchip-host.c   |   4 +-
> >>  drivers/pci/controller/pcie-rockchip.c        |  21 +-
> >>  drivers/pci/controller/pcie-rockchip.h        |  24 +-
> >>  6 files changed, 406 insertions(+), 96 deletions(-)
> >>  create mode 100644 arch/arm64/boot/dts/rockchip/rk3399-rockpro64-pcie-ep.dtso
> >>
> > 
> > 
> 
> 
> -- 
> Damien Le Moal
> Western Digital Research
Krzysztof Wilczyński Nov. 13, 2024, 8:49 p.m. UTC | #4
Hello,

> This patch series fix the PCI address mapping handling of the Rockchip
> PCI 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 .align_addr() operation to make the RK3399
>    endpoint controller driver fully functional with the new
>    pci_epc_mem_map() function
>  - Patch 6 uses the new align_addr operation function to fix the ATU
>    programming for MSI IRQ data mapping
>  - Patch 7, 8, 9 and 10 refactor the driver code to make it more
>    readable
>  - Patch 11 introduces the .stop() endpoint controller operation to
>    correctly disable the endpopint controller after use
>  - Patch 12 improves link training
>  - Patch 13 implements handling of the #PERST signal
>  - Patch 14 adds a DT overlay file to enable EP mode and define the
>    PERST# GPIO (reset-gpios) property.
> 
> These 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.

Applied to controller/rockchip, thank you!

[01/13] PCI: rockchip-ep: Fix address translation unit programming
        https://git.kernel.org/pci/pci/c/289cd5c0db35

[02/13] PCI: rockchip-ep: Use a macro to define EP controller .align feature
        https://git.kernel.org/pci/pci/c/8ba3b41eb7ec

[03/13] PCI: rockchip-ep: Improve rockchip_pcie_ep_unmap_addr()
        https://git.kernel.org/pci/pci/c/db68baa5d884

[04/13] PCI: rockchip-ep: Improve rockchip_pcie_ep_map_addr()
        https://git.kernel.org/pci/pci/c/c5b097d2a295

[05/13] PCI: rockchip-ep: Implement the pci_epc_ops::align_addr() operation
        https://git.kernel.org/pci/pci/c/75b011d9006e

[06/13] PCI: rockchip-ep: Fix MSI IRQ data mapping
        https://git.kernel.org/pci/pci/c/42c55124c3b2

[07/13] PCI: rockchip-ep: Rename rockchip_pcie_parse_ep_dt()
        https://git.kernel.org/pci/pci/c/c8b915ec5338

[08/13] PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() memory allocations
        https://git.kernel.org/pci/pci/c/c0473caa87f1

[09/13] PCI: rockchip-ep: Refactor rockchip_pcie_ep_probe() MSI-X hiding
        https://git.kernel.org/pci/pci/c/48e848c8727c

[10/13] PCI: rockchip-ep: Refactor endpoint link training enable
        https://git.kernel.org/pci/pci/c/c6de5dd3fd0c

[11/13] PCI: rockship-ep: Implement the pci_epc_ops::stop_link() operation
        https://git.kernel.org/pci/pci/c/8a9424d83e20

	Krzysztof
Krzysztof Wilczyński Nov. 13, 2024, 8:59 p.m. UTC | #5
Hello,

> > >> These 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.
> > > 
> > > Ping ? If there are no issues, can we get this queued up ?
> > 
> > Mani,
> > 
> > Ping AGAIN !!!!
> > 
> > I do not see anything queued in pci/next. What is the blocker ?
> > These patches have been sitting on the list for nearly a month now, PLEASE DO
> > SOMETHING. Comment or apply, but please reply something.
> > 
> 
> Damien,
> 
> Sorry for the late reply. Things got a bit hectic due to company onsite meeting.
> I'm going through my queue now.

Thank you, Mani!  I took this over and pulled this series.

You and Bjorn can have a look over the changes, if you have a moment.  That
said, at least to me, the changes looked good.

	Krzysztof
Damien Le Moal Nov. 14, 2024, 4:14 a.m. UTC | #6
On 11/14/24 05:59, Krzysztof Wilczyński wrote:
> Hello,
> 
>>>>> These 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.
>>>>
>>>> Ping ? If there are no issues, can we get this queued up ?
>>>
>>> Mani,
>>>
>>> Ping AGAIN !!!!
>>>
>>> I do not see anything queued in pci/next. What is the blocker ?
>>> These patches have been sitting on the list for nearly a month now, PLEASE DO
>>> SOMETHING. Comment or apply, but please reply something.
>>>
>>
>> Damien,
>>
>> Sorry for the late reply. Things got a bit hectic due to company onsite meeting.
>> I'm going through my queue now.
> 
> Thank you, Mani!  I took this over and pulled this series.
> 
> You and Bjorn can have a look over the changes, if you have a moment.  That
> said, at least to me, the changes looked good.

Krzysztof,

Thanks. But the kernel test robot already complained about a build failure for
the rockchip branch. The series needs to go through the endpoint branch as the
.align_addr method is only defined in that branch at the moment.

> 
> 	Krzysztof
Krzysztof Wilczyński Nov. 14, 2024, 5:24 p.m. UTC | #7
Hello,

> >> Sorry for the late reply. Things got a bit hectic due to company onsite meeting.
> >> I'm going through my queue now.
> > 
> > Thank you, Mani!  I took this over and pulled this series.
> > 
> > You and Bjorn can have a look over the changes, if you have a moment.  That
> > said, at least to me, the changes looked good.
> 
> Thanks. But the kernel test robot already complained about a build failure for
> the rockchip branch.

We did see this in our local testing environment, too.  Albeit, I didn't
get around to moving the commits before bot picked the branch up.

> The series needs to go through the endpoint branch as the .align_addr
> method is only defined in that branch at the moment.

The changes are now together.  Thank you!

	Krzysztof