[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230907052639.16491-4-W_Armin@gmx.de>
Date: Thu, 7 Sep 2023 07:26:37 +0200
From: Armin Wolf <W_Armin@....de>
To: hdegoede@...hat.com
Cc: jdelvare@...e.com, linux@...ck-us.net, linux-hwmon@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: [PATCH v2 3/5] 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@....de>
---
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
Powered by blists - more mailing lists