Re: [RFT PATCH v2 2/3] hwmon: (spd5118) Use spd5118 specific read/write operations

[Paul Menzel <pmenzel@molgen.mpg.de>]

[continue tests from thread *Re: [PATCH v4 5/6] i2c: smbus: Support DDR5 
SPD EEPROMs* [1] here]

[Cc: +Jean Delvare]

Dear Guenter,


Am 18.06.24 um 21:53 schrieb Guenter Roeck:
> regmap write operations on i801 controllers were observed to fail with
> -ENXIO errors. It appears that the i801 controller and/or at least some
> spd5118 compatible chips do not support the I2C_FUNC_SMBUS_I2C_BLOCK
> operation used by the regmap-i2c core if I2C_FUNC_SMBUS_I2C_BLOCK is
> supported by the I2C/SMBus controller.
> 
> Stop using the regmap-i2c core and always use basic SMBus operations
> instead.
> 
> Reported-by: Paul Menzel <pmenzel@molgen.mpg.de>
> Closes: https://lore.kernel.org/linux-hwmon/33f369c1-1098-458e-9398-30037bd8c5aa@molgen.mpg.de/
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: Added patch
> 
>   drivers/hwmon/Kconfig   |  2 +-
>   drivers/hwmon/spd5118.c | 28 +++++++++++++++++++++++++++-
>   2 files changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index d5eced417fc3..fdfa778a965d 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2184,7 +2184,7 @@ config SENSORS_INA3221
>   config SENSORS_SPD5118
>   	tristate "SPD5118 Compliant Temperature Sensors"
>   	depends on I2C
> -	select REGMAP_I2C
> +	select REGMAP
>   	help
>   	  If you say yes here you get support for SPD5118 (JEDEC JESD300)
>   	  compliant temperature sensors. Such sensors are found on DDR5 memory
> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
> index d405c5ef755d..995c45e2a997 100644
> --- a/drivers/hwmon/spd5118.c
> +++ b/drivers/hwmon/spd5118.c
> @@ -489,6 +489,31 @@ static bool spd5118_volatile_reg(struct device *dev, unsigned int reg)
>   	}
>   }
>   
> +static int spd5118_regmap_reg_read(void *context, unsigned int reg,
> +				   unsigned int *val)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(context, reg);
> +	if (ret < 0)
> +		return ret;
> +
> +	*val = ret;
> +
> +	return 0;
> +}
> +
> +static int spd5118_regmap_reg_write(void *context, unsigned int reg,
> +				    unsigned int val)
> +{
> +	return i2c_smbus_write_byte_data(context, reg, val);
> +}
> +
> +static const struct regmap_bus spd5118_regmap_bus = {
> +	.reg_write = spd5118_regmap_reg_write,
> +	.reg_read = spd5118_regmap_reg_read,
> +};
> +
>   static const struct regmap_range_cfg spd5118_regmap_range_cfg[] = {
>   	{
>   	.selector_reg   = SPD5118_REG_I2C_LEGACY_MODE,
> @@ -526,7 +551,8 @@ static int spd5118_probe(struct i2c_client *client)
>   	if (!data)
>   		return -ENOMEM;
>   
> -	regmap = devm_regmap_init_i2c(client, &spd5118_regmap_config);
> +	regmap = devm_regmap_init(dev, &spd5118_regmap_bus, client,
> +				  &spd5118_regmap_config);
>   	if (IS_ERR(regmap))
>   		return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
>   

Unfortunately, it still fails:

     $ git log --no-decorate --oneline -4
     7ddcff2d44ae3 hwmon: (spd5118) Add support for Renesas/ITD SPD5118 
hub controllers
     e89136743324f hwmon: (spd5118) Use spd5118 specific read/write 
operations
     0fcc7279f0cc4 hwmon: (spd5118) Use regmap to implement paging
     801b6aad6fa7a hwmon: (spd5118) Add configuration option for 
auto-detection

     $ uname -r
     6.10.0-rc4.mx64.461-00050-g7ddcff2d44ae
     $ sudo bash -c 'echo 56000 > /sys/class/hwmon/hwmon3/temp1_max'
     bash: line 1: echo: write error: No such device or address


Kind regards,

Paul


[1]: 
https://lore.kernel.org/all/342dae24-56c5-4b81-9591-dc23ddbb2806@roeck-us.net/