Message-ID: <f175dcae-2729-2bac-d2e8-088537354160@linaro.org>
Date:   Fri, 3 Mar 2023 11:06:15 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 08/16] PCI: samsung: Rename exynos_pcie to samsung_pcie
Content-Language: en-US
To:     Pankaj Dubey <pankaj.dubey@samsung.com>,
        'Shradha Todi' <shradha.t@samsung.com>, 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
Cc:     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
References: <20230214121333.1837-1-shradha.t@samsung.com>
 <CGME20230214121436epcas5p1641de02220bea5edb23bc875b6311270@epcas5p1.samsung.com>
 <20230214121333.1837-9-shradha.t@samsung.com>
 <a4bfad9d-b9df-28a5-6bee-5cbbca4dd23f@linaro.org>
 <146301d94d03$002ba040$0082e0c0$@samsung.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <146301d94d03$002ba040$0082e0c0$@samsung.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 02/03/2023 13:32, Pankaj Dubey wrote:
> 
> 
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: Thursday, February 16, 2023 4:37 PM
>> To: Shradha Todi <shradha.t@samsung.com>; 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
>> Cc: 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 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@samsung.com>
>>> Signed-off-by: Shradha Todi <shradha.t@samsung.com>
>>> ---
>>>  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. 

But the patches does not do it, at least mostly. It only renames which
does not bring any support... what's more, such renames without actual
context - support for the new devices - is a bit pointless.

> 
> 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.

Without context it's impossible to judge whether this makes any sense.

Best regards,
Krzysztof