| Message ID | 20200317205017.28280-15-michael@walle.cc (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add support for Kontron sl28cpld | expand |
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@walle.cc> > --- > 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"); >
Hi Guenter, thanks for the review Am 2020-03-18 04:27, schrieb Guenter Roeck: > 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@walle.cc> >> --- >> 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. ok, I guess that applies to all of the new files. >> +#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 ? ok. like explained on another patch review comment, I had that together with the following dev_get_regmap() which uses this. But I was already not sure while writing the code whether I should do it first and don't have to undo the devm_ in case of an error. Well since already two reviewers stumbled on this, I'll do it first on all the patches. >> + 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. There was one, but, since I use mfd_add_devices() which uses the platform driver name (or the id_table) I removed it. While it doesn't really matter here in the hwmon part for now, the GPIO driver uses the id_table data to distinguish between different flavors of the GPIO controller. So there I'd have to duplicate data in the tables (eg. the id_table as well as the of_match_table) and get that data in the _probe(), although only the id_table data is used. -michael > >> + .driver = { >> + .name = "sl28cpld-hwmon", >> + }, >> +}; >> +module_platform_driver(sl28cpld_hwmon_driver); >> + >> +MODULE_DESCRIPTION("sl28cpld Hardware Monitoring Driver"); >> +MODULE_LICENSE("GPL"); >>
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> + +#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; + + 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, + .driver = { + .name = "sl28cpld-hwmon", + }, +}; +module_platform_driver(sl28cpld_hwmon_driver); + +MODULE_DESCRIPTION("sl28cpld Hardware Monitoring Driver"); +MODULE_LICENSE("GPL");
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@walle.cc> --- drivers/hwmon/Kconfig | 10 +++ drivers/hwmon/Makefile | 1 + drivers/hwmon/sl28cpld-hwmon.c | 146 +++++++++++++++++++++++++++++++++ 3 files changed, 157 insertions(+) create mode 100644 drivers/hwmon/sl28cpld-hwmon.c