All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Fenglin Wu" <fenglin.wu@oss.qualcomm.com>
Cc: linux-rtc@vger.kernel.org,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Subject: Re: [PATCH 1/3] soc: qcom: Add pmic-gio GIO framework driver
Date: Wed, 11 Nov 2026 03:47:10 +0000	[thread overview]
Message-ID: <sashiko-outbox-208223@kernel.org> (raw)
In-Reply-To: <20261110-gio-framework-rtc-driver-2026-10-12-v1-1-e4164178183a@oss.qualcomm.com>

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] soc: qcom: pmic_gio: UCSI MESSAGE_OUT blocks marked read-only
  in pmic_gio_reg_blocks
- [Medium] soc: qcom: pmic_gio: broken auxiliary_device release lifetime
  in devm_pmic_gio_add_consumer()
--

commit 0534b1e914edfb726f83b9ccdae4428780afd1fd
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>

soc: qcom: Add pmic-gio GIO framework driver

This commit adds a transport-agnostic PMIC GIO framework driver that exposes
the SoCCP battery-management firmware register space via regmap, routes
alerts through a virtual IRQ domain, and provides helpers for registering
transports, state notifiers, and auxiliary consumer devices.

> diff --git a/drivers/soc/qcom/pmic_gio.c b/drivers/soc/qcom/pmic_gio.c
> new file mode 100644
> index 0000000000000..7237bf1c549cf
> --- /dev/null
> +++ b/drivers/soc/qcom/pmic_gio.c
[ ... ]
> +static const struct pmic_gio_reg_block pmic_gio_reg_blocks[] = {
[ ... ]
> +	/* USBC */
> +	{ GIO_USBC_IF_VERSION_ADDR, GIO_USBC_IF_VERSION_SIZE, false },
> +	{ GIO_USBC_ALERT_STATUS_ADDR, GIO_USBC_ALERT_STATUS_SIZE, false },
> +	{ GIO_USBC_LAST_ALERT_ADDR, GIO_USBC_LAST_ALERT_SIZE, false },
> +	{ GIO_USBC_UCSI_VERSION_REG, GIO_USBC_UCSI_VERSION_SIZE, false },
> +	{ GIO_USBC_UCSI_CCI_REG, GIO_USBC_UCSI_CCI_SIZE, false },
> +	{ GIO_USBC_UCSI_CONTROL_REG, GIO_USBC_UCSI_CONTROL_SIZE, true },
> +	{ GIO_USBC_UCSI_MESSAGE_IN_REG, GIO_USBC_UCSI_MESSAGE_IN_SIZE, false },
> +	{ GIO_USBC_UCSI_MSGINX_REG, GIO_USBC_UCSI_MSGINX_SIZE, false },
> +	{ GIO_USBC_UCSI_MESSAGE_OUT_REG, GIO_USBC_UCSI_MESSAGE_OUT_SIZE, false },
> +	{ GIO_USBC_UCSI_MSGOUTX_ADDR, GIO_USBC_UCSI_MSGOUTX_SIZE, false },

[Severity: Medium]
Should GIO_USBC_UCSI_MESSAGE_OUT_REG and GIO_USBC_UCSI_MSGOUTX_ADDR have
writeable set to true?  In UCSI, the OS writes outbound command payloads to
MESSAGE_OUT and MSGOUTX before writing CONTROL, but
pmic_gio_regmap_bus_gather_write() and pmic_gio_writeable_reg() reject
writes when writeable is false.

[ ... ]

> +static void pmic_gio_aux_release(struct device *dev)
> +{
> +	of_node_put(dev->of_node);
> +	dev->of_node = NULL;
> +}
> +
> +static void pmic_gio_del_aux_device(void *data)
> +{
> +	struct auxiliary_device *aux = data;
> +
> +	auxiliary_device_delete(aux);
> +	auxiliary_device_uninit(aux);
> +}

[ ... ]

> +int devm_pmic_gio_add_consumer(struct pmic_gio_device *gio, struct auxiliary_device *aux,
> +			       const char *name)
> +{
> +	int ret;
> +
> +	aux->name = name;
> +	aux->dev.parent = gio->dev;
> +	aux->dev.release = pmic_gio_aux_release;
> +	aux->dev.platform_data = gio;
> +	device_set_of_node_from_dev(&aux->dev, gio->dev);
> +	ret = auxiliary_device_init(aux);
> +	if (ret) {
> +		of_node_put(aux->dev.of_node);
> +		aux->dev.of_node = NULL;
> +		return ret;
> +	}
> +
> +	ret = auxiliary_device_add(aux);
> +	if (ret) {
> +		auxiliary_device_uninit(aux);
> +		return ret;
> +	}
> +
> +	return devm_add_action_or_reset(gio->dev, pmic_gio_del_aux_device, aux);
> +}
> +EXPORT_SYMBOL_GPL(devm_pmic_gio_add_consumer);

[Severity: Medium]
Does pmic_gio_aux_release() violate the struct device lifetime rules by not
freeing aux?  A reference-counted struct auxiliary_device must be freed in
its dev.release callback when the last reference drops; if the caller
allocates aux with devm_kzalloc(), unbinding the parent frees aux while a
reference to aux->dev may still be held, causing a use-after-free.

Could devm_pmic_gio_add_consumer() wrap devm_auxiliary_device_create(gio->dev,
name, gio) instead of taking a caller-allocated struct auxiliary_device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110-gio-framework-rtc-driver-2026-10-12-v1-0-e4164178183a@oss.qualcomm.com?part=1

  reply	other threads:[~2026-11-11  3:47 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-11  3:28 [PATCH 0/3] soc: qcom: Add Qualcomm PMIC GIO framework and RTC driver using it Fenglin Wu
2026-11-11  3:28 ` [PATCH 1/3] soc: qcom: Add pmic-gio GIO framework driver Fenglin Wu
2026-11-11  3:47   ` sashiko-bot [this message]
2026-11-11  3:28 ` [PATCH 2/3] soc: qcom: pmic_glink: Add GIO pmic_glink instance Fenglin Wu
2026-11-11  4:07   ` sashiko-bot
2026-11-11 13:14   ` Jeff Roach
2026-11-11 13:41     ` Jeff Roach
2026-11-12  1:11     ` Fenglin Wu
2026-11-12  7:10       ` Fenglin Wu
2026-11-12 21:11         ` Jeff Roach
2026-11-11  3:28 ` [PATCH 3/3] rtc: Add Qualcomm GIO RTC driver Fenglin Wu
2026-11-11  4:20   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=sashiko-outbox-208223@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=fenglin.wu@oss.qualcomm.com \
    --cc=linux-rtc@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.