lkml.org 
[lkml]   [2020]   [May]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] hwmon:(adm1275) Enable adm1278 ADM1278_TEMP1_EN
From
Date
On 5/29/20 10:42 AM, Patrick Williams wrote:
> Hi Guenter,
>
> Thanks for the initial look at this.
>
> One question for you below...
>
> On Fri, May 29, 2020 at 10:30:16AM -0700, Guenter Roeck wrote:
>> On 5/29/20 5:46 AM, Manikandan Elumalai wrote:
>>> + /* 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.
>
> 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?
>

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

>>> /* 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;
>>>
>>
>


[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-05-29 20:53    [W:0.086 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site