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

[Naresh Solanki <naresh.solanki@xxxxxxxxxxxxx>]

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@xxxxxxxxxxxxx>
>
> 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