Re: [PATCH v2] pwm: mc33xs2410: add support for temperature sensors

[Uwe Kleine-König]

Hello Dimitri,

On Thu, May 15, 2025 at 02:40:54PM +0200, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@xxxxxxxxxxxx>
> 
> The MC33XS2410 provides temperature sensors for the central die temperature
> and the four outputs. Additionally a common temperature warning threshold
> can be configured for the outputs. Add hwmon support for the sensors.
> 
> Signed-off-by: Dimitri Fedrau <dimitri.fedrau@xxxxxxxxxxxx>
> ---
> Changes in v2:
> - Remove helper mc33xs2410_hwmon_read_out_status and report the last
>   latched status.
> - Link to v1: https://lore.kernel.org/r/20250512-mc33xs2410-hwmon-v1-1-addba77c78f9@xxxxxxxxxxxx
> ---

Mostly fine from my POV. I suggest to squash the following change into
your patch:

diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index a0c077af9c98..d9bcd1e8413e 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -425,7 +425,6 @@ config PWM_LPSS_PLATFORM
 
 config PWM_MC33XS2410
 	tristate "MC33XS2410 PWM support"
-	depends on HWMON || HWMON=n
 	depends on OF
 	depends on SPI
 	help
diff --git a/drivers/pwm/pwm-mc33xs2410.c b/drivers/pwm/pwm-mc33xs2410.c
index c1b99b114314..f5bba1a7bcc5 100644
--- a/drivers/pwm/pwm-mc33xs2410.c
+++ b/drivers/pwm/pwm-mc33xs2410.c
@@ -163,7 +163,6 @@ static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val
 	return mc33xs2410_write_reg(spi, reg, tmp);
 }
 
-#if IS_ENABLED(CONFIG_HWMON)
 static const struct hwmon_channel_info * const mc33xs2410_hwmon_info[] = {
 	HWMON_CHANNEL_INFO(temp,
 			   HWMON_T_LABEL | HWMON_T_INPUT,
@@ -286,21 +285,20 @@ static const struct hwmon_chip_info mc33xs2410_hwmon_chip_info = {
 static int mc33xs2410_hwmon_probe(struct spi_device *spi)
 {
 	struct device *dev = &spi->dev;
-	struct device *hwmon;
 
-	hwmon = devm_hwmon_device_register_with_info(dev, NULL, spi,
-						     &mc33xs2410_hwmon_chip_info,
-						     NULL);
+	if (IS_REACHABLE(CONFIG_HWMON)) {
+		struct device *hwmon;
 
-	return PTR_ERR_OR_ZERO(hwmon);
-}
+		hwmon = devm_hwmon_device_register_with_info(dev, NULL, spi,
+							     &mc33xs2410_hwmon_chip_info,
+							     NULL);
 
-#else
-static int mc33xs2410_hwmon_probe(struct spi_device *spi)
-{
-	return 0;
+		return PTR_ERR_OR_ZERO(hwmon);
+	} else {
+		dev_dbg(dev, "Not registering hwmon sensors\n");
+		return 0;
+	}
 }
-#endif
 
 static u8 mc33xs2410_pwm_get_freq(u64 period)
 {
@@ -523,7 +521,11 @@ static int mc33xs2410_probe(struct spi_device *spi)
 	if (ret < 0)
 		return dev_err_probe(dev, ret, "Failed to add pwm chip\n");
 
-	return mc33xs2410_hwmon_probe(spi);
+	ret = mc33xs2410_hwmon_probe(spi);
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "Failed to register hwmon sensors\n");
+
+	return 0;
 }
 
 static const struct spi_device_id mc33xs2410_spi_id[] = {
Best regards
Uwe

Attachment: signature.asc
Description: PGP signature