lkml.org 
[lkml]   [2023]   [Dec]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 1/3] hwmon: max31827: Add PEC support
From
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 = &regmap_i2c_smbus_i2c_block;
drivers/base/regmap/regmap-i2c.c: bus = &regmap_i2c_smbus_i2c_block_reg16;
drivers/base/regmap/regmap-i2c.c: bus = &regmap_smbus_word;
drivers/base/regmap/regmap-i2c.c: bus = &regmap_smbus_word_swapped;
drivers/base/regmap/regmap-i2c.c: bus = &regmap_smbus_byte;

If that doesn't work for some reason, I'd rather figure out why instead of
starting to drop regmap support.

Guenter

\
 
 \ /
  Last update: 2023-12-18 20:02    [W:0.076 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site