* [PATCH v4 0/2] hwmon: add support for MC33XS2410 hardware
@ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay
0 siblings, 0 replies; 10+ messages in thread
From: Dimitri Fedrau @ 2025-07-08 16:13 UTC (permalink / raw)
To: Uwe Kleine-König, Jean Delvare, Guenter Roeck
Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau,
Dimitri Fedrau
The device is able to monitor temperature, voltage and current of each of
the four outputs. Add basic support for monitoring the temperature of the
four outputs and the die temperature.
Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com>
---
Changes in v4:
- Move hwmon functionality into separate driver residing in hwmon subsystem
(auxiliary device)
- Link to v3: https://lore.kernel.org/r/20250619-mc33xs2410-hwmon-v3-1-301731e49f8f@liebherr.com
Changes in v3:
- Add changes suggested by Uwe Kleine-König.
Remove "#if IS_ENABLED(CONFIG_HWMON)" and add
"if (IS_REACHABLE(CONFIG_HWMON))" in mc33xs2410_hwmon_probe.
- Link to v2: https://lore.kernel.org/r/20250515-mc33xs2410-hwmon-v2-1-8d2e78f7e30d@liebherr.com
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
---
Dimitri Fedrau (2):
pwm: mc33xs2410: add hwmon support
hwmon: add support for MC33XS2410 hardware monitoring
drivers/hwmon/Kconfig | 10 +++
drivers/hwmon/Makefile | 1 +
drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++
drivers/pwm/Kconfig | 1 +
drivers/pwm/pwm-mc33xs2410.c | 64 +++++++++++++-
include/linux/mc33xs2410.h | 14 +++
6 files changed, 267 insertions(+), 2 deletions(-)
---
base-commit: 446d36aa71b56e8a926e6ff066d83b12130fda59
change-id: 20250507-mc33xs2410-hwmon-a5ff9efec005
Best regards,
--
Dimitri Fedrau <dimitri.fedrau@liebherr.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH v4 0/2] hwmon: add support for MC33XS2410 hardware @ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 0 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau via B4 Relay @ 2025-07-08 16:13 UTC (permalink / raw) To: Uwe Kleine-König, Jean Delvare, Guenter Roeck Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau, Dimitri Fedrau The device is able to monitor temperature, voltage and current of each of the four outputs. Add basic support for monitoring the temperature of the four outputs and the die temperature. Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> --- Changes in v4: - Move hwmon functionality into separate driver residing in hwmon subsystem (auxiliary device) - Link to v3: https://lore.kernel.org/r/20250619-mc33xs2410-hwmon-v3-1-301731e49f8f@liebherr.com Changes in v3: - Add changes suggested by Uwe Kleine-König. Remove "#if IS_ENABLED(CONFIG_HWMON)" and add "if (IS_REACHABLE(CONFIG_HWMON))" in mc33xs2410_hwmon_probe. - Link to v2: https://lore.kernel.org/r/20250515-mc33xs2410-hwmon-v2-1-8d2e78f7e30d@liebherr.com 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 --- Dimitri Fedrau (2): pwm: mc33xs2410: add hwmon support hwmon: add support for MC33XS2410 hardware monitoring drivers/hwmon/Kconfig | 10 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++ drivers/pwm/Kconfig | 1 + drivers/pwm/pwm-mc33xs2410.c | 64 +++++++++++++- include/linux/mc33xs2410.h | 14 +++ 6 files changed, 267 insertions(+), 2 deletions(-) --- base-commit: 446d36aa71b56e8a926e6ff066d83b12130fda59 change-id: 20250507-mc33xs2410-hwmon-a5ff9efec005 Best regards, -- Dimitri Fedrau <dimitri.fedrau@liebherr.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] pwm: mc33xs2410: add hwmon support 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay @ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay -1 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau @ 2025-07-08 16:13 UTC (permalink / raw) To: Uwe Kleine-König, Jean Delvare, Guenter Roeck Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau, Dimitri Fedrau Support for hwmon is provided by a separate driver residing in hwmon subsystem which is implemented as auxiliary device. Add handling of this device. Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> --- drivers/pwm/Kconfig | 1 + drivers/pwm/pwm-mc33xs2410.c | 64 ++++++++++++++++++++++++++++++++++++++++++-- include/linux/mc33xs2410.h | 14 ++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 6e113f8b4baf293284f017bc22fbc162633bb2fc..5b20230c5ccd99137efcd968efe603b8c8e2253f 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -427,6 +427,7 @@ config PWM_MC33XS2410 tristate "MC33XS2410 PWM support" depends on OF depends on SPI + select AUXILIARY_BUS help NXP MC33XS2410 high-side switch driver. The MC33XS2410 is a four channel high-side switch. The device is operational from 3.0 V diff --git a/drivers/pwm/pwm-mc33xs2410.c b/drivers/pwm/pwm-mc33xs2410.c index a1ac3445ccdb4709d92e0075d424a8abc1416eee..e70ed90bfdac77f5c777f0ba66d670331a515d12 100644 --- a/drivers/pwm/pwm-mc33xs2410.c +++ b/drivers/pwm/pwm-mc33xs2410.c @@ -18,10 +18,12 @@ * rather something in between. */ +#include <linux/auxiliary_bus.h> #include <linux/bitfield.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/math64.h> +#include <linux/mc33xs2410.h> #include <linux/minmax.h> #include <linux/module.h> #include <linux/of.h> @@ -120,12 +122,19 @@ static int mc33xs2410_read_reg(struct spi_device *spi, u8 reg, u16 *val, u8 flag return mc33xs2410_read_regs(spi, ®, flag, val, 1); } -static int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val) +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); } +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_ctrl, "PWM_MC33XS2410"); -static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val) +{ + return mc33xs2410_read_reg(spi, reg, val, 0); +} +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_diag, "PWM_MC33XS2410"); + +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) { u16 tmp; int ret; @@ -139,6 +148,7 @@ static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val return mc33xs2410_write_reg(spi, reg, tmp); } +EXPORT_SYMBOL_NS_GPL(mc33xs2410_modify_reg, "PWM_MC33XS2410"); static u8 mc33xs2410_pwm_get_freq(u64 period) { @@ -297,6 +307,52 @@ static const struct pwm_ops mc33xs2410_pwm_ops = { .get_state = mc33xs2410_pwm_get_state, }; +static void mc33xs2410_adev_release(struct device *dev) +{ + struct auxiliary_device *adev = to_auxiliary_dev(dev); + + kfree(adev); +} + +static void mc33xs2410_unregister_adev(void *_adev) +{ + struct auxiliary_device *adev = _adev; + + auxiliary_device_delete(adev); + auxiliary_device_uninit(adev); +} + +static int mc33xs2410_hwmon_register(struct device *dev) +{ + struct auxiliary_device *adev; + int ret; + + adev = kzalloc(sizeof(*adev), GFP_KERNEL); + if (!adev) + return -ENOMEM; + + adev->name = "hwmon"; + adev->dev.parent = dev; + adev->dev.release = mc33xs2410_adev_release; + adev->id = 0; + + ret = auxiliary_device_init(adev); + if (ret) + return ret; + + ret = auxiliary_device_add(adev); + if (ret) { + auxiliary_device_uninit(adev); + return ret; + } + + ret = devm_add_action_or_reset(dev, mc33xs2410_unregister_adev, adev); + if (ret) + return ret; + + return 0; +} + static int mc33xs2410_reset(struct device *dev) { struct gpio_desc *reset_gpio; @@ -361,6 +417,10 @@ static int mc33xs2410_probe(struct spi_device *spi) if (ret < 0) return dev_err_probe(dev, ret, "Failed to add pwm chip\n"); + ret = mc33xs2410_hwmon_register(dev); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to register hwmon device\n"); + return 0; } diff --git a/include/linux/mc33xs2410.h b/include/linux/mc33xs2410.h new file mode 100644 index 0000000000000000000000000000000000000000..15a0b0b595fe00a369cee45f2d30b2d912b612bb --- /dev/null +++ b/include/linux/mc33xs2410.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH + */ +#ifndef _MC33XS2410_H +#define _MC33XS2410_H + +#include <linux/spi/spi.h> + +int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val); +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val); +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val); + +#endif /* _MC33XS2410_H */ -- 2.39.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 1/2] pwm: mc33xs2410: add hwmon support @ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 0 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau via B4 Relay @ 2025-07-08 16:13 UTC (permalink / raw) To: Uwe Kleine-König, Jean Delvare, Guenter Roeck Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau, Dimitri Fedrau From: Dimitri Fedrau <dimitri.fedrau@liebherr.com> Support for hwmon is provided by a separate driver residing in hwmon subsystem which is implemented as auxiliary device. Add handling of this device. Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> --- drivers/pwm/Kconfig | 1 + drivers/pwm/pwm-mc33xs2410.c | 64 ++++++++++++++++++++++++++++++++++++++++++-- include/linux/mc33xs2410.h | 14 ++++++++++ 3 files changed, 77 insertions(+), 2 deletions(-) diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig index 6e113f8b4baf293284f017bc22fbc162633bb2fc..5b20230c5ccd99137efcd968efe603b8c8e2253f 100644 --- a/drivers/pwm/Kconfig +++ b/drivers/pwm/Kconfig @@ -427,6 +427,7 @@ config PWM_MC33XS2410 tristate "MC33XS2410 PWM support" depends on OF depends on SPI + select AUXILIARY_BUS help NXP MC33XS2410 high-side switch driver. The MC33XS2410 is a four channel high-side switch. The device is operational from 3.0 V diff --git a/drivers/pwm/pwm-mc33xs2410.c b/drivers/pwm/pwm-mc33xs2410.c index a1ac3445ccdb4709d92e0075d424a8abc1416eee..e70ed90bfdac77f5c777f0ba66d670331a515d12 100644 --- a/drivers/pwm/pwm-mc33xs2410.c +++ b/drivers/pwm/pwm-mc33xs2410.c @@ -18,10 +18,12 @@ * rather something in between. */ +#include <linux/auxiliary_bus.h> #include <linux/bitfield.h> #include <linux/delay.h> #include <linux/err.h> #include <linux/math64.h> +#include <linux/mc33xs2410.h> #include <linux/minmax.h> #include <linux/module.h> #include <linux/of.h> @@ -120,12 +122,19 @@ static int mc33xs2410_read_reg(struct spi_device *spi, u8 reg, u16 *val, u8 flag return mc33xs2410_read_regs(spi, ®, flag, val, 1); } -static int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val) +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); } +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_ctrl, "PWM_MC33XS2410"); -static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val) +{ + return mc33xs2410_read_reg(spi, reg, val, 0); +} +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_diag, "PWM_MC33XS2410"); + +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) { u16 tmp; int ret; @@ -139,6 +148,7 @@ static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val return mc33xs2410_write_reg(spi, reg, tmp); } +EXPORT_SYMBOL_NS_GPL(mc33xs2410_modify_reg, "PWM_MC33XS2410"); static u8 mc33xs2410_pwm_get_freq(u64 period) { @@ -297,6 +307,52 @@ static const struct pwm_ops mc33xs2410_pwm_ops = { .get_state = mc33xs2410_pwm_get_state, }; +static void mc33xs2410_adev_release(struct device *dev) +{ + struct auxiliary_device *adev = to_auxiliary_dev(dev); + + kfree(adev); +} + +static void mc33xs2410_unregister_adev(void *_adev) +{ + struct auxiliary_device *adev = _adev; + + auxiliary_device_delete(adev); + auxiliary_device_uninit(adev); +} + +static int mc33xs2410_hwmon_register(struct device *dev) +{ + struct auxiliary_device *adev; + int ret; + + adev = kzalloc(sizeof(*adev), GFP_KERNEL); + if (!adev) + return -ENOMEM; + + adev->name = "hwmon"; + adev->dev.parent = dev; + adev->dev.release = mc33xs2410_adev_release; + adev->id = 0; + + ret = auxiliary_device_init(adev); + if (ret) + return ret; + + ret = auxiliary_device_add(adev); + if (ret) { + auxiliary_device_uninit(adev); + return ret; + } + + ret = devm_add_action_or_reset(dev, mc33xs2410_unregister_adev, adev); + if (ret) + return ret; + + return 0; +} + static int mc33xs2410_reset(struct device *dev) { struct gpio_desc *reset_gpio; @@ -361,6 +417,10 @@ static int mc33xs2410_probe(struct spi_device *spi) if (ret < 0) return dev_err_probe(dev, ret, "Failed to add pwm chip\n"); + ret = mc33xs2410_hwmon_register(dev); + if (ret < 0) + return dev_err_probe(dev, ret, "Failed to register hwmon device\n"); + return 0; } diff --git a/include/linux/mc33xs2410.h b/include/linux/mc33xs2410.h new file mode 100644 index 0000000000000000000000000000000000000000..15a0b0b595fe00a369cee45f2d30b2d912b612bb --- /dev/null +++ b/include/linux/mc33xs2410.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH + */ +#ifndef _MC33XS2410_H +#define _MC33XS2410_H + +#include <linux/spi/spi.h> + +int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val); +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val); +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val); + +#endif /* _MC33XS2410_H */ -- 2.39.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] pwm: mc33xs2410: add hwmon support 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay (?) @ 2025-07-16 6:39 ` Uwe Kleine-König 2025-07-16 7:06 ` Dimitri Fedrau -1 siblings, 1 reply; 10+ messages in thread From: Uwe Kleine-König @ 2025-07-16 6:39 UTC (permalink / raw) To: dimitri.fedrau Cc: Jean Delvare, Guenter Roeck, linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau [-- Attachment #1: Type: text/plain, Size: 4733 bytes --] Hello Dimitri, On Tue, Jul 08, 2025 at 06:13:03PM +0200, Dimitri Fedrau via B4 Relay wrote: > diff --git a/drivers/pwm/pwm-mc33xs2410.c b/drivers/pwm/pwm-mc33xs2410.c > index a1ac3445ccdb4709d92e0075d424a8abc1416eee..e70ed90bfdac77f5c777f0ba66d670331a515d12 100644 > --- a/drivers/pwm/pwm-mc33xs2410.c > +++ b/drivers/pwm/pwm-mc33xs2410.c > @@ -18,10 +18,12 @@ > * rather something in between. > */ > > +#include <linux/auxiliary_bus.h> > #include <linux/bitfield.h> > #include <linux/delay.h> > #include <linux/err.h> > #include <linux/math64.h> > +#include <linux/mc33xs2410.h> > #include <linux/minmax.h> > #include <linux/module.h> > #include <linux/of.h> > @@ -120,12 +122,19 @@ static int mc33xs2410_read_reg(struct spi_device *spi, u8 reg, u16 *val, u8 flag > return mc33xs2410_read_regs(spi, ®, flag, val, 1); > } > > -static int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val) > +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); > } > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_ctrl, "PWM_MC33XS2410"); To reduce repetition (a bit) you can consider to define DEFAULT_SYMBOL_NAMESPACE. > -static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) > +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val) > +{ > + return mc33xs2410_read_reg(spi, reg, val, 0); > +} > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_diag, "PWM_MC33XS2410"); > + > +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) > { > u16 tmp; > int ret; > @@ -139,6 +148,7 @@ static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val > > return mc33xs2410_write_reg(spi, reg, tmp); > } > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_modify_reg, "PWM_MC33XS2410"); > > static u8 mc33xs2410_pwm_get_freq(u64 period) > { > @@ -297,6 +307,52 @@ static const struct pwm_ops mc33xs2410_pwm_ops = { > .get_state = mc33xs2410_pwm_get_state, > }; > > +static void mc33xs2410_adev_release(struct device *dev) > +{ > + struct auxiliary_device *adev = to_auxiliary_dev(dev); > + > + kfree(adev); > +} > + > +static void mc33xs2410_unregister_adev(void *_adev) > +{ > + struct auxiliary_device *adev = _adev; > + > + auxiliary_device_delete(adev); > + auxiliary_device_uninit(adev); > +} This is a copy of auxiliary_device_destroy(). But see below. > +static int mc33xs2410_hwmon_register(struct device *dev) > +{ > + struct auxiliary_device *adev; > + int ret; > + > + adev = kzalloc(sizeof(*adev), GFP_KERNEL); > + if (!adev) > + return -ENOMEM; > + > + adev->name = "hwmon"; > + adev->dev.parent = dev; > + adev->dev.release = mc33xs2410_adev_release; > + adev->id = 0; > + > + ret = auxiliary_device_init(adev); > + if (ret) > + return ret; > + > + ret = auxiliary_device_add(adev); > + if (ret) { > + auxiliary_device_uninit(adev); > + return ret; > + } > + > + ret = devm_add_action_or_reset(dev, mc33xs2410_unregister_adev, adev); > + if (ret) > + return ret; > + > + return 0; > +} This function is equivalent to devm_auxiliary_device_create(dev, "hwmon", NULL); > + > static int mc33xs2410_reset(struct device *dev) > { > struct gpio_desc *reset_gpio; > @@ -361,6 +417,10 @@ static int mc33xs2410_probe(struct spi_device *spi) > if (ret < 0) > return dev_err_probe(dev, ret, "Failed to add pwm chip\n"); > > + ret = mc33xs2410_hwmon_register(dev); > + if (ret < 0) > + return dev_err_probe(dev, ret, "Failed to register hwmon device\n"); > + > return 0; > } > > diff --git a/include/linux/mc33xs2410.h b/include/linux/mc33xs2410.h > new file mode 100644 > index 0000000000000000000000000000000000000000..15a0b0b595fe00a369cee45f2d30b2d912b612bb > --- /dev/null > +++ b/include/linux/mc33xs2410.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH > + */ > +#ifndef _MC33XS2410_H > +#define _MC33XS2410_H > + > +#include <linux/spi/spi.h> > + > +int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val); > +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val); > +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val); > + > +#endif /* _MC33XS2410_H */ I consider it elegant to have the MODULE_IMPORT_NS("PWM_MC33XS2410") in the header. This is nice because the namespacing is completely transparant to consumers and all they need it the right #include as if there was no namespacing at all. Best regards Uwe [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 1/2] pwm: mc33xs2410: add hwmon support 2025-07-16 6:39 ` Uwe Kleine-König @ 2025-07-16 7:06 ` Dimitri Fedrau 0 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau @ 2025-07-16 7:06 UTC (permalink / raw) To: Uwe Kleine-König Cc: dimitri.fedrau, Jean Delvare, Guenter Roeck, linux-pwm, linux-kernel, linux-hwmon Hi Uwe, Am Wed, Jul 16, 2025 at 08:39:14AM +0200 schrieb Uwe Kleine-König: > Hello Dimitri, > > On Tue, Jul 08, 2025 at 06:13:03PM +0200, Dimitri Fedrau via B4 Relay wrote: > > diff --git a/drivers/pwm/pwm-mc33xs2410.c b/drivers/pwm/pwm-mc33xs2410.c > > index a1ac3445ccdb4709d92e0075d424a8abc1416eee..e70ed90bfdac77f5c777f0ba66d670331a515d12 100644 > > --- a/drivers/pwm/pwm-mc33xs2410.c > > +++ b/drivers/pwm/pwm-mc33xs2410.c > > @@ -18,10 +18,12 @@ > > * rather something in between. > > */ > > > > +#include <linux/auxiliary_bus.h> > > #include <linux/bitfield.h> > > #include <linux/delay.h> > > #include <linux/err.h> > > #include <linux/math64.h> > > +#include <linux/mc33xs2410.h> > > #include <linux/minmax.h> > > #include <linux/module.h> > > #include <linux/of.h> > > @@ -120,12 +122,19 @@ static int mc33xs2410_read_reg(struct spi_device *spi, u8 reg, u16 *val, u8 flag > > return mc33xs2410_read_regs(spi, ®, flag, val, 1); > > } > > > > -static int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val) > > +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); > > } > > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_ctrl, "PWM_MC33XS2410"); > > To reduce repetition (a bit) you can consider to define > DEFAULT_SYMBOL_NAMESPACE. > Will add it in V5. > > -static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) > > +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val) > > +{ > > + return mc33xs2410_read_reg(spi, reg, val, 0); > > +} > > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_read_reg_diag, "PWM_MC33XS2410"); > > + > > +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val) > > { > > u16 tmp; > > int ret; > > @@ -139,6 +148,7 @@ static int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val > > > > return mc33xs2410_write_reg(spi, reg, tmp); > > } > > +EXPORT_SYMBOL_NS_GPL(mc33xs2410_modify_reg, "PWM_MC33XS2410"); > > > > static u8 mc33xs2410_pwm_get_freq(u64 period) > > { > > @@ -297,6 +307,52 @@ static const struct pwm_ops mc33xs2410_pwm_ops = { > > .get_state = mc33xs2410_pwm_get_state, > > }; > > > > +static void mc33xs2410_adev_release(struct device *dev) > > +{ > > + struct auxiliary_device *adev = to_auxiliary_dev(dev); > > + > > + kfree(adev); > > +} > > + > > +static void mc33xs2410_unregister_adev(void *_adev) > > +{ > > + struct auxiliary_device *adev = _adev; > > + > > + auxiliary_device_delete(adev); > > + auxiliary_device_uninit(adev); > > +} > > This is a copy of auxiliary_device_destroy(). But see below. > Yes, you are right. > > +static int mc33xs2410_hwmon_register(struct device *dev) > > +{ > > + struct auxiliary_device *adev; > > + int ret; > > + > > + adev = kzalloc(sizeof(*adev), GFP_KERNEL); > > + if (!adev) > > + return -ENOMEM; > > + > > + adev->name = "hwmon"; > > + adev->dev.parent = dev; > > + adev->dev.release = mc33xs2410_adev_release; > > + adev->id = 0; > > + > > + ret = auxiliary_device_init(adev); > > + if (ret) > > + return ret; > > + > > + ret = auxiliary_device_add(adev); > > + if (ret) { > > + auxiliary_device_uninit(adev); > > + return ret; > > + } > > + > > + ret = devm_add_action_or_reset(dev, mc33xs2410_unregister_adev, adev); > > + if (ret) > > + return ret; > > + > > + return 0; > > +} > > This function is equivalent to devm_auxiliary_device_create(dev, "hwmon", NULL); > Thanks for finding this, will implement it in V5. > > + > > static int mc33xs2410_reset(struct device *dev) > > { > > struct gpio_desc *reset_gpio; > > @@ -361,6 +417,10 @@ static int mc33xs2410_probe(struct spi_device *spi) > > if (ret < 0) > > return dev_err_probe(dev, ret, "Failed to add pwm chip\n"); > > > > + ret = mc33xs2410_hwmon_register(dev); > > + if (ret < 0) > > + return dev_err_probe(dev, ret, "Failed to register hwmon device\n"); > > + > > return 0; > > } > > > > diff --git a/include/linux/mc33xs2410.h b/include/linux/mc33xs2410.h > > new file mode 100644 > > index 0000000000000000000000000000000000000000..15a0b0b595fe00a369cee45f2d30b2d912b612bb > > --- /dev/null > > +++ b/include/linux/mc33xs2410.h > > @@ -0,0 +1,14 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2024 Liebherr-Electronics and Drives GmbH > > + */ > > +#ifndef _MC33XS2410_H > > +#define _MC33XS2410_H > > + > > +#include <linux/spi/spi.h> > > + > > +int mc33xs2410_read_reg_ctrl(struct spi_device *spi, u8 reg, u16 *val); > > +int mc33xs2410_read_reg_diag(struct spi_device *spi, u8 reg, u16 *val); > > +int mc33xs2410_modify_reg(struct spi_device *spi, u8 reg, u8 mask, u8 val); > > + > > +#endif /* _MC33XS2410_H */ > > I consider it elegant to have the > > MODULE_IMPORT_NS("PWM_MC33XS2410") > > in the header. This is nice because the namespacing is completely > transparant to consumers and all they need it the right #include as if > there was no namespacing at all. > Yes, will implement it as you suggested. Thanks for your input. Best regards, Dimitri Fedrau ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] hwmon: add support for MC33XS2410 hardware monitoring 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay @ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay -1 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau @ 2025-07-08 16:13 UTC (permalink / raw) To: Uwe Kleine-König, Jean Delvare, Guenter Roeck Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau, Dimitri Fedrau The device is able to monitor temperature, voltage and current of each of the four outputs. Add basic support for monitoring the temperature of the four outputs and the die temperature. Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> --- drivers/hwmon/Kconfig | 10 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 079620dd42862ef5e026697e9e1b1fcd5b8be298..9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -700,6 +700,16 @@ config SENSORS_MC13783_ADC help Support for the A/D converter on MC13783 and MC13892 PMIC. +config SENSORS_MC33XS2410 + tristate "MC33XS2410 HWMON support" + depends on PWM_MC33XS2410 + help + If you say yes here you get hardware monitoring support for + MC33XS2410. + + This driver can also be built as a module. If so, the module + will be called mc33xs2410_hwmon. + config SENSORS_FSCHMD tristate "Fujitsu Siemens Computers sensor chips" depends on (X86 || COMPILE_TEST) && I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 48e5866c0c9a7677089d1001a9c5ae4adebff5d5..cd8bc4752b4dbf015c6eb46157626f4e8f87dfae 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -165,6 +165,7 @@ obj-$(CONFIG_SENSORS_MAX31790) += max31790.o obj-$(CONFIG_MAX31827) += max31827.o obj-$(CONFIG_SENSORS_MAX77705) += max77705-hwmon.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_MC33XS2410) += mc33xs2410_hwmon.o obj-$(CONFIG_SENSORS_MC34VR500) += mc34vr500.o obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o obj-$(CONFIG_SENSORS_TC654) += tc654.o diff --git a/drivers/hwmon/mc33xs2410_hwmon.c b/drivers/hwmon/mc33xs2410_hwmon.c new file mode 100644 index 0000000000000000000000000000000000000000..71a14932c6ccaf8d15b045b55093238d924160ea --- /dev/null +++ b/drivers/hwmon/mc33xs2410_hwmon.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2025 Liebherr-Electronics and Drives GmbH + */ + +#include <linux/auxiliary_bus.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/hwmon.h> +#include <linux/mc33xs2410.h> +#include <linux/module.h> + +/* ctrl registers */ + +#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) + +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, ®_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), + ®_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, ®_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 auxiliary_device *adev, + const struct auxiliary_device_id *id) +{ + struct device *dev = &adev->dev; + struct spi_device *spi = container_of(dev->parent, struct spi_device, 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); +} + +static const struct auxiliary_device_id mc33xs2410_hwmon_ids[] = { + { + .name = "pwm_mc33xs2410.hwmon", + }, + { } +}; +MODULE_DEVICE_TABLE(auxiliary, mc33xs2410_hwmon_ids); + +static struct auxiliary_driver mc33xs2410_hwmon_driver = { + .probe = mc33xs2410_hwmon_probe, + .id_table = mc33xs2410_hwmon_ids, +}; +module_auxiliary_driver(mc33xs2410_hwmon_driver); + +MODULE_DESCRIPTION("NXP MC33XS2410 hwmon driver"); +MODULE_AUTHOR("Dimitri Fedrau <dimitri.fedrau@liebherr.com>"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("PWM_MC33XS2410"); -- 2.39.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/2] hwmon: add support for MC33XS2410 hardware monitoring @ 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 0 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau via B4 Relay @ 2025-07-08 16:13 UTC (permalink / raw) To: Uwe Kleine-König, Jean Delvare, Guenter Roeck Cc: linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau, Dimitri Fedrau From: Dimitri Fedrau <dimitri.fedrau@liebherr.com> The device is able to monitor temperature, voltage and current of each of the four outputs. Add basic support for monitoring the temperature of the four outputs and the die temperature. Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> --- drivers/hwmon/Kconfig | 10 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 190 insertions(+) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index 079620dd42862ef5e026697e9e1b1fcd5b8be298..9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -700,6 +700,16 @@ config SENSORS_MC13783_ADC help Support for the A/D converter on MC13783 and MC13892 PMIC. +config SENSORS_MC33XS2410 + tristate "MC33XS2410 HWMON support" + depends on PWM_MC33XS2410 + help + If you say yes here you get hardware monitoring support for + MC33XS2410. + + This driver can also be built as a module. If so, the module + will be called mc33xs2410_hwmon. + config SENSORS_FSCHMD tristate "Fujitsu Siemens Computers sensor chips" depends on (X86 || COMPILE_TEST) && I2C diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile index 48e5866c0c9a7677089d1001a9c5ae4adebff5d5..cd8bc4752b4dbf015c6eb46157626f4e8f87dfae 100644 --- a/drivers/hwmon/Makefile +++ b/drivers/hwmon/Makefile @@ -165,6 +165,7 @@ obj-$(CONFIG_SENSORS_MAX31790) += max31790.o obj-$(CONFIG_MAX31827) += max31827.o obj-$(CONFIG_SENSORS_MAX77705) += max77705-hwmon.o obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o +obj-$(CONFIG_SENSORS_MC33XS2410) += mc33xs2410_hwmon.o obj-$(CONFIG_SENSORS_MC34VR500) += mc34vr500.o obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o obj-$(CONFIG_SENSORS_TC654) += tc654.o diff --git a/drivers/hwmon/mc33xs2410_hwmon.c b/drivers/hwmon/mc33xs2410_hwmon.c new file mode 100644 index 0000000000000000000000000000000000000000..71a14932c6ccaf8d15b045b55093238d924160ea --- /dev/null +++ b/drivers/hwmon/mc33xs2410_hwmon.c @@ -0,0 +1,179 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2025 Liebherr-Electronics and Drives GmbH + */ + +#include <linux/auxiliary_bus.h> +#include <linux/bitfield.h> +#include <linux/bitops.h> +#include <linux/hwmon.h> +#include <linux/mc33xs2410.h> +#include <linux/module.h> + +/* ctrl registers */ + +#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) + +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, ®_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), + ®_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, ®_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 auxiliary_device *adev, + const struct auxiliary_device_id *id) +{ + struct device *dev = &adev->dev; + struct spi_device *spi = container_of(dev->parent, struct spi_device, 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); +} + +static const struct auxiliary_device_id mc33xs2410_hwmon_ids[] = { + { + .name = "pwm_mc33xs2410.hwmon", + }, + { } +}; +MODULE_DEVICE_TABLE(auxiliary, mc33xs2410_hwmon_ids); + +static struct auxiliary_driver mc33xs2410_hwmon_driver = { + .probe = mc33xs2410_hwmon_probe, + .id_table = mc33xs2410_hwmon_ids, +}; +module_auxiliary_driver(mc33xs2410_hwmon_driver); + +MODULE_DESCRIPTION("NXP MC33XS2410 hwmon driver"); +MODULE_AUTHOR("Dimitri Fedrau <dimitri.fedrau@liebherr.com>"); +MODULE_LICENSE("GPL"); +MODULE_IMPORT_NS("PWM_MC33XS2410"); -- 2.39.5 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] hwmon: add support for MC33XS2410 hardware monitoring 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay (?) @ 2025-07-08 16:21 ` Guenter Roeck 2025-07-11 5:45 ` Dimitri Fedrau -1 siblings, 1 reply; 10+ messages in thread From: Guenter Roeck @ 2025-07-08 16:21 UTC (permalink / raw) To: dimitri.fedrau Cc: Uwe Kleine-König, Jean Delvare, linux-pwm, linux-kernel, linux-hwmon, Dimitri Fedrau On Tue, Jul 08, 2025 at 06:13:04PM +0200, Dimitri Fedrau via B4 Relay wrote: > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com> > > The device is able to monitor temperature, voltage and current of each of > the four outputs. Add basic support for monitoring the temperature of the > four outputs and the die temperature. > > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> > --- > drivers/hwmon/Kconfig | 10 +++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++ Code looks good, except the documentation is missing. Please add to Documentation/hwmon/. Thanks, Guenter > 3 files changed, 190 insertions(+) > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 079620dd42862ef5e026697e9e1b1fcd5b8be298..9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -700,6 +700,16 @@ config SENSORS_MC13783_ADC > help > Support for the A/D converter on MC13783 and MC13892 PMIC. > > +config SENSORS_MC33XS2410 > + tristate "MC33XS2410 HWMON support" > + depends on PWM_MC33XS2410 > + help > + If you say yes here you get hardware monitoring support for > + MC33XS2410. > + > + This driver can also be built as a module. If so, the module > + will be called mc33xs2410_hwmon. > + > config SENSORS_FSCHMD > tristate "Fujitsu Siemens Computers sensor chips" > depends on (X86 || COMPILE_TEST) && I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 48e5866c0c9a7677089d1001a9c5ae4adebff5d5..cd8bc4752b4dbf015c6eb46157626f4e8f87dfae 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -165,6 +165,7 @@ obj-$(CONFIG_SENSORS_MAX31790) += max31790.o > obj-$(CONFIG_MAX31827) += max31827.o > obj-$(CONFIG_SENSORS_MAX77705) += max77705-hwmon.o > obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o > +obj-$(CONFIG_SENSORS_MC33XS2410) += mc33xs2410_hwmon.o > obj-$(CONFIG_SENSORS_MC34VR500) += mc34vr500.o > obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o > obj-$(CONFIG_SENSORS_TC654) += tc654.o > diff --git a/drivers/hwmon/mc33xs2410_hwmon.c b/drivers/hwmon/mc33xs2410_hwmon.c > new file mode 100644 > index 0000000000000000000000000000000000000000..71a14932c6ccaf8d15b045b55093238d924160ea > --- /dev/null > +++ b/drivers/hwmon/mc33xs2410_hwmon.c > @@ -0,0 +1,179 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2025 Liebherr-Electronics and Drives GmbH > + */ > + > +#include <linux/auxiliary_bus.h> > +#include <linux/bitfield.h> > +#include <linux/bitops.h> > +#include <linux/hwmon.h> > +#include <linux/mc33xs2410.h> > +#include <linux/module.h> > + > +/* ctrl registers */ > + > +#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) > + > +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, ®_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), > + ®_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, ®_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 auxiliary_device *adev, > + const struct auxiliary_device_id *id) > +{ > + struct device *dev = &adev->dev; > + struct spi_device *spi = container_of(dev->parent, struct spi_device, 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); > +} > + > +static const struct auxiliary_device_id mc33xs2410_hwmon_ids[] = { > + { > + .name = "pwm_mc33xs2410.hwmon", > + }, > + { } > +}; > +MODULE_DEVICE_TABLE(auxiliary, mc33xs2410_hwmon_ids); > + > +static struct auxiliary_driver mc33xs2410_hwmon_driver = { > + .probe = mc33xs2410_hwmon_probe, > + .id_table = mc33xs2410_hwmon_ids, > +}; > +module_auxiliary_driver(mc33xs2410_hwmon_driver); > + > +MODULE_DESCRIPTION("NXP MC33XS2410 hwmon driver"); > +MODULE_AUTHOR("Dimitri Fedrau <dimitri.fedrau@liebherr.com>"); > +MODULE_LICENSE("GPL"); > +MODULE_IMPORT_NS("PWM_MC33XS2410"); > > -- > 2.39.5 > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 2/2] hwmon: add support for MC33XS2410 hardware monitoring 2025-07-08 16:21 ` Guenter Roeck @ 2025-07-11 5:45 ` Dimitri Fedrau 0 siblings, 0 replies; 10+ messages in thread From: Dimitri Fedrau @ 2025-07-11 5:45 UTC (permalink / raw) To: Guenter Roeck Cc: dimitri.fedrau, Uwe Kleine-König, Jean Delvare, linux-pwm, linux-kernel, linux-hwmon Hi Guenter, thanks for the quicke review, will add the documentation in v5. Best regards, Dimitri Fedrau Am Tue, Jul 08, 2025 at 09:21:19AM -0700 schrieb Guenter Roeck: > On Tue, Jul 08, 2025 at 06:13:04PM +0200, Dimitri Fedrau via B4 Relay wrote: > > From: Dimitri Fedrau <dimitri.fedrau@liebherr.com> > > > > The device is able to monitor temperature, voltage and current of each of > > the four outputs. Add basic support for monitoring the temperature of the > > four outputs and the die temperature. > > > > Signed-off-by: Dimitri Fedrau <dimitri.fedrau@liebherr.com> > > --- > > drivers/hwmon/Kconfig | 10 +++ > > drivers/hwmon/Makefile | 1 + > > drivers/hwmon/mc33xs2410_hwmon.c | 179 +++++++++++++++++++++++++++++++++++++++ > > Code looks good, except the documentation is missing. > Please add to Documentation/hwmon/. > > Thanks, > Guenter > > > 3 files changed, 190 insertions(+) > > > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > > index 079620dd42862ef5e026697e9e1b1fcd5b8be298..9d28fcf7cd2a6f9e2f54694a717bd85ff4047b46 100644 > > --- a/drivers/hwmon/Kconfig > > +++ b/drivers/hwmon/Kconfig > > @@ -700,6 +700,16 @@ config SENSORS_MC13783_ADC > > help > > Support for the A/D converter on MC13783 and MC13892 PMIC. > > > > +config SENSORS_MC33XS2410 > > + tristate "MC33XS2410 HWMON support" > > + depends on PWM_MC33XS2410 > > + help > > + If you say yes here you get hardware monitoring support for > > + MC33XS2410. > > + > > + This driver can also be built as a module. If so, the module > > + will be called mc33xs2410_hwmon. > > + > > config SENSORS_FSCHMD > > tristate "Fujitsu Siemens Computers sensor chips" > > depends on (X86 || COMPILE_TEST) && I2C > > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > > index 48e5866c0c9a7677089d1001a9c5ae4adebff5d5..cd8bc4752b4dbf015c6eb46157626f4e8f87dfae 100644 > > --- a/drivers/hwmon/Makefile > > +++ b/drivers/hwmon/Makefile > > @@ -165,6 +165,7 @@ obj-$(CONFIG_SENSORS_MAX31790) += max31790.o > > obj-$(CONFIG_MAX31827) += max31827.o > > obj-$(CONFIG_SENSORS_MAX77705) += max77705-hwmon.o > > obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o > > +obj-$(CONFIG_SENSORS_MC33XS2410) += mc33xs2410_hwmon.o > > obj-$(CONFIG_SENSORS_MC34VR500) += mc34vr500.o > > obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o > > obj-$(CONFIG_SENSORS_TC654) += tc654.o > > diff --git a/drivers/hwmon/mc33xs2410_hwmon.c b/drivers/hwmon/mc33xs2410_hwmon.c > > new file mode 100644 > > index 0000000000000000000000000000000000000000..71a14932c6ccaf8d15b045b55093238d924160ea > > --- /dev/null > > +++ b/drivers/hwmon/mc33xs2410_hwmon.c > > @@ -0,0 +1,179 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2025 Liebherr-Electronics and Drives GmbH > > + */ > > + > > +#include <linux/auxiliary_bus.h> > > +#include <linux/bitfield.h> > > +#include <linux/bitops.h> > > +#include <linux/hwmon.h> > > +#include <linux/mc33xs2410.h> > > +#include <linux/module.h> > > + > > +/* ctrl registers */ > > + > > +#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) > > + > > +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, ®_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), > > + ®_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, ®_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 auxiliary_device *adev, > > + const struct auxiliary_device_id *id) > > +{ > > + struct device *dev = &adev->dev; > > + struct spi_device *spi = container_of(dev->parent, struct spi_device, 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); > > +} > > + > > +static const struct auxiliary_device_id mc33xs2410_hwmon_ids[] = { > > + { > > + .name = "pwm_mc33xs2410.hwmon", > > + }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(auxiliary, mc33xs2410_hwmon_ids); > > + > > +static struct auxiliary_driver mc33xs2410_hwmon_driver = { > > + .probe = mc33xs2410_hwmon_probe, > > + .id_table = mc33xs2410_hwmon_ids, > > +}; > > +module_auxiliary_driver(mc33xs2410_hwmon_driver); > > + > > +MODULE_DESCRIPTION("NXP MC33XS2410 hwmon driver"); > > +MODULE_AUTHOR("Dimitri Fedrau <dimitri.fedrau@liebherr.com>"); > > +MODULE_LICENSE("GPL"); > > +MODULE_IMPORT_NS("PWM_MC33XS2410"); > > > > -- > > 2.39.5 > > > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-07-16 7:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-07-08 16:13 [PATCH v4 0/2] hwmon: add support for MC33XS2410 hardware Dimitri Fedrau 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 2025-07-08 16:13 ` [PATCH v4 1/2] pwm: mc33xs2410: add hwmon support Dimitri Fedrau 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 2025-07-16 6:39 ` Uwe Kleine-König 2025-07-16 7:06 ` Dimitri Fedrau 2025-07-08 16:13 ` [PATCH v4 2/2] hwmon: add support for MC33XS2410 hardware monitoring Dimitri Fedrau 2025-07-08 16:13 ` Dimitri Fedrau via B4 Relay 2025-07-08 16:21 ` Guenter Roeck 2025-07-11 5:45 ` Dimitri Fedrau
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.