| Message ID | 20240604040237.1064024-5-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: Add support for SPD5118 compliant chips | expand |
Am 04.06.24 um 06:02 schrieb Guenter Roeck: > Add support for reading SPD NVMEM data from SPD5118 (Jedec JESD300) > compliant memory modules. NVMEM write operation is not supported. > > NVMEM support is optional. If CONFIG_NVMEM is disabled, the driver will > still instantiate but not provide NVMEM attribute files. > > Signed-off-by: Guenter Roeck <linux@roeck-us.net> > --- > v4: Use NVMEM_DEVID_NONE instead of NVMEM_DEVID_AUTO > Ignore nvmem registration failure if nvmem support is disabled > > v3: New patch > > Documentation/hwmon/spd5118.rst | 8 ++ > drivers/hwmon/spd5118.c | 147 +++++++++++++++++++++++++++++++- > 2 files changed, 151 insertions(+), 4 deletions(-) > > diff --git a/Documentation/hwmon/spd5118.rst b/Documentation/hwmon/spd5118.rst > index a15d75aa2066..ef7338f46575 100644 > --- a/Documentation/hwmon/spd5118.rst > +++ b/Documentation/hwmon/spd5118.rst > @@ -53,3 +53,11 @@ temp1_crit_alarm Temperature critical alarm > > Alarm attributes are sticky until read and will be cleared afterwards > unless the alarm condition still applies. > + > + > +SPD (Serial Presence Detect) support > +------------------------------------ > + > +The driver also supports reading the SPD NVRAM on SPD5118 compatible chips. > +SPD data is available from the 'eeprom' binary attribute file attached to the > +chip's I2C device. > diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c > index d55c073ff5fd..5cb5e52c0a38 100644 > --- a/drivers/hwmon/spd5118.c > +++ b/drivers/hwmon/spd5118.c > @@ -20,6 +20,8 @@ > #include <linux/i2c.h> > #include <linux/hwmon.h> > #include <linux/module.h> > +#include <linux/mutex.h> > +#include <linux/nvmem-provider.h> > #include <linux/pm.h> > #include <linux/regmap.h> > #include <linux/units.h> > @@ -53,12 +55,31 @@ static const unsigned short normal_i2c[] = { > > #define SPD5118_TS_DISABLE BIT(0) /* temperature sensor disable */ > > +#define SPD5118_LEGACY_MODE_ADDR BIT(3) > +#define SPD5118_LEGACY_PAGE_MASK GENMASK(2, 0) > +#define SPD5118_LEGACY_MODE_MASK (SPD5118_LEGACY_MODE_ADDR | SPD5118_LEGACY_PAGE_MASK) > + > + > +#define SPD5118_NUM_PAGES 8 > +#define SPD5118_PAGE_SIZE 128 > +#define SPD5118_PAGE_SHIFT 7 > +#define SPD5118_PAGE_MASK GENMASK(6, 0) > +#define SPD5118_EEPROM_BASE 0x80 > +#define SPD5118_EEPROM_SIZE (SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES) > + > /* Temperature unit in millicelsius */ > #define SPD5118_TEMP_UNIT (MILLIDEGREE_PER_DEGREE / 4) > /* Representable temperature range in millicelsius */ > #define SPD5118_TEMP_RANGE_MIN -256000 > #define SPD5118_TEMP_RANGE_MAX 255750 > > +struct spd5118_data { > + struct regmap *regmap; > + struct mutex nvmem_lock; > +}; > + > +/* hwmon */ > + > static int spd5118_temp_from_reg(u16 reg) > { > int temp = sign_extend32(reg >> 2, 10); > @@ -360,9 +381,111 @@ static const struct hwmon_chip_info spd5118_chip_info = { > .info = spd5118_info, > }; > > +/* nvmem */ > + > +static int spd5118_nvmem_set_page(struct regmap *regmap, int page) > +{ > + unsigned int old_page; > + int err; > + > + err = regmap_read(regmap, SPD5118_REG_I2C_LEGACY_MODE, &old_page); > + if (err) > + return err; > + > + if (page != (old_page & SPD5118_LEGACY_MODE_MASK)) { > + /* Update page and explicitly select 1-byte addressing */ > + err = regmap_update_bits(regmap, SPD5118_REG_I2C_LEGACY_MODE, > + SPD5118_LEGACY_MODE_MASK, page); > + if (err) > + return err; > + > + /* Selected new NVMEM page, drop cached data */ > + regcache_drop_region(regmap, SPD5118_EEPROM_BASE, 0xff); > + } > + > + return 0; > +} > + > +static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf, > + unsigned int offset, size_t count) > +{ > + int err; > + > + err = spd5118_nvmem_set_page(regmap, offset >> SPD5118_PAGE_SHIFT); > + if (err) > + return err; > + > + offset &= SPD5118_PAGE_MASK; > + > + /* Can't cross page boundaries */ > + if (offset + count > SPD5118_PAGE_SIZE) > + count = SPD5118_PAGE_SIZE - offset; > + > + err = regmap_bulk_read(regmap, SPD5118_EEPROM_BASE + offset, buf, count); > + if (err) > + return err; > + > + return count; > +} > + > +static int spd5118_nvmem_read(void *priv, unsigned int off, void *val, size_t count) > +{ > + struct spd5118_data *data = priv; > + char *buf = val; > + int ret; > + > + if (unlikely(!count)) > + return count; > + > + if (off + count > SPD5118_EEPROM_SIZE) > + return -EINVAL; > + > + mutex_lock(&data->nvmem_lock); > + > + while (count) { > + ret = spd5118_nvmem_read_page(data->regmap, buf, off, count); > + if (ret < 0) { > + mutex_unlock(&data->nvmem_lock); > + return ret; > + } > + buf += ret; > + off += ret; > + count -= ret; > + } > + mutex_unlock(&data->nvmem_lock); > + return 0; > +} > + > +static int spd5118_nvmem_init(struct device *dev, struct spd5118_data *data) > +{ > + struct nvmem_config nvmem_config = { > + .type = NVMEM_TYPE_EEPROM, > + .name = dev_name(dev), > + .id = NVMEM_DEVID_NONE, > + .dev = dev, > + .base_dev = dev, > + .read_only = true, > + .root_only = false, > + .owner = THIS_MODULE, > + .compat = true, Hi, do we really need this setting here? > + .reg_read = spd5118_nvmem_read, > + .priv = data, > + .stride = 1, > + .word_size = 1, > + .size = SPD5118_EEPROM_SIZE, > + }; > + struct nvmem_device *nvmem; > + > + nvmem = devm_nvmem_register(dev, &nvmem_config); > + return PTR_ERR_OR_ZERO(nvmem); > +} > + > +/* regmap */ > + > static bool spd5118_writeable_reg(struct device *dev, unsigned int reg) > { > switch (reg) { > + case SPD5118_REG_I2C_LEGACY_MODE: > case SPD5118_REG_TEMP_CLR: > case SPD5118_REG_TEMP_CONFIG: > case SPD5118_REG_TEMP_MAX: > @@ -396,7 +519,7 @@ static bool spd5118_volatile_reg(struct device *dev, unsigned int reg) > static const struct regmap_config spd5118_regmap_config = { > .reg_bits = 8, > .val_bits = 8, > - .max_register = SPD5118_REG_TEMP_STATUS, > + .max_register = 0xff, > .writeable_reg = spd5118_writeable_reg, > .volatile_reg = spd5118_volatile_reg, > .cache_type = REGCACHE_MAPLE, > @@ -406,10 +529,15 @@ static int spd5118_probe(struct i2c_client *client) > { > struct device *dev = &client->dev; > unsigned int regval, revision, vendor, bank; > + struct spd5118_data *data; > struct device *hwmon_dev; > struct regmap *regmap; > int err; > > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > regmap = devm_regmap_init_i2c(client, &spd5118_regmap_config); > if (IS_ERR(regmap)) > return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n"); > @@ -433,7 +561,16 @@ static int spd5118_probe(struct i2c_client *client) > if (!spd5118_vendor_valid(bank, vendor)) > return -ENODEV; > > - dev_set_drvdata(dev, regmap); > + data->regmap = regmap; > + mutex_init(&data->nvmem_lock); > + dev_set_drvdata(dev, data); > + > + err = spd5118_nvmem_init(dev, data); > + /* Ignore if NVMEM support is disabled */ > + if (err && err != -EOPNOTSUPP) { Maybe we can use IS_REACHABLE(CONFIG_NVMEM) here? Thanks, Armin Wolf > + dev_err_probe(dev, err, "failed to register nvmem\n"); > + return err; > + } > > hwmon_dev = devm_hwmon_device_register_with_info(dev, "spd5118", > regmap, &spd5118_chip_info, > @@ -454,7 +591,8 @@ static int spd5118_probe(struct i2c_client *client) > > static int spd5118_suspend(struct device *dev) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct spd5118_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > u32 regval; > int err; > > @@ -479,7 +617,8 @@ static int spd5118_suspend(struct device *dev) > > static int spd5118_resume(struct device *dev) > { > - struct regmap *regmap = dev_get_drvdata(dev); > + struct spd5118_data *data = dev_get_drvdata(dev); > + struct regmap *regmap = data->regmap; > > regcache_cache_only(regmap, false); > return regcache_sync(regmap);
On 6/4/24 04:58, Armin Wolf wrote: > Am 04.06.24 um 06:02 schrieb Guenter Roeck: > >> Add support for reading SPD NVMEM data from SPD5118 (Jedec JESD300) >> compliant memory modules. NVMEM write operation is not supported. >> >> NVMEM support is optional. If CONFIG_NVMEM is disabled, the driver will >> still instantiate but not provide NVMEM attribute files. >> >> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >> --- >> v4: Use NVMEM_DEVID_NONE instead of NVMEM_DEVID_AUTO >> Ignore nvmem registration failure if nvmem support is disabled >> >> v3: New patch >> >> Documentation/hwmon/spd5118.rst | 8 ++ >> drivers/hwmon/spd5118.c | 147 +++++++++++++++++++++++++++++++- [ ... ] >> +static int spd5118_nvmem_init(struct device *dev, struct spd5118_data *data) >> +{ >> + struct nvmem_config nvmem_config = { >> + .type = NVMEM_TYPE_EEPROM, >> + .name = dev_name(dev), >> + .id = NVMEM_DEVID_NONE, >> + .dev = dev, >> + .base_dev = dev, >> + .read_only = true, >> + .root_only = false, >> + .owner = THIS_MODULE, >> + .compat = true, > > Hi, > > do we really need this setting here? > The "eeprom" file is only created if both "base_dev" and "compat" are set. decode-dimms depends on it. While decode-dimms has to be updated anyway, I did not want to make that more complicated than necessary. Another option would be not to use the nvmem subsystem in the first place, similar to the ee1004 driver, but my understanding is that the use of the nvmem subsystem is preferred. [ ... ] >> + >> + err = spd5118_nvmem_init(dev, data); >> + /* Ignore if NVMEM support is disabled */ >> + if (err && err != -EOPNOTSUPP) { > > Maybe we can use IS_REACHABLE(CONFIG_NVMEM) here? > We could, but I prefer to avoid conditionals in the code if possible, the dummy devm_nvmem_register() is there specifically to cover that situation, and no other driver does that. Also, since the underlying functions are dummy, the compiler should optimize it all away if CONFIG_NVMEM=n. Thanks, Guenter
Am 04.06.24 um 16:30 schrieb Guenter Roeck: > On 6/4/24 04:58, Armin Wolf wrote: >> Am 04.06.24 um 06:02 schrieb Guenter Roeck: >> >>> Add support for reading SPD NVMEM data from SPD5118 (Jedec JESD300) >>> compliant memory modules. NVMEM write operation is not supported. >>> >>> NVMEM support is optional. If CONFIG_NVMEM is disabled, the driver will >>> still instantiate but not provide NVMEM attribute files. >>> >>> Signed-off-by: Guenter Roeck <linux@roeck-us.net> >>> --- >>> v4: Use NVMEM_DEVID_NONE instead of NVMEM_DEVID_AUTO >>> Ignore nvmem registration failure if nvmem support is disabled >>> >>> v3: New patch >>> >>> Documentation/hwmon/spd5118.rst | 8 ++ >>> drivers/hwmon/spd5118.c | 147 >>> +++++++++++++++++++++++++++++++- > > > [ ... ] > >>> +static int spd5118_nvmem_init(struct device *dev, struct >>> spd5118_data *data) >>> +{ >>> + struct nvmem_config nvmem_config = { >>> + .type = NVMEM_TYPE_EEPROM, >>> + .name = dev_name(dev), >>> + .id = NVMEM_DEVID_NONE, >>> + .dev = dev, >>> + .base_dev = dev, >>> + .read_only = true, >>> + .root_only = false, >>> + .owner = THIS_MODULE, >>> + .compat = true, >> >> Hi, >> >> do we really need this setting here? >> > > The "eeprom" file is only created if both "base_dev" and "compat" are > set. > decode-dimms depends on it. While decode-dimms has to be updated anyway, > I did not want to make that more complicated than necessary. > I understand. > Another option would be not to use the nvmem subsystem in the first > place, > similar to the ee1004 driver, but my understanding is that the use of the > nvmem subsystem is preferred. > > [ ... ] > >>> + >>> + err = spd5118_nvmem_init(dev, data); >>> + /* Ignore if NVMEM support is disabled */ >>> + if (err && err != -EOPNOTSUPP) { >> >> Maybe we can use IS_REACHABLE(CONFIG_NVMEM) here? >> > > We could, but I prefer to avoid conditionals in the code if possible, > the dummy devm_nvmem_register() is there specifically to cover that > situation, and no other driver does that. Also, since the underlying > functions are dummy, the compiler should optimize it all away if > CONFIG_NVMEM=n. > > Thanks, > Guenter > Ok, then i am ok with with this patch. Tested-by: Armin Wolf <W_Armin@gmx.de>
diff --git a/Documentation/hwmon/spd5118.rst b/Documentation/hwmon/spd5118.rst index a15d75aa2066..ef7338f46575 100644 --- a/Documentation/hwmon/spd5118.rst +++ b/Documentation/hwmon/spd5118.rst @@ -53,3 +53,11 @@ temp1_crit_alarm Temperature critical alarm Alarm attributes are sticky until read and will be cleared afterwards unless the alarm condition still applies. + + +SPD (Serial Presence Detect) support +------------------------------------ + +The driver also supports reading the SPD NVRAM on SPD5118 compatible chips. +SPD data is available from the 'eeprom' binary attribute file attached to the +chip's I2C device. diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c index d55c073ff5fd..5cb5e52c0a38 100644 --- a/drivers/hwmon/spd5118.c +++ b/drivers/hwmon/spd5118.c @@ -20,6 +20,8 @@ #include <linux/i2c.h> #include <linux/hwmon.h> #include <linux/module.h> +#include <linux/mutex.h> +#include <linux/nvmem-provider.h> #include <linux/pm.h> #include <linux/regmap.h> #include <linux/units.h> @@ -53,12 +55,31 @@ static const unsigned short normal_i2c[] = { #define SPD5118_TS_DISABLE BIT(0) /* temperature sensor disable */ +#define SPD5118_LEGACY_MODE_ADDR BIT(3) +#define SPD5118_LEGACY_PAGE_MASK GENMASK(2, 0) +#define SPD5118_LEGACY_MODE_MASK (SPD5118_LEGACY_MODE_ADDR | SPD5118_LEGACY_PAGE_MASK) + + +#define SPD5118_NUM_PAGES 8 +#define SPD5118_PAGE_SIZE 128 +#define SPD5118_PAGE_SHIFT 7 +#define SPD5118_PAGE_MASK GENMASK(6, 0) +#define SPD5118_EEPROM_BASE 0x80 +#define SPD5118_EEPROM_SIZE (SPD5118_PAGE_SIZE * SPD5118_NUM_PAGES) + /* Temperature unit in millicelsius */ #define SPD5118_TEMP_UNIT (MILLIDEGREE_PER_DEGREE / 4) /* Representable temperature range in millicelsius */ #define SPD5118_TEMP_RANGE_MIN -256000 #define SPD5118_TEMP_RANGE_MAX 255750 +struct spd5118_data { + struct regmap *regmap; + struct mutex nvmem_lock; +}; + +/* hwmon */ + static int spd5118_temp_from_reg(u16 reg) { int temp = sign_extend32(reg >> 2, 10); @@ -360,9 +381,111 @@ static const struct hwmon_chip_info spd5118_chip_info = { .info = spd5118_info, }; +/* nvmem */ + +static int spd5118_nvmem_set_page(struct regmap *regmap, int page) +{ + unsigned int old_page; + int err; + + err = regmap_read(regmap, SPD5118_REG_I2C_LEGACY_MODE, &old_page); + if (err) + return err; + + if (page != (old_page & SPD5118_LEGACY_MODE_MASK)) { + /* Update page and explicitly select 1-byte addressing */ + err = regmap_update_bits(regmap, SPD5118_REG_I2C_LEGACY_MODE, + SPD5118_LEGACY_MODE_MASK, page); + if (err) + return err; + + /* Selected new NVMEM page, drop cached data */ + regcache_drop_region(regmap, SPD5118_EEPROM_BASE, 0xff); + } + + return 0; +} + +static ssize_t spd5118_nvmem_read_page(struct regmap *regmap, char *buf, + unsigned int offset, size_t count) +{ + int err; + + err = spd5118_nvmem_set_page(regmap, offset >> SPD5118_PAGE_SHIFT); + if (err) + return err; + + offset &= SPD5118_PAGE_MASK; + + /* Can't cross page boundaries */ + if (offset + count > SPD5118_PAGE_SIZE) + count = SPD5118_PAGE_SIZE - offset; + + err = regmap_bulk_read(regmap, SPD5118_EEPROM_BASE + offset, buf, count); + if (err) + return err; + + return count; +} + +static int spd5118_nvmem_read(void *priv, unsigned int off, void *val, size_t count) +{ + struct spd5118_data *data = priv; + char *buf = val; + int ret; + + if (unlikely(!count)) + return count; + + if (off + count > SPD5118_EEPROM_SIZE) + return -EINVAL; + + mutex_lock(&data->nvmem_lock); + + while (count) { + ret = spd5118_nvmem_read_page(data->regmap, buf, off, count); + if (ret < 0) { + mutex_unlock(&data->nvmem_lock); + return ret; + } + buf += ret; + off += ret; + count -= ret; + } + mutex_unlock(&data->nvmem_lock); + return 0; +} + +static int spd5118_nvmem_init(struct device *dev, struct spd5118_data *data) +{ + struct nvmem_config nvmem_config = { + .type = NVMEM_TYPE_EEPROM, + .name = dev_name(dev), + .id = NVMEM_DEVID_NONE, + .dev = dev, + .base_dev = dev, + .read_only = true, + .root_only = false, + .owner = THIS_MODULE, + .compat = true, + .reg_read = spd5118_nvmem_read, + .priv = data, + .stride = 1, + .word_size = 1, + .size = SPD5118_EEPROM_SIZE, + }; + struct nvmem_device *nvmem; + + nvmem = devm_nvmem_register(dev, &nvmem_config); + return PTR_ERR_OR_ZERO(nvmem); +} + +/* regmap */ + static bool spd5118_writeable_reg(struct device *dev, unsigned int reg) { switch (reg) { + case SPD5118_REG_I2C_LEGACY_MODE: case SPD5118_REG_TEMP_CLR: case SPD5118_REG_TEMP_CONFIG: case SPD5118_REG_TEMP_MAX: @@ -396,7 +519,7 @@ static bool spd5118_volatile_reg(struct device *dev, unsigned int reg) static const struct regmap_config spd5118_regmap_config = { .reg_bits = 8, .val_bits = 8, - .max_register = SPD5118_REG_TEMP_STATUS, + .max_register = 0xff, .writeable_reg = spd5118_writeable_reg, .volatile_reg = spd5118_volatile_reg, .cache_type = REGCACHE_MAPLE, @@ -406,10 +529,15 @@ static int spd5118_probe(struct i2c_client *client) { struct device *dev = &client->dev; unsigned int regval, revision, vendor, bank; + struct spd5118_data *data; struct device *hwmon_dev; struct regmap *regmap; int err; + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); + if (!data) + return -ENOMEM; + regmap = devm_regmap_init_i2c(client, &spd5118_regmap_config); if (IS_ERR(regmap)) return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n"); @@ -433,7 +561,16 @@ static int spd5118_probe(struct i2c_client *client) if (!spd5118_vendor_valid(bank, vendor)) return -ENODEV; - dev_set_drvdata(dev, regmap); + data->regmap = regmap; + mutex_init(&data->nvmem_lock); + dev_set_drvdata(dev, data); + + err = spd5118_nvmem_init(dev, data); + /* Ignore if NVMEM support is disabled */ + if (err && err != -EOPNOTSUPP) { + dev_err_probe(dev, err, "failed to register nvmem\n"); + return err; + } hwmon_dev = devm_hwmon_device_register_with_info(dev, "spd5118", regmap, &spd5118_chip_info, @@ -454,7 +591,8 @@ static int spd5118_probe(struct i2c_client *client) static int spd5118_suspend(struct device *dev) { - struct regmap *regmap = dev_get_drvdata(dev); + struct spd5118_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; u32 regval; int err; @@ -479,7 +617,8 @@ static int spd5118_suspend(struct device *dev) static int spd5118_resume(struct device *dev) { - struct regmap *regmap = dev_get_drvdata(dev); + struct spd5118_data *data = dev_get_drvdata(dev); + struct regmap *regmap = data->regmap; regcache_cache_only(regmap, false); return regcache_sync(regmap);
Add support for reading SPD NVMEM data from SPD5118 (Jedec JESD300) compliant memory modules. NVMEM write operation is not supported. NVMEM support is optional. If CONFIG_NVMEM is disabled, the driver will still instantiate but not provide NVMEM attribute files. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- v4: Use NVMEM_DEVID_NONE instead of NVMEM_DEVID_AUTO Ignore nvmem registration failure if nvmem support is disabled v3: New patch Documentation/hwmon/spd5118.rst | 8 ++ drivers/hwmon/spd5118.c | 147 +++++++++++++++++++++++++++++++- 2 files changed, 151 insertions(+), 4 deletions(-)