diff mbox series

hwmon: (pmbus/core): Generalize pmbus status flag map

Message ID 20230118111536.3276540-1-Naresh.Solanki@9elements.com (mailing list archive)
State Changes Requested
Headers show
Series hwmon: (pmbus/core): Generalize pmbus status flag map | expand

Commit Message

Naresh Solanki Jan. 18, 2023, 11:15 a.m. UTC 
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>
---
 drivers/hwmon/pmbus/pmbus_core.c | 94 ++++++++++++++++----------------
 1 file changed, 47 insertions(+), 47 deletions(-)


base-commit: 774dccfe77dcd8cf1d82df1f61fe95a720dc76e4

Comments

Guenter Roeck Jan. 18, 2023, 1:43 p.m. UTC | #1 
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.

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
Naresh Solanki Jan. 18, 2023, 2:36 p.m. UTC | #2 
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
>
Guenter Roeck Jan. 18, 2023, 4:24 p.m. UTC | #3 
On 1/18/23 06:36, Naresh Solanki wrote:
> 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.
> 

Please submit the necessary patches together. I can not accept patches
introducing structural changes like this without use case.

Thanks,
Guenter
diff mbox series

Patch

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;