>functions as a separate file
>
>EXTERNAL MAIL
>
>
>On Tue, Aug 19, 2025 at 07:52:31PM GMT, hans.zhang@xxxxxxxxxxx wrote:
>> From: Manikandan K Pillai <mpillai@xxxxxxxxxxx>
>>
>> Move the functions for platform common tasks to a separate file. The
>> common library functions and functions specific to platform are
>> now in different files.
>>
>
>Why do we have too many library files? What is the need to split
>pcie-cadence-common.c which itself is a library?
>
Hi Mani,
I plan to drop this patch and that address all your comments.
- Mani
>> Signed-off-by: Manikandan K Pillai <mpillai@xxxxxxxxxxx>
>> Co-developed-by: Hans Zhang <hans.zhang@xxxxxxxxxxx>
>> Signed-off-by: Hans Zhang <hans.zhang@xxxxxxxxxxx>
>> ---
>> drivers/pci/controller/cadence/Makefile | 2 +-
>> .../controller/cadence/pcie-cadence-common.c | 141 ++++++++++++++++++
>> drivers/pci/controller/cadence/pcie-cadence.c | 129 ----------------
>> 3 files changed, 142 insertions(+), 130 deletions(-)
>> create mode 100644 drivers/pci/controller/cadence/pcie-cadence-common.c
>>
>> diff --git a/drivers/pci/controller/cadence/Makefile
>b/drivers/pci/controller/cadence/Makefile
>> index e45f72388bbb..b104562fb86a 100644
>> --- a/drivers/pci/controller/cadence/Makefile
>> +++ b/drivers/pci/controller/cadence/Makefile
>> @@ -1,5 +1,5 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -obj-$(CONFIG_PCIE_CADENCE) += pcie-cadence.o
>> +obj-$(CONFIG_PCIE_CADENCE) += pcie-cadence-common.o pcie-cadence.o
>> obj-$(CONFIG_PCIE_CADENCE_HOST) += pcie-cadence-host-common.o pcie-
>cadence-host.o
>> obj-$(CONFIG_PCIE_CADENCE_EP) += pcie-cadence-ep-common.o pcie-
>cadence-ep.o
>> obj-$(CONFIG_PCIE_CADENCE_PLAT) += pcie-cadence-plat.o
>> diff --git a/drivers/pci/controller/cadence/pcie-cadence-common.c
>b/drivers/pci/controller/cadence/pcie-cadence-common.c
>> new file mode 100644
>> index 000000000000..e14d53d64bf1
>> --- /dev/null
>> +++ b/drivers/pci/controller/cadence/pcie-cadence-common.c
>> @@ -0,0 +1,141 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2017 Cadence
>> +// Cadence PCIe controller driver.
>> +// Author: Cyrille Pitchen <cyrille.pitchen@xxxxxxxxxxxxxxxxxx>
>
>We use below style for multi line comments:
>
> /*
> *
> */
>
>So even though this style existed, you should change it in all new files.
>
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/of.h>
>> +
>> +#include "pcie-cadence.h"
>> +
>> +void cdns_pcie_disable_phy(struct cdns_pcie *pcie)
>> +{
>> + int i = pcie->phy_count;
>> +
>> + while (i--) {
>> + phy_power_off(pcie->phy[i]);
>> + phy_exit(pcie->phy[i]);
>> + }
>> +}
>> +EXPORT_SYMBOL_GPL(cdns_pcie_disable_phy);
>> +
>> +int cdns_pcie_enable_phy(struct cdns_pcie *pcie)
>> +{
>> + int ret;
>> + int i;
>> +
>> + for (i = 0; i < pcie->phy_count; i++) {
>> + ret = phy_init(pcie->phy[i]);
>> + if (ret < 0)
>> + goto err_phy;
>> +
>> + ret = phy_power_on(pcie->phy[i]);
>> + if (ret < 0) {
>> + phy_exit(pcie->phy[i]);
>> + goto err_phy;
>> + }
>> + }
>> +
>> + return 0;
>> +
>> +err_phy:
>> + while (--i >= 0) {
>> + phy_power_off(pcie->phy[i]);
>> + phy_exit(pcie->phy[i]);
>> + }
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(cdns_pcie_enable_phy);
>> +
>> +int cdns_pcie_init_phy(struct device *dev, struct cdns_pcie *pcie)
>> +{
>> + struct device_node *np = dev->of_node;
>> + int phy_count;
>> + struct phy **phy;
>> + struct device_link **link;
>> + int i;
>> + int ret;
>> + const char *name;
>> +
>> + phy_count = of_property_count_strings(np, "phy-names");
>> + if (phy_count < 1) {
>> + dev_info(dev, "no \"phy-names\" property found; PHY will not
>be initialized\n");
>> + pcie->phy_count = 0;
>> + return 0;
>> + }
>> +
>> + phy = devm_kcalloc(dev, phy_count, sizeof(*phy), GFP_KERNEL);
>> + if (!phy)
>> + return -ENOMEM;
>> +
>> + link = devm_kcalloc(dev, phy_count, sizeof(*link), GFP_KERNEL);
>> + if (!link)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < phy_count; i++) {
>> + of_property_read_string_index(np, "phy-names", i, &name);
>> + phy[i] = devm_phy_get(dev, name);
>> + if (IS_ERR(phy[i])) {
>> + ret = PTR_ERR(phy[i]);
>> + goto err_phy;
>> + }
>> + link[i] = device_link_add(dev, &phy[i]->dev,
>DL_FLAG_STATELESS);
>> + if (!link[i]) {
>> + devm_phy_put(dev, phy[i]);
>> + ret = -EINVAL;
>> + goto err_phy;
>> + }
>> + }
>> +
>> + pcie->phy_count = phy_count;
>> + pcie->phy = phy;
>> + pcie->link = link;
>> +
>> + ret = cdns_pcie_enable_phy(pcie);
>
>Double space after =
>
>- Mani
>
>--
>மணிவண்ணன் சதாசிவம்
