On Tue, 16 Oct 2018 17:16:15 -0500 Paresh Chaudhary <paresh.chaudhary@xxxxxxxxxxxxxxxxxxx> wrote: > Hi Jonathan, > > Please find my comment inline. > > Thanks, > Paresh > > On Sat, Oct 13, 2018 at 8:54 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote: > > > > On Thu, 11 Oct 2018 11:38:10 -0500 > > Matt Weber <matthew.weber@xxxxxxxxxxxxxxxxxxx> wrote: > > > > > From: Paresh Chaudhary <paresh.chaudhary@xxxxxxxxxxxxxxxxxxx> > > > > > > This patch added device tree binding info for MAX31856 driver. > > > > > > Signed-off-by: Paresh Chaudhary <paresh.chaudhary@xxxxxxxxxxxxxxxxxxx> > > > Signed-off-by: Matt Weber <matthew.weber@xxxxxxxxxxxxxxxxxxx> > > Device tree bindings should be cc'd to the devicetree list and maintainers. > > > > Thanks, > > > > Jonathan > > > > > --- > > > Changes v1 -> v2 > > > [Matt > > > - Removed comment block and added possibilities of > > > thermocouple type in device tree binding doc. > > > > > > --- > > > .../bindings/iio/temperature/max31856.txt | 32 ++++++++++++++++++++++ > > > MAINTAINERS | 1 + > > > 2 files changed, 33 insertions(+) > > > create mode 100644 Documentation/devicetree/bindings/iio/temperature/max31856.txt > > > > > > diff --git a/Documentation/devicetree/bindings/iio/temperature/max31856.txt b/Documentation/devicetree/bindings/iio/temperature/max31856.txt > > > new file mode 100644 > > > index 0000000..e1def9f > > > --- /dev/null > > > +++ b/Documentation/devicetree/bindings/iio/temperature/max31856.txt > > > @@ -0,0 +1,32 @@ > > > +Maxim MAX31856 thermocouple support > > > + > > > +https://datasheets.maximintegrated.com/en/ds/MAX31856.pdf > > > + > > > +Required properties: > > > + - compatible: must be "max31856" > > > + - reg: SPI chip select number for the device > > > + - spi-max-frequency: As per datasheet max. supported freq is 5000000 > > > + - spi-cpha: must be defined for max31856 to enable SPI mode 1 > > > + - type: Type of thermocouple (By default is K-Type) > > > + 0x00 : TYPE_B > > > + 0x01 : TYPE_E > > > + 0x02 : TYPE_J > > > + 0x03 : TYPE_K (default) > > > + 0x04 : TYPE_N > > > + 0x05 : TYPE_R > > > + 0x06 : TYPE_S > > > + 0x07 : TYPE_T > > > + > > > + Refer to spi/spi-bus.txt for generic SPI slave bindings. > > > + > > > +Optional properties: > > > + - one-shot: Enable one-shot Conversion mode (By default mode is auto) > > > > Why is this something that should be in devicetree rather than controlled > > in the driver in response to some userspace interaction? > > > > > There is no specific reason. This is just providing info to the driver > so the driver can configure chip while probe for specified mode. > I understand your concern but as of now, the driver does not have > support to decide mode based on userspace interaction. We will work on > that when we reopen for enhancement (e.g threshold for temp. range) Hmm. The problem is that this should not be a devicetree decision and if we allow it to be so, then we end up stuck with having to support the interface here going forward. It also sets a precedence that will get picked up by other drivers. So, sorry but I really don't think this is a good idea. For now you may just have pick a default and go with it until you have the time to add some automated switching between them at some later date. Jonathan > > > + > > > + Example: > > > + max31856@0 { > > > + compatible = "max31856"; > > > + reg = <0>; > > > + spi-max-frequency = <5000000>; > > > + spi-cpha; > > > + type = <0x03>; > > > + }; > > > diff --git a/MAINTAINERS b/MAINTAINERS > > > index dd9a83d..6482ae8 100644 > > > --- a/MAINTAINERS > > > +++ b/MAINTAINERS > > > @@ -7161,6 +7161,7 @@ MAX31856 IIO DRIVER > > > M: Matthew Weber <matthew.weber@xxxxxxxxxxxxxxxxxxx> > > > S: Maintained > > > F: drivers/iio/temperature/max31856.c > > > +F: Documentation/devicetree/bindings/iio/temperature/max31856.txt > > > > > > IIO UNIT CONVERTER > > > M: Peter Rosin <peda@xxxxxxxxxx> > >
