[PATCH 11/12] hwmon: spd5118: Add I3C support
Guenter Roeck
linux at roeck-us.net
Thu Mar 19 07:34:18 PDT 2026
On 3/18/26 21:35, Akhil R wrote:
> On Wed, 18 Mar 2026 11:53:49 -0700, Guenter Roeck wrote:
>> On 3/18/26 10:27, Akhil R wrote:
>>> Add a regmap config and a probe function to support for I3C based
>>> communication to SPD5118 devices.
>>>
>>> On an I3C bus, SPD5118 are enumerated via SETAASA and always require an
>>> ACPI or device tree entry. The device matching is hence through the OF
>>> match tables only and do not need an I3C class match table. The device
>>> identity is verified in the type registers before proceeding to the
>>> common probe function.
>>>
>>> Signed-off-by: Akhil R <akhilrajeev at nvidia.com>
>>> ---
>>> drivers/hwmon/Kconfig | 7 +++--
>>> drivers/hwmon/spd5118.c | 66 ++++++++++++++++++++++++++++++++++++++++-
>>> 2 files changed, 70 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>>> index 8af80e17d25e..23604c05ad22 100644
>>> --- a/drivers/hwmon/Kconfig
>>> +++ b/drivers/hwmon/Kconfig
>>> @@ -2300,10 +2300,13 @@ config SENSORS_SPD5118
>>> tristate "SPD5118 Compliant Temperature Sensors"
>>> depends on I2C
>>> select REGMAP_I2C
>>
>> I also had
>> depends on I3C || I3C=n
>> in my version at
>>
>> https://patchwork.kernel.org/project/linux-hwmon/patch/20250419161356.2528986-6-linux@roeck-us.net/
>>
>> which I guess matches the more recent "depends on I3C_OR_I2C".
>
> Ack. Will update.
>
>>
>>> + select REGMAP_I3C if I3C
>>> help
>>> If you say yes here you get support for SPD5118 (JEDEC JESD300)
>>> - compliant temperature sensors. Such sensors are found on DDR5 memory
>>> - modules.
>>> + compliant temperature sensors using I2C or I3C bus interface.
>>> + Such sensors are found on DDR5 memory modules.
>>> +
>>> + This driver supports both I2C and I3C interfaces.
>>>
>>> This driver can also be built as a module. If so, the module
>>> will be called spd5118.
>>> diff --git a/drivers/hwmon/spd5118.c b/drivers/hwmon/spd5118.c
>>> index 5da44571b6a0..d70123e10616 100644
>>> --- a/drivers/hwmon/spd5118.c
>>> +++ b/drivers/hwmon/spd5118.c
>>> @@ -18,6 +18,7 @@
>>> #include <linux/bits.h>
>>> #include <linux/err.h>
>>> #include <linux/i2c.h>
>>> +#include <linux/i3c/device.h>
>>> #include <linux/hwmon.h>
>>> #include <linux/module.h>
>>> #include <linux/mutex.h>
>>> @@ -482,6 +483,25 @@ static const struct regmap_config spd5118_regmap16_config = {
>>> .cache_type = REGCACHE_MAPLE,
>>> };
>>>
>>> +/*
>>> + * I3C uses 2-byte register addressing -
>>> + * Byte 1: MemReg | BlkAddr[0] | Address[5:0]
>>> + * Byte 2: 0000 | BlkAddr[4:1]
>>> + *
>>> + * The low byte carries the register/NVM address and the high byte carries the
>>> + * upper block address bits, so little-endian format is required. No range
>>> + * config is needed since I3C does not use MR11 page switching.
>>> + */
>>> +static const struct regmap_config spd5118_regmap_i3c_config = {
>>> + .reg_bits = 16,
>>> + .val_bits = 8,
>>> + .max_register = 0x7ff,
>>> + .reg_format_endian = REGMAP_ENDIAN_LITTLE,
>>
>> Should this be added to spd5118_regmap16_config instead, or is there reason
>> to assume that I2C 16-bit addressing differs from I3C addressing ?
>
> I did not see any difference for I2C in the specification, but I assumed the
> existing format would have been working and I thought not to change them.
> Changing the I2C format would also require a change in the is_16bit nvmem_read
> formula.
>
>>
>>> + .writeable_reg = spd5118_writeable_reg,
>>> + .volatile_reg = spd5118_volatile_reg,
>>> + .cache_type = REGCACHE_MAPLE,
>>> +};
>>> +
>>> static int spd5118_suspend(struct device *dev)
>>> {
>>> struct spd5118_data *data = dev_get_drvdata(dev);
>>> @@ -770,7 +790,51 @@ static struct i2c_driver spd5118_i2c_driver = {
>>> .address_list = IS_ENABLED(CONFIG_SENSORS_SPD5118_DETECT) ? normal_i2c : NULL,
>>> };
>>>
>>> -module_i2c_driver(spd5118_i2c_driver);
>>> +/* I3C */
>>> +
>>> +static int spd5118_i3c_probe(struct i3c_device *i3cdev)
>>> +{
>>> + struct device *dev = i3cdev_to_dev(i3cdev);
>>> + struct regmap *regmap;
>>> + unsigned int regval;
>>> + int err;
>>> +
>>> + regmap = devm_regmap_init_i3c(i3cdev, &spd5118_regmap_i3c_config);
>>> + if (IS_ERR(regmap))
>>> + return dev_err_probe(dev, PTR_ERR(regmap), "regmap init failed\n");
>>> +
>>> + /* Verify this is a SPD5118 device */
>>> + err = regmap_read(regmap, SPD5118_REG_TYPE, ®val);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (regval != 0x51) {
>>> + dev_err(dev, "unexpected device type 0x%02x, expected 0x51\n", regval);
>>> + return -ENODEV;
>>> + }
>>> +
>>> + err = regmap_read(regmap, SPD5118_REG_TYPE + 1, ®val);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (regval != 0x18) {
>>> + dev_err(dev, "unexpected device type 0x%02x, expected 0x18\n", regval);
>>> + return -ENODEV;
>>> + }
>>> +
>>
>> I don't think this should dump error messages. Also, it might be desirable
>> to use a single regmap operation to read both values.
>
> Ack. Will use regmap_bulk_read() and will remove the error dump.
>
>>
>>> + return spd5118_common_probe(dev, regmap, false);
>>
>> Why is_16bit=false ?
>
> We don't need the encoding formula for the nvmem address with I3C. Since it
> uses little-endian, (page * 0x100 + SPD5118_EEPROM_BASE) translates to the
> correct address. Or did I overlook something?
>
Testing of the 16-bit code was limited: I had to set the SPD on a system
manually to 16-bit mode to get it working, and that only worked until the system
was reset. Its whole point was to prepare for I3C mode. If that fails, the entire
16-bit code in the driver is potentially wrong and should be pulled out before
adding I3C code. It can be added back later if/when a system actually utilizing
it is found.
Thanks,
Guenter
>>
>>> +}
>>> +
>>> +static struct i3c_driver spd5118_i3c_driver = {
>>> + .driver = {
>>> + .name = "spd5118_i3c",
>>> + .of_match_table = spd5118_of_ids,
>>> + .pm = pm_sleep_ptr(&spd5118_pm_ops),
>>> + },
>>> + .probe = spd5118_i3c_probe,
>>> +};
>>> +
>>> +module_i3c_i2c_driver(spd5118_i3c_driver, &spd5118_i2c_driver);
>>>
>>> MODULE_AUTHOR("René Rebe <rene at exactcode.de>");
>>> MODULE_AUTHOR("Guenter Roeck <linux at roeck-us.net>");
>
> Best Regards,
> Akhil
More information about the linux-i3c
mailing list