Date:   Fri, 3 Mar 2023 15:38:29 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Shradha Todi <shradha.t@samsung.com>
Cc:     lpieralisi@kernel.org, kw@linux.com, robh@kernel.org,
        bhelgaas@google.com, krzysztof.kozlowski+dt@linaro.org,
        alim.akhtar@samsung.com, jingoohan1@gmail.com,
        Sergey.Semin@baikalelectronics.ru, lukas.bulwahn@gmail.com,
        hongxing.zhu@nxp.com, tglx@linutronix.de, m.szyprowski@samsung.com,
        jh80.chung@samsung.co, pankaj.dubey@samsung.com,
        linux-pci@vger.kernel.org, devicetree@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        linux-samsung-soc@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/16] Refactor Exynos PCIe driver to make it generic
Message-ID: <20230303123829.er626hqa562sal3t@mobilestation>
References: <CGME20230214121348epcas5p48a3b2b225f616d748cc20622d01edb97@epcas5p4.samsung.com>
 <20230214121333.1837-1-shradha.t@samsung.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230214121333.1837-1-shradha.t@samsung.com>
Precedence: bulk

Hi Shradha

On Tue, Feb 14, 2023 at 05:43:17PM +0530, Shradha Todi wrote:
> Currently pci-exynos is being used as a PCIe driver for Exynos5433
> only. This patch set refactors the driver to make it extensible to
> other Samsung manufactured SoCs having DWC PCIe controllers.
> The major change points are:
> - Renaming all common functions/structures to use "samsung" instead
>   of "exynos". Make common probe/remove/suspend/resume
> - Making clock/regulator get/enable/disable generic
> - Adding private struct to hold platform specific function ops

Just a general note regarding the DT-bindings. If you're willing to fix
some names or most importantly add new ones please follow as much as
possible to the generic interface defined in the common part of the
DW PCIe bindings schema:
Documentation/devicetree/bindings/pci/snps,dw-pcie-common.yaml
Documentation/devicetree/bindings/pci/snps,dw-pcie.yaml
for instance the generic "reg-names" are "elbi" or "app" defined for
the application-dependent registers map (normally implemented via the
ELBI interface in hardware), the "appl" name is marked as vendor-specific
and should be avoided.

-Serge(y)

> 
> Shradha Todi (16):
>   dt-bindings: PCI: Rename Exynos PCIe binding to Samsung PCIe
>   PCI: exynos: Rename Exynos PCIe driver to Samsung PCIe
>   PCI: samsung: Change macro names to exynos specific
>   PCI: samsung: Use clock bulk API to get clocks
>   dt-bindings: PCI: Rename the term elbi to appl
>   arm64: dts: exynos: Rename the term elbi to appl
>   PCI: samsung: Rename the term elbi to appl
>   PCI: samsung: Rename exynos_pcie to samsung_pcie
>   PCI: samsung: Make common appl readl/writel functions
>   dt-bindings: PCI: Add phy-names as required property
>   arm64: dts: exynos: Add phy-names as DT property
>   PCI: samsung: Get PHY using non-DT version
>   PCI: samsung: Rename common functions to samsung
>   PCI: samsung: Add platform device private data
>   PCI: samsung: Add structure to hold resource operations
>   PCI: samsung: Make handling of regulators generic
> 
>  ...ung,exynos-pcie.yaml => samsung,pcie.yaml} |  15 +-
>  MAINTAINERS                                   |   4 +-
>  arch/arm64/boot/dts/exynos/exynos5433.dtsi    |   3 +-
>  drivers/pci/controller/dwc/Kconfig            |   6 +-
>  drivers/pci/controller/dwc/Makefile           |   2 +-
>  drivers/pci/controller/dwc/pci-samsung.c      | 508 ++++++++++++++++++
>  6 files changed, 526 insertions(+), 12 deletions(-)
>  rename Documentation/devicetree/bindings/pci/{samsung,exynos-pcie.yaml => samsung,pcie.yaml} (89%)
>  create mode 100644 drivers/pci/controller/dwc/pci-samsung.c
> 
> -- 
> 2.17.1
> 
>