| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <c4df3803-854a-8c88-f174-18eb98fed195@systec-electronic.com>
Date: Fri, 18 Aug 2023 07:06:03 +0200 (CEST)
From: Andre Werner <andre.werner@...tec-electronic.com>
To: Lee Jones <lee@...nel.org>
cc: werneazc@...il.com, lgirdwood@...il.com, broonie@...nel.org,
linux-kernel@...r.kernel.org,
Andre Werner <andre.werner@...tec-electronic.com>
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@...il.com wrote:
>
>> From: Andre Werner <andre.werner@...tec-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@...tec-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é
Powered by blists - more mailing lists