Accessing virtual registers is very inefficient, so pwm map values should be cached when possible, else userspace could effectively do a DOS attack by reading pwm map values in a while loop. Use the regmap cache to cache those values. Tested on a Fujitsu Esprimo P720. Signed-off-by: Armin Wolf <W_Armin@xxxxxx> --- drivers/hwmon/Kconfig | 1 + drivers/hwmon/sch5627.c | 72 +++++++++++++++++++++++++------- drivers/hwmon/sch56xx-common.c | 76 ++++++++++++++++++++++++++++++++++ drivers/hwmon/sch56xx-common.h | 3 ++ 4 files changed, 138 insertions(+), 14 deletions(-) diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig index ec38c8892158..1c672195b5b3 100644 --- a/drivers/hwmon/Kconfig +++ b/drivers/hwmon/Kconfig @@ -1909,6 +1909,7 @@ config SENSORS_SMSC47B397 config SENSORS_SCH56XX_COMMON tristate + select REGMAP config SENSORS_SCH5627 tristate "SMSC SCH5627" diff --git a/drivers/hwmon/sch5627.c b/drivers/hwmon/sch5627.c index bf408e35e2c3..85f8352cb3cf 100644 --- a/drivers/hwmon/sch5627.c +++ b/drivers/hwmon/sch5627.c @@ -9,7 +9,9 @@ #include <linux/bits.h> #include <linux/module.h> #include <linux/mod_devicetable.h> +#include <linux/pm.h> #include <linux/init.h> +#include <linux/regmap.h> #include <linux/slab.h> #include <linux/jiffies.h> #include <linux/platform_device.h> @@ -72,6 +74,7 @@ static const char * const SCH5627_IN_LABELS[SCH5627_NO_IN] = { "VCC", "VTT", "VBAT", "VTR", "V_IN" }; struct sch5627_data { + struct regmap *regmap; unsigned short addr; u8 control; u8 temp_max[SCH5627_NO_TEMPS]; @@ -91,6 +94,26 @@ struct sch5627_data { u16 in[SCH5627_NO_IN]; }; +static const struct regmap_range sch5627_tunables_ranges[] = { + regmap_reg_range(0xA0, 0xA3), +}; + +static const struct regmap_access_table sch5627_tunables_table = { + .yes_ranges = sch5627_tunables_ranges, + .n_yes_ranges = ARRAY_SIZE(sch5627_tunables_ranges), +}; + +static const struct regmap_config sch5627_regmap_config = { + .reg_bits = 16, + .val_bits = 8, + .wr_table = &sch5627_tunables_table, + .rd_table = &sch5627_tunables_table, + .cache_type = REGCACHE_RBTREE, + .use_single_read = true, + .use_single_write = true, + .can_sleep = true, +}; + static int sch5627_update_temp(struct sch5627_data *data) { int ret = 0; @@ -250,7 +273,7 @@ static int sch5627_read(struct device *dev, enum hwmon_sensor_types type, u32 at long *val) { struct sch5627_data *data = dev_get_drvdata(dev); - int ret; + int ret, value; switch (type) { case hwmon_temp: @@ -301,15 +324,11 @@ static int sch5627_read(struct device *dev, enum hwmon_sensor_types type, u32 at case hwmon_pwm: switch (attr) { case hwmon_pwm_auto_channels_temp: - mutex_lock(&data->update_lock); - ret = sch56xx_read_virtual_reg(data->addr, SCH5627_REG_PWM_MAP[channel]); - mutex_unlock(&data->update_lock); - + ret = regmap_read(data->regmap, SCH5627_REG_PWM_MAP[channel], &value); if (ret < 0) return ret; - *val = ret; - + *val = value; return 0; default: break; @@ -359,7 +378,6 @@ static int sch5627_write(struct device *dev, enum hwmon_sensor_types type, u32 a long val) { struct sch5627_data *data = dev_get_drvdata(dev); - int ret; switch (type) { case hwmon_pwm: @@ -369,12 +387,7 @@ static int sch5627_write(struct device *dev, enum hwmon_sensor_types type, u32 a if (val > U8_MAX || val < 0) return -EINVAL; - mutex_lock(&data->update_lock); - ret = sch56xx_write_virtual_reg(data->addr, SCH5627_REG_PWM_MAP[channel], - val); - mutex_unlock(&data->update_lock); - - return ret; + return regmap_write(data->regmap, SCH5627_REG_PWM_MAP[channel], val); default: break; } @@ -501,6 +514,12 @@ static int sch5627_probe(struct platform_device *pdev) pr_err("hardware monitoring not enabled\n"); return -ENODEV; } + + data->regmap = devm_regmap_init_sch56xx(&pdev->dev, &data->update_lock, data->addr, + &sch5627_regmap_config); + if (IS_ERR(data->regmap)) + return PTR_ERR(data->regmap); + /* Trigger a Vbat voltage measurement, so that we get a valid reading the first time we read Vbat */ sch56xx_write_virtual_reg(data->addr, SCH5627_REG_CTRL, data->control | SCH5627_CTRL_VBAT); @@ -531,6 +550,30 @@ static int sch5627_probe(struct platform_device *pdev) return 0; } +static int sch5627_suspend(struct device *dev) +{ + struct sch5627_data *data = dev_get_drvdata(dev); + + regcache_cache_only(data->regmap, true); + regcache_mark_dirty(data->regmap); + + return 0; +} + +static int sch5627_resume(struct device *dev) +{ + struct sch5627_data *data = dev_get_drvdata(dev); + + regcache_cache_only(data->regmap, false); + /* We must not access the virtual registers when the lock bit is set */ + if (data->control & SCH5627_CTRL_LOCK) + return regcache_drop_region(data->regmap, 0, U16_MAX); + + return regcache_sync(data->regmap); +} + +static DEFINE_SIMPLE_DEV_PM_OPS(sch5627_dev_pm_ops, sch5627_suspend, sch5627_resume); + static const struct platform_device_id sch5627_device_id[] = { { .name = "sch5627", @@ -542,6 +585,7 @@ MODULE_DEVICE_TABLE(platform, sch5627_device_id); static struct platform_driver sch5627_driver = { .driver = { .name = DRVNAME, + .pm = pm_sleep_ptr(&sch5627_dev_pm_ops), }, .probe = sch5627_probe, .id_table = sch5627_device_id, diff --git a/drivers/hwmon/sch56xx-common.c b/drivers/hwmon/sch56xx-common.c index de3a0886c2f7..a3abafdaa3e6 100644 --- a/drivers/hwmon/sch56xx-common.c +++ b/drivers/hwmon/sch56xx-common.c @@ -10,6 +10,7 @@ #include <linux/mod_devicetable.h> #include <linux/init.h> #include <linux/platform_device.h> +#include <linux/regmap.h> #include <linux/dmi.h> #include <linux/err.h> #include <linux/io.h> @@ -64,6 +65,11 @@ struct sch56xx_watchdog_data { u8 watchdog_output_enable; }; +struct sch56xx_bus_context { + struct mutex *lock; /* Used to serialize access to the mailbox registers */ + u16 addr; +}; + static struct platform_device *sch56xx_pdev; /* Super I/O functions */ @@ -243,6 +249,76 @@ int sch56xx_read_virtual_reg12(u16 addr, u16 msb_reg, u16 lsn_reg, } EXPORT_SYMBOL(sch56xx_read_virtual_reg12); +/* + * Regmap support + */ + +static int sch56xx_reg_write(void *context, unsigned int reg, unsigned int val) +{ + struct sch56xx_bus_context *bus = context; + int ret; + + mutex_lock(bus->lock); + ret = sch56xx_write_virtual_reg(bus->addr, (u16)reg, (u8)val); + mutex_unlock(bus->lock); + + return ret; +} + +static int sch56xx_reg_read(void *context, unsigned int reg, unsigned int *val) +{ + struct sch56xx_bus_context *bus = context; + int ret; + + mutex_lock(bus->lock); + ret = sch56xx_read_virtual_reg(bus->addr, (u16)reg); + mutex_unlock(bus->lock); + + if (ret < 0) + return ret; + + *val = ret; + + return 0; +} + +static void sch56xx_free_context(void *context) +{ + kfree(context); +} + +static const struct regmap_bus sch56xx_bus = { + .reg_write = sch56xx_reg_write, + .reg_read = sch56xx_reg_read, + .free_context = sch56xx_free_context, + .reg_format_endian_default = REGMAP_ENDIAN_LITTLE, + .val_format_endian_default = REGMAP_ENDIAN_LITTLE, +}; + +struct regmap *devm_regmap_init_sch56xx(struct device *dev, struct mutex *lock, u16 addr, + const struct regmap_config *config) +{ + struct sch56xx_bus_context *context; + struct regmap *map; + + if (config->reg_bits != 16 && config->val_bits != 8) + return ERR_PTR(-EOPNOTSUPP); + + context = kzalloc(sizeof(*context), GFP_KERNEL); + if (!context) + return ERR_PTR(-ENOMEM); + + context->lock = lock; + context->addr = addr; + + map = devm_regmap_init(dev, &sch56xx_bus, context, config); + if (IS_ERR(map)) + kfree(context); + + return map; +} +EXPORT_SYMBOL(devm_regmap_init_sch56xx); + /* * Watchdog routines */ diff --git a/drivers/hwmon/sch56xx-common.h b/drivers/hwmon/sch56xx-common.h index e907d9da0dd5..3fb1cddbf977 100644 --- a/drivers/hwmon/sch56xx-common.h +++ b/drivers/hwmon/sch56xx-common.h @@ -5,9 +5,12 @@ ***************************************************************************/ #include <linux/mutex.h> +#include <linux/regmap.h> struct sch56xx_watchdog_data; +struct regmap *devm_regmap_init_sch56xx(struct device *dev, struct mutex *lock, u16 addr, + const struct regmap_config *config); int sch56xx_read_virtual_reg(u16 addr, u16 reg); int sch56xx_write_virtual_reg(u16 addr, u16 reg, u8 val); int sch56xx_read_virtual_reg16(u16 addr, u16 reg); -- 2.39.2