hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN

PATCH Superseded Manikandan Elumalai 2 participants 4 messages Jun 8, 2020 – Jun 9, 2020 Hardware monitoring

Share & tools

Manikandan Elumalai

author

2020-06-08 10:43 UTC #0 lore raw

PATCH v3 1/1 hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN

The adm1278 temp attribute need it for openbmc platform . This feature not enabled by default, so PMON_CONFIG needs to enable it.

v3:
----
fix invalid signed-off.
removed checkpath warnings.
write ADM1278_TEMP1_EN and ADM1278_VOUT_EN conf in single line operation.

v2:
----
add Signed-off-by.
removed ADM1278_TEMP1_EN check.

Signed-off-by Manikandan Elumalai <[email protected]>

---
 drivers/hwmon/pmbus/adm1275.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/hwmon/pmbus/adm1275.c +4 -6

index 5caa37fb..4782e31 100644
@@ -666,11 +666,12 @@ static int adm1275_probe(struct i2c_client *client,
 		tindex = 3;
 
 		info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
-			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
+			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-		/* Enable VOUT if not enabled (it is disabled by default) */
+		/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
 		if (!(config & ADM1278_VOUT_EN)) {
-			config |= ADM1278_VOUT_EN;
+			config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
 			ret = i2c_smbus_write_byte_data(client,
 							ADM1275_PMON_CONFIG,
 							config);

@@ -681,9 +682,6 @@ static int adm1275_probe(struct i2c_client *client,
 			}
 		}
 
-		if (config & ADM1278_TEMP1_EN)
-			info->func[0] |=
-				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		if (config & ADM1278_VIN_EN)
 			info->func[0] |= PMBUS_HAVE_VIN;
 		break;

Signature · 1 line

2.7.4

Guenter Roeck

maintainer

2020-06-08 13:49 UTC #1 lore raw

Quoted 24 lines · Manikandan Elumalai

The adm1278 temp attribute need it for openbmc platform . This feature not enabled by default, so PMON_CONFIG needs to enable it.
v3:
----
fix invalid signed-off.
removed checkpath warnings.
write ADM1278_TEMP1_EN and ADM1278_VOUT_EN conf in single line operation.
v2:
----
add Signed-off-by.
removed ADM1278_TEMP1_EN check.
Signed-off-by Manikandan Elumalai <[email protected]>
---
 drivers/hwmon/pmbus/adm1275.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/hwmon/pmbus/adm1275.c +0 -0
index 5caa37fb..4782e31 100644

drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 673

Quoted 10 lines

drivers/hwmon/pmbus/adm1275.c +3 -2

@@ -666,11 +666,12 @@ static int adm1275_probe(struct i2c_client *client,
 		tindex = 3;
 
 		info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
-			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
+			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
-		/* Enable VOUT if not enabled (it is disabled by default) */
+		/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
 		if (!(config & ADM1278_VOUT_EN)) {

This if statement needs to be

		if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN)

Quoted 16 lines

drivers/hwmon/pmbus/adm1275.c +1 -4

-			config |= ADM1278_VOUT_EN;
+			config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
 			ret = i2c_smbus_write_byte_data(client,
 							ADM1275_PMON_CONFIG,
 							config);

@@ -681,9 +682,6 @@ static int adm1275_probe(struct i2c_client *client,
 			}
 		}
 
-		if (config & ADM1278_TEMP1_EN)
-			info->func[0] |=
-				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		if (config & ADM1278_VIN_EN)
 			info->func[0] |= PMBUS_HAVE_VIN;
 		break;

Manikandan

author

2020-06-09 15:26 UTC #2 lore raw

Quoted 32 lines · Guenter Roeck

Quoted 31 lines · Manikandan Elumalai
The adm1278 temp attribute need it for openbmc platform . This feature not enabled by default, so PMON_CONFIG needs to enable it.
v3:
----
fix invalid signed-off.
removed checkpath warnings.
write ADM1278_TEMP1_EN and ADM1278_VOUT_EN conf in single line operation.
v2:
----
add Signed-off-by.
removed ADM1278_TEMP1_EN check.
Signed-off-by Manikandan Elumalai <[email protected]>
---
 drivers/hwmon/pmbus/adm1275.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/hwmon/pmbus/adm1275.c +2 -1
index 5caa37fb..4782e31 100644
@@ -666,11 +666,12 @@ static int adm1275_probe(struct i2c_client *client,
 		tindex = 3;
 
 		info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
-			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
+			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 

Quoted 7 lines

drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 673
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +1 -1
-		/* Enable VOUT if not enabled (it is disabled by default) */
+		/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
 		if (!(config & ADM1278_VOUT_EN)) {
This if statement needs to be
		if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN)

Hi Guenter,

The below warning shown by checkpatch after changes,

WARNING: line over 80 characters
#38: FILE: drivers/hwmon/pmbus/adm1275.c:672:
+ if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN) {

total: 0 errors, 1 warnings, 24 lines checked

I didn't see any if() condition made as two line in the driver . Is this acceptable warning ?

Thanks
Mani.E

Quoted 17 lines

Quoted 16 lines

drivers/hwmon/pmbus/adm1275.c +1 -4

-			config |= ADM1278_VOUT_EN;
+			config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
 			ret = i2c_smbus_write_byte_data(client,
 							ADM1275_PMON_CONFIG,
 							config);

@@ -681,9 +682,6 @@ static int adm1275_probe(struct i2c_client *client,
 			}
 		}
 
-		if (config & ADM1278_TEMP1_EN)
-			info->func[0] |=
-				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		if (config & ADM1278_VIN_EN)
 			info->func[0] |= PMBUS_HAVE_VIN;
 		break;

Guenter Roeck

maintainer

2020-06-09 16:56 UTC #3 lore raw

Quoted 44 lines · Manikandan

Quoted 32 lines · Guenter Roeck
Quoted 31 lines · Manikandan Elumalai
The adm1278 temp attribute need it for openbmc platform . This feature not enabled by default, so PMON_CONFIG needs to enable it.
v3:
----
fix invalid signed-off.
removed checkpath warnings.
write ADM1278_TEMP1_EN and ADM1278_VOUT_EN conf in single line operation.
v2:
----
add Signed-off-by.
removed ADM1278_TEMP1_EN check.
Signed-off-by Manikandan Elumalai <[email protected]>
---
 drivers/hwmon/pmbus/adm1275.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/hwmon/pmbus/adm1275.c +2 -1
index 5caa37fb..4782e31 100644
@@ -666,11 +666,12 @@ static int adm1275_probe(struct i2c_client *client,
 		tindex = 3;
 
 		info->func[0] |= PMBUS_HAVE_PIN | PMBUS_HAVE_STATUS_INPUT |
-			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT;
+			PMBUS_HAVE_VOUT | PMBUS_HAVE_STATUS_VOUT |
+			PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 
Quoted 7 lines
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 673
Quoted 3 lines
drivers/hwmon/pmbus/adm1275.c +1 -1
-		/* Enable VOUT if not enabled (it is disabled by default) */
+		/* Enable VOUT & TEMP1 if not enabled (disabled by default) */
 		if (!(config & ADM1278_VOUT_EN)) {
This if statement needs to be
		if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN)
Hi Guenter,
The below warning shown by checkpatch after changes,

Quoted 8 lines

WARNING: line over 80 characters
#38: FILE: drivers/hwmon/pmbus/adm1275.c:672:
+ if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN) {
total: 0 errors, 1 warnings, 24 lines checked
I didn't see any if() condition made as two line in the driver . Is this acceptable warning ?

The warning is (or should be) gone in the latest upstream kernel - the line length limit is now 100 bytes. Just ignore that warning, or rebase your patch to the latest upstream kernel.

Guenter

Quoted 19 lines

Thanks
Mani.E

Quoted 17 lines

Quoted 16 lines

drivers/hwmon/pmbus/adm1275.c +1 -4

-			config |= ADM1278_VOUT_EN;
+			config |= ADM1278_VOUT_EN | ADM1278_TEMP1_EN;
 			ret = i2c_smbus_write_byte_data(client,
 							ADM1275_PMON_CONFIG,
 							config);

@@ -681,9 +682,6 @@ static int adm1275_probe(struct i2c_client *client,
 			}
 		}
 
-		if (config & ADM1278_TEMP1_EN)
-			info->func[0] |=
-				PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;
 		if (config & ADM1278_VIN_EN)
 			info->func[0] |= PMBUS_HAVE_VIN;
 		break;