Re: [PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
o...@mm-sol.com>, Krzysztof Kozlowski , Masami
Hiramatsu , Pengutronix Kernel Team
, Gustavo Pimentel ,
Shawn Guo , Lucas Stach
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Fri, Jun 10, 2022 at 11:25:31AM +0300, Serge Semin wrote:
> All of the DW PCIe core driver entities have names with the dw_ prefix in
> order to easily distinguish local and common PCIe name spaces. All except
> the pcie_port structure which contains the DW PCIe Root Port descriptor.
> For historical reason the structure has retained the original name since
> commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
> the DW PCIe IP-core support was added to the kernel. Let's finally fix
> that by adding the dw_ prefix to the structure name and by adding the _rp
> suffix to be similar to the EP counterpart. Thus the name will be coherent
> with the common driver naming policy. It shall make the driver code more
> readable eliminating visual confusion between the local and generic PCI
> name spaces.
>
> Signed-off-by: Serge Semin
>
> ---
>
> Changelog v4:
> - This is a new patch created on the v4 lap of the series.
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
> drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
> drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
> drivers/pci/controller/dwc/pci-keystone.c | 20 +--
> drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> drivers/pci/controller/dwc/pci-meson.c| 2 +-
> drivers/pci/controller/dwc/pcie-al.c | 6 ++--
> drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
> drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
> .../pci/controller/dwc/pcie-designware-host.c | 36 +--
> .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 30
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
> drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
> drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
> drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
> drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
> drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
> drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
> drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
> drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
> drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
> 23 files changed, 103 insertions(+), 103 deletions(-)
Reviewed-by: Rob Herring
Re: [PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
gkai Hu , "linux-arm-ker...@lists.infradead.org"
, Roy Zang , Jingoo Han
, "linuxppc-dev@lists.ozlabs.org"
, Heiko Stuebner ,
"linux-ker...@vger.kernel.org" , Serge Semin
, Stanimir Varbanov , Krzysztof
Kozlowski , Masami Hiramatsu
, Pengutronix Kernel Team , Gustavo
Pimentel , Shawn Guo ,
Lucas Stach
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Fri, Jun 10, 2022 at 10:25:31AM +0200, Serge Semin wrote:
> All of the DW PCIe core driver entities have names with the dw_ prefix in
> order to easily distinguish local and common PCIe name spaces. All except
> the pcie_port structure which contains the DW PCIe Root Port descriptor.
> For historical reason the structure has retained the original name since
> commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
> the DW PCIe IP-core support was added to the kernel. Let's finally fix
> that by adding the dw_ prefix to the structure name and by adding the _rp
> suffix to be similar to the EP counterpart. Thus the name will be coherent
> with the common driver naming policy. It shall make the driver code more
> readable eliminating visual confusion between the local and generic PCI
> name spaces.
Hi Serge,
I think that most variable and parameters of this type is named "pp" for
"pcie_port".
If this is the way we want to go, those should be changed also to "rp", right?
/Jesper
> Signed-off-by: Serge Semin
>
> ---
>
> Changelog v4:
> - This is a new patch created on the v4 lap of the series.
> ---
> drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
> drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
> drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
> drivers/pci/controller/dwc/pci-keystone.c | 20 +--
> drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> drivers/pci/controller/dwc/pci-meson.c| 2 +-
> drivers/pci/controller/dwc/pcie-al.c | 6 ++--
> drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
> drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
> .../pci/controller/dwc/pcie-designware-host.c | 36 +--
> .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> drivers/pci/controller/dwc/pcie-designware.h | 30
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
> drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
> drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
> drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
> drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
> drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
> drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
> drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
> drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
> drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
> drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
> 23 files changed, 103 insertions(+), 103 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
> b/drivers/pci/controller/dwc/pci-dra7xx.c
> index dfcdeb432dc8..a174b680b2a7 100644
> --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> @@ -178,7 +178,7 @@ static void dra7xx_pcie_enable_interrupts(struct
> dra7xx_pcie *dra7xx)
> dra7xx_pcie_enable_msi_interrupts(dra7xx);
> }
>
> -static int dra7xx_pcie_host_init(struct pcie_port *pp)
> +static int dra7xx_pcie_host_init(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
> @@ -202,7 +202,7 @@ static const struct irq_domain_ops intx_domain_ops = {
> .xlate = pci_irqd_intx_xlate,
> };
>
> -static int dra7xx_pcie_handle_msi(struct pcie_port *pp, int index)
> +static int dra7xx_pcie_handle_msi(struct dw_pcie_rp *pp, int index)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> unsigned long val;
> @@ -224,7 +224,7 @@ static int dra7xx_pcie_handle_msi(struct pcie_port *pp,
> int index)
> return 1;
> }
>
> -static void dra7xx_pcie_handle_msi_irq(struct pcie_port *pp)
> +static void dra7xx_pcie_handle_msi_irq(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> int ret, i, count, num_ctrls;
> @@ -255,8 +255,8 @@ static void dra7xx_pcie_msi_irq_handler(struct irq_desc
> *desc)
> {
> struct irq_chip *chip = irq_desc_get_chip(desc);
> struct dra7xx_pcie *dra7xx;
> + struct dw_pcie_rp *pp;
> struct dw_pcie *pci;
> - struct pcie_port *pp;
> unsigned long reg;
> u32 bit;
>
> @@ -344,7 +344,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void
> *arg)
> return IRQ_HANDLED;
> }
>
> -static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
> +static int dra7xx_pcie_init_irq_domain(struct dw_pcie_rp *pp)
> {
> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> struct device *dev =
Re: [PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
gkai Hu , "linux-arm-ker...@lists.infradead.org"
, Roy Zang , Jingoo Han
, "linuxppc-dev@lists.ozlabs.org"
, Heiko Stuebner ,
"linux-ker...@vger.kernel.org" , Serge Semin
, Krzysztof Kozlowski
, Masami Hiramatsu ,
Pengutronix Kernel Team , Gustavo Pimentel
, Shawn Guo , Lucas Stach
Errors-To: linuxppc-dev-bounces+archive=mail-archive@lists.ozlabs.org
Sender: "Linuxppc-dev"
On Fri, Jun 10, 2022 at 04:16:42PM +0200, Jesper Nilsson wrote:
> On Fri, Jun 10, 2022 at 10:25:31AM +0200, Serge Semin wrote:
> > All of the DW PCIe core driver entities have names with the dw_ prefix in
> > order to easily distinguish local and common PCIe name spaces. All except
> > the pcie_port structure which contains the DW PCIe Root Port descriptor.
> > For historical reason the structure has retained the original name since
> > commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
> > the DW PCIe IP-core support was added to the kernel. Let's finally fix
> > that by adding the dw_ prefix to the structure name and by adding the _rp
> > suffix to be similar to the EP counterpart. Thus the name will be coherent
> > with the common driver naming policy. It shall make the driver code more
> > readable eliminating visual confusion between the local and generic PCI
> > name spaces.
>
> Hi Serge,
Hi Jesper
>
> I think that most variable and parameters of this type is named "pp" for
> "pcie_port".
> If this is the way we want to go, those should be changed also to "rp", right?
Basically you may be right, but the change you suggest is much harder
to provide and may cause additional problems I have much doubts it is
really required. One thing is to update the struct name, but a whole
another story is to change the variables definition especially across
all the platform drivers involved here and especially of such
frequently used object as the DW PCIe Root Port descriptor.
First of all what you suggest will affect much-much-much more code
lines than this one, which in its turn will eventually cause problems
with the backporting of the new patches to the older stable kernels
released before the one with the updated names. Secondly it is a
matter of a separate patch, which can be added by someone who would
think it was really required. So to speak I don't think that changing
the variable names worth it especially seeing the driver naming
convention isn't perfect at all in many other aspects like using name
"pci" of the dw_pcie structure instance.
-Sergey
>
> /Jesper
>
> > Signed-off-by: Serge Semin
> >
> > ---
> >
> > Changelog v4:
> > - This is a new patch created on the v4 lap of the series.
> > ---
> > drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
> > drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
> > drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
> > drivers/pci/controller/dwc/pci-keystone.c | 20 +--
> > drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
> > drivers/pci/controller/dwc/pci-meson.c| 2 +-
> > drivers/pci/controller/dwc/pcie-al.c | 6 ++--
> > drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
> > drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
> > .../pci/controller/dwc/pcie-designware-host.c | 36 +--
> > .../pci/controller/dwc/pcie-designware-plat.c | 2 +-
> > drivers/pci/controller/dwc/pcie-designware.h | 30
> > drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
> > drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
> > drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
> > drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
> > drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
> > drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
> > drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
> > drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
> > drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
> > drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
> > drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
> > 23 files changed, 103 insertions(+), 103 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
> > b/drivers/pci/controller/dwc/pci-dra7xx.c
> > index dfcdeb432dc8..a174b680b2a7 100644
> > --- a/drivers/pci/controller/dwc/pci-dra7xx.c
> > +++ b/drivers/pci/controller/dwc/pci-dra7xx.c
> > @@ -178,7 +178,7 @@ static void dra7xx_pcie_enable_interrupts(struct
> > dra7xx_pcie *dra7xx)
> > dra7xx_pcie_enable_msi_interrupts(dra7xx);
> > }
> >
> > -static int dra7xx_pcie_host_init(struct pcie_port *pp)
> > +static int dra7xx_pcie_host_init(struct dw_pcie_rp *pp)
> > {
> > struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> > struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
> > @@ -202,7 +202,7 @@ static const struct irq_domain_ops intx_domain_ops = {
> > .xlate = pci_irqd_intx_xlate,
> > };
> >
> > -static int dra7xx_pcie_handle_msi(struct pcie_port
[PATCH v4 15/18] PCI: dwc: Add dw_ prefix to the pcie_port structure name
All of the DW PCIe core driver entities have names with the dw_ prefix in
order to easily distinguish local and common PCIe name spaces. All except
the pcie_port structure which contains the DW PCIe Root Port descriptor.
For historical reason the structure has retained the original name since
commit 340cba6092c2 ("pci: Add PCIe driver for Samsung Exynos") when
the DW PCIe IP-core support was added to the kernel. Let's finally fix
that by adding the dw_ prefix to the structure name and by adding the _rp
suffix to be similar to the EP counterpart. Thus the name will be coherent
with the common driver naming policy. It shall make the driver code more
readable eliminating visual confusion between the local and generic PCI
name spaces.
Signed-off-by: Serge Semin
---
Changelog v4:
- This is a new patch created on the v4 lap of the series.
---
drivers/pci/controller/dwc/pci-dra7xx.c | 12 +++
drivers/pci/controller/dwc/pci-exynos.c | 6 ++--
drivers/pci/controller/dwc/pci-imx6.c | 6 ++--
drivers/pci/controller/dwc/pci-keystone.c | 20 +--
drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
drivers/pci/controller/dwc/pci-meson.c| 2 +-
drivers/pci/controller/dwc/pcie-al.c | 6 ++--
drivers/pci/controller/dwc/pcie-armada8k.c| 4 +--
drivers/pci/controller/dwc/pcie-artpec6.c | 4 +--
.../pci/controller/dwc/pcie-designware-host.c | 36 +--
.../pci/controller/dwc/pcie-designware-plat.c | 2 +-
drivers/pci/controller/dwc/pcie-designware.h | 30
drivers/pci/controller/dwc/pcie-dw-rockchip.c | 4 +--
drivers/pci/controller/dwc/pcie-fu740.c | 2 +-
drivers/pci/controller/dwc/pcie-histb.c | 10 +++---
drivers/pci/controller/dwc/pcie-intel-gw.c| 6 ++--
drivers/pci/controller/dwc/pcie-keembay.c | 4 +--
drivers/pci/controller/dwc/pcie-kirin.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom.c| 4 +--
drivers/pci/controller/dwc/pcie-spear13xx.c | 6 ++--
drivers/pci/controller/dwc/pcie-tegra194.c| 22 ++--
drivers/pci/controller/dwc/pcie-uniphier.c| 10 +++---
drivers/pci/controller/dwc/pcie-visconti.c| 6 ++--
23 files changed, 103 insertions(+), 103 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c
b/drivers/pci/controller/dwc/pci-dra7xx.c
index dfcdeb432dc8..a174b680b2a7 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -178,7 +178,7 @@ static void dra7xx_pcie_enable_interrupts(struct
dra7xx_pcie *dra7xx)
dra7xx_pcie_enable_msi_interrupts(dra7xx);
}
-static int dra7xx_pcie_host_init(struct pcie_port *pp)
+static int dra7xx_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct dra7xx_pcie *dra7xx = to_dra7xx_pcie(pci);
@@ -202,7 +202,7 @@ static const struct irq_domain_ops intx_domain_ops = {
.xlate = pci_irqd_intx_xlate,
};
-static int dra7xx_pcie_handle_msi(struct pcie_port *pp, int index)
+static int dra7xx_pcie_handle_msi(struct dw_pcie_rp *pp, int index)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
unsigned long val;
@@ -224,7 +224,7 @@ static int dra7xx_pcie_handle_msi(struct pcie_port *pp, int
index)
return 1;
}
-static void dra7xx_pcie_handle_msi_irq(struct pcie_port *pp)
+static void dra7xx_pcie_handle_msi_irq(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
int ret, i, count, num_ctrls;
@@ -255,8 +255,8 @@ static void dra7xx_pcie_msi_irq_handler(struct irq_desc
*desc)
{
struct irq_chip *chip = irq_desc_get_chip(desc);
struct dra7xx_pcie *dra7xx;
+ struct dw_pcie_rp *pp;
struct dw_pcie *pci;
- struct pcie_port *pp;
unsigned long reg;
u32 bit;
@@ -344,7 +344,7 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void
*arg)
return IRQ_HANDLED;
}
-static int dra7xx_pcie_init_irq_domain(struct pcie_port *pp)
+static int dra7xx_pcie_init_irq_domain(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
struct device *dev = pci->dev;
@@ -475,7 +475,7 @@ static int dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
{
int ret;
struct dw_pcie *pci = dra7xx->pci;
- struct pcie_port *pp = >pp;
+ struct dw_pcie_rp *pp = >pp;
struct device *dev = pci->dev;
pp->irq = platform_get_irq(pdev, 1);
diff --git a/drivers/pci/controller/dwc/pci-exynos.c
b/drivers/pci/controller/dwc/pci-exynos.c
index 467c8d1cd7e4..2044d191fba6 100644
--- a/drivers/pci/controller/dwc/pci-exynos.c
+++ b/drivers/pci/controller/dwc/pci-exynos.c
@@ -249,7 +249,7 @@ static int exynos_pcie_link_up(struct dw_pcie *pci)
return (val & PCIE_ELBI_XMLH_LINKUP);
}
-static int exynos_pcie_host_init(struct pcie_port *pp)
+static int exynos_pcie_host_init(struct dw_pcie_rp *pp)
{
struct dw_pcie *pci =
