* [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map @ 2022-11-30 16:58 Naresh Solanki 2022-11-30 16:58 ` [PATCH v3 1/4] hwmon: (pmbus/core): Update " Naresh Solanki 2022-11-30 18:06 ` [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to " Guenter Roeck 0 siblings, 2 replies; 8+ messages in thread From: Naresh Solanki @ 2022-11-30 16:58 UTC (permalink / raw) To: devicetree, Guenter Roeck, Jean Delvare, Liam Girdwood, Mark Brown Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Naresh Solanki Add PMBus status byte to regulator flag map. Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> --- Changes: - Remove status input - Add comment for PMBUS status byte. --- drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c index 95e95783972a..a7b4ae0f1f3b 100644 --- a/drivers/hwmon/pmbus/pmbus_core.c +++ b/drivers/hwmon/pmbus/pmbus_core.c @@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category { }; static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { - { + { /* STATUS byte is always present. */ + .func = -1, + .reg = PMBUS_STATUS_BYTE, + .bits = (const struct pmbus_regulator_status_assoc[]) { + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, + { }, + }, + }, { .func = PMBUS_HAVE_STATUS_VOUT, .reg = PMBUS_STATUS_VOUT, .bits = (const struct pmbus_regulator_status_assoc[]) { @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { 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 }, + { PB_POUT_OP_FAULT, REGULATOR_ERROR_FAIL }, { }, }, }, { @@ -2834,14 +2844,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned if (status & PB_STATUS_POWER_GOOD_N) *flags |= REGULATOR_ERROR_REGULATION_OUT; } - /* - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are - * defined strictly as fault indicators (not warnings). - */ - if (status & PB_STATUS_IOUT_OC) - *flags |= REGULATOR_ERROR_OVER_CURRENT; - if (status & PB_STATUS_VOUT_OV) - *flags |= REGULATOR_ERROR_REGULATION_OUT; /* * If we haven't discovered any thermal faults or warnings via base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c -- 2.37.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3 1/4] hwmon: (pmbus/core): Update regulator flag map 2022-11-30 16:58 [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map Naresh Solanki @ 2022-11-30 16:58 ` Naresh Solanki 2022-11-30 18:09 ` Guenter Roeck 2022-11-30 18:06 ` [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to " Guenter Roeck 1 sibling, 1 reply; 8+ messages in thread From: Naresh Solanki @ 2022-11-30 16:58 UTC (permalink / raw) To: devicetree, Guenter Roeck, Jean Delvare, Liam Girdwood, Mark Brown Cc: linux-kernel, linux-hwmon, Patrick Rudolph, Naresh Solanki Add regulator flag map for PMBUS status byte & status input. Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> --- drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c index 95e95783972a..f5caceaaef2a 100644 --- a/drivers/hwmon/pmbus/pmbus_core.c +++ b/drivers/hwmon/pmbus/pmbus_core.c @@ -2752,6 +2752,15 @@ struct pmbus_regulator_status_category { static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { { + .func = -1, + .reg = PMBUS_STATUS_BYTE, + .bits = (const struct pmbus_regulator_status_assoc[]) { + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, + { }, + }, + }, { .func = PMBUS_HAVE_STATUS_VOUT, .reg = PMBUS_STATUS_VOUT, .bits = (const struct pmbus_regulator_status_assoc[]) { @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { 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 }, + { PB_POUT_OP_FAULT, REGULATOR_ERROR_OVER_CURRENT }, { }, }, }, { @@ -2778,6 +2788,18 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP }, { }, }, + }, { + .func = PMBUS_HAVE_STATUS_INPUT, + .reg = PMBUS_STATUS_INPUT, + .bits = (const struct pmbus_regulator_status_assoc[]) { + { PB_IIN_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT }, + { PB_IIN_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN }, + { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE }, + { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN }, + { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, + { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, + { }, + }, }, }; @@ -2834,14 +2856,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned if (status & PB_STATUS_POWER_GOOD_N) *flags |= REGULATOR_ERROR_REGULATION_OUT; } - /* - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are - * defined strictly as fault indicators (not warnings). - */ - if (status & PB_STATUS_IOUT_OC) - *flags |= REGULATOR_ERROR_OVER_CURRENT; - if (status & PB_STATUS_VOUT_OV) - *flags |= REGULATOR_ERROR_REGULATION_OUT; /* * If we haven't discovered any thermal faults or warnings via base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c -- 2.37.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Update regulator flag map 2022-11-30 16:58 ` [PATCH v3 1/4] hwmon: (pmbus/core): Update " Naresh Solanki @ 2022-11-30 18:09 ` Guenter Roeck 2022-11-30 18:15 ` Naresh Solanki 0 siblings, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2022-11-30 18:09 UTC (permalink / raw) To: Naresh Solanki Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph On Wed, Nov 30, 2022 at 05:58:29PM +0100, Naresh Solanki wrote: > Add regulator flag map for PMBUS status byte & status input. > Now you lost me. Two 1/4 patches with overlapping and different content ??? Guenter > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > --- > drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++-------- > 1 file changed, 22 insertions(+), 8 deletions(-) > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index 95e95783972a..f5caceaaef2a 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -2752,6 +2752,15 @@ struct pmbus_regulator_status_category { > > static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { > { > + .func = -1, > + .reg = PMBUS_STATUS_BYTE, > + .bits = (const struct pmbus_regulator_status_assoc[]) { > + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, > + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, > + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, > + { }, > + }, > + }, { > .func = PMBUS_HAVE_STATUS_VOUT, > .reg = PMBUS_STATUS_VOUT, > .bits = (const struct pmbus_regulator_status_assoc[]) { > @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = > { 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 }, > + { PB_POUT_OP_FAULT, REGULATOR_ERROR_OVER_CURRENT }, > { }, > }, > }, { > @@ -2778,6 +2788,18 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = > { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP }, > { }, > }, > + }, { > + .func = PMBUS_HAVE_STATUS_INPUT, > + .reg = PMBUS_STATUS_INPUT, > + .bits = (const struct pmbus_regulator_status_assoc[]) { > + { PB_IIN_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT }, > + { PB_IIN_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN }, > + { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE }, > + { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN }, > + { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, > + { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, > + { }, > + }, > }, > }; > > @@ -2834,14 +2856,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned > if (status & PB_STATUS_POWER_GOOD_N) > *flags |= REGULATOR_ERROR_REGULATION_OUT; > } > - /* > - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are > - * defined strictly as fault indicators (not warnings). > - */ > - if (status & PB_STATUS_IOUT_OC) > - *flags |= REGULATOR_ERROR_OVER_CURRENT; > - if (status & PB_STATUS_VOUT_OV) > - *flags |= REGULATOR_ERROR_REGULATION_OUT; > > /* > * If we haven't discovered any thermal faults or warnings via > > base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c > -- > 2.37.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Update regulator flag map 2022-11-30 18:09 ` Guenter Roeck @ 2022-11-30 18:15 ` Naresh Solanki 0 siblings, 0 replies; 8+ messages in thread From: Naresh Solanki @ 2022-11-30 18:15 UTC (permalink / raw) To: Guenter Roeck Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph Hi, On 30-11-2022 11:39 pm, Guenter Roeck wrote: > On Wed, Nov 30, 2022 at 05:58:29PM +0100, Naresh Solanki wrote: >> Add regulator flag map for PMBUS status byte & status input. >> > > Now you lost me. Two 1/4 patches with overlapping and different > content ??? > This patch wasn't suppose to be. will check what went wrong & avoid these overlap in future. > Guenter > >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> --- >> drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++-------- >> 1 file changed, 22 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c >> index 95e95783972a..f5caceaaef2a 100644 >> --- a/drivers/hwmon/pmbus/pmbus_core.c >> +++ b/drivers/hwmon/pmbus/pmbus_core.c >> @@ -2752,6 +2752,15 @@ struct pmbus_regulator_status_category { >> >> static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { >> { >> + .func = -1, >> + .reg = PMBUS_STATUS_BYTE, >> + .bits = (const struct pmbus_regulator_status_assoc[]) { >> + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, >> + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, >> + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, >> + { }, >> + }, >> + }, { >> .func = PMBUS_HAVE_STATUS_VOUT, >> .reg = PMBUS_STATUS_VOUT, >> .bits = (const struct pmbus_regulator_status_assoc[]) { >> @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = >> { 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 }, >> + { PB_POUT_OP_FAULT, REGULATOR_ERROR_OVER_CURRENT }, >> { }, >> }, >> }, { >> @@ -2778,6 +2788,18 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = >> { PB_TEMP_OT_FAULT, REGULATOR_ERROR_OVER_TEMP }, >> { }, >> }, >> + }, { >> + .func = PMBUS_HAVE_STATUS_INPUT, >> + .reg = PMBUS_STATUS_INPUT, >> + .bits = (const struct pmbus_regulator_status_assoc[]) { >> + { PB_IIN_OC_FAULT, REGULATOR_ERROR_OVER_CURRENT }, >> + { PB_IIN_OC_WARNING, REGULATOR_ERROR_OVER_CURRENT_WARN }, >> + { PB_VOLTAGE_UV_FAULT, REGULATOR_ERROR_UNDER_VOLTAGE }, >> + { PB_VOLTAGE_UV_WARNING, REGULATOR_ERROR_UNDER_VOLTAGE_WARN }, >> + { PB_VOLTAGE_OV_WARNING, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, >> + { PB_VOLTAGE_OV_FAULT, REGULATOR_ERROR_OVER_VOLTAGE_WARN }, >> + { }, >> + }, >> }, >> }; >> >> @@ -2834,14 +2856,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned >> if (status & PB_STATUS_POWER_GOOD_N) >> *flags |= REGULATOR_ERROR_REGULATION_OUT; >> } >> - /* >> - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are >> - * defined strictly as fault indicators (not warnings). >> - */ >> - if (status & PB_STATUS_IOUT_OC) >> - *flags |= REGULATOR_ERROR_OVER_CURRENT; >> - if (status & PB_STATUS_VOUT_OV) >> - *flags |= REGULATOR_ERROR_REGULATION_OUT; >> >> /* >> * If we haven't discovered any thermal faults or warnings via >> >> base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c >> -- >> 2.37.3 >> Regards, Naresh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map 2022-11-30 16:58 [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map Naresh Solanki 2022-11-30 16:58 ` [PATCH v3 1/4] hwmon: (pmbus/core): Update " Naresh Solanki @ 2022-11-30 18:06 ` Guenter Roeck 2022-11-30 18:12 ` Naresh Solanki 1 sibling, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2022-11-30 18:06 UTC (permalink / raw) To: Naresh Solanki Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph On Wed, Nov 30, 2022 at 05:58:28PM +0100, Naresh Solanki wrote: > Add PMBus status byte to regulator flag map. > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > > --- > Changes: > - Remove status input Not really. > - Add comment for PMBUS status byte. > --- > drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++--------- > 1 file changed, 11 insertions(+), 9 deletions(-) > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > index 95e95783972a..a7b4ae0f1f3b 100644 > --- a/drivers/hwmon/pmbus/pmbus_core.c > +++ b/drivers/hwmon/pmbus/pmbus_core.c > @@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category { > }; > > static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { > - { > + { /* STATUS byte is always present. */ > + .func = -1, > + .reg = PMBUS_STATUS_BYTE, > + .bits = (const struct pmbus_regulator_status_assoc[]) { > + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, > + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, > + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, Still there. > + { }, > + }, > + }, { > .func = PMBUS_HAVE_STATUS_VOUT, > .reg = PMBUS_STATUS_VOUT, > .bits = (const struct pmbus_regulator_status_assoc[]) { > @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = > { 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 }, > + { PB_POUT_OP_FAULT, REGULATOR_ERROR_FAIL }, Please document your changes in the description. There are two functional changes hidden under "Add PMBus status byte to regulator flag map". That is inappropriate. Those, as I have pointed out, should be separate patches. Guenter > { }, > }, > }, { > @@ -2834,14 +2844,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned > if (status & PB_STATUS_POWER_GOOD_N) > *flags |= REGULATOR_ERROR_REGULATION_OUT; > } > - /* > - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are > - * defined strictly as fault indicators (not warnings). > - */ > - if (status & PB_STATUS_IOUT_OC) > - *flags |= REGULATOR_ERROR_OVER_CURRENT; > - if (status & PB_STATUS_VOUT_OV) > - *flags |= REGULATOR_ERROR_REGULATION_OUT; > > /* > * If we haven't discovered any thermal faults or warnings via > > base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c > -- > 2.37.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map 2022-11-30 18:06 ` [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to " Guenter Roeck @ 2022-11-30 18:12 ` Naresh Solanki 2022-11-30 18:19 ` Guenter Roeck 0 siblings, 1 reply; 8+ messages in thread From: Naresh Solanki @ 2022-11-30 18:12 UTC (permalink / raw) To: Guenter Roeck Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph Hi, On 30-11-2022 11:36 pm, Guenter Roeck wrote: > On Wed, Nov 30, 2022 at 05:58:28PM +0100, Naresh Solanki wrote: >> Add PMBus status byte to regulator flag map. >> >> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >> >> --- >> Changes: >> - Remove status input > > Not really. > This was about PMBUS_STATUS_INPUT & not STATUS_BYTE >> - Add comment for PMBUS status byte. >> --- >> drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++--------- >> 1 file changed, 11 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c >> index 95e95783972a..a7b4ae0f1f3b 100644 >> --- a/drivers/hwmon/pmbus/pmbus_core.c >> +++ b/drivers/hwmon/pmbus/pmbus_core.c >> @@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category { >> }; >> >> static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { >> - { >> + { /* STATUS byte is always present. */ >> + .func = -1, >> + .reg = PMBUS_STATUS_BYTE, >> + .bits = (const struct pmbus_regulator_status_assoc[]) { >> + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, >> + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, >> + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, > > Still there. > STATUS_INPUT remove & STATUS_BYTE retained. >> + { }, >> + }, >> + }, { >> .func = PMBUS_HAVE_STATUS_VOUT, >> .reg = PMBUS_STATUS_VOUT, >> .bits = (const struct pmbus_regulator_status_assoc[]) { >> @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = >> { 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 }, >> + { PB_POUT_OP_FAULT, REGULATOR_ERROR_FAIL }, > > Please document your changes in the description. There are two functional > changes hidden under "Add PMBus status byte to regulator flag map". > That is inappropriate. Those, as I have pointed out, should be separate > patches. My bad. I kept this change in this patch. Will remove this from this patch. > > Guenter > >> { }, >> }, >> }, { >> @@ -2834,14 +2844,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned >> if (status & PB_STATUS_POWER_GOOD_N) >> *flags |= REGULATOR_ERROR_REGULATION_OUT; >> } >> - /* >> - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are >> - * defined strictly as fault indicators (not warnings). >> - */ >> - if (status & PB_STATUS_IOUT_OC) >> - *flags |= REGULATOR_ERROR_OVER_CURRENT; >> - if (status & PB_STATUS_VOUT_OV) >> - *flags |= REGULATOR_ERROR_REGULATION_OUT; >> >> /* >> * If we haven't discovered any thermal faults or warnings via >> >> base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c >> -- >> 2.37.3 >> Regards, Naresh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map 2022-11-30 18:12 ` Naresh Solanki @ 2022-11-30 18:19 ` Guenter Roeck 2022-11-30 18:25 ` Naresh Solanki 0 siblings, 1 reply; 8+ messages in thread From: Guenter Roeck @ 2022-11-30 18:19 UTC (permalink / raw) To: Naresh Solanki Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph On Wed, Nov 30, 2022 at 11:42:32PM +0530, Naresh Solanki wrote: > Hi, > > On 30-11-2022 11:36 pm, Guenter Roeck wrote: > > On Wed, Nov 30, 2022 at 05:58:28PM +0100, Naresh Solanki wrote: > > > Add PMBus status byte to regulator flag map. > > > > > > Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> > > > > > > --- > > > Changes: > > > - Remove status input > > > > Not really. > > > This was about PMBUS_STATUS_INPUT & not STATUS_BYTE > > > - Add comment for PMBUS status byte. > > > --- > > > drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++--------- > > > 1 file changed, 11 insertions(+), 9 deletions(-) > > > > > > diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c > > > index 95e95783972a..a7b4ae0f1f3b 100644 > > > --- a/drivers/hwmon/pmbus/pmbus_core.c > > > +++ b/drivers/hwmon/pmbus/pmbus_core.c > > > @@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category { > > > }; > > > static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { > > > - { > > > + { /* STATUS byte is always present. */ > > > + .func = -1, > > > + .reg = PMBUS_STATUS_BYTE, > > > + .bits = (const struct pmbus_regulator_status_assoc[]) { > > > + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, > > > + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, > > > + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, > > > > Still there. > > > STATUS_INPUT remove & STATUS_BYTE retained. You added a check for PB_STATUS_VIN_UV which reports an input failure which was not reported before. You do so silently without mentioning the change in the description. In the change log you claim "Remove status input". Yet, the above adds reporting on an input voltage error. Maybe the change log was supposed to refer to the input status register, but that doesn't mean you can silently add a check for PB_STATUS_VIN_UV. Guenter > > > + { }, > > > + }, > > > + }, { > > > .func = PMBUS_HAVE_STATUS_VOUT, > > > .reg = PMBUS_STATUS_VOUT, > > > .bits = (const struct pmbus_regulator_status_assoc[]) { > > > @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = > > > { 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 }, > > > + { PB_POUT_OP_FAULT, REGULATOR_ERROR_FAIL }, > > > > Please document your changes in the description. There are two functional > > changes hidden under "Add PMBus status byte to regulator flag map". > > That is inappropriate. Those, as I have pointed out, should be separate > > patches. > My bad. I kept this change in this patch. Will remove this from this patch. > > > > Guenter > > > > > { }, > > > }, > > > }, { > > > @@ -2834,14 +2844,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned > > > if (status & PB_STATUS_POWER_GOOD_N) > > > *flags |= REGULATOR_ERROR_REGULATION_OUT; > > > } > > > - /* > > > - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are > > > - * defined strictly as fault indicators (not warnings). > > > - */ > > > - if (status & PB_STATUS_IOUT_OC) > > > - *flags |= REGULATOR_ERROR_OVER_CURRENT; > > > - if (status & PB_STATUS_VOUT_OV) > > > - *flags |= REGULATOR_ERROR_REGULATION_OUT; > > > /* > > > * If we haven't discovered any thermal faults or warnings via > > > > > > base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c > > > -- > > > 2.37.3 > > > > > Regards, > Naresh ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map 2022-11-30 18:19 ` Guenter Roeck @ 2022-11-30 18:25 ` Naresh Solanki 0 siblings, 0 replies; 8+ messages in thread From: Naresh Solanki @ 2022-11-30 18:25 UTC (permalink / raw) To: Guenter Roeck Cc: devicetree, Jean Delvare, Liam Girdwood, Mark Brown, linux-kernel, linux-hwmon, Patrick Rudolph Hi, On 30-11-2022 11:49 pm, Guenter Roeck wrote: > On Wed, Nov 30, 2022 at 11:42:32PM +0530, Naresh Solanki wrote: >> Hi, >> >> On 30-11-2022 11:36 pm, Guenter Roeck wrote: >>> On Wed, Nov 30, 2022 at 05:58:28PM +0100, Naresh Solanki wrote: >>>> Add PMBus status byte to regulator flag map. >>>> >>>> Signed-off-by: Naresh Solanki <Naresh.Solanki@9elements.com> >>>> >>>> --- >>>> Changes: >>>> - Remove status input >>> >>> Not really. >>> >> This was about PMBUS_STATUS_INPUT & not STATUS_BYTE >>>> - Add comment for PMBUS status byte. >>>> --- >>>> drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++--------- >>>> 1 file changed, 11 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c >>>> index 95e95783972a..a7b4ae0f1f3b 100644 >>>> --- a/drivers/hwmon/pmbus/pmbus_core.c >>>> +++ b/drivers/hwmon/pmbus/pmbus_core.c >>>> @@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category { >>>> }; >>>> static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = { >>>> - { >>>> + { /* STATUS byte is always present. */ >>>> + .func = -1, >>>> + .reg = PMBUS_STATUS_BYTE, >>>> + .bits = (const struct pmbus_regulator_status_assoc[]) { >>>> + { PB_STATUS_IOUT_OC, REGULATOR_ERROR_OVER_CURRENT }, >>>> + { PB_STATUS_VOUT_OV, REGULATOR_ERROR_REGULATION_OUT }, >>>> + { PB_STATUS_VIN_UV, REGULATOR_ERROR_UNDER_VOLTAGE }, >>> >>> Still there. >>> >> STATUS_INPUT remove & STATUS_BYTE retained. > > You added a check for PB_STATUS_VIN_UV which reports an input failure > which was not reported before. You do so silently without mentioning the > change in the description. In the change log you claim "Remove status input". > Yet, the above adds reporting on an input voltage error. Maybe the > change log was supposed to refer to the input status register, but that > doesn't mean you can silently add a check for PB_STATUS_VIN_UV. Right. I removed STATUS_INPUT but overlooked VIN in STATUS_BYTE. Will fix this in next revision. > > Guenter > >>>> + { }, >>>> + }, >>>> + }, { >>>> .func = PMBUS_HAVE_STATUS_VOUT, >>>> .reg = PMBUS_STATUS_VOUT, >>>> .bits = (const struct pmbus_regulator_status_assoc[]) { >>>> @@ -2768,6 +2777,7 @@ static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = >>>> { 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 }, >>>> + { PB_POUT_OP_FAULT, REGULATOR_ERROR_FAIL }, >>> >>> Please document your changes in the description. There are two functional >>> changes hidden under "Add PMBus status byte to regulator flag map". >>> That is inappropriate. Those, as I have pointed out, should be separate >>> patches. >> My bad. I kept this change in this patch. Will remove this from this patch. >>> >>> Guenter >>> >>>> { }, >>>> }, >>>> }, { >>>> @@ -2834,14 +2844,6 @@ static int pmbus_regulator_get_error_flags(struct regulator_dev *rdev, unsigned >>>> if (status & PB_STATUS_POWER_GOOD_N) >>>> *flags |= REGULATOR_ERROR_REGULATION_OUT; >>>> } >>>> - /* >>>> - * Unlike most other status bits, PB_STATUS_{IOUT_OC,VOUT_OV} are >>>> - * defined strictly as fault indicators (not warnings). >>>> - */ >>>> - if (status & PB_STATUS_IOUT_OC) >>>> - *flags |= REGULATOR_ERROR_OVER_CURRENT; >>>> - if (status & PB_STATUS_VOUT_OV) >>>> - *flags |= REGULATOR_ERROR_REGULATION_OUT; >>>> /* >>>> * If we haven't discovered any thermal faults or warnings via >>>> >>>> base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c >>>> -- >>>> 2.37.3 >>>> >> >> Regards, >> Naresh Regards, Naresh ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-11-30 18:25 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-30 16:58 [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to regulator flag map Naresh Solanki 2022-11-30 16:58 ` [PATCH v3 1/4] hwmon: (pmbus/core): Update " Naresh Solanki 2022-11-30 18:09 ` Guenter Roeck 2022-11-30 18:15 ` Naresh Solanki 2022-11-30 18:06 ` [PATCH v3 1/4] hwmon: (pmbus/core): Add status byte to " Guenter Roeck 2022-11-30 18:12 ` Naresh Solanki 2022-11-30 18:19 ` Guenter Roeck 2022-11-30 18:25 ` Naresh Solanki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).