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
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