hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
PATCH v2 1/1 hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, so PMON_CONFIG needs to be modified in order to enable it.
v2:
- Add Signed-off-by.
- Removed ADM1278_TEMP1_EN check.
--- drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)
drivers/hwmon/pmbus/adm1275.c +17 -4
@@ -666,7 +666,23 @@ 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 ; + + config =i2c_smbus_read_byte_data (client,ADM1275_PMON_CONFIG ); +if (config <0 ) +return config; + +/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config); +if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {
@@ -681,9 +697,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
maintainerQuoted 4 lines · Manikandan Elumalai
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, soPMON_CONFIGneeds to be modified in order to enable it.Signed-off-by Manikandan Elumalai <[email protected]>
This is not valid.
Quoted 4 lines
v2:
- Add Signed-off-by.
- RemovedADM1278_TEMP1_ENcheck.
checkpatch reports:
Quoted 1 line
---WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
CHECK: Alignment should match open parenthesis
#45: FILE: drivers/hwmon/pmbus/adm1275.c:679:
+ ret = i2c_smbus_write_byte_data(client,
+ ADM1275_PMON_CONFIG,
WARNING: suspect code indent for conditional statements (16, 16) #47: FILE: drivers/hwmon/pmbus/adm1275.c:681:
+ if (ret < 0) {
+ dev_err(&client->dev,
ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 2 warnings, 1 checks, 33 lines checked
Please follow published guidelines when submitting patches.
Quoted 19 lines
drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)drivers/hwmon/pmbus/adm1275.c +7
-1@@ -666,7 +666,23 @@ 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 ; + + config =i2c_smbus_read_byte_data (client,ADM1275_PMON_CONFIG ); +if (config <0 ) +return config; +
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 10 lines
drivers/hwmon/pmbus/adm1275.c +10
-0+/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config); +if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }
This can be handled in a single operation, together with ADM1278_VOUT_EN below. There is no need for two separate write operations.
Guenter
Quoted 14 lines
drivers/hwmon/pmbus/adm1275.c +0
-3/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,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 ;
Hi Guenter,
Thanks for the initial look at this.
One question for you below...
Quoted 14 lines · Guenter Roeck
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 10 lines · Manikandan Elumalai
drivers/hwmon/pmbus/adm1275.c +10
-0+/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config); +if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }This can be handled in a single operation, together with
ADM1278_VOUT_ENbelow. There is no need for two separate write operations.
I don't know if you noticed here but the change ends up enabling TEMP1_EN in all cases. Is this acceptable? If not, do you have any preference on how it is selected for enablement?
Quoted 14 lines
Quoted 13 lines
drivers/hwmon/pmbus/adm1275.c +0
-3/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,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 · 8 lines
Patrick Williams -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 833 bytes Desc: not available URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20200529/9450a289/attachment-0001.sig>
?On 5/29/20, 5:47 AM, "Manikandan Elumalai" <[email protected]> wrote:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, so PMON_CONFIG needs to be modified in order to enable it.
Signed-off-by : Manikandan Elumalai <[email protected]>
v2:
- Add Signed-off-by.
- Removed ADM1278_TEMP1_EN check.
---
drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
index 5caa37fb..ab5fceb 100644
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -666,7 +666,23 @@ 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;
+
+ config = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG);
+ if (config < 0)
+ return config;
+
+ /* Enable TEMP1 by default */
+ config |= ADM1278_TEMP1_EN;
+ ret = i2c_smbus_write_byte_data(client,
+ ADM1275_PMON_CONFIG,
+ config);
+ if (ret < 0) {
+ dev_err(&client->dev,
+ "Failed to enable temperature config\n");
+ return -ENODEV;
+ }You don't need this above code removing check as below should be enough to populate sysfs entry you need.
/* Enable VOUT if not enabled (it is disabled by default) */
if (!(config & ADM1278_VOUT_EN)) {
@@ -681,9 +697,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;
--
2.7.4Guenter Roeck
maintainerQuoted 22 lines · Patrick Williams
Hi Guenter,
Thanks for the initial look at this.
One question for you below...
Quoted 14 lines · Guenter Roeck
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 10 lines · Manikandan Elumalai
drivers/hwmon/pmbus/adm1275.c +10
-0+/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config); +if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }This can be handled in a single operation, together with
ADM1278_VOUT_ENbelow. There is no need for two separate write operations.
Quoted 4 lines
I don't know if you noticed here but the change ends up enabling
TEMP1_ENin all cases. Is this acceptable? If not, do you have any preference on how it is selected for enablement?
I did. We are doing the same for output voltage already, so I am not that much concerned about it. If it is, we might consider adding _enable attribute support (see Documentation/hwmon/sysfs-interface.rst) to the PMBus core (presumably as virtual PMBus commands) and let the user enable/disable individual attributes as needed.
What _should_ really be done, of course, is that the BIOS/ROMMON configures the chip as desired. Obviously that is not happening here.
Guenter
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
drivers/hwmon/pmbus/adm1275.c +0
-0/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,6 @@ static int adm1275_probe(struct i2c_client *client, } }
Quoted 9 lines
Quoted 8 lines
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 702
Quoted 7 lines
drivers/hwmon/pmbus/adm1275.c +0
-3-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 ;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://lists.ozlabs.org/pipermail/linux-aspeed/attachments/20200529/64248c86/attachment.sig>
Guenter Roeck
maintainerQuoted 42 lines · Vijay Khemka
?On 5/29/20, 5:47 AM, "Manikandan Elumalai" <[email protected]> wrote:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform . This functionality is not enabled by default, so PMON_CONFIG needs to be modified in order to enable it. Signed-off-by : Manikandan Elumalai <[email protected]> v2: - Add Signed-off-by. - Removed ADM1278_TEMP1_EN check. --- drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c index 5caa37fb..ab5fceb 100644 --- a/drivers/hwmon/pmbus/adm1275.c +++ b/drivers/hwmon/pmbus/adm1275.c @@ -666,7 +666,23 @@ 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; + + config = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG); + if (config < 0) + return config; + + /* Enable TEMP1 by default */ + config |= ADM1278_TEMP1_EN; + ret = i2c_smbus_write_byte_data(client, + ADM1275_PMON_CONFIG, + config); + if (ret < 0) { + dev_err(&client->dev, + "Failed to enable temperature config\n"); + return -ENODEV; + }
Quoted 3 lines
You don't need this above code removing check as below should be enough to populate sysfs entry you need.
You mean you are only interested in having the attribute, even if it doesn't report anything useful because monitoring is disabled in the chip ? Sorry, I don't understand. Can you elaborate ?
Thanks,
Guenter
Quoted 17 lines
/* Enable VOUT if not enabled (it is disabled by default) */
if (!(config & ADM1278_VOUT_EN)) {
@@ -681,9 +697,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;
--
2.7.4?On 5/29/20, 11:56 AM, "Guenter Roeck" <groeck7 at gmail.com on behalf of [email protected]> wrote:
On 5/29/20 10:57 AM, Vijay Khemka wrote:
>
>
> On 5/29/20, 5:47 AM, "Manikandan Elumalai" <[email protected]> wrote:
>
> The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
> This functionality is not enabled by default, so PMON_CONFIG needs to be modified in order to enable it.
>
> Signed-off-by : Manikandan Elumalai <[email protected]>
>
> v2:
> - Add Signed-off-by.
> - Removed ADM1278_TEMP1_EN check.
> ---
> drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwmon/pmbus/adm1275.c b/drivers/hwmon/pmbus/adm1275.c
> index 5caa37fb..ab5fceb 100644
> --- a/drivers/hwmon/pmbus/adm1275.c
> +++ b/drivers/hwmon/pmbus/adm1275.c
> @@ -666,7 +666,23 @@ 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;
> +
> + config = i2c_smbus_read_byte_data(client, ADM1275_PMON_CONFIG);
> + if (config < 0)
> + return config;
> +
> + /* Enable TEMP1 by default */
> + config |= ADM1278_TEMP1_EN;
> + ret = i2c_smbus_write_byte_data(client,
> + ADM1275_PMON_CONFIG,
> + config);
> + if (ret < 0) {
> + dev_err(&client->dev,
> + "Failed to enable temperature config\n");
> + return -ENODEV;
> + }
> You don't need this above code removing check as below should be enough to
> populate sysfs entry you need.
>
You mean you are only interested in having the attribute, even if it doesn't
report anything useful because monitoring is disabled in the chip ?
Sorry, I don't understand. Can you elaborate ?Sorry for misinterpretation, No I don't mean that. This should be fine.
Thanks,
Guenter
> /* Enable VOUT if not enabled (it is disabled by default) */
> if (!(config & ADM1278_VOUT_EN)) {
> @@ -681,9 +697,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;
> --
> 2.7.4
>
>
> Quoted 59 lines · Guenter Roeck
Quoted 4 lines · Manikandan Elumalai
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, soPMON_CONFIGneeds to be modified in order to enable it.Signed-off-by Manikandan Elumalai <[email protected]>This is not valid.
Quoted 4 lines
v2:
- Add Signed-off-by.
- RemovedADM1278_TEMP1_ENcheck.checkpatch reports:
Quoted 1 line
---WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .CHECK: Alignment should match open parenthesis
#45: FILE:drivers/hwmon/pmbus/adm1275.c:679:drivers/hwmon/pmbus/adm1275.c +2
-0+ ret = i2c_smbus_write_byte_data(client, + ADM1275_PMON_CONFIG,WARNING: suspect code indent for conditional statements (16, 16) #47: FILE:
drivers/hwmon/pmbus/adm1275.c:681:drivers/hwmon/pmbus/adm1275.c +2
-0+if (ret <0 ) { + dev_err(&client->dev,ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 2 warnings, 1 checks, 33 lines checked
Please follow published guidelines when submitting patches.
Quoted 24 lines
drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)drivers/hwmon/pmbus/adm1275.c +12
-1@@ -666,7 +666,23 @@ 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 ; + + config =i2c_smbus_read_byte_data (client,ADM1275_PMON_CONFIG ); +if (config <0 ) +return config; + +/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config);
Quoted 9 lines
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 5 lines
drivers/hwmon/pmbus/adm1275.c +5
-0+if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }This can be handled in a single operation, together with
ADM1278_VOUT_ENbelow. There is no need for two separate write operations.
Thanks for review Guenter, Patrick and Vijay.
Sorry for delay response.
I have made changes to write ADM1278_VOUT_EN and ADM1278_TEMP1_EN in single operation and tested in platfrom .
The changes given for quick look and will help if any misunderstand.
--- a/drivers/hwmon/pmbus/adm1275.c
+++ b/drivers/hwmon/pmbus/adm1275.c
@@ -666,11 +666,11 @@ 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) */
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);
@@ -680,10 +680,6 @@ static int adm1275_probe(struct i2c_client *client,
return -ENODEV;
}
}
-
- 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;
Quoted 17 lines
Guenter
Quoted 14 lines
drivers/hwmon/pmbus/adm1275.c +0
-3/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,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
maintainerQuoted 85 lines · Manikandan
Quoted 59 lines · Guenter Roeck
Quoted 4 lines · Manikandan Elumalai
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, soPMON_CONFIGneeds to be modified in order to enable it.Signed-off-by Manikandan Elumalai <[email protected]>This is not valid.
Quoted 4 lines
v2:
- Add Signed-off-by.
- RemovedADM1278_TEMP1_ENcheck.checkpatch reports:
Quoted 1 line
---WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .CHECK: Alignment should match open parenthesis
#45: FILE:drivers/hwmon/pmbus/adm1275.c:679:drivers/hwmon/pmbus/adm1275.c +2
-0+ ret = i2c_smbus_write_byte_data(client, + ADM1275_PMON_CONFIG,WARNING: suspect code indent for conditional statements (16, 16) #47: FILE:
drivers/hwmon/pmbus/adm1275.c:681:drivers/hwmon/pmbus/adm1275.c +2
-0+if (ret <0 ) { + dev_err(&client->dev,ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 2 warnings, 1 checks, 33 lines checked
Please follow published guidelines when submitting patches.
Quoted 24 lines
drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)drivers/hwmon/pmbus/adm1275.c +12
-1@@ -666,7 +666,23 @@ 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 ; + + config =i2c_smbus_read_byte_data (client,ADM1275_PMON_CONFIG ); +if (config <0 ) +return config; + +/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config);Quoted 9 lines
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 5 lines
drivers/hwmon/pmbus/adm1275.c +5
-0+if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }This can be handled in a single operation, together with
ADM1278_VOUT_ENbelow. There is no need for two separate write operations.Thanks for review Guenter, Patrick and Vijay.
Sorry for delay response.
I have made changes to writeADM1278_VOUT_ENandADM1278_TEMP1_ENin single operation and tested in platfrom .
The changes given for quick look and will help if any misunderstand.---
a/drivers/hwmon/pmbus/adm1275.c
+++b/drivers/hwmon/pmbus/adm1275.c
@@ -666,11 +666,11 @@ static int adm1275_probe(structi2c_client*client,drivers/hwmon/pmbus/adm1275.c +0
-0tindex =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;
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 688
Quoted 2 lines
drivers/hwmon/pmbus/adm1275.c +0
-0/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {
if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN)
Quoted 2 lines
- config |= ADM1278_VOUT_EN; + config |= (ADM1278_VOUT_EN | ADM1278_TEMP1_EN);
( ) is unnecessary here.
Quoted 31 lines
(quoted diff) +0
-0ret = i2c_smbus_write_byte_data(client, ADM1275_PMON_CONFIG, config);@@ -680,10 +680,6 @@ static int adm1275_probe(struct i2c_client *client,(quoted diff) +0
-0return -ENODEV; } }- - if (config & ADM1278_TEMP1_EN) - info->func[0] |= - PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;drivers/hwmon/pmbus/adm1275.c +0
-0if (config &ADM1278_VIN_EN ) info->func[0 ] |=PMBUS_HAVE_VIN ;break ;Quoted 17 lines
Guenter
Quoted 14 lines
drivers/hwmon/pmbus/adm1275.c +0
-3/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,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 ;
Quoted 89 lines · Guenter Roeck
Quoted 85 lines · Manikandan
Quoted 59 lines · Guenter Roeck
Quoted 4 lines · Manikandan Elumalai
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .
This functionality is not enabled by default, soPMON_CONFIGneeds to be modified in order to enable it.Signed-off-by Manikandan Elumalai <[email protected]>This is not valid.
Quoted 4 lines
v2:
- Add Signed-off-by.
- RemovedADM1278_TEMP1_ENcheck.checkpatch reports:
Quoted 1 line
---WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14:
The adm1278 temperature sysfs attribute need it for one of the openbmc platform .CHECK: Alignment should match open parenthesis
#45: FILE:drivers/hwmon/pmbus/adm1275.c:679:drivers/hwmon/pmbus/adm1275.c +2
-0+ ret = i2c_smbus_write_byte_data(client, + ADM1275_PMON_CONFIG,WARNING: suspect code indent for conditional statements (16, 16) #47: FILE:
drivers/hwmon/pmbus/adm1275.c:681:drivers/hwmon/pmbus/adm1275.c +2
-0+if (ret <0 ) { + dev_err(&client->dev,ERROR: Missing Signed-off-by: line(s)
total: 1 errors, 2 warnings, 1 checks, 33 lines checked
Please follow published guidelines when submitting patches.
Quoted 24 lines
drivers/hwmon/pmbus/adm1275.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-)drivers/hwmon/pmbus/adm1275.c +12
-1@@ -666,7 +666,23 @@ 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 ; + + config =i2c_smbus_read_byte_data (client,ADM1275_PMON_CONFIG ); +if (config <0 ) +return config; + +/* Enable TEMP1 by default */ + config |=ADM1278_TEMP1_EN ; + ret =i2c_smbus_write_byte_data (client, +ADM1275_PMON_CONFIG , + config);Quoted 9 lines
drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 685
Quoted 5 lines
drivers/hwmon/pmbus/adm1275.c +5
-0+if (ret <0 ) { +dev_err (&client->dev, +"Failed to enable temperature config\n" ); +return -ENODEV ; + }This can be handled in a single operation, together with
ADM1278_VOUT_ENbelow. There is no need for two separate write operations.Thanks for review Guenter, Patrick and Vijay.
Sorry for delay response.
I have made changes to writeADM1278_VOUT_ENandADM1278_TEMP1_ENin single operation and tested in platfrom .
The changes given for quick look and will help if any misunderstand.---
a/drivers/hwmon/pmbus/adm1275.c
+++b/drivers/hwmon/pmbus/adm1275.c
@@ -666,11 +666,11 @@ static int adm1275_probe(structi2c_client*client,drivers/hwmon/pmbus/adm1275.c +0
-0tindex =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;drivers/hwmon/pmbus/adm1275.c static int adm1275_probe(struct i2c_client *client, · line 688
Quoted 2 lines
drivers/hwmon/pmbus/adm1275.c +0
-0/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {
Quoted 7 lines
if (config & (ADM1278_VOUT_EN | ADM1278_TEMP1_EN) != ADM1278_VOUT_EN | ADM1278_TEMP1_EN)Quoted 2 lines
- config |= ADM1278_VOUT_EN; + config |= (ADM1278_VOUT_EN | ADM1278_TEMP1_EN);( ) is unnecessary here.
Thanks for quick review Guenter. I will submit all changes in v3.
Quoted 32 lines
Quoted 32 lines
(quoted diff) +0
-0ret = i2c_smbus_write_byte_data(client, ADM1275_PMON_CONFIG, config);@@ -680,10 +680,6 @@ static int adm1275_probe(struct i2c_client *client,(quoted diff) +0
-0return -ENODEV; } }- - if (config & ADM1278_TEMP1_EN) - info->func[0] |= - PMBUS_HAVE_TEMP | PMBUS_HAVE_STATUS_TEMP;drivers/hwmon/pmbus/adm1275.c +0
-0if (config &ADM1278_VIN_EN ) info->func[0 ] |=PMBUS_HAVE_VIN ;break ;Quoted 17 lines
Guenter
Quoted 14 lines
drivers/hwmon/pmbus/adm1275.c +0
-3/* Enable VOUT if not enabled (it is disabled by default) */ if (!(config &ADM1278_VOUT_EN )) {@@ -681,9 +697,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 ;