lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <55789f60-fd7b-44d6-a757-2b4ca0f727c1@roeck-us.net>
Date: Thu, 15 May 2025 18:33:20 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: dimitri.fedrau@...bherr.com, Uwe Kleine-König
 <ukleinek@...nel.org>, Jean Delvare <jdelvare@...e.com>
Cc: linux-pwm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-hwmon@...r.kernel.org, Dimitri Fedrau <dima.fedrau@...il.com>
Subject: Re: [PATCH v2] pwm: mc33xs2410: add support for temperature sensors

On 5/15/25 05:40, Dimitri Fedrau via B4 Relay wrote:
> From: Dimitri Fedrau <dimitri.fedrau@...bherr.com>
> 
> 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@...bherr.com>

 From hwmon perspective:

Acked-by: Guenter Roeck <linux@...ck-us.net>

> ---
> 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@liebherr.com
> ---
>   drivers/pwm/Kconfig          |   1 +
>   drivers/pwm/pwm-mc33xs2410.c | 164 ++++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 164 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 6faa8b2ec0a4844f667a84335f30bde44d52378e..0deaf8447f4302e7cfd3b4cb35c7d46ef19e006c 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -425,6 +425,7 @@ 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 a1ac3445ccdb4709d92e0075d424a8abc1416eee..c1b99b1143141242ce99782162ae05536dd88163 100644
> --- a/drivers/pwm/pwm-mc33xs2410.c
> +++ b/drivers/pwm/pwm-mc33xs2410.c
> @@ -21,6 +21,7 @@
>   #include <linux/bitfield.h>
>   #include <linux/delay.h>
>   #include <linux/err.h>
> +#include <linux/hwmon.h>
>   #include <linux/math64.h>
>   #include <linux/minmax.h>
>   #include <linux/module.h>
> @@ -29,6 +30,8 @@
>   
>   #include <linux/spi/spi.h>
>   
> +/* ctrl registers */
> +
>   #define MC33XS2410_GLB_CTRL			0x00
>   #define MC33XS2410_GLB_CTRL_MODE		GENMASK(7, 6)
>   #define MC33XS2410_GLB_CTRL_MODE_NORMAL		FIELD_PREP(MC33XS2410_GLB_CTRL_MODE, 1)
> @@ -51,6 +54,21 @@
>   
>   #define MC33XS2410_WDT				0x14
>   
> +#define MC33XS2410_TEMP_WT			0x29
> +#define MC33XS2410_TEMP_WT_MASK			GENMASK(7, 0)
> +
> +/* diag registers */
> +
> +/* chan in { 1 ... 4 } */
> +#define MC33XS2410_OUT_STA(chan)		(0x02 + (chan) - 1)
> +#define MC33XS2410_OUT_STA_OTW			BIT(8)
> +
> +#define MC33XS2410_TS_TEMP_DIE			0x26
> +#define MC33XS2410_TS_TEMP_MASK			GENMASK(9, 0)
> +
> +/* chan in { 1 ... 4 } */
> +#define MC33XS2410_TS_TEMP(chan)		(0x2f + (chan) - 1)
> +
>   #define MC33XS2410_PWM_MIN_PERIOD		488282
>   /* step in { 0 ... 3 } */
>   #define MC33XS2410_PWM_MAX_PERIOD(step)		(2000000000 >> (2 * (step)))
> @@ -125,6 +143,11 @@ static int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val)
>   	return mc33xs2410_read_reg(spi, reg, val, MC33XS2410_FRAME_IN_DATA_RD);
>   }
>   
> +static int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val)
> +{
> +	return mc33xs2410_read_reg(spi, reg, val, 0);
> +}
> +
>   static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val)
>   {
>   	u16 tmp;
> @@ -140,6 +163,145 @@ 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,
> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_MAX |
> +			   HWMON_T_ALARM,
> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_MAX |
> +			   HWMON_T_ALARM,
> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_MAX |
> +			   HWMON_T_ALARM,
> +			   HWMON_T_LABEL | HWMON_T_INPUT | HWMON_T_MAX |
> +			   HWMON_T_ALARM),
> +	NULL,
> +};
> +
> +static umode_t mc33xs2410_hwmon_is_visible(const void *data,
> +					   enum hwmon_sensor_types type,
> +					   u32 attr, int channel)
> +{
> +	switch (attr) {
> +	case hwmon_temp_input:
> +	case hwmon_temp_alarm:
> +	case hwmon_temp_label:
> +		return 0444;
> +	case hwmon_temp_max:
> +		return 0644;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static int mc33xs2410_hwmon_read(struct device *dev,
> +				 enum hwmon_sensor_types type,
> +				 u32 attr, int channel, long *val)
> +{
> +	struct spi_device *spi = dev_get_drvdata(dev);
> +	u16 reg_val;
> +	int ret;
> +	u8 reg;
> +
> +	switch (attr) {
> +	case hwmon_temp_input:
> +		reg = (channel == 0) ? MC33XS2410_TS_TEMP_DIE :
> +				       MC33XS2410_TS_TEMP(channel);
> +		ret = mc33xs2410_read_reg_diag(spi, reg, &reg_val);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* LSB is 0.25 degree celsius */
> +		*val = FIELD_GET(MC33XS2410_TS_TEMP_MASK, reg_val) * 250 - 40000;
> +		return 0;
> +	case hwmon_temp_alarm:
> +		ret = mc33xs2410_read_reg_diag(spi, MC33XS2410_OUT_STA(channel),
> +					       &reg_val);
> +		if (ret < 0)
> +			return ret;
> +
> +		*val = FIELD_GET(MC33XS2410_OUT_STA_OTW, reg_val);
> +		return 0;
> +	case hwmon_temp_max:
> +		ret = mc33xs2410_read_reg_ctrl(spi, MC33XS2410_TEMP_WT, &reg_val);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* LSB is 1 degree celsius */
> +		*val = FIELD_GET(MC33XS2410_TEMP_WT_MASK, reg_val) * 1000 - 40000;
> +		return 0;
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static int mc33xs2410_hwmon_write(struct device *dev,
> +				  enum hwmon_sensor_types type, u32 attr,
> +				  int channel, long val)
> +{
> +	struct spi_device *spi = dev_get_drvdata(dev);
> +
> +	switch (attr) {
> +	case hwmon_temp_max:
> +		val = clamp_val(val, -40000, 215000);
> +
> +		/* LSB is 1 degree celsius */
> +		val = (val / 1000) + 40;
> +		return mc33xs2410_modify_reg(spi, MC33XS2410_TEMP_WT,
> +					     MC33XS2410_TEMP_WT_MASK, val);
> +	default:
> +		return -EOPNOTSUPP;
> +	}
> +}
> +
> +static const char *const mc33xs2410_temp_label[] = {
> +	"Central die temperature",
> +	"Channel 1 temperature",
> +	"Channel 2 temperature",
> +	"Channel 3 temperature",
> +	"Channel 4 temperature",
> +};
> +
> +static int mc33xs2410_read_string(struct device *dev,
> +				  enum hwmon_sensor_types type,
> +				  u32 attr, int channel, const char **str)
> +{
> +	*str = mc33xs2410_temp_label[channel];
> +
> +	return 0;
> +}
> +
> +static const struct hwmon_ops mc33xs2410_hwmon_hwmon_ops = {
> +	.is_visible = mc33xs2410_hwmon_is_visible,
> +	.read = mc33xs2410_hwmon_read,
> +	.read_string = mc33xs2410_read_string,
> +	.write = mc33xs2410_hwmon_write,
> +};
> +
> +static const struct hwmon_chip_info mc33xs2410_hwmon_chip_info = {
> +	.ops = &mc33xs2410_hwmon_hwmon_ops,
> +	.info = mc33xs2410_hwmon_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);
> +
> +	return PTR_ERR_OR_ZERO(hwmon);
> +}
> +
> +#else
> +static int mc33xs2410_hwmon_probe(struct spi_device *spi)
> +{
> +	return 0;
> +}
> +#endif
> +
>   static u8 mc33xs2410_pwm_get_freq(u64 period)
>   {
>   	u8 step, count;
> @@ -361,7 +523,7 @@ static int mc33xs2410_probe(struct spi_device *spi)
>   	if (ret < 0)
>   		return dev_err_probe(dev, ret, "Failed to add pwm chip\n");
>   
> -	return 0;
> +	return mc33xs2410_hwmon_probe(spi);
>   }
>   
>   static const struct spi_device_id mc33xs2410_spi_id[] = {
> 
> ---
> base-commit: 7395eb13e3a85067de3e083d3781630ea303c0c4
> change-id: 20250507-mc33xs2410-hwmon-a5ff9efec005
> 
> Best regards,


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