hwmon: (pmbus/core): Add status byte to regulator flag map

PATCH Superseded Naresh Solanki 2 participants 8 messages Nov 30, 2022 Hardware monitoring

lore

Share & tools

lore

Naresh Solanki

author

2022-11-30 17:03 UTC #0 lore raw

PATCH v3 1/4 hwmon: (pmbus/core): Add status byte to regulator flag map

Add PMBus status byte to regulator flag map.

Signed-off-by Naresh Solanki <[email protected]>

---
Changes:
- Remove status input
- Add comment for PMBUS status byte.

---
 drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +11 -9

index 95e95783972a..a7b4ae0f1f3b 100644
@@ -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

Signature · 1 line

2.37.3

Naresh Solanki

author

2022-11-30 17:03 UTC #1 lore raw

PATCH v3 1/4 hwmon: (pmbus/core): Update regulator flag map

Add regulator flag map for PMBUS status byte & status input.

Signed-off-by Naresh Solanki <[email protected]>

---
 drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +22 -8

index 95e95783972a..f5caceaaef2a 100644
@@ -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

Signature · 1 line

2.37.3

Guenter Roeck

maintainer

2022-11-30 18:09 UTC #2 lore raw

Re: [PATCH v3 1/4] hwmon: (pmbus/core): Update regulator flag map

Quoted 2 lines · Naresh Solanki

Add regulator flag map for PMBUS status byte & status input.

Now you lost me. Two 1/4 patches with overlapping and different content ???

Guenter

Quoted 72 lines

Signed-off-by Naresh Solanki <[email protected]>

---
 drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +22 -8

index 95e95783972a..f5caceaaef2a 100644
@@ -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

Signature · 1 line

2.37.3

Naresh Solanki

author

2022-11-30 18:18 UTC #3 lore raw

Re: [PATCH v3 1/4] hwmon: (pmbus/core): Update regulator flag map

Hi,

Quoted 7 lines · Guenter Roeck

Quoted 2 lines · Naresh Solanki
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.

Quoted 67 lines

Guenter

Quoted 65 lines

Signed-off-by Naresh Solanki <[email protected]>

---
  drivers/hwmon/pmbus/pmbus_core.c | 30 ++++++++++++++++++++++--------
  1 file changed, 22 insertions(+), 8 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +22 -8

index 95e95783972a..f5caceaaef2a 100644
@@ -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;

