lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
 <CH2PPF4D26F8E1C00BA70B94B327B46E794A207A@CH2PPF4D26F8E1C.namprd07.prod.outlook.com>
Date: Mon, 1 Sep 2025 04:30:59 +0000
From: Manikandan Karunakaran Pillai <mpillai@...ence.com>
To: Manivannan Sadhasivam <mani@...nel.org>,
        "hans.zhang@...tech.com"
	<hans.zhang@...tech.com>
CC: "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "lpieralisi@...nel.org"
	<lpieralisi@...nel.org>,
        "kw@...ux.com" <kw@...ux.com>, "robh@...nel.org"
	<robh@...nel.org>,
        "kwilczynski@...nel.org" <kwilczynski@...nel.org>,
        "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
        "conor+dt@...nel.org"
	<conor+dt@...nel.org>,
        "fugang.duan@...tech.com" <fugang.duan@...tech.com>,
        "guoyin.chen@...tech.com" <guoyin.chen@...tech.com>,
        "peter.chen@...tech.com"
	<peter.chen@...tech.com>,
        "cix-kernel-upstream@...tech.com"
	<cix-kernel-upstream@...tech.com>,
        "linux-pci@...r.kernel.org"
	<linux-pci@...r.kernel.org>,
        "devicetree@...r.kernel.org"
	<devicetree@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v8 07/15] PCI: cadence: Move PCIe controller common
 functions as a separate file

>functions as a separate file
>
>EXTERNAL MAIL
>
>
>On Tue, Aug 19, 2025 at 07:52:31PM GMT, hans.zhang@...tech.com wrote:
>> From: Manikandan K Pillai <mpillai@...ence.com>
>>
>> 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@...ence.com>
>> Co-developed-by: Hans Zhang <hans.zhang@...tech.com>
>> Signed-off-by: Hans Zhang <hans.zhang@...tech.com>
>> ---
>>  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@...e-electrons.com>
>
>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
>
>--
>மணிவண்ணன் சதாசிவம்

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