All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Shiv Prakash Gupta" <shivprakash.gupta@nxp.com>,
	linux-rtc@vger.kernel.org, linux-hwmon@vger.kernel.org,
	linux-watchdog@vger.kernel.org, devicetree@vger.kernel.org
Cc: robh@kernel.org, conor+dt@kernel.org,
	"Alexandre Belloni" <alexandre.belloni@bootlin.com>
Subject: Re: [PATCH v4 4/4] rtc: pcf8525: Add temperature sensor support via hwmon
Date: Fri, 30 Oct 2026 11:11:55 +0000	[thread overview]
Message-ID: <sashiko-outbox-190925@kernel.org> (raw)
In-Reply-To: <20261030110139.1929109-5-shivprakash.gupta@nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] rtc: pcf8525: use devm_release_action() on hwmon error in pcf8525_hwmon_register()
--

commit 657bb5e33615f2ed31527354e958f8aeecac64a5
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

rtc: pcf8525: Add temperature sensor support via hwmon

This commit adds optional hwmon support for the PCF8525 on-chip temperature
sensor, exposing temperature readings and a configurable update interval.

> diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
> index f8fbc00bb464d..5cb984958f286 100644
> --- a/drivers/rtc/rtc-pcf8525.c
> +++ b/drivers/rtc/rtc-pcf8525.c
[ ... ]
> @@ -346,6 +349,217 @@ static int pcf8525_rtc_set_offset(struct device *dev, long offset)
[ ... ]
> +static void pcf8525_hwmon_register(struct device *dev,
> +				   struct pcf8525 *pcf8525)
> +{
> +	struct device *hwmon_dev;
> +	int ret;
> +
> +	if (!IS_ENABLED(CONFIG_RTC_DRV_PCF8525_HWMON))
> +		return;
> +
> +	/* Enable the temperature readout before exposing hwmon attributes. */
> +	ret = regmap_update_bits(pcf8525->regmap, PCF8525_REG_CTRL5,
> +				 PCF8525_CTRL5_TEMP_RD_EN,
> +				 PCF8525_CTRL5_TEMP_RD_EN);
> +	if (ret) {
> +		dev_warn(dev, "failed to enable temperature readout: %d\n", ret);
> +		return;
> +	}
> +
> +	/*
> +	 * Register a devm cleanup action to disable the digital
> +	 * temperature readout when the driver is unbound.
> +	 */
> +	if (devm_add_action_or_reset(dev, pcf8525_hwmon_disable_temp, pcf8525)) {
> +		dev_warn(dev, "failed to register HWMON cleanup action\n");
> +		return;
> +	}
> +
> +	/* Only expose sysfs attributes once the hardware is fully ready. */
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "pcf8525",
> +							 pcf8525,
> +							 &pcf8525_hwmon_chip_info,
> +							 NULL);
> +	if (IS_ERR(hwmon_dev)) {
> +		dev_warn(dev, "failed to register HWMON device: %ld\n",
> +			 PTR_ERR(hwmon_dev));
> +		pcf8525_hwmon_disable_temp(pcf8525);
> +		return;
> +	}

[Severity: Low]
In pcf8525_hwmon_register() in drivers/rtc/rtc-pcf8525.c, if
devm_hwmon_device_register_with_info() fails after devm_add_action_or_reset()
has already registered pcf8525_hwmon_disable_temp() on dev, calling
pcf8525_hwmon_disable_temp(pcf8525) directly leaves the devres action queued
while pcf8525_probe() succeeds. When the device is later unbound, devres will
invoke pcf8525_hwmon_disable_temp() a second time.

Should this error path call devm_release_action(dev,
pcf8525_hwmon_disable_temp, pcf8525) instead of calling
pcf8525_hwmon_disable_temp(pcf8525) directly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030110139.1929109-1-shivprakash.gupta@nxp.com?part=4

      reply	other threads:[~2026-10-30 11:11 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-30 11:01 [PATCH v4 0/4] rtc: Add NXP PCF8525 RTC driver with watchdog and hwmon Shiv Prakash Gupta
2026-10-30 11:01 ` [PATCH v4 1/4] dt-bindings: rtc: Add NXP PCF8525 RTC Shiv Prakash Gupta
2026-10-30 11:04   ` sashiko-bot
2026-10-30 11:01 ` [PATCH v4 2/4] rtc: pcf8525: Add NXP PCF8525 RTC driver Shiv Prakash Gupta
2026-10-30 11:18   ` sashiko-bot
2026-10-30 11:01 ` [PATCH v4 3/4] rtc: pcf8525: Add watchdog support Shiv Prakash Gupta
2026-10-30 11:19   ` sashiko-bot
2026-10-30 11:01 ` [PATCH v4 4/4] rtc: pcf8525: Add temperature sensor support via hwmon Shiv Prakash Gupta
2026-10-30 11:11   ` sashiko-bot [this message]

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-190925@kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=alexandre.belloni@bootlin.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-rtc@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=shivprakash.gupta@nxp.com \
    /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.