mbox series

[v1,0/9] Refactoring Microchip PolarFire PCIe driver

Message ID 20230719102057.22329-1-minda.chen@starfivetech.com (mailing list archive)
Headers show
Series Refactoring Microchip PolarFire PCIe driver | expand

Message

Minda Chen July 19, 2023, 10:20 a.m. UTC 
This patchset final purpose is add PCIe driver for StarFive JH7110 SoC.
JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the
same IP and have commit their codes, which are mixed with PLDA
controller codes and Microchip platform codes.

For re-use the PLDA controller codes, I request refactoring microchip
codes, move PLDA common codes to PLDA files.
Desigware and Cadence is good example for refactoring codes.

So first step is extract the PLDA common codes from microchip, and
refactoring the microchip codes.(patch1 - 4)
Then add the PLDA platform codes. (patch5, 6)
At last, add Starfive codes. (patch7 - 9)

This patchset is base on v6.5-rc1

patch1 is add PLDA XpressRICH PCIe host common properties dt-binding
       docs, most are extracted from microchip,pcie-host.yaml
patch2 is add plda,xpressrich-pcie-common.yaml(patch1 file) reference
       and remove the PLDA common properties.
patch3 is extracting the PLDA common codes from microchip Polarfire PCIe
       codes. The change list in the commit message.
patch4 is move microchip driver to PLDA directory and remove the PLDA
       common codes.
patch5 is add PLDA Xpressrich platform driver dt-binding doc.
patch6 is PLDA Xpressrich platform driver.
patch7 is add StarFive JH7110 PCIe dt-binding doc.
patch8 is add StarFive JH7110 Soc PCIe platform codes.
patch9 is StarFive JH7110 device tree configuration.

I have noticed that Daire have changed microchip's codes.
https://patchwork.kernel.org/project/linux-pci/cover/20230630154859.2049521-1-daire.mcnamara@microchip.com/
I have changed patch3 and patch4 base on their commits. StarFive
PCIe driver still can work. But their codes is under reviewed and 
maybe changing. Do not base on their changes first.
I will base on their commit to change patch3 and patch4 as soon as
their commits are accepted.

List below is old patchset and is dropped, which is non-refractored version.
https://patchwork.kernel.org/project/linux-pci/cover/20230406111142.74410-1-minda.chen@starfivetech.com/

Minda Chen (9):
  dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
  dt-bindings: PCI: microchip: Remove the PLDA common properties
  PCI: PLDA: Get PLDA common codes from Microchip PolarFire host
  PCI: microchip: Move PCIe driver to PLDA directory
  dt-bindings: PLDA: Add PLDA XpressRICH PCIe host controller
  PCI: PLDA: Add host conroller platform driver
  dt-bindings: PCI: Add StarFive JH7110 PCIe controller
  PCI: PLDA: starfive: Add JH7110 PCIe controller
  riscv: dts: starfive: add PCIe dts configuration for JH7110

 .../bindings/pci/microchip,pcie-host.yaml     |  45 +-
 .../pci/plda,xpressrich-pcie-common.yaml      |  72 ++
 .../pci/plda,xpressrich-pcie-host.yaml        |  66 ++
 .../bindings/pci/starfive,jh7110-pcie.yaml    | 138 ++++
 MAINTAINERS                                   |  19 +-
 .../jh7110-starfive-visionfive-2.dtsi         |  44 ++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  88 +++
 drivers/pci/controller/Kconfig                |   9 +-
 drivers/pci/controller/Makefile               |   2 +-
 drivers/pci/controller/plda/Kconfig           |  35 +
 drivers/pci/controller/plda/Makefile          |   5 +
 .../{ => plda}/pcie-microchip-host.c          | 594 ++--------------
 drivers/pci/controller/plda/pcie-plda-host.c  | 665 ++++++++++++++++++
 drivers/pci/controller/plda/pcie-plda-plat.c  |  64 ++
 drivers/pci/controller/plda/pcie-plda.h       | 230 ++++++
 drivers/pci/controller/plda/pcie-starfive.c   | 415 +++++++++++
 16 files changed, 1885 insertions(+), 606 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich-pcie-common.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich-pcie-host.yaml
 create mode 100644 Documentation/devicetree/bindings/pci/starfive,jh7110-pcie.yaml
 create mode 100644 drivers/pci/controller/plda/Kconfig
 create mode 100644 drivers/pci/controller/plda/Makefile
 rename drivers/pci/controller/{ => plda}/pcie-microchip-host.c (50%)
 create mode 100644 drivers/pci/controller/plda/pcie-plda-host.c
 create mode 100644 drivers/pci/controller/plda/pcie-plda-plat.c
 create mode 100644 drivers/pci/controller/plda/pcie-plda.h
 create mode 100644 drivers/pci/controller/plda/pcie-starfive.c


