[PATCH v2 3/5] hwmon: (sch5627) Use regmap for pwm map register caching

[Armin Wolf]

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