[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <zfe2q5sw5pphfyzidhe6d564j6yw32lnxzv4etxcr3hhi2ychy@34qka6dllzxe>
Date: Wed, 17 Sep 2025 18:24:26 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Cc: sboyd@...nel.org, jic23@...nel.org, dlechner@...libre.com,
nuno.sa@...log.com, andy@...nel.org, arnd@...db.de, gregkh@...uxfoundation.org,
srini@...nel.org, vkoul@...nel.org, kishon@...nel.org,
krzysztof.kozlowski@...aro.org, u.kleine-koenig@...libre.com, linux-arm-msm@...r.kernel.org,
linux-iio@...r.kernel.org, linux-kernel@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-pm@...r.kernel.org, kernel@...labora.com, wenst@...omium.org,
casey.connolly@...aro.org, Jonathan Cameron <jonathan.cameron@...wei.com>,
Neil Armstrong <neil.armstrong@...aro.org>
Subject: Re: [PATCH v4 1/7] spmi: Implement spmi_subdevice_alloc_and_add()
and devm variant
Hi,
On Tue, Sep 16, 2025 at 10:44:39AM +0200, AngeloGioacchino Del Regno wrote:
> Some devices connected over the SPMI bus may be big, in the sense
> that those may be a complex of devices managed by a single chip
> over the SPMI bus, reachable through a single SID.
>
> Add new functions aimed at managing sub-devices of a SPMI device
> spmi_subdevice_alloc_and_add() and a spmi_subdevice_put_and_remove()
> for adding a new subdevice and removing it respectively, and also
> add their devm_* variants.
>
> The need for such functions comes from the existance of those
> complex Power Management ICs (PMICs), which feature one or many
> sub-devices, in some cases with these being even addressable on
> the chip in form of SPMI register ranges.
>
> Examples of those devices can be found in both Qualcomm platforms
> with their PMICs having PON, RTC, SDAM, GPIO controller, and other
> sub-devices, and in newer MediaTek platforms showing similar HW
> features and a similar layout with those also having many subdevs.
>
> Also, instead of generally exporting symbols, export them with a
> new "SPMI" namespace: all users will have to import this namespace
> to make use of the newly introduced exports.
>
> Link: https://lore.kernel.org/r/20250722101317.76729-2-angelogioacchino.delregno@collabora.com
> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
> Tested-by: Neil Armstrong <neil.armstrong@...aro.org> # on SM8650-QRD
> Link: https://lore.kernel.org/r/20250730112645.542179-2-angelogioacchino.delregno@collabora.com
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> ---
It would be nice, if this patch lands in the 6.18 merge window. That
way we don't need to do an immutable branch for the different involved
subsystems.
Greetings,
-- Sebastian
> drivers/spmi/spmi-devres.c | 24 ++++++++++++
> drivers/spmi/spmi.c | 79 ++++++++++++++++++++++++++++++++++++++
> include/linux/spmi.h | 16 ++++++++
> 3 files changed, 119 insertions(+)
>
> diff --git a/drivers/spmi/spmi-devres.c b/drivers/spmi/spmi-devres.c
> index 62c4b3f24d06..8feebab0365b 100644
> --- a/drivers/spmi/spmi-devres.c
> +++ b/drivers/spmi/spmi-devres.c
> @@ -60,5 +60,29 @@ int devm_spmi_controller_add(struct device *parent, struct spmi_controller *ctrl
> }
> EXPORT_SYMBOL_GPL(devm_spmi_controller_add);
>
> +static void devm_spmi_subdevice_remove(void *res)
> +{
> + spmi_subdevice_remove(res);
> +}
> +
> +struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
> + struct spmi_device *sparent)
> +{
> + struct spmi_subdevice *sub_sdev;
> + int ret;
> +
> + sub_sdev = spmi_subdevice_alloc_and_add(sparent);
> + if (IS_ERR(sub_sdev))
> + return sub_sdev;
> +
> + ret = devm_add_action_or_reset(dev, devm_spmi_subdevice_remove, sub_sdev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + return sub_sdev;
> +}
> +EXPORT_SYMBOL_NS_GPL(devm_spmi_subdevice_alloc_and_add, "SPMI");
> +
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("SPMI devres helpers");
> +MODULE_IMPORT_NS("SPMI");
> diff --git a/drivers/spmi/spmi.c b/drivers/spmi/spmi.c
> index 3cf8d9bd4566..e011876c3187 100644
> --- a/drivers/spmi/spmi.c
> +++ b/drivers/spmi/spmi.c
> @@ -19,6 +19,7 @@
>
> static bool is_registered;
> static DEFINE_IDA(ctrl_ida);
> +static DEFINE_IDA(spmi_subdevice_ida);
>
> static void spmi_dev_release(struct device *dev)
> {
> @@ -31,6 +32,19 @@ static const struct device_type spmi_dev_type = {
> .release = spmi_dev_release,
> };
>
> +static void spmi_subdev_release(struct device *dev)
> +{
> + struct spmi_device *sdev = to_spmi_device(dev);
> + struct spmi_subdevice *sub_sdev = container_of(sdev, struct spmi_subdevice, sdev);
> +
> + ida_free(&spmi_subdevice_ida, sub_sdev->devid);
> + kfree(sub_sdev);
> +}
> +
> +static const struct device_type spmi_subdev_type = {
> + .release = spmi_subdev_release,
> +};
> +
> static void spmi_ctrl_release(struct device *dev)
> {
> struct spmi_controller *ctrl = to_spmi_controller(dev);
> @@ -90,6 +104,18 @@ void spmi_device_remove(struct spmi_device *sdev)
> }
> EXPORT_SYMBOL_GPL(spmi_device_remove);
>
> +/**
> + * spmi_subdevice_remove() - Remove an SPMI subdevice
> + * @sub_sdev: spmi_device to be removed
> + */
> +void spmi_subdevice_remove(struct spmi_subdevice *sub_sdev)
> +{
> + struct spmi_device *sdev = &sub_sdev->sdev;
> +
> + device_unregister(&sdev->dev);
> +}
> +EXPORT_SYMBOL_NS_GPL(spmi_subdevice_remove, "SPMI");
> +
> static inline int
> spmi_cmd(struct spmi_controller *ctrl, u8 opcode, u8 sid)
> {
> @@ -431,6 +457,59 @@ struct spmi_device *spmi_device_alloc(struct spmi_controller *ctrl)
> }
> EXPORT_SYMBOL_GPL(spmi_device_alloc);
>
> +/**
> + * spmi_subdevice_alloc_and_add(): Allocate and add a new SPMI sub-device
> + * @sparent: SPMI parent device with previously registered SPMI controller
> + *
> + * Returns:
> + * Pointer to newly allocated SPMI sub-device for success or negative ERR_PTR.
> + */
> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent)
> +{
> + struct spmi_subdevice *sub_sdev;
> + struct spmi_device *sdev;
> + int ret;
> +
> + sub_sdev = kzalloc(sizeof(*sub_sdev), GFP_KERNEL);
> + if (!sub_sdev)
> + return ERR_PTR(-ENOMEM);
> +
> + ret = ida_alloc(&spmi_subdevice_ida, GFP_KERNEL);
> + if (ret < 0) {
> + kfree(sub_sdev);
> + return ERR_PTR(ret);
> + }
> +
> + sdev = &sub_sdev->sdev;
> + sdev->ctrl = sparent->ctrl;
> + device_initialize(&sdev->dev);
> + sdev->dev.parent = &sparent->dev;
> + sdev->dev.bus = &spmi_bus_type;
> + sdev->dev.type = &spmi_subdev_type;
> +
> + sub_sdev->devid = ret;
> + sdev->usid = sparent->usid;
> +
> + ret = dev_set_name(&sdev->dev, "%d-%02x.%d.auto",
> + sdev->ctrl->nr, sdev->usid, sub_sdev->devid);
> + if (ret)
> + goto err_put_dev;
> +
> + ret = device_add(&sdev->dev);
> + if (ret) {
> + dev_err(&sdev->dev, "Can't add %s, status %d\n",
> + dev_name(&sdev->dev), ret);
> + goto err_put_dev;
> + }
> +
> + return sub_sdev;
> +
> +err_put_dev:
> + put_device(&sdev->dev);
> + return ERR_PTR(ret);
> +}
> +EXPORT_SYMBOL_NS_GPL(spmi_subdevice_alloc_and_add, "SPMI");
> +
> /**
> * spmi_controller_alloc() - Allocate a new SPMI controller
> * @parent: parent device
> diff --git a/include/linux/spmi.h b/include/linux/spmi.h
> index 28e8c8bd3944..7cea0a5b034b 100644
> --- a/include/linux/spmi.h
> +++ b/include/linux/spmi.h
> @@ -69,6 +69,22 @@ int spmi_device_add(struct spmi_device *sdev);
>
> void spmi_device_remove(struct spmi_device *sdev);
>
> +/**
> + * struct spmi_subdevice - Basic representation of an SPMI sub-device
> + * @sdev: Sub-device representation of an SPMI device
> + * @devid: Platform Device ID of an SPMI sub-device
> + */
> +struct spmi_subdevice {
> + struct spmi_device sdev;
> + unsigned int devid;
> +};
> +
> +struct spmi_subdevice *spmi_subdevice_alloc_and_add(struct spmi_device *sparent);
> +void spmi_subdevice_remove(struct spmi_subdevice *sdev);
> +
> +struct spmi_subdevice *devm_spmi_subdevice_alloc_and_add(struct device *dev,
> + struct spmi_device *sparent);
> +
> /**
> * struct spmi_controller - interface to the SPMI master controller
> * @dev: Driver model representation of the device.
> --
> 2.51.0
>
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists