linux-kernel - Re: [PATCH 1/4] PCI/pwrctrl: Move creation of pwrctrl devices to pci_scan

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: <Z18Pmq7_rK3pvuT4@wunner.de>
Date: Sun, 15 Dec 2024 18:19:22 +0100
From: Lukas Wunner <lukas@...ner.de>
To: manivannan.sadhasivam@...aro.org
Cc: Bjorn Helgaas <bhelgaas@...gle.com>,
	Bartosz Golaszewski <brgl@...ev.pl>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
	Bjorn Andersson <andersson@...nel.org>,
	Konrad Dybcio <konradybcio@...nel.org>,
	Qiang Yu <quic_qianyu@...cinc.com>
Subject: Re: [PATCH 1/4] PCI/pwrctrl: Move creation of pwrctrl devices to
 pci_scan_device()

On Tue, Dec 10, 2024 at 03:25:24PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2446,6 +2448,36 @@ bool pci_bus_read_dev_vendor_id(struct pci_bus *bus, int devfn, u32 *l,
>  }
>  EXPORT_SYMBOL(pci_bus_read_dev_vendor_id);
>  
> +/*
> + * Create pwrctrl devices (if required) for the PCI devices to handle the power
> + * state.
> + */
> +static void pci_pwrctrl_create_devices(struct pci_bus *bus, int devfn)

Nit:  AFAICS this only creates a *single* platform device, so the
"devices" (plural) in the function name and in the code comment
doesn't seem to be accurate anymore.


> diff --git a/drivers/pci/pwrctrl/core.c b/drivers/pci/pwrctrl/core.c
> index 2fb174db91e5..9cc7e2b7f2b5 100644
> --- a/drivers/pci/pwrctrl/core.c
> +++ b/drivers/pci/pwrctrl/core.c
> @@ -44,7 +44,7 @@ static void rescan_work_func(struct work_struct *work)
>  						   struct pci_pwrctrl, work);
>  
>  	pci_lock_rescan_remove();
> -	pci_rescan_bus(to_pci_dev(pwrctrl->dev->parent)->bus);
> +	pci_rescan_bus(to_pci_host_bridge(pwrctrl->dev->parent)->bus);
>  	pci_unlock_rescan_remove();
>  }

Remind me, what's the purpose of this?  I'm guessing that it
recursively creates the platform devices below the newly
powered up device, is that correct?  If so, is it still
necessary?  Doesn't the new approach automatically create
those devices upon their enumeration?

Overall it looks like a significant improvement, thanks for doing this!

Lukas