[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <95e41f16-4b5f-4f2a-bc31-17273032312b@roeck-us.net>
Date: Tue, 31 Oct 2023 09:01:16 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Antoniu Miclaus <antoniu.miclaus@...log.com>,
Alessandro Zummo <a.zummo@...ertech.it>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Rob Herring <robh+dt@...nel.org>,
Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
Conor Dooley <conor+dt@...nel.org>,
Jean Delvare <jdelvare@...e.com>, linux-rtc@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-hwmon@...r.kernel.org
Subject: Re: [PATCH v3 2/2] rtc: max31335: add driver support
On 10/31/23 08:30, Antoniu Miclaus wrote:
> RTC driver for MAX31335 ±2ppm Automotive Real-Time Clock with
> Integrated MEMS Resonator.
>
> Signed-off-by: Antoniu Miclaus <antoniu.miclaus@...log.com>
> ---
> changes in v3:
> - drop MAX31335_STATUS1 register check inside probe function.
> drivers/rtc/Kconfig | 11 +
> drivers/rtc/Makefile | 1 +
> drivers/rtc/rtc-max31335.c | 765 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 777 insertions(+)
> create mode 100644 drivers/rtc/rtc-max31335.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index d7502433c78a..11c7d7fe1e85 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -373,6 +373,17 @@ config RTC_DRV_MAX8997
> This driver can also be built as a module. If so, the module
> will be called rtc-max8997.
>
> +config RTC_DRV_MAX31335
> + tristate "Analog Devices MAX31335"
> + depends on I2C
> + select REGMAP_I2C
> + help
> + If you say yes here you get support for the Analog Devices
> + MAX31335.
> +
> + This driver can also be built as a module. If so, the module
> + will be called rtc-max31335.
> +
Just out of curiosity, is this an unreleased chip ? I only find
MAX311331 and MAX31334 on the Analog website, but the register
map for those is different, and they don't support reporting
the chip temperature.
[ ... ]
> +
> +static const struct hwmon_channel_info *max31335_info[] = {
> + HWMON_CHANNEL_INFO(temp, HWMON_T_INPUT),
According to the register map above, the chip does support
low and high temperature limits as well as over- and undertemperature
alarms and interrupts. I would suggest to add support for all of those.
You might also consider adding support for temperature alarm interrupts
and report temperature alarm events by calling hwmon_notify_event()
if a thermal event occurs.
[ ... ]
> +
> + hwmon = devm_hwmon_device_register_with_info(&client->dev, client->name,
> + max31335,
> + &max31335_chip_info,
> + NULL);
There is no "depends on HWMON" in the Kconfig entry, meaning this will fail
to compile if HWMON=n or if HWMON=m and RTC_DRV_MAX31335=y.
Guenter
Powered by blists - more mailing lists