Re: [PATCH] hwmon: (pmbus/core): Generalize pmbus status flag map

[Naresh Solanki <naresh.solanki@9elements.com>]

Hi Guenter,

On 18-01-2023 07:13 pm, Guenter Roeck wrote:
> On 1/18/23 03:15, Naresh Solanki wrote:
>> Move the PMBus status flag map (pmbus_regulator_status_flag_map)
>> outside of the regulator #if block and update the associated
>> variable/struct name to reflect a generic PMBus status. Also retain
>> the regulator error flag for use in determining regulator specific error.
>>
>> This will make the PMBus status flag map more versatile and easier to
>> incorporate into different contexts and functions.
>>
>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com>
> 
> Sorry, I don't see the point of moving a structure including
> regulator error codes outside regulator code.
I'm working on adding pmbus irq handler.
Since irq handler is regulator independent, this change is needed.

Naresh
> 
> Guenter
> 
>> ---
>>   drivers/hwmon/pmbus/pmbus_core.c | 94 ++++++++++++++++----------------
>>   1 file changed, 47 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c 
>> b/drivers/hwmon/pmbus/pmbus_core.c
>> index 95e95783972a..1b70cf3be313 100644
>> --- a/drivers/hwmon/pmbus/pmbus_core.c
>> +++ b/drivers/hwmon/pmbus/pmbus_core.c
>> @@ -2692,6 +2692,49 @@ static int pmbus_init_common(struct i2c_client 
>> *client, struct pmbus_data *data,
>>       return 0;
>>   }
>> +/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
>> +struct pmbus_status_assoc {
>> +    int pflag, rflag;
>> +};
>> +
>> +/* PMBus->regulator bit mappings for a PMBus status register */
>> +struct pmbus_status_category {
>> +    int func;
>> +    int reg;
>> +    const struct pmbus_status_assoc *bits; /* zero-terminated */
>> +};
>> +
>> +static const struct pmbus_status_category __maybe_unused 
>> pmbus_status_flag_map[] = {
>> +    {
>> +        .func = PMBUS_HAVE_STATUS_VOUT,
>> +        .reg = PMBUS_STATUS_VOUT,
>> +        .bits = (const struct pmbus_status_assoc[]) {
>> +            { PB_VOLTAGE_UV_WARNING, 
>> REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
>> +            { PB_VOLTAGE_UV_FAULT,   REGULATOR_ERROR_UNDER_VOLTAGE },
>> +            { PB_VOLTAGE_OV_WARNING, 
>> REGULATOR_ERROR_OVER_VOLTAGE_WARN },
>> +            { PB_VOLTAGE_OV_FAULT,   REGULATOR_ERROR_REGULATION_OUT },
>> +            { },
>> +        },
>> +    }, {
>> +        .func = PMBUS_HAVE_STATUS_IOUT,
>> +        .reg = PMBUS_STATUS_IOUT,
>> +        .bits = (const struct pmbus_status_assoc[]) {
>> +            { PB_IOUT_OC_WARNING,    
>> REGULATOR_ERROR_OVER_CURRENT_WARN },
>> +            { PB_IOUT_OC_FAULT,      REGULATOR_ERROR_OVER_CURRENT },
>> +            { PB_IOUT_OC_LV_FAULT,   REGULATOR_ERROR_OVER_CURRENT },
>> +            { },
>> +        },
>> +    }, {
>> +        .func = PMBUS_HAVE_STATUS_TEMP,
>> +        .reg = PMBUS_STATUS_TEMPERATURE,
>> +        .bits = (const struct pmbus_status_assoc[]) {
>> +            { PB_TEMP_OT_WARNING,    REGULATOR_ERROR_OVER_TEMP_WARN },
>> +            { PB_TEMP_OT_FAULT,      REGULATOR_ERROR_OVER_TEMP },
>> +            { },
>> +        },
>> +    },
>> +};
>> +
>>   #if IS_ENABLED(CONFIG_REGULATOR)
>>   static int pmbus_regulator_is_enabled(struct regulator_dev *rdev)
>>   {
>> @@ -2738,54 +2781,11 @@ static int pmbus_regulator_disable(struct 
>> regulator_dev *rdev)
>>       return _pmbus_regulator_on_off(rdev, 0);
>>   }
>> -/* A PMBus status flag and the corresponding REGULATOR_ERROR_* flag */
>> -struct pmbus_regulator_status_assoc {
>> -    int pflag, rflag;
>> -};
>> -
>> -/* PMBus->regulator bit mappings for a PMBus status register */
>> -struct pmbus_regulator_status_category {
>> -    int func;
>> -    int reg;
>> -    const struct pmbus_regulator_status_assoc *bits; /* 
>> zero-terminated */
>> -};
>> -
>> -static const struct pmbus_regulator_status_category 
>> pmbus_regulator_flag_map[] = {
>> -    {
>> -        .func = PMBUS_HAVE_STATUS_VOUT,
>> -        .reg = PMBUS_STATUS_VOUT,
>> -        .bits = (const struct pmbus_regulator_status_assoc[]) {
>> -            { PB_VOLTAGE_UV_WARNING, 
>> REGULATOR_ERROR_UNDER_VOLTAGE_WARN },
>> -            { PB_VOLTAGE_UV_FAULT,   REGULATOR_ERROR_UNDER_VOLTAGE },
>> -            { PB_VOLTAGE_OV_WARNING, 
>> REGULATOR_ERROR_OVER_VOLTAGE_WARN },
>> -            { PB_VOLTAGE_OV_FAULT,   REGULATOR_ERROR_REGULATION_OUT },
>> -            { },
>> -        },
>> -    }, {
>> -        .func = PMBUS_HAVE_STATUS_IOUT,
>> -        .reg = PMBUS_STATUS_IOUT,
>> -        .bits = (const struct pmbus_regulator_status_assoc[]) {
>> -            { PB_IOUT_OC_WARNING,    
>> REGULATOR_ERROR_OVER_CURRENT_WARN },
>> -            { PB_IOUT_OC_FAULT,      REGULATOR_ERROR_OVER_CURRENT },
>> -            { PB_IOUT_OC_LV_FAULT,   REGULATOR_ERROR_OVER_CURRENT },
>> -            { },
>> -        },
>> -    }, {
>> -        .func = PMBUS_HAVE_STATUS_TEMP,
>> -        .reg = PMBUS_STATUS_TEMPERATURE,
>> -        .bits = (const struct pmbus_regulator_status_assoc[]) {
>> -            { PB_TEMP_OT_WARNING,    REGULATOR_ERROR_OVER_TEMP_WARN },
>> -            { PB_TEMP_OT_FAULT,      REGULATOR_ERROR_OVER_TEMP },
>> -            { },
>> -        },
>> -    },
>> -};
>> -
>>   static int pmbus_regulator_get_error_flags(struct regulator_dev 
>> *rdev, unsigned int *flags)
>>   {
>>       int i, status;
>> -    const struct pmbus_regulator_status_category *cat;
>> -    const struct pmbus_regulator_status_assoc *bit;
>> +    const struct pmbus_status_category *cat;
>> +    const struct pmbus_status_assoc *bit;
>>       struct device *dev = rdev_get_dev(rdev);
>>       struct i2c_client *client = to_i2c_client(dev->parent);
>>       struct pmbus_data *data = i2c_get_clientdata(client);
>> @@ -2796,8 +2796,8 @@ static int 
>> pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned
>>       mutex_lock(&data->update_lock);
>> -    for (i = 0; i < ARRAY_SIZE(pmbus_regulator_flag_map); i++) {
>> -        cat = &pmbus_regulator_flag_map[i];
>> +    for (i = 0; i < ARRAY_SIZE(pmbus_status_flag_map); i++) {
>> +        cat = &pmbus_status_flag_map[i];
>>           if (!(func & cat->func))
>>               continue;
>>
>> base-commit: 774dccfe77dcd8cf1d82df1f61fe95a720dc76e4
>