Hi, On 1/20/2025 6:25 AM, Roger Quadros wrote: > > On 14/01/2025 01:07, Shree Ramamoorthy wrote: >> The chipid macro/variable and regmap_read function call is not needed >> because the TPS65219_REG_TI_DEV_ID register value is not a consistent value >> across TPS65219 PMIC config versions. Reading from the DEV_ID register >> without a consistent value to compare it to isn't useful. There isn't a >> way to verify the match data ID is the same ID read from the DEV_ID device >> register. 0xF0 isn't a DEV_ID value consistent across TPS65219 NVM >> configurations. >> >> For TPS65215, there is a consistent value in bits 5-0 of the DEV_ID >> register. However, there are other error checks in place within probe() >> that apply to both PMICs rather than keeping this isolated check for one >> PMIC. >> >> Signed-off-by: Shree Ramamoorthy <s-ramamoorthy@xxxxxx> > subject should be "mfd: tps65219:.."? > >> --- >> drivers/mfd/tps65219.c | 6 ------ >> include/linux/mfd/tps65219.h | 7 +++++-- >> 2 files changed, 5 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/mfd/tps65219.c b/drivers/mfd/tps65219.c >> index 081c5a30b04a..15b874ee59e5 100644 >> --- a/drivers/mfd/tps65219.c >> +++ b/drivers/mfd/tps65219.c >> @@ -246,12 +246,6 @@ static int tps65219_probe(struct i2c_client *client) >> if (ret) >> return ret; >> >> - ret = regmap_read(tps->regmap, TPS65219_REG_TI_DEV_ID, &chipid); >> - if (ret) { >> - dev_err(tps->dev, "Failed to read device ID: %d\n", ret); >> - return ret; >> - } >> - > But now chipid has no users. So please drop that variable as well. > >> ret = devm_mfd_add_devices(tps->dev, PLATFORM_DEVID_AUTO, >> tps65219_cells, ARRAY_SIZE(tps65219_cells), >> NULL, 0, regmap_irq_get_domain(tps->irq_data)); >> diff --git a/include/linux/mfd/tps65219.h b/include/linux/mfd/tps65219.h >> index 546bceec7173..0d88e92ff8f2 100644 >> --- a/include/linux/mfd/tps65219.h >> +++ b/include/linux/mfd/tps65219.h >> @@ -13,8 +13,11 @@ >> #include <linux/regmap.h> >> #include <linux/regulator/driver.h> >> >> -/* TPS chip id list */ >> -#define TPS65219 0xF0 >> +/* Chip id list*/ >> +enum pmic_id { >> + TPS65215, >> + TPS65219, >> +}; > This change is not part of subject. Please merge it with next patch. Thank you for reviewing! Will make these 3 changes & include it in the next version. >> >> /* I2C ID for TPS65219 part */ >> #define TPS65219_I2C_ID 0x24