Re: [PATCH 4/4] rtc: pcf8525: Add temperature sensor support via hwmon

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Subsystem Layering Violation: Registering hwmon functionality directly from the RTC driver.
- [Medium] Missing hardware state rollback when hwmon registration fails.
--

--- Patch [4]: [PATCH 4/4] rtc: pcf8525: Add temperature sensor support via hwmon ---
commit 36335f5669e8a341626815565d14236fae935a84
Author: Shiv Prakash Gupta <shivprakash.gupta@nxp.com>

rtc: pcf8525: Add temperature sensor support via hwmon
    
Register the PCF8525 internal temperature sensor through the hwmon
interface when CONFIG_RTC_DRV_PCF8525_HWMON is enabled.

> diff --git a/drivers/rtc/rtc-pcf8525.c b/drivers/rtc/rtc-pcf8525.c
> index 4ef648bdfc5b9..92bab3231ce7f 100644
> --- a/drivers/rtc/rtc-pcf8525.c
> +++ b/drivers/rtc/rtc-pcf8525.c
[ ... ]
> +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 only the digital readout; preserve TSIE, CL and XTL_TYP. */
> +	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;
> +	}
> +
> +	hwmon_dev = devm_hwmon_device_register_with_info(dev, "pcf8525",
> +							 pcf8525,
> +							 &pcf8525_hwmon_chip_info,
> +							 NULL);

[Severity: Medium]
Is it permitted to register the hwmon device directly from this RTC driver?

The hwmon subsystem guidelines specify that if the main functionality of a
device is not hardware monitoring, the hwmon features should be implemented
as an auxiliary device driver residing in drivers/hwmon/.

> +	if (IS_ERR(hwmon_dev))
> +		dev_warn(dev, "failed to register HWMON device: %ld\n",
> +			 PTR_ERR(hwmon_dev));

[Severity: Medium]
If devm_hwmon_device_register_with_info() fails, should the hardware state
be rolled back?

The code sets the PCF8525_CTRL5_TEMP_RD_EN bit in the register before
calling the registration function. If registration fails, the error path
logs a warning and returns, leaving the temperature sensor enabled in the
hardware. Could this cause unnecessary power consumption?

> +}

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