hwmon: (sch5627) Use regmap for pwm map register caching

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@gmx.de>
Link: https://lore.kernel.org/r/20230907052639.16491-4-W_Armin@gmx.de
Signed-off-by: Guenter Roeck <linux@roeck-us.net>

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 12af9f9cfd9f58d8d07d8b1aea5021c5ba8c3bf5..ea390da7bc751a2a2c313b31f28127d99fd201f5 100644 (file)
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1919,6 +1919,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 bf408e35e2c3295be4f0a653b8b7cf382d4ef5b2..85f8352cb3cf468a08db184e99dad2d637eafefe 100644 (file)
--- 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 ac1f725807155166b9918367c4b37389e252cf5b..ebdeb3a20b87dde66d462b863ce81f791ef29021 100644 (file)
--- a/drivers/hwmon/sch56xx-common.c
+++ b/drivers/hwmon/sch56xx-common.c
@@ -9,6 +9,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/platform_device.h>
+#include <linux/regmap.h>
 #include <linux/err.h>
 #include <linux/io.h>
 #include <linux/acpi.h>
@@ -59,6 +60,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 */
@@ -238,6 +244,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 e907d9da0dd56dab9e59fa53fe37b1f8415827e9..3fb1cddbf9774efe9bb65ed358339fee7e0e0180 100644 (file)
--- 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);