add max31732 driver #2959
add max31732 driver #2959
Conversation
Add support for Analog Devices MAX31732, a five-channel temperature monitor with per-channel alarms, calibration helpers, optional IRQ handling, and MTP configuration. Signed-off-by: Sinan Divarci <sinan.divarci@analog.com>
Add documentation for the Analog Devices MAX31732 temperature sensor driver under Documentation/hwmon. Signed-off-by: Sinan Divarci <sinan.divarci@analog.com>
Add device tree bindings for the Analog Devices MAX31732 temperature sensor driver under Documentation/devicetree/bindings/hwmon/. Signed-off-by: Sinan Divarci <sinan.divarci@analog.com>
sdivarci
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
|
|
||
| * All temperatures reported via sysfs are in milli-degree Celsius. | ||
| * ``stop`` halts conversions but does not power-cycle the part; use ``soft_por`` | ||
| to restore all the registers to their default values. |
There was a problem hiding this comment.
this patch can typically be squashed with the one adding the driver
| static const struct attribute_group mtp_attr_group = { | ||
| .name = "mtp", | ||
| .attrs = mtp_attrs, | ||
| }; |
There was a problem hiding this comment.
The above needs to be re-thought. The above is not the way to expose the hwmon interface. The below is what you need to be using (an example):
https://elixir.bootlin.com/linux/v6.17/source/drivers/hwmon/ltc4282.c#L1563
Then you need to look at the standard hwmon ABI:
https://www.kernel.org/doc/Documentation/hwmon/sysfs-interface
and try to map as much as you can. The reason is that you'll have to justify very carefully why you need some non standard interface to be exposed to userspace. Often we don't really need to expose everything.
| s32 ret; | ||
| u32 alarm_que; | ||
|
|
||
| if (fwnode_property_read_bool(dev_fwnode(dev), "adi,alarm1-interrupt-mode")) |
There was a problem hiding this comment.
just as an hint for future versions of the patches you have device_property_read_bool(dev, ...). So no need for all of those dev_fwnode()
|
|
||
| ret = devm_device_add_group(dev, &mtp_attr_group); | ||
| if (ret) | ||
| return dev_err_probe(dev, ret, "failed to create mtp sysfs group\n"); |
There was a problem hiding this comment.
not how you should be adding attr groups. Needs to be done through the hwmon API (when registering the device)
| if (reg_val == 0) | ||
| ret = regmap_set_bits(data->regmap, MAX31732_REG_CONF1, MAX31732_CONF1_STOP); | ||
| else | ||
| ret = regmap_clear_bits(data->regmap, MAX31732_REG_CONF1, MAX31732_CONF1_STOP); |
There was a problem hiding this comment.
for the above you can use regmap_update_bits()
| }; | ||
|
|
||
| MODULE_DEVICE_TABLE(i2c, max31732_ids); | ||
|
|
| { .compatible = "adi,max31732", }, | ||
| { }, | ||
| }; | ||
|
|
| .class = I2C_CLASS_HWMON, | ||
| .driver = { | ||
| .name = "max31732-driver", | ||
| .of_match_table = of_match_ptr(max31732_of_match), |
| .driver = { | ||
| .name = "max31732-driver", | ||
| .of_match_table = of_match_ptr(max31732_of_match), | ||
| .pm = &max31732_pm_ops, |
There was a problem hiding this comment.
use pm_sleep_ptr() together with DEFINE_SIMPLE_DEV_PM_OPS() and then no need of __maybe_unused
| MODULE_DESCRIPTION("MAX31732 driver"); | ||
| MODULE_LICENSE("GPL"); | ||
| MODULE_VERSION("1.0"); | ||
| MODULE_SOFTDEP("pre: regmap_i2c"); |
There was a problem hiding this comment.
typically we dont really use the above two but I guess it does not harm
PR Description
Add MAX31732 hwmod driver and documentation.
PR Type
PR Checklist