Quoted 7 lines

Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -0
  	/*
  	 * If we haven't discovered any thermal faults or warnings via
base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c
Signature · 1 line
2.37.3

Regards,
Naresh

Guenter Roeck

maintainer

2022-11-30 18:06 UTC #4 lore raw

In reply to Naresh Solanki

Quoted 7 lines · Naresh Solanki

Add PMBus status byte to regulator flag map.
Signed-off-by Naresh Solanki <[email protected]>
---
Changes:
- Remove status input

Not really.

Quoted 11 lines

- Add comment for PMBUS status byte.

---
 drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +0 -0

index 95e95783972a..a7b4ae0f1f3b 100644
@@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category {
 };

drivers/hwmon/pmbus/pmbus_core.c struct pmbus_regulator_status_category { · line 2760

Quoted 10 lines

drivers/hwmon/pmbus/pmbus_core.c +7 -1

 
 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.

drivers/hwmon/pmbus/pmbus_core.c static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = · line 2780

Quoted 11 lines

drivers/hwmon/pmbus/pmbus_core.c +4 -0

+			{ },
+		},
+	}, {
 		.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

Quoted 23 lines

drivers/hwmon/pmbus/pmbus_core.c +0 -8

 			{ },
 		},
 	}, {

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

Signature · 1 line

2.37.3

Naresh Solanki

author

2022-11-30 18:12 UTC #5 lore raw

Hi,

Quoted 11 lines · Guenter Roeck

Quoted 7 lines · Naresh Solanki
Add PMBus status byte to regulator flag map.
Signed-off-by Naresh Solanki <[email protected]>
---
Changes:
- Remove status input
Not really.

This was about PMBUS_STATUS_INPUT & not STATUS_BYTE

Quoted 12 lines

- Add comment for PMBUS status byte.

---
  drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++---------
  1 file changed, 11 insertions(+), 9 deletions(-)

drivers/hwmon/pmbus/pmbus_core.c +0 -0

index 95e95783972a..a7b4ae0f1f3b 100644
@@ -2751,7 +2751,16 @@ struct pmbus_regulator_status_category {
  };

Quoted 12 lines

drivers/hwmon/pmbus/pmbus_core.c struct pmbus_regulator_status_category { · line 2760

Quoted 9 lines

drivers/hwmon/pmbus/pmbus_core.c +7 -1

  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.

Quoted 8 lines

drivers/hwmon/pmbus/pmbus_core.c +3 -0

+			{ },
+		},
+	}, {
  		.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 },

Quoted 8 lines

drivers/hwmon/pmbus/pmbus_core.c static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = · line 2780
Quoted 3 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -0
  			{ 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.

Quoted 19 lines

Guenter

Quoted 16 lines

drivers/hwmon/pmbus/pmbus_core.c +0 -8

  			{ },
  		},
  	}, {

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

Quoted 7 lines

Quoted 7 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -0
  	/*
  	 * If we haven't discovered any thermal faults or warnings via
base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c
Signature · 1 line
2.37.3

Regards,
Naresh

Guenter Roeck

maintainer

2022-11-30 18:21 UTC #6 lore raw

Quoted 38 lines · Naresh Solanki

Hi,
Quoted 11 lines · Guenter Roeck
Quoted 7 lines · Naresh Solanki
Add PMBus status byte to regulator flag map.
Signed-off-by Naresh Solanki <[email protected]>
---
Changes:
- Remove status input
Not really.
This was about PMBUS_STATUS_INPUT & not STATUS_BYTE
Quoted 14 lines
Quoted 14 lines
- Add comment for PMBUS status byte.
---
  drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++---------
  1 file changed, 11 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +1 -1
index 95e95783972a..a7b4ae0f1f3b 100644
@@ -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. */
Quoted 9 lines
drivers/hwmon/pmbus/pmbus_core.c struct pmbus_regulator_status_category { · line 2760
Quoted 6 lines
drivers/hwmon/pmbus/pmbus_core.c +6 -0
+		.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.

Quoted 1 line

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

Quoted 45 lines

Quoted 8 lines
Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -0
+			{ },
+		},
+	}, {
  		.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 },
Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = · line 2780
Quoted 3 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -0
  			{ 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.
Quoted 21 lines
Guenter
Quoted 18 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -8
  			{ },
  		},
  	}, {
@@ -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
 
Quoted 4 lines
Quoted 4 lines
base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c
Signature · 1 line
2.37.3
Regards,
Naresh

Naresh Solanki

author

2022-11-30 18:25 UTC #7 lore raw

Hi,

Quoted 39 lines · Guenter Roeck

Quoted 38 lines · Naresh Solanki
Hi,
Quoted 11 lines · Guenter Roeck
Quoted 7 lines · Naresh Solanki
Add PMBus status byte to regulator flag map.
Signed-off-by Naresh Solanki <[email protected]>
---
Changes:
- Remove status input
Not really.
This was about PMBUS_STATUS_INPUT & not STATUS_BYTE
Quoted 23 lines
Quoted 10 lines
- Add comment for PMBUS status byte.
---
   drivers/hwmon/pmbus/pmbus_core.c | 20 +++++++++++---------
   1 file changed, 11 insertions(+), 9 deletions(-)
drivers/hwmon/pmbus/pmbus_core.c +0 -0
index 95e95783972a..a7b4ae0f1f3b 100644
drivers/hwmon/pmbus/pmbus_core.c struct pmbus_regulator_status_category { · line 2759
Quoted 10 lines
drivers/hwmon/pmbus/pmbus_core.c +7 -1
@@ -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.

Quoted 8 lines

Quoted 1 line
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.

Quoted 48 lines

Guenter
Quoted 45 lines
Quoted 8 lines
Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c +3 -0
+			{ },
+		},
+	}, {
   		.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 },
Quoted 8 lines
drivers/hwmon/pmbus/pmbus_core.c static const struct pmbus_regulator_status_category pmbus_regulator_flag_map[] = · line 2780
Quoted 3 lines
drivers/hwmon/pmbus/pmbus_core.c +1 -0
   			{ 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.
Quoted 21 lines
Guenter
Quoted 18 lines
drivers/hwmon/pmbus/pmbus_core.c +0 -8
   			{ },
   		},
   	}, {
@@ -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
 
Quoted 4 lines
Quoted 4 lines
base-commit: 9494c53e1389b120ba461899207ac8a3aab2632c
Signature · 1 line
2.37.3
Regards,
Naresh

Regards,
Naresh