Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1630587 > unrolled thread

Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

Started byCédric Le Goater <clg@kaod.org>
First post2017-04-25 17:10 +0200
Last post2017-04-25 18:00 +0200
Articles 2 — 1 participant

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current  sensors Cédric Le Goater <clg@kaod.org> - 2017-04-25 17:10 +0200
    Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current  sensors Cédric Le Goater <clg@kaod.org> - 2017-04-25 18:00 +0200

#1630587 — Re: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors

FromCédric Le Goater <clg@kaod.org>
Date2017-04-25 17:10 +0200
SubjectRe: [PATCH] hwmon: (ibmpowernv) Add min/max attributes and current sensors
Message-ID<tAagi-6jY-35@gated-at.bofh.it>
On 04/22/2017 08:11 AM, Michael Ellerman wrote:
> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
>> On 04/21/2017 05:17 PM, Cédric Le Goater wrote:
>>> On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
>>>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>>>> index 6d2e660..1f329fa8 100644
>>>> --- a/drivers/hwmon/ibmpowernv.c
>>>> +++ b/drivers/hwmon/ibmpowernv.c
>>>> @@ -65,7 +66,8 @@ enum sensors {
>>>>  	{"fan", "ibm,opal-sensor-cooling-fan"},
>>>>  	{"temp", "ibm,opal-sensor-amb-temp"},
>>>>  	{"in", "ibm,opal-sensor-power-supply"},
>>>> -	{"power", "ibm,opal-sensor-power"}
>>>> +	{"power", "ibm,opal-sensor-power"},
>>>> +	{"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */
>>>
>>> why isn't there a compatible string ? 
>>
>> Skiboot exports "ibm,opal-sensor" as compatible string for all the inband
>> sensors. Now if I define that as the compatible string here, then all the
>> sensors would get identified as "curr" type of sensor by the driver.
> 
> So fix it in skiboot?


After a memory refresh, this table is to maintain compatibility with 
the support of the FSP sensors in old firmware. These have peculiar
device node names and properties.  

So What the code is doing looks correct. But, you should also modify 
the 'enum sensors' to include a CURRENT value.

Thanks,

C.

[toc] | [next] | [standalone]


#1630717

FromCédric Le Goater <clg@kaod.org>
Date2017-04-25 18:00 +0200
Message-ID<tAb2F-6Ff-1@gated-at.bofh.it>
In reply to#1630587
On 04/25/2017 04:28 PM, Cédric Le Goater wrote:
> On 04/22/2017 08:11 AM, Michael Ellerman wrote:
>> Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
>>> On 04/21/2017 05:17 PM, Cédric Le Goater wrote:
>>>> On 04/21/2017 06:31 AM, Shilpasri G Bhat wrote:
>>>>> diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
>>>>> index 6d2e660..1f329fa8 100644
>>>>> --- a/drivers/hwmon/ibmpowernv.c
>>>>> +++ b/drivers/hwmon/ibmpowernv.c
>>>>> @@ -65,7 +66,8 @@ enum sensors {
>>>>>  	{"fan", "ibm,opal-sensor-cooling-fan"},
>>>>>  	{"temp", "ibm,opal-sensor-amb-temp"},
>>>>>  	{"in", "ibm,opal-sensor-power-supply"},
>>>>> -	{"power", "ibm,opal-sensor-power"}
>>>>> +	{"power", "ibm,opal-sensor-power"},
>>>>> +	{"curr"}, /* Follows newer device tree compatible ibm,opal-sensor */
>>>>
>>>> why isn't there a compatible string ? 
>>>
>>> Skiboot exports "ibm,opal-sensor" as compatible string for all the inband
>>> sensors. Now if I define that as the compatible string here, then all the
>>> sensors would get identified as "curr" type of sensor by the driver.
>>
>> So fix it in skiboot?
> 
> 
> After a memory refresh, this table is to maintain compatibility with 
> the support of the FSP sensors in old firmware. These have peculiar
> device node names and properties.  
> 
> So What the code is doing looks correct. But, you should also modify 
> the 'enum sensors' to include a CURRENT value.

But the patch does that already. I was missing context. This is fine
then.

Thanks,

C.

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web