Re: [PATCH v2 4/9] power: sequencing: pcie-m2: Create serdev for PCI devices present before probe

[Bartosz Golaszewski]

On Thu, 7 May 2026 18:06:12 +0200, Manivannan Sadhasivam via B4 Relay
<devnull+manivannan.sadhasivam.oss.qualcomm.com@xxxxxxxxxx> said:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
>
> So far, the driver is registering a notifier to create serdev for the PCI
> devices that are going to be attached after probe. But it doesn't handle
> the devices present before probe. Due to this, serdev is not getting
> created for those existing devices.
>
> Hence, create serdev for PCI devices available before probe as well.
>
> Note that the serdev for available devices are created before
> registering the notifier. There is a small window where a device could
> appear after pwrseq_pcie_m2_create_serdev(), before notifier registration.
> But since M.2 cards are fixed to a slot, they are mostly added either
> before booting the host or after using hotplug. So this window is mostly
> theoretical.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> ---
>  drivers/power/sequencing/pwrseq-pcie-m2.c | 83 ++++++++++++++++++++++++++-----
>  1 file changed, 70 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 038271207a27..0a37a375a89d 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -236,7 +236,7 @@ static int pwrseq_pcie_m2_create_bt_node(struct pwrseq_pcie_m2_ctx *ctx,
>  	return ret;
>  }
>
> -static int pwrseq_pcie_m2_create_serdev(struct pwrseq_pcie_m2_ctx *ctx,
> +static int __pwrseq_pcie_m2_create_serdev(struct pwrseq_pcie_m2_ctx *ctx,
>  					struct pci_dev *pdev)

It may just be a personal preference but I dislike the __ prefix that doesn't
really indicate how the function is different from the one without it. I prefer
the name to reflect that - if it's because the function expects a mutex to be
already taken then it should be called something_something_unlocked() etc.

Maybe call it pwrseq_pcie_m2_create_serdev_for_pci()? Or at least:
pwrseq_pcie_m2_do_create_serdev()?

>  {
>  	struct serdev_controller *serdev_ctrl;
> @@ -259,6 +259,16 @@ static int pwrseq_pcie_m2_create_serdev(struct pwrseq_pcie_m2_ctx *ctx,
>  		return 0;
>  	}
>
> +	/* Bail out if the serdev device was already created for the PCI dev */
> +	mutex_lock(&ctx->list_lock);
> +	list_for_each_entry(pci_dev, &ctx->pci_devices, list) {
> +		if (pci_dev->pdev == pdev) {
> +			mutex_unlock(&ctx->list_lock);
> +			return 0;
> +		}
> +	}
> +	mutex_unlock(&ctx->list_lock);

Is there any reason to not use guard() here?

Bart