base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5

Comments

Bjorn Helgaas July 19, 2023, 3:26 p.m. UTC | #1 
On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:
> This patchset final purpose is add PCIe driver for StarFive JH7110 SoC.
>   dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
> JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the
> same IP and have commit their codes, which are mixed with PLDA
> controller codes and Microchip platform codes.

I guess this actually adds TWO drivers: PCIE_PLDA_PLAT_HOST (claims
"plda,xpressrich-pcie-host" devices) and PCIE_STARFIVE_HOST (claims
"starfive,jh7110-pcie" devices), right?

> For re-use the PLDA controller codes, I request refactoring microchip
> codes, move PLDA common codes to PLDA files.
> Desigware and Cadence is good example for refactoring codes.
> 
> So first step is extract the PLDA common codes from microchip, and
> refactoring the microchip codes.(patch1 - 4)
> Then add the PLDA platform codes. (patch5, 6)
> At last, add Starfive codes. (patch7 - 9)
> 
> This patchset is base on v6.5-rc1

Doesn't quite apply cleanly for me:

  10:10:15 ~/linux (main)$ git checkout -b wip/minda-starfive-v1 v6.5-rc1
  Switched to a new branch 'wip/minda-starfive-v1'
  10:10:33 ~/linux (wip/minda-starfive-v1)$ git am m/20230719_minda_chen_refactoring_microchip_polarfire_pcie_driver.mbx
  Applying: dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
  Applying: dt-bindings: PCI: microchip: Remove the PLDA common properties
  Applying: PCI: PLDA: Get PLDA common codes from Microchip PolarFire host
  Applying: PCI: microchip: Move PCIe driver to PLDA directory
  Applying: dt-bindings: PLDA: Add PLDA XpressRICH PCIe host controller
  Applying: PCI: PLDA: Add host conroller platform driver
  Applying: dt-bindings: PCI: Add StarFive JH7110 PCIe controller
  Applying: PCI: PLDA: starfive: Add JH7110 PCIe controller
  Applying: riscv: dts: starfive: add PCIe dts configuration for JH7110
  error: patch failed: arch/riscv/boot/dts/starfive/jh7110.dtsi:629
  error: arch/riscv/boot/dts/starfive/jh7110.dtsi: patch does not apply
  Patch failed at 0009 riscv: dts: starfive: add PCIe dts configuration for JH7110

>   dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
>   dt-bindings: PCI: microchip: Remove the PLDA common properties
>   PCI: PLDA: Get PLDA common codes from Microchip PolarFire host
>   PCI: microchip: Move PCIe driver to PLDA directory
>   dt-bindings: PLDA: Add PLDA XpressRICH PCIe host controller
>   PCI: PLDA: Add host conroller platform driver

"controller"

>   dt-bindings: PCI: Add StarFive JH7110 PCIe controller
>   PCI: PLDA: starfive: Add JH7110 PCIe controller
>   riscv: dts: starfive: add PCIe dts configuration for JH7110

Use "PCI: plda: " prefix for PLDA things that are shared across
multiple drivers.

Use "PCI: starfive: " prefix for starfive-specific things.

