Messages in this thread | | | Date | Mon, 18 Dec 2023 11:01:12 -0800 | Subject | Re: [PATCH 1/3] hwmon: max31827: Add PEC support | From | Guenter Roeck <> |
| |
On 12/18/23 09:59, Matyas, Daniel wrote: > > > ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ > *Von:* Guenter Roeck <groeck7@gmail.com> im Auftrag von Guenter Roeck <linux@roeck-us.net> > *Gesendet:* Montag, Dezember 18, 2023 6:26:57 nachm. > *An:* Matyas, Daniel <Daniel.Matyas@analog.com> > *Cc:* Jean Delvare <jdelvare@suse.com>; Rob Herring <robh+dt@kernel.org>; Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>; Conor Dooley <conor+dt@kernel.org>; Jonathan Corbet <corbet@lwn.net>; linux-hwmon@vger.kernel.org <linux-hwmon@vger.kernel.org>; devicetree@vger.kernel.org <devicetree@vger.kernel.org>; linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>; linux-doc@vger.kernel.org <linux-doc@vger.kernel.org> > *Betreff:* Re: [PATCH 1/3] hwmon: max31827: Add PEC support > > [External] > > On 12/18/23 06:55, Matyas, Daniel wrote: > [ ... ] >>> On top of that, it is not clear why regmap can't be used in the first place. >>> It seems that the major change is that one needs to read the configuration >>> register after a write to see if there was a PEC error. It is not immediately >>> obvious why that additional read (if indeed necessary) would require >>> regmap support to be dropped. >>> >> >> I tried out writing and and reading with regmap, but it is not working properly. Even if I modify the client flag, I still receive only 2 bytes of data (a word). I should be receiving 2+1 bytes = data + CRC-8. >> >> With i2c_smbus reads and writes, when I set the flag, I receive the 2+1 bytes, as expected. >> > > The SMBus code in drivers/i2c/i2c-core-smbus.c is supposed to check > if the received PEC is correct for SMBus transfers. Are you saying > that this doesn't work, or that regmap doesn't use SMBus functions > to communicate with the chip ? > > Thanks, > Guenter > > > I am 70% sure, that the regmap does not use SMBus functions. >
It should.
$ git grep smbus drivers/base/regmap/regmap-i2c.c drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_byte_reg_read(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_read_byte_data(i2c, reg); drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_byte_reg_write(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: return i2c_smbus_write_byte_data(i2c, reg, val); drivers/base/regmap/regmap-i2c.c:static const struct regmap_bus regmap_smbus_byte = { drivers/base/regmap/regmap-i2c.c: .reg_write = regmap_smbus_byte_reg_write, drivers/base/regmap/regmap-i2c.c: .reg_read = regmap_smbus_byte_reg_read, drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_word_reg_read(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_read_word_data(i2c, reg); drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_word_reg_write(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: return i2c_smbus_write_word_data(i2c, reg, val); drivers/base/regmap/regmap-i2c.c:static const struct regmap_bus regmap_smbus_word = { drivers/base/regmap/regmap-i2c.c: .reg_write = regmap_smbus_word_reg_write, drivers/base/regmap/regmap-i2c.c: .reg_read = regmap_smbus_word_reg_read, drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_word_read_swapped(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_read_word_swapped(i2c, reg); drivers/base/regmap/regmap-i2c.c:static int regmap_smbus_word_write_swapped(void *context, unsigned int reg, drivers/base/regmap/regmap-i2c.c: return i2c_smbus_write_word_swapped(i2c, reg, val); drivers/base/regmap/regmap-i2c.c:static const struct regmap_bus regmap_smbus_word_swapped = { drivers/base/regmap/regmap-i2c.c: .reg_write = regmap_smbus_word_write_swapped, drivers/base/regmap/regmap-i2c.c: .reg_read = regmap_smbus_word_read_swapped, drivers/base/regmap/regmap-i2c.c:static int regmap_i2c_smbus_i2c_write(void *context, const void *data, drivers/base/regmap/regmap-i2c.c: return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count, drivers/base/regmap/regmap-i2c.c:static int regmap_i2c_smbus_i2c_read(void *context, const void *reg, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_read_i2c_block_data(i2c, ((u8 *)reg)[0], val_size, val); drivers/base/regmap/regmap-i2c.c:static const struct regmap_bus regmap_i2c_smbus_i2c_block = { drivers/base/regmap/regmap-i2c.c: .write = regmap_i2c_smbus_i2c_write, drivers/base/regmap/regmap-i2c.c: .read = regmap_i2c_smbus_i2c_read, drivers/base/regmap/regmap-i2c.c:static int regmap_i2c_smbus_i2c_write_reg16(void *context, const void *data, drivers/base/regmap/regmap-i2c.c: return i2c_smbus_write_i2c_block_data(i2c, ((u8 *)data)[0], count, drivers/base/regmap/regmap-i2c.c:static int regmap_i2c_smbus_i2c_read_reg16(void *context, const void *reg, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_write_byte_data(i2c, ((u16 *)reg)[0] & 0xff, drivers/base/regmap/regmap-i2c.c: ret = i2c_smbus_read_byte(i2c); drivers/base/regmap/regmap-i2c.c:static const struct regmap_bus regmap_i2c_smbus_i2c_block_reg16 = { drivers/base/regmap/regmap-i2c.c: .write = regmap_i2c_smbus_i2c_write_reg16, drivers/base/regmap/regmap-i2c.c: .read = regmap_i2c_smbus_i2c_read_reg16, drivers/base/regmap/regmap-i2c.c: bus = ®map_i2c_smbus_i2c_block; drivers/base/regmap/regmap-i2c.c: bus = ®map_i2c_smbus_i2c_block_reg16; drivers/base/regmap/regmap-i2c.c: bus = ®map_smbus_word; drivers/base/regmap/regmap-i2c.c: bus = ®map_smbus_word_swapped; drivers/base/regmap/regmap-i2c.c: bus = ®map_smbus_byte;
If that doesn't work for some reason, I'd rather figure out why instead of starting to drop regmap support.
Guenter
| |