All of lore.kernel.org
 help / color / mirror / Atom feed
From: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
To: Guenter Roeck <linux@roeck-us.net>
Cc: mpe@ellerman.id.au, linuxppc-dev@lists.ozlabs.org,
	linux-hwmon@vger.kernel.org, linux-kernel@vger.kernel.org,
	stewart@linux.vnet.ibm.com
Subject: Re: [PATCH 0/3] Add support to disable sensor groups in P9
Date: Thu, 17 May 2018 11:40:35 +0530	[thread overview]
Message-ID: <2ceb6b30-60b7-d985-ee0d-6c5784eda273@linux.vnet.ibm.com> (raw)
In-Reply-To: <20180515150205.GA29321@roeck-us.net>



On 05/15/2018 08:32 PM, Guenter Roeck wrote:
> On Thu, Mar 22, 2018 at 04:24:32PM +0530, Shilpasri G Bhat wrote:
>> This patch series adds support to enable/disable OCC based
>> inband-sensor groups at runtime. The environmental sensor groups are
>> managed in HWMON and the remaining platform specific sensor groups are
>> managed in /sys/firmware/opal.
>>
>> The firmware changes required for this patch is posted below:
>> https://lists.ozlabs.org/pipermail/skiboot/2018-March/010812.html
>>
> 
> Sorry for not getting back earlier. This is a tough one.
> 

Thanks for the reply. I have tried to answer your questions according to my
understanding below:

> Key problem is that you are changing the ABI with those new attributes.
> On top of that, the attributes _do_ make some sense (many chips support
> enabling/disabling of individual sensors), suggesting that those or
> similar attributes may or even should at some point be added to the ABI.
> 
> At the same time, returning "0" as measurement values when sensors are
> disabled does not seem like a good idea, since "0" is a perfectly valid
> measurement, at least for most sensors.

I agree.

> 
> Given that, we need to have a discussion about adding _enable attributes to
> the ABI 

> what is the scope,
IIUC the scope should be RW and the attribute is defined for each supported
sensor group

> when should the attributes exist and when not,
We control this currently via device-tree

> do we want/need power_enable or powerX_enable or both, and so on), and 
We need power_enable right now

> what to return if a sensor is disabled (such as -ENODATA).
-ENODATA sounds good.

Thanks and Regards,
Shilpa

 Once we have an
> agreement, we can continue with an implementation.
> 
> Guenter
> 
>> Shilpasri G Bhat (3):
>>   powernv:opal-sensor-groups: Add support to enable sensor groups
>>   hwmon: ibmpowernv: Add attributes to enable/disable sensor groups
>>   powernv: opal-sensor-groups: Add attributes to disable/enable sensors
>>
>>  .../ABI/testing/sysfs-firmware-opal-sensor-groups  |  34 ++++++
>>  Documentation/hwmon/ibmpowernv                     |  31 ++++-
>>  arch/powerpc/include/asm/opal-api.h                |   4 +-
>>  arch/powerpc/include/asm/opal.h                    |   2 +
>>  .../powerpc/platforms/powernv/opal-sensor-groups.c | 104 ++++++++++++-----
>>  arch/powerpc/platforms/powernv/opal-wrappers.S     |   1 +
>>  drivers/hwmon/ibmpowernv.c                         | 127 +++++++++++++++++++--
>>  7 files changed, 265 insertions(+), 38 deletions(-)
>>  create mode 100644 Documentation/ABI/testing/sysfs-firmware-opal-sensor-groups
>>
>> -- 
>> 1.8.3.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

  reply	other threads:[~2018-05-17  6:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-22 10:54 [PATCH 0/3] Add support to disable sensor groups in P9 Shilpasri G Bhat
2018-03-22 10:54 ` [PATCH 1/3] powernv:opal-sensor-groups: Add support to enable sensor groups Shilpasri G Bhat
2018-03-22 10:54 ` [PATCH 2/3] hwmon: ibmpowernv: Add attributes to enable/disable " Shilpasri G Bhat
2018-03-22 10:54 ` [PATCH 3/3] powernv: opal-sensor-groups: Add attributes to disable/enable sensors Shilpasri G Bhat
2018-05-15 15:02 ` [PATCH 0/3] Add support to disable sensor groups in P9 Guenter Roeck
2018-05-17  6:10   ` Shilpasri G Bhat [this message]
2018-05-17 12:38     ` Guenter Roeck
2018-05-18  4:03       ` Shilpasri G Bhat

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2ceb6b30-60b7-d985-ee0d-6c5784eda273@linux.vnet.ibm.com \
    --to=shilpa.bhat@linux.vnet.ibm.com \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mpe@ellerman.id.au \
    --cc=stewart@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.