From: Andre Werner <[email protected]>
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 <[email protected]>
---
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;
+ 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
On Wed, 09 Aug 2023, [email protected] wrote:
> From: Andre Werner <[email protected]>
>
> 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 <[email protected]>
> ---
> 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?
> + 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 [李琼斯]
On Thu, 17 Aug 2023, Lee Jones wrote:
> On Wed, 09 Aug 2023, [email protected] wrote:
>
>> From: Andre Werner <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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é