Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230727103949.26149-1-minda.chen@starfivetech.com>
In-Reply-To: <20230727103949.26149-1-minda.chen@starfivetech.com>
From:   Emil Renner Berthing <emil.renner.berthing@canonical.com>
Date:   Sat, 5 Aug 2023 15:05:16 +0200
Message-ID: <CAJM55Z-r3EtMGui=g8PhwbkYbuPFJgk8OcAF6S_1LcTRAqbOdw@mail.gmail.com>
Subject: Re: [PATCH v2 0/4] Refactoring Microchip PCIe driver and add StarFive PCIe
To:     Minda Chen <minda.chen@starfivetech.com>
Cc:     =?UTF-8?Q?Krzysztof_Wilczy=C5=84ski?= <kw@linux.com>,
        Daire McNamara <daire.mcnamara@microchip.com>,
        Conor Dooley <conor@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Bjorn Helgaas <bhelgaas@google.com>,
        Lorenzo Pieralisi <lpieralisi@kernel.org>,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-pci@vger.kernel.org, Philipp Zabel <p.zabel@pengutronix.de>,
        Mason Huo <mason.huo@starfivetech.com>,
        Leyfoon Tan <leyfoon.tan@starfivetech.com>,
        Kevin Xie <kevin.xie@starfivetech.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, 27 Jul 2023 at 12:40, Minda Chen <minda.chen@starfivetech.com> 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 - 2)
> Then, add Starfive codes. (patch3 - 4)
>
> This patchset is base on v6.5-rc1
>
> patch1 is move PLDA XpressRICH PCIe host common properties dt-binding
>        docs from microchip,pcie-host.yaml
> patch2 is extracting the PLDA common codes from microchip Polarfire PCIe
>        codes. The change list in the commit message.
> patch3 is add StarFive JH7110 PCIe dt-binding doc.
> patch4 is add StarFive JH7110 Soc PCIe codes.

Hi Minda,

To test this series properly it needs matching nodes in the VisionFive
2 device trees, but it seems to be missing from this version of the
patch series. If I apply the device tree patch from v1 I get errors
like this:

  pcie-starfive 2b000000.pcie: invalid resource (null)
  pcie-starfive 2b000000.pcie: error -EINVAL: failed to map reg memory

It would be great if you included the device tree patch in the next
series so this can actually be tested.

/Emil

> 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 patch2 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 patch2 as soon as
> their commits are accepted.
>
> previous version:
> v1:https://patchwork.kernel.org/project/linux-pci/cover/20230719102057.22329-1-minda.chen@starfivetech.com/
>
> change:
>   v2:
>     patch1:
>       - squash dt-bindings patches to patch1
>       - add 'required' list.
>       - plda doc rename to plda,xpressrich-axi-common.yaml
>     patch2:
>       - squash the microchip modification patch to patch 2.
>     patch3:
>       - remove the plda common required property.
>     patch4:
>       - Sync the hide rc bar ops with config read function.
>       - Revert the T_PVPERL to 100ms and add comments for the source.
>       - Replace the link check function by the standard link ops.
>       - Convert to new pm ops marcos.
>       - Some formats modification.
>       - pcie-plda-host modification merge to patch4.
>     other:
>       - remove the pcie-plda-plat.c
>       - remove the starfive dts patch first. for it depends on
>         stg clock and syscon setting.
>
> Minda Chen (4):
>   dt-bindings: PCI: Add PLDA XpressRICH PCIe host common properties
>   PCI: plda: Get common codes from Microchip PolarFire host
>   dt-bindings: PCI: Add StarFive JH7110 PCIe controller
>   PCI: starfive: Add JH7110 PCIe controller
>
>  .../bindings/pci/microchip,pcie-host.yaml     |  49 +-
>  .../pci/plda,xpressrich3-axi-common.yaml      |  69 ++
>  .../bindings/pci/starfive,jh7110-pcie.yaml    | 133 ++++
>  MAINTAINERS                                   |  19 +-
>  drivers/pci/controller/Kconfig                |   9 +-
>  drivers/pci/controller/Makefile               |   2 +-
>  drivers/pci/controller/plda/Kconfig           |  31 +
>  drivers/pci/controller/plda/Makefile          |   4 +
>  .../{ => plda}/pcie-microchip-host.c          | 594 ++--------------
>  drivers/pci/controller/plda/pcie-plda-host.c  | 665 ++++++++++++++++++
>  drivers/pci/controller/plda/pcie-plda.h       | 242 +++++++
>  drivers/pci/controller/plda/pcie-starfive.c   | 438 ++++++++++++
>  12 files changed, 1645 insertions(+), 610 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/pci/plda,xpressrich3-axi-common.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.h
>  create mode 100644 drivers/pci/controller/plda/pcie-starfive.c
>
>
> base-commit: 06c2afb862f9da8dc5efa4b6076a0e48c3fbaaa5
> --
> 2.17.1
>