On 3/17/20 1:50 PM, Michael Walle wrote: > This adds support for the hardware monitoring controller of the sl28cpld > board management controller. This driver is part of a multi-function > device. > > Signed-off-by: Michael Walle <michael@xxxxxxxx> > --- > drivers/hwmon/Kconfig | 10 +++ > drivers/hwmon/Makefile | 1 + > drivers/hwmon/sl28cpld-hwmon.c | 146 +++++++++++++++++++++++++++++++++ Please also provide Documentation/hwmon/sl28cpld. > 3 files changed, 157 insertions(+) > create mode 100644 drivers/hwmon/sl28cpld-hwmon.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 05a30832c6ba..c98716f78cfa 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -1412,6 +1412,16 @@ config SENSORS_RASPBERRYPI_HWMON > This driver can also be built as a module. If so, the module > will be called raspberrypi-hwmon. > > +config SENSORS_SL28CPLD > + tristate "Kontron's SMARC-sAL28 hardware monitoring driver" > + depends on MFD_SL28CPLD > + help > + If you say yes here you get support for a fan connected to the > + input of the SMARC connector of Kontron's SMARC-sAL28 module. > + > + This driver can also be built as a module. If so, the module > + will be called sl28cpld-hwmon. > + > config SENSORS_SHT15 > tristate "Sensiron humidity and temperature sensors. SHT15 and compat." > depends on GPIOLIB || COMPILE_TEST > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index b0b9c8e57176..dfb0f8cda2dd 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -155,6 +155,7 @@ obj-$(CONFIG_SENSORS_S3C) += s3c-hwmon.o > obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o > obj-$(CONFIG_SENSORS_SCH5627) += sch5627.o > obj-$(CONFIG_SENSORS_SCH5636) += sch5636.o > +obj-$(CONFIG_SENSORS_SL28CPLD) += sl28cpld-hwmon.o > obj-$(CONFIG_SENSORS_SHT15) += sht15.o > obj-$(CONFIG_SENSORS_SHT21) += sht21.o > obj-$(CONFIG_SENSORS_SHT3x) += sht3x.o > diff --git a/drivers/hwmon/sl28cpld-hwmon.c b/drivers/hwmon/sl28cpld-hwmon.c > new file mode 100644 > index 000000000000..7ac42bb0a48c > --- /dev/null > +++ b/drivers/hwmon/sl28cpld-hwmon.c > @@ -0,0 +1,146 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * SMARC-sAL28 fan hardware monitoring driver. > + * > + * Copyright 2019 Kontron Europe GmbH > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/bitfield.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_address.h> > +#include <linux/regmap.h> > +#include <linux/platform_device.h> > +#include <linux/hwmon.h> > + Alphabetic order of include files, please. > +#define FAN_INPUT 0 > +#define FAN_SCALE_X8 BIT(7) > +#define FAN_VALUE_MASK GENMASK(6, 0) > + > +struct sl28cpld_hwmon { > + struct regmap *regmap; > + u32 offset; > +}; > + > +static umode_t sl28cpld_hwmon_is_visible(const void *data, > + enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + return 0444; > +} > + > +static int sl28cpld_hwmon_read(struct device *dev, > + enum hwmon_sensor_types type, u32 attr, > + int channel, long *input) > +{ > + struct sl28cpld_hwmon *hwmon = dev_get_drvdata(dev); > + unsigned int value; > + int ret; > + > + switch (attr) { > + case hwmon_fan_input: > + ret = regmap_read(hwmon->regmap, hwmon->offset + FAN_INPUT, > + &value); > + if (ret) > + return ret; > + /* > + * The register has a 7 bit value and 1 bit which indicates the > + * scale. If the MSB is set, then the lower 7 bit has to be > + * multiplied by 8, to get the correct reading. > + */ > + if (value & FAN_SCALE_X8) > + value = FIELD_GET(FAN_VALUE_MASK, value) << 3; > + > + /* > + * The counter period is 1000ms and the sysfs specification > + * says we should asssume 2 pulses per revolution. > + */ > + value *= 60 / 2; > + > + break; > + default: > + return -EOPNOTSUPP; > + } > + > + *input = value; > + return 0; > +} > + > +static const u32 sl28cpld_hwmon_fan_config[] = { > + HWMON_F_INPUT, > + 0 > +}; > + > +static const struct hwmon_channel_info sl28cpld_hwmon_fan = { > + .type = hwmon_fan, > + .config = sl28cpld_hwmon_fan_config, > +}; > + > +static const struct hwmon_channel_info *sl28cpld_hwmon_info[] = { > + &sl28cpld_hwmon_fan, > + NULL > +}; > + > +static const struct hwmon_ops sl28cpld_hwmon_ops = { > + .is_visible = sl28cpld_hwmon_is_visible, > + .read = sl28cpld_hwmon_read, > +}; > + > +static const struct hwmon_chip_info sl28cpld_hwmon_chip_info = { > + .ops = &sl28cpld_hwmon_ops, > + .info = sl28cpld_hwmon_info, > +}; > + > +static int sl28cpld_hwmon_probe(struct platform_device *pdev) > +{ > + struct device *hwmon_dev; > + struct sl28cpld_hwmon *hwmon; > + struct resource *res; > + > + hwmon = devm_kzalloc(&pdev->dev, sizeof(*hwmon), GFP_KERNEL); > + if (!hwmon) > + return -ENOMEM; > + > + if (!pdev->dev.parent) > + return -ENODEV; > + Maybe do this first ? > + hwmon->regmap = dev_get_regmap(pdev->dev.parent, NULL); > + if (!hwmon->regmap) > + return -ENODEV; > + > + res = platform_get_resource(pdev, IORESOURCE_REG, 0); > + if (!res) > + return -EINVAL; > + hwmon->offset = res->start; > + > + hwmon_dev = devm_hwmon_device_register_with_info(&pdev->dev, > + "sl28cpld_hwmon", > + hwmon, > + &sl28cpld_hwmon_chip_info, > + NULL); > + if (IS_ERR(hwmon_dev)) { > + dev_err(&pdev->dev, "failed to register as hwmon device"); > + return PTR_ERR(hwmon_dev); > + } > + > + return 0; > +} > + > +static const struct platform_device_id sl28cpld_hwmon_id_table[] = { > + {"sl28cpld-fan", 0}, > +}; > +MODULE_DEVICE_TABLE(platform, sl28cpld_hwmon_id_table); > + > +static struct platform_driver sl28cpld_hwmon_driver = { > + .probe = sl28cpld_hwmon_probe, > + .id_table = sl28cpld_hwmon_id_table, I'd have expected an of_match_table. > + .driver = { > + .name = "sl28cpld-hwmon", > + }, > +}; > +module_platform_driver(sl28cpld_hwmon_driver); > + > +MODULE_DESCRIPTION("sl28cpld Hardware Monitoring Driver"); > +MODULE_LICENSE("GPL"); >