RE: [PATCH 08/16] PCI: samsung: Rename exynos_pcie to samsung_pcie

[Pankaj Dubey]

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@xxxxxxxxxx>
> Sent: Thursday, February 16, 2023 4:37 PM
> To: Shradha Todi <shradha.t@xxxxxxxxxxx>; lpieralisi@xxxxxxxxxx;
> kw@xxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> krzysztof.kozlowski+dt@xxxxxxxxxx; alim.akhtar@xxxxxxxxxxx;
> jingoohan1@xxxxxxxxx; Sergey.Semin@xxxxxxxxxxxxxxxxxxxx;
> lukas.bulwahn@xxxxxxxxx; hongxing.zhu@xxxxxxx; tglx@xxxxxxxxxxxxx;
> m.szyprowski@xxxxxxxxxxx; jh80.chung@xxxxxxxxxx;
> pankaj.dubey@xxxxxxxxxxx
> Cc: linux-pci@xxxxxxxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; linux-arm-
> kernel@xxxxxxxxxxxxxxxxxxx; linux-samsung-soc@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH 08/16] PCI: samsung: Rename exynos_pcie to
> samsung_pcie
> 
> On 14/02/2023 13:13, Shradha Todi wrote:
> > The platform specific structure being used is named exynos_pcie.
> > Changing it to samsung_pcie for making it generic.
> >
> > Suggested-by: Pankaj Dubey <pankaj.dubey@xxxxxxxxxxx>
> > Signed-off-by: Shradha Todi <shradha.t@xxxxxxxxxxx>
> > ---
> >  drivers/pci/controller/dwc/pci-samsung.c | 190
> > +++++++++++------------
> >  1 file changed, 95 insertions(+), 95 deletions(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-samsung.c
> > b/drivers/pci/controller/dwc/pci-samsung.c
> > index d5adf1017a05..be0177fcd763 100644
> > --- a/drivers/pci/controller/dwc/pci-samsung.c
> > +++ b/drivers/pci/controller/dwc/pci-samsung.c
> > @@ -23,7 +23,7 @@
> >
> >  #include "pcie-designware.h"
> >
> > -#define to_exynos_pcie(x)	dev_get_drvdata((x)->dev)
> > +#define to_samsung_pcie(x)	dev_get_drvdata((x)->dev)
> >
> >  /* PCIe APPL registers */
> >  #define EXYNOS_PCIE_IRQ_PULSE			0x000
> > @@ -51,7 +51,7 @@
> >  #define EXYNOS_PCIE_APPL_SLV_ARMISC		0x120
> >  #define EXYNOS_PCIE_APPL_SLV_DBI_ENABLE	BIT(21)
> >
> > -struct exynos_pcie {
> > +struct samsung_pcie {
> 
> No, I don't see benefit of this at all. How we call stuff inside driver is not related
> whether this is for Tesla or Exynos. We could even call it "pony". :) Thus
> renamings just to support new variant of Samsung device is not a good reason.
> 
Whole intention of this whole series was to make exynos-pcie driver to support for all Samsung manufactured SoCs be it Exynos series or custom ASIC such as fsd, artpect-v8. 

While doing so, we feel for better readability and conveying better names for files, structs, internal APIs will help developers for understanding and reusing it. For example we know that clock initialization will remain common (thanks for bulk_clk_xxx APIs) so we kept APIs for handling clocks starting with samsung_clk_xxxx, but if we have to implement two variant of APIs, or struct targeting different platforms it would be good if they have platform specific prefixes. This will help in grep or future code maintenance. 

Though technically all these can be done even without renaming, but if we see no impact as such, so why not use better names?
> Unless all of the old "exynos" names will be soon needed for some exynos-
> specific variants?
> 

No we don't have any such plans.

> Best regards,
> Krzysztof