Re: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Combining device allocation and addition into a single `alloc_and_add` function breaks device model configuration semantics for `spmi_bus_type` devices.
--

--- Patch [5]: [PATCH v12 05/12] spmi: Implement spmi_subdevice_alloc_and_add() and devm variant ---
commit 07c884decc8feb83970dc7a2fe54a4fbf80be610
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

spmi: Implement spmi_subdevice_alloc_and_add() and devm variant

This patch adds new functions to manage sub-devices of an SPMI device,
including spmi_subdevice_alloc_and_add() and spmi_subdevice_remove().
These are intended to support complex PMICs that feature multiple
sub-devices addressable via SPMI register ranges.

> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 4050f3614dab8..e218c81405458 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c

[ ... ]

> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
> +{
> +	struct spmi_subdevice *sub_sdev;
> +	struct spmi_device *sdev;
> +	int ret;

[ ... ]

> +	device_initialize(&sdev->dev);
> +	sdev->dev.parent = &sparent->dev;
> +	sdev->dev.bus = &spmi_bus_type;
> +	sdev->dev.type = &spmi_subdev_type;
> +
> +	ret = dev_set_name(&sdev->dev, "%u-%02x.%u.auto",
> +			   sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
> +	if (ret)
> +		goto err_put_dev;
> +
> +	ret = device_add(&sdev->dev);

[Severity: High]
Does combining allocation and registration into a single function prevent
callers from properly configuring device properties?

Since device_add() is called immediately here, the driver core executes bus
matching and broadcasts the KOBJ_ADD uevent before the caller receives the
device pointer. This leaves no window to set properties like of_node or
fwnode.

Will this cause the uevent to be sent without a MODALIAS, breaking userspace
module autoloading and automatic driver probing for these subdevices?

> +	if (ret) {
> +		dev_err(&sdev->dev, "Can't add device, status %pe\n", ERR_PTR(ret));
> +		goto err_put_dev;
> +	}
> +
> +	return sub_sdev;
> +
> +err_put_dev:
> +	put_device(&sdev->dev);
> +	return ERR_PTR(ret);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707111829.180158-1-angelogioacchino.delregno@collabora.com?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy