On Thu, Oct 18, 2018 at 08:47:43AM -0500, Rob Herring wrote: > On Tue, Oct 16, 2018 at 6:38 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > > > On Tue, Oct 16, 2018 at 01:02:08PM -0500, Rob Herring wrote: > > > On Mon, Oct 15, 2018 at 2:54 PM Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > > > > > > > On Mon, Oct 15, 2018 at 07:55:58AM +0200, Wojciech Sleńska wrote: > > > > > pt., 12 paź 2018 o 22:28 Rob Herring <robh@xxxxxxxxxx> napisał(a): > > > > > > > > > > > > On Fri, Oct 05, 2018 at 07:38:35AM +0200, Wojciech Slenska wrote: > > > > > > > > > > > > Commit msg? > > > > > > > > > > > > > Signed-off-by: Wojciech Slenska <wojciech.slenska@xxxxxxxxx> > > > > > > > --- > > > > > > > Documentation/devicetree/bindings/hwmon/sht3x.txt | 16 +++++++++++++ > > > > > > > > > > > > Please split bindings to separate patch. > > > > > > > > > > I will do this. > > > > > > > > > > > > > > > > > > drivers/hwmon/sht3x.c | 28 ++++++++++++++++++++--- > > > > > > > 2 files changed, 41 insertions(+), 3 deletions(-) > > > > > > > create mode 100644 Documentation/devicetree/bindings/hwmon/sht3x.txt > > > > > > > > > > > > > > diff --git a/Documentation/devicetree/bindings/hwmon/sht3x.txt b/Documentation/devicetree/bindings/hwmon/sht3x.txt > > > > > > > new file mode 100644 > > > > > > > index 0000000..80b117e > > > > > > > --- /dev/null > > > > > > > +++ b/Documentation/devicetree/bindings/hwmon/sht3x.txt > > > > > > > @@ -0,0 +1,16 @@ > > > > > > > +Sensirion SHT3x Humidity and Temperature Sensor > > > > > > > + > > > > > > > +Required node properties: > > > > > > > +- compatible: "sensirion,sht3x" or "sensirion,sts3x" > > > > > > > +- reg: I2C bus address of the device > > > > > > > + > > > > > > > +Optional properties: > > > > > > > +- sensirion,blocking-io: enable blocking mode on i2c > > > > > > > > > > > > This is not a h/w parameter and shouldn't be in DT. > > > > > > > > > > > > > +- sensirion,no-high-precision: disable high accuracy > > > > > > > > > > > > Maybe this one is okay, but couldn't the user want to set this? If so, > > > > > > then it should be a sysfs attr. > > > > > > > > > > Those two parameters have been just moved from > > > > > linux/include/linux/platform_data/sht3x.h > > > > > Currently, those two parameters can be set in board file, so for me > > > > > was natural to move it to dts. > > > > > Of course, I can remove it from dts completely. > > > > > > > > > > > > > I wonder if there is an authoritative document explaining the current policy > > > > in respect to devicetree properties. > > > > > > No. In the end it is often a judgement call. > > > > > > > Sometimes I hear that platform > > > > configuration parameters are now permitted, sometimes I hear that we are > > > > back to hardware description only. > > > > > > Yes, configuration is permitted, but there are some constraints. The > > > questions I ask are is it OS specific or specific to current > > > implementation of an OS, who or what decides what the setting is? The > > > last question results in lots of things dropped in favor of a sysfs > > > control. > > > > > In this driver, both parameters were considered system / platform parameters. > > One controls if the chip uses i2c clock stretching or if the system has to poll > > for results. This is determined by the system's i2c controller and may also be > > influenced by considerations such as if there is another chip on the same bus. > > Okay, makes sense. As it is named, it sounded like picking a s/w API > to use (sync vs. async). So please improve the description based on > Guenter's explanation. > > > The other parameter controls accuracy which directly translates into power > > consumption. While there could be applications where both parameters are > > controlled by user space, that would be the exception. In the expected use > > case, ie for hardware monitoring, user space control would neither be feasible > > nor desirable. As such, sysfs controls for the parameters don't even exist. > > I could imagine you may want to switch modes based on battery powered > vs. plugged in. But that's not to say you couldn't want it one way or > the other for a system. > > > I don't mind if you reject adding those dt properties for now, but I would > > resist adding sysfs attributes unless someone provides me with an actual > > and specific use case. > > I'm not rejecting them. My next question though is should these be > common properties? No, they are chip specific. Guenter
