Messages in this thread | | | Date | Fri, 18 Aug 2023 07:06:03 +0200 (CEST) | From | Andre Werner <> | Subject | Re: [PATCH 1/2] mfd: (tps65086): Read DEVICE ID register 1 from device |
| |
On Thu, 17 Aug 2023, Lee Jones wrote:
> On Wed, 09 Aug 2023, werneazc@gmail.com wrote: > >> From: Andre Werner <andre.werner@systec-electronic.com> >> >> This commit prepares a following commit for the regulator part of the MFD. >> The driver should support different device chips that differ in their >> register definitions, for instance to control LDOA1 and SWB2. >> So it is necessary to use a dedicated regulator description for a >> specific device variant. Thus, the content from DEVICEID Register 1 is >> used to choose a dedicated configuration between the different device >> variants. >> >> Signed-off-by: Andre Werner <andre.werner@systec-electronic.com> >> --- >> drivers/mfd/tps65086.c | 37 ++++++++++++++++++++++++++++++------ >> include/linux/mfd/tps65086.h | 27 ++++++++++++++++++++------ >> 2 files changed, 52 insertions(+), 12 deletions(-) >> >> diff --git a/drivers/mfd/tps65086.c b/drivers/mfd/tps65086.c >> index 6a21000aad4a..38f8572c265e 100644 >> --- a/drivers/mfd/tps65086.c >> +++ b/drivers/mfd/tps65086.c >> @@ -64,7 +64,7 @@ MODULE_DEVICE_TABLE(of, tps65086_of_match_table); >> static int tps65086_probe(struct i2c_client *client) >> { >> struct tps65086 *tps; >> - unsigned int version; >> + unsigned int version, id; >> int ret; >> >> tps = devm_kzalloc(&client->dev, sizeof(*tps), GFP_KERNEL); >> @@ -81,16 +81,41 @@ static int tps65086_probe(struct i2c_client *client) >> return PTR_ERR(tps->regmap); >> } >> >> - ret = regmap_read(tps->regmap, TPS65086_DEVICEID, &version); >> + ret = regmap_read(tps->regmap, TPS65086_DEVICEID1, &id); >> if (ret) { >> - dev_err(tps->dev, "Failed to read revision register\n"); >> + dev_err(tps->dev, "Failed to read revision register 1\n"); >> + return ret; >> + } >> + >> + /* Store device ID to load regulator configuration that fit to IC variant */ >> + switch (id) { >> + case TPS6508640_ID: >> + tps->chip_id = TPS6508640; > > Why not use the meaningful TPS6508640_ID for the chip_id instead of an > arbitrary enum?
In the regulator part for this MFD I use this enum ID to select the right configuration from an array. So the intention is using the enum as the index for this table. I can move this selection into the regulator part and store the meaningful TPS65086 IDs in the MFD data if you prefer?
> >> + break; >> + case TPS65086401_ID: >> + tps->chip_id = TPS65086401; >> + break; >> + case TPS6508641_ID: >> + tps->chip_id = TPS6508641; >> + break; >> + case TPS65086470_ID: >> + tps->chip_id = TPS65086470; >> + break; >> + default: >> + dev_err(tps->dev, "Unknown device ID. Cannot determine regulator config.\n"); >> + return -ENODEV; >> + } >> + >> + ret = regmap_read(tps->regmap, TPS65086_DEVICEID2, &version); >> + if (ret) { >> + dev_err(tps->dev, "Failed to read revision register 2\n"); >> return ret; >> } >> >> dev_info(tps->dev, "Device: TPS65086%01lX, OTP: %c, Rev: %ld\n", >> - (version & TPS65086_DEVICEID_PART_MASK), >> - (char)((version & TPS65086_DEVICEID_OTP_MASK) >> 4) + 'A', >> - (version & TPS65086_DEVICEID_REV_MASK) >> 6); >> + (version & TPS65086_DEVICEID2_PART_MASK), >> + (char)((version & TPS65086_DEVICEID2_OTP_MASK) >> 4) + 'A', >> + (version & TPS65086_DEVICEID2_REV_MASK) >> 6); >> >> if (tps->irq > 0) { >> ret = regmap_add_irq_chip(tps->regmap, tps->irq, IRQF_ONESHOT, 0, >> diff --git a/include/linux/mfd/tps65086.h b/include/linux/mfd/tps65086.h >> index 16f87cccc003..88df344b38df 100644 >> --- a/include/linux/mfd/tps65086.h >> +++ b/include/linux/mfd/tps65086.h >> @@ -13,8 +13,9 @@ >> #include <linux/regmap.h> >> >> /* List of registers for TPS65086 */ >> -#define TPS65086_DEVICEID 0x01 >> -#define TPS65086_IRQ 0x02 >> +#define TPS65086_DEVICEID1 0x00 >> +#define TPS65086_DEVICEID2 0x01 >> +#define TPS65086_IRQ 0x02 >> #define TPS65086_IRQ_MASK 0x03 >> #define TPS65086_PMICSTAT 0x04 >> #define TPS65086_SHUTDNSRC 0x05 >> @@ -75,16 +76,29 @@ >> #define TPS65086_IRQ_SHUTDN_MASK BIT(3) >> #define TPS65086_IRQ_FAULT_MASK BIT(7) >> >> -/* DEVICEID Register field definitions */ >> -#define TPS65086_DEVICEID_PART_MASK GENMASK(3, 0) >> -#define TPS65086_DEVICEID_OTP_MASK GENMASK(5, 4) >> -#define TPS65086_DEVICEID_REV_MASK GENMASK(7, 6) >> +/* DEVICEID1 Register field definitions */ >> +#define TPS6508640_ID 0x00 >> +#define TPS65086401_ID 0x01 >> +#define TPS6508641_ID 0x10 >> +#define TPS65086470_ID 0x70 >> + >> +/* DEVICEID2 Register field definitions */ >> +#define TPS65086_DEVICEID2_PART_MASK GENMASK(3, 0) >> +#define TPS65086_DEVICEID2_OTP_MASK GENMASK(5, 4) >> +#define TPS65086_DEVICEID2_REV_MASK GENMASK(7, 6) >> >> /* VID Masks */ >> #define BUCK_VID_MASK GENMASK(7, 1) >> #define VDOA1_VID_MASK GENMASK(4, 1) >> #define VDOA23_VID_MASK GENMASK(3, 0) >> >> +enum tps65086_ids { >> + TPS6508640, >> + TPS65086401, >> + TPS6508641, >> + TPS65086470, >> +}; >> + >> /* Define the TPS65086 IRQ numbers */ >> enum tps65086_irqs { >> TPS65086_IRQ_DIETEMP, >> @@ -100,6 +114,7 @@ enum tps65086_irqs { >> struct tps65086 { >> struct device *dev; >> struct regmap *regmap; >> + unsigned int chip_id; >> >> /* IRQ Data */ >> int irq; >> -- >> 2.41.0 >> > > -- > Lee Jones [李琼斯] >
Regards,
André | |