Thread (18 messages) 18 messages, 2 authors, 2018-09-27

Re: [PATCH 2/2] hwmon: ina3221: Add enable sysfs nodes

From: Nicolin Chen <nicoleotsuka@gmail.com>
Date: 2018-09-26 18:02:52
Also in: linux-hwmon, lkml

On Wed, Sep 26, 2018 at 06:06:32AM -0700, Guenter Roeck wrote:
On 09/25/2018 11:42 PM, Nicolin Chen wrote:
quoted
The inX_enable interface allows user space to enable or disable
the corresponding channel. Meanwhile, according to hwmon ABI, a
disabled channel/sensor should return -ENODATA as a read result.

However, there're configurable nodes sharing the same __show()
functions. So this change also adds to check if the attribute is
read-only to make sure it's not reading a configuration but the
sensor data.
 
One necessary high level change I don't see below: With this change,
we should no longer drop a channel entirely if it is disabled from
devicetree. All channels should be visible but report -ENODATA if
disabled. In other words, it should be possible for the 'enable' flag
to override settings in DT.
Hmm...I don't feel so convinced here. The status in DT binding isn't
exactly a setting but a physical status: if a hardware design leaves
a channel to be disconnected, I don't really see a point in enabling
it in the runtime. Or maybe you can shed some light on it?

Meanwhile, I believe the enable nodes are necessary in either way as
users could decide to disable the connected channels, based on their
use cases, to save power.

Thanks
Nicolin
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help