This is the same as how drivers/pci/controller/dwc/* looks.

Bjorn
Conor Dooley July 19, 2023, 4:58 p.m. UTC | #2 
Hey Minda,

On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:
> This patchset final purpose is add PCIe driver for StarFive JH7110 SoC.
> JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the
> same IP and have commit their codes, which are mixed with PLDA
> controller codes and Microchip platform codes.
> 
> For re-use the PLDA controller codes, I request refactoring microchip
> codes, move PLDA common codes to PLDA files.
> Desigware and Cadence is good example for refactoring codes.
> 
> So first step is extract the PLDA common codes from microchip, and
> refactoring the microchip codes.(patch1 - 4)
> Then add the PLDA platform codes. (patch5, 6)
> At last, add Starfive codes. (patch7 - 9)

Thanks for sending this, I'll try to have a look through it tomorrow, or
if not, early next week. As pointed out off-list, the gist of what you
have here looked good to myself and Daire.

> This patchset is base on v6.5-rc1
> 
> patch1 is add PLDA XpressRICH PCIe host common properties dt-binding
>        docs, most are extracted from microchip,pcie-host.yaml
> patch2 is add plda,xpressrich-pcie-common.yaml(patch1 file) reference
>        and remove the PLDA common properties.
> patch3 is extracting the PLDA common codes from microchip Polarfire PCIe
>        codes. The change list in the commit message.
> patch4 is move microchip driver to PLDA directory and remove the PLDA
>        common codes.
> patch5 is add PLDA Xpressrich platform driver dt-binding doc.
> patch6 is PLDA Xpressrich platform driver.
> patch7 is add StarFive JH7110 PCIe dt-binding doc.
> patch8 is add StarFive JH7110 Soc PCIe platform codes.
> patch9 is StarFive JH7110 device tree configuration.
> 
> I have noticed that Daire have changed microchip's codes.
> https://patchwork.kernel.org/project/linux-pci/cover/20230630154859.2049521-1-daire.mcnamara@microchip.com/

I'll go and ping this, it's been a few weeks with no movement :)

Thanks,
Conor.
Minda Chen July 20, 2023, 2:15 a.m. UTC | #3 
On 2023/7/19 23:26, Bjorn Helgaas wrote:
> On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:
>> This patchset final purpose is add PCIe driver for StarFive JH7110 SoC.
>>   dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
>> JH7110 using PLDA XpressRICH PCIe IP. Microchip PolarFire Using the
>> same IP and have commit their codes, which are mixed with PLDA
>> controller codes and Microchip platform codes.
> 
> I guess this actually adds TWO drivers: PCIE_PLDA_PLAT_HOST (claims
> "plda,xpressrich-pcie-host" devices) and PCIE_STARFIVE_HOST (claims
> "starfive,jh7110-pcie" devices), right?
> 
Yes, plda,xpressrich-pcie-host is IP controller driver. Do it like designware/cadence/mobiveil, (pcie-(ip)-plat.c)
But I can't test it. I don't whether need it. If it not required, I will delete it.
>> For re-use the PLDA controller codes, I request refactoring microchip
>> codes, move PLDA common codes to PLDA files.
>> Desigware and Cadence is good example for refactoring codes.
>> 
>> So first step is extract the PLDA common codes from microchip, and
>> refactoring the microchip codes.(patch1 - 4)
>> Then add the PLDA platform codes. (patch5, 6)
>> At last, add Starfive codes. (patch7 - 9)
>> 
>> This patchset is base on v6.5-rc1
> 
> Doesn't quite apply cleanly for me:
> 
I am sorry, The driver need stg clk and syscon driver, which are have't be merge to main line.
mainly dts is(patch9) rejected, Must apply this series patch first. (I forget add this link in cover letter)
https://patchwork.kernel.org/project/linux-riscv/cover/20230712092007.31013-1-xingyu.wu@starfivetech.com/
and this syscon patch 
https://patchwork.kernel.org/project/linux-riscv/patch/20230717023040.78860-7-xingyu.wu@starfivetech.com/
>   10:10:15 ~/linux (main)$ git checkout -b wip/minda-starfive-v1 v6.5-rc1
>   Switched to a new branch 'wip/minda-starfive-v1'
>   10:10:33 ~/linux (wip/minda-starfive-v1)$ git am m/20230719_minda_chen_refactoring_microchip_polarfire_pcie_driver.mbx
>   Applying: dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
>   Applying: dt-bindings: PCI: microchip: Remove the PLDA common properties
>   Applying: PCI: PLDA: Get PLDA common codes from Microchip PolarFire host
>   Applying: PCI: microchip: Move PCIe driver to PLDA directory
>   Applying: dt-bindings: PLDA: Add PLDA XpressRICH PCIe host controller
>   Applying: PCI: PLDA: Add host conroller platform driver
>   Applying: dt-bindings: PCI: Add StarFive JH7110 PCIe controller
>   Applying: PCI: PLDA: starfive: Add JH7110 PCIe controller
>   Applying: riscv: dts: starfive: add PCIe dts configuration for JH7110
>   error: patch failed: arch/riscv/boot/dts/starfive/jh7110.dtsi:629
>   error: arch/riscv/boot/dts/starfive/jh7110.dtsi: patch does not apply
>   Patch failed at 0009 riscv: dts: starfive: add PCIe dts configuration for JH7110
> 
>>   dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
>>   dt-bindings: PCI: microchip: Remove the PLDA common properties
>>   PCI: PLDA: Get PLDA common codes from Microchip PolarFire host
>>   PCI: microchip: Move PCIe driver to PLDA directory
>>   dt-bindings: PLDA: Add PLDA XpressRICH PCIe host controller
>>   PCI: PLDA: Add host conroller platform driver
> 
> "controller"
>ok
>>   dt-bindings: PCI: Add StarFive JH7110 PCIe controller
>>   PCI: PLDA: starfive: Add JH7110 PCIe controller
>>   riscv: dts: starfive: add PCIe dts configuration for JH7110
> 
> Use "PCI: plda: " prefix for PLDA things that are shared across
> multiple drivers.
> 
> Use "PCI: starfive: " prefix for starfive-specific things.
> 
> This is the same as how drivers/pci/controller/dwc/* looks.
> 
ok, thanks.
> Bjorn
Conor Dooley July 20, 2023, 12:12 p.m. UTC | #4 
On Thu, Jul 20, 2023 at 10:15:51AM +0800, Minda Chen wrote:
> On 2023/7/19 23:26, Bjorn Helgaas wrote:
> > On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:

> >> This patchset is base on v6.5-rc1
> > 
> > Doesn't quite apply cleanly for me:
> > 
> I am sorry, The driver need stg clk and syscon driver, which are have't be merge to main line.
> mainly dts is(patch9) rejected, Must apply this series patch first. (I forget add this link in cover letter)
> https://patchwork.kernel.org/project/linux-riscv/cover/20230712092007.31013-1-xingyu.wu@starfivetech.com/
> and this syscon patch 
> https://patchwork.kernel.org/project/linux-riscv/patch/20230717023040.78860-7-xingyu.wu@starfivetech.com/

You could detach the dts patch from the series & send it independently once
everything it depends on is in place. I'm going to pick up both of the
patches you've linked for v6.6 in the next day or two.
Minda Chen July 21, 2023, 9:34 a.m. UTC | #5 
On 2023/7/20 20:12, Conor Dooley wrote:
> On Thu, Jul 20, 2023 at 10:15:51AM +0800, Minda Chen wrote:
>> On 2023/7/19 23:26, Bjorn Helgaas wrote:
>> > On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:
> 
>> >> This patchset is base on v6.5-rc1
>> > 
>> > Doesn't quite apply cleanly for me:
>> > 
>> I am sorry, The driver need stg clk and syscon driver, which are have't be merge to main line.
>> mainly dts is(patch9) rejected, Must apply this series patch first. (I forget add this link in cover letter)
>> https://patchwork.kernel.org/project/linux-riscv/cover/20230712092007.31013-1-xingyu.wu@starfivetech.com/
>> and this syscon patch 
>> https://patchwork.kernel.org/project/linux-riscv/patch/20230717023040.78860-7-xingyu.wu@starfivetech.com/
> 
> You could detach the dts patch from the series & send it independently once
> everything it depends on is in place. I'm going to pick up both of the
> patches you've linked for v6.6 in the next day or two.
Thanks very much. Yes， I have considered remove the dts patch. But I think the maintainers also
will review the dts file. So I add this....
Minda Chen July 21, 2023, 9:55 a.m. UTC | #6 
On 2023/7/20 20:12, Conor Dooley wrote:
> On Thu, Jul 20, 2023 at 10:15:51AM +0800, Minda Chen wrote:
>> On 2023/7/19 23:26, Bjorn Helgaas wrote:
>> > On Wed, Jul 19, 2023 at 06:20:48PM +0800, Minda Chen wrote:
> 
>> >> This patchset is base on v6.5-rc1
>> > 
>> > Doesn't quite apply cleanly for me:
>> > 
>> I am sorry, The driver need stg clk and syscon driver, which are have't be merge to main line.
>> mainly dts is(patch9) rejected, Must apply this series patch first. (I forget add this link in cover letter)
>> https://patchwork.kernel.org/project/linux-riscv/cover/20230712092007.31013-1-xingyu.wu@starfivetech.com/
>> and this syscon patch 
>> https://patchwork.kernel.org/project/linux-riscv/patch/20230717023040.78860-7-xingyu.wu@starfivetech.com/
> 
> You could detach the dts patch from the series & send it independently once
> everything it depends on is in place. I'm going to pick up both of the
> patches you've linked for v6.6 in the next day or two.
Thanks very much. I have considered removing the dts patch. But I think Maintainers
also review the dts patch. So I add this.....