| Message ID | 87644503b0397248c06fbe0058f292955dd10f79.1503407738.git.lukas@wunner.de |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: > All chips supported by this driver clock data out on the falling edge > and latch data in on the rising edge, hence SPI mode (0,0) or (1,1) > must be used. > > Furthermore, none of the chips has an internal reference voltage > regulator, so an external supply is always required and needs to be > specified in the device tree lest the IIO "scale" in sysfs cannot be > calculated. > > Document these requirements in the device tree binding, together with > the new "microchip,continuous-conversion" property added specifically > for the mcp3550/1/3. > > Cc: Mathias Duckeck <m.duckeck@kunbus.de> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > .../devicetree/bindings/iio/adc/mcp320x.txt | 23 ++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > index bcd3ac8e6e0c..cf28af9ec0ac 100644 > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > @@ -29,15 +29,38 @@ Required properties: > "microchip,mcp3204" > "microchip,mcp3208" > "microchip,mcp3301" > + "microchip,mcp3550-50" > + "microchip,mcp3550-60" > + "microchip,mcp3551" > + "microchip,mcp3553" > > NOTE: The use of the compatibles with no vendor prefix > is deprecated and only listed because old DT use them. > > + - spi-cpha, spi-cpol (boolean): > + Either SPI mode (0,0) or (1,1) must be used, so specify > + none or both of spi-cpha, spi-cpol. The MCP3550/1/3 > + is more efficient in mode (1,1) as only 3 instead of > + 4 bytes need to be read from the ADC, but not all SPI > + masters support it. > + > + - vref-supply: Phandle to the external reference voltage supply. > + > +Optional properties: > + - microchip,continuous-conversion (boolean): Second binding I have seen today with a continuous property. Make this common (or maybe we already have one). > + Only applicable to MCP3550/1/3: These ADCs have long > + conversion times and therefore support "continuous > + conversion mode" to allow retrieval of conversions > + at any time without observing a delay. The mode is > + enabled by permanently driving CS low, e.g. by wiring > + it to ground. > + > Examples: > spi_controller { > mcp3x0x@0 { > compatible = "mcp3002"; > reg = <0>; > spi-max-frequency = <1000000>; > + vref-supply = <&vref_reg>; > }; > }; > -- > 2.11.0 > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 25, 2017 at 02:59:41PM -0500, Rob Herring wrote: > On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: > > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > +Optional properties: > > + - microchip,continuous-conversion (boolean): > > + Only applicable to MCP3550/1/3: These ADCs have long > > + conversion times and therefore support "continuous > > + conversion mode" to allow retrieval of conversions > > + at any time without observing a delay. The mode is > > + enabled by permanently driving CS low, e.g. by wiring > > + it to ground. > > Second binding I have seen today with a continuous property. Make this > common (or maybe we already have one). The other one was the TI LMP92001 ADC driver posted by Abhisit Sangjan (cc), however looking at the datasheet of that chip reveals that continuous versus one-shot mode is selected by flipping a bit in the chip's register map. So it is configurable at run-time. It's not something that's hardwired. (Which is the case with the MCP3550 in my patch.) My understanding was that run-time configurable options should not be listed in the device-tree at all, only hardware features. If that is correct then that device-tree property should be dropped from Abhisit Sangjan's patch. Configuring the feature via sysfs is fine I guess. However we do have another driver supporting continuous versus one-shot mode and that is drivers/iio/light/us5182d.c by Adriana Reus (cc). The feature was added with commit c3304c212326. I'm not sure if it's hardwired or runtime-configurable, the datasheet is gone from the manufacturer's website. I agree that a common "continuous" property makes sense. We haven't defined any common IIO properties so far and that has already led to inconsistencies. E.g. most ADC/DAC drivers name the reference regulator "vref-supply", but e.g. drivers/iio/dac/ad7303.c calls it "REF-supply". What do you think of this: -- >8 -- Subject: [PATCH] dt-bindings: iio: Document common properties It's about time we standardize on common names for frequently used IIO properties. For starters, document "vref-supply" and "continuous". Suggested-by: Rob Herring <robh@kernel.org> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt index 68d6f8c..c3e87e15 100644 --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. io-channels = <&adc 10>, <&adc 11>; io-channel-names = "adc1", "adc2"; }; + +==Common IIO properties== + +Reference voltage: +ADCs, DACs and several other IIO devices require a reference voltage. +By convention the property specifying this regulator is named "vref-supply". +If the chip lacks a dedicated Vref pin and instead uses its own power supply +as reference, the property specifying the regulator is commonly named +"vdd-supply" or "vcc-supply". + +Continuous mode: +Some sensors can be configured to perform continuous (versus one-shot) +measurements. Continuous mode may require more energy in return for faster +or more reliable measurements. A boolean property named "continuous" +signifies that the device is configured for this mode.
Hi, On Sun, Aug 27, 2017 at 6:34 PM, Lukas Wunner <lukas@wunner.de> wrote: > On Fri, Aug 25, 2017 at 02:59:41PM -0500, Rob Herring wrote: >> On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: >> > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt >> > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt >> > +Optional properties: >> > + - microchip,continuous-conversion (boolean): >> > + Only applicable to MCP3550/1/3: These ADCs have long >> > + conversion times and therefore support "continuous >> > + conversion mode" to allow retrieval of conversions >> > + at any time without observing a delay. The mode is >> > + enabled by permanently driving CS low, e.g. by wiring >> > + it to ground. >> >> Second binding I have seen today with a continuous property. Make this >> common (or maybe we already have one). > > The other one was the TI LMP92001 ADC driver posted by Abhisit Sangjan > (cc), however looking at the datasheet of that chip reveals that > continuous versus one-shot mode is selected by flipping a bit in the > chip's register map. > > So it is configurable at run-time. It's not something that's hardwired. > (Which is the case with the MCP3550 in my patch.) > > My understanding was that run-time configurable options should not be > listed in the device-tree at all, only hardware features. If that is > correct then that device-tree property should be dropped from Abhisit > Sangjan's patch. Configuring the feature via sysfs is fine I guess. > > However we do have another driver supporting continuous versus one-shot > mode and that is drivers/iio/light/us5182d.c by Adriana Reus (cc). > The feature was added with commit c3304c212326. I'm not sure if it's > hardwired or runtime-configurable, the datasheet is gone from the > manufacturer's website. For the us5182 continuous versus one-shot is also a configuration in the chip's registers. I think it would be fine also as a sysfs property instead. > > I agree that a common "continuous" property makes sense. We haven't > defined any common IIO properties so far and that has already led to > inconsistencies. E.g. most ADC/DAC drivers name the reference regulator > "vref-supply", but e.g. drivers/iio/dac/ad7303.c calls it "REF-supply". > > What do you think of this: > > -- >8 -- > Subject: [PATCH] dt-bindings: iio: Document common properties > > It's about time we standardize on common names for frequently used IIO > properties. For starters, document "vref-supply" and "continuous". > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt > index 68d6f8c..c3e87e15 100644 > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt > @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. > io-channels = <&adc 10>, <&adc 11>; > io-channel-names = "adc1", "adc2"; > }; > + > +==Common IIO properties== > + > +Reference voltage: > +ADCs, DACs and several other IIO devices require a reference voltage. > +By convention the property specifying this regulator is named "vref-supply". > +If the chip lacks a dedicated Vref pin and instead uses its own power supply > +as reference, the property specifying the regulator is commonly named > +"vdd-supply" or "vcc-supply". > + > +Continuous mode: > +Some sensors can be configured to perform continuous (versus one-shot) > +measurements. Continuous mode may require more energy in return for faster > +or more reliable measurements. A boolean property named "continuous" > +signifies that the device is configured for this mode. > -- > 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 29 Aug 2017 10:21:13 +0300 Adriana Reus <adi.reus@gmail.com> wrote: +cc Mark Brown and linux-spi to address question about how to represent a hardwired chip select. See below for why I think that is what we should be representing rather than the fact it puts it in 'continuous mode'. > Hi, > > On Sun, Aug 27, 2017 at 6:34 PM, Lukas Wunner <lukas@wunner.de> wrote: > > On Fri, Aug 25, 2017 at 02:59:41PM -0500, Rob Herring wrote: > >> On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: > >> > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > >> > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > >> > +Optional properties: > >> > + - microchip,continuous-conversion (boolean): > >> > + Only applicable to MCP3550/1/3: These ADCs have long > >> > + conversion times and therefore support "continuous > >> > + conversion mode" to allow retrieval of conversions > >> > + at any time without observing a delay. The mode is > >> > + enabled by permanently driving CS low, e.g. by wiring > >> > + it to ground. hmm. This is odd. We probably need to make the SPI subsystem aware of this. It is possible to ask for exclusive use of an SPI bus and I think we should be doing this here. It may be wired low on your board, but it may be wired to a controllable chip select on other boards and we can still force it low to trigger this mode if it makes sense for the current application. Datasheet says that this mode kicks in whenever we don't raise the cs rather than it being a one time thing. So I'd argue what we actually need to represent here is that the CS line is not controllable. What this means to the driver should be handled in the driver - ideally also dealing with the case where it is controllable appropriately (via exclusive bus usage). Spi devices have the SPI_NO_CS bit in the mode member of the spi device but I'm not sure about bindings. Mark or other SPI people (cc'd) any thoughts on how to handle this cleanly? > >> > >> Second binding I have seen today with a continuous property. Make this > >> common (or maybe we already have one). > > > > The other one was the TI LMP92001 ADC driver posted by Abhisit Sangjan > > (cc), however looking at the datasheet of that chip reveals that > > continuous versus one-shot mode is selected by flipping a bit in the > > chip's register map. > > > > So it is configurable at run-time. It's not something that's hardwired. > > (Which is the case with the MCP3550 in my patch.) For the runtime configurable ones, the vast majority of the time this should not be exposed to userspace at all. It's a fairly esoteric option and exactly what it means varies hugely between different boards. Most of the time userspace has not idea what to do with it - if 'explicitly' provided as a control. It might make sense for a particular application with tightly coupled userspace and kernelspace, but that's not what we are targeting in IIO. So it is up to the driver to make sensible decisions based on what is going on. Kind of like auto suspend for runtime pm. The one case where we normally want to flip to continuous modes is when we have a chardev access going on to the device. In IIO that reflects the fact we are in a push mode rather than userspace polling for new data. A lot of hardware supports continuous and one shot capture in this fashion! Pretty much anything with an internal sequencing clock. > > > > My understanding was that run-time configurable options should not be > > listed in the device-tree at all, only hardware features. If that is > > correct then that device-tree property should be dropped from Abhisit > > Sangjan's patch. Configuring the feature via sysfs is fine I guess. > > > > However we do have another driver supporting continuous versus one-shot > > mode and that is drivers/iio/light/us5182d.c by Adriana Reus (cc). > > The feature was added with commit c3304c212326. I'm not sure if it's > > hardwired or runtime-configurable, the datasheet is gone from the > > manufacturer's website. > For the us5182 continuous versus one-shot is also a configuration in the > chip's registers. I think it would be fine also as a sysfs property instead. > > > > I agree that a common "continuous" property makes sense. We haven't > > defined any common IIO properties so far and that has already led to > > inconsistencies. E.g. most ADC/DAC drivers name the reference regulator > > "vref-supply", but e.g. drivers/iio/dac/ad7303.c calls it "REF-supply". > > > > What do you think of this: Personally I don't think we are in a position yet to make this a generic property - this is the first device where it is actually to do with the physical circuit (and arguably it isn't really - see above). It might become common, but I actually think it unlikely. It's odd to have a device that sits in the sweet spot between being dumb enough to not make it software configurable and clever enough to implement this stuff at all. Reference voltages are an oddity as the supply naming typically should match that on the datasheet. It's 'fairly' consistent but some devices have a set of relatively obscure references to different parts of the input circuitry. We can document it as a 'default' assuming nothing strange is going on though. This is why we have the vagueness below on VDD and VCC. > > > > -- >8 -- > > Subject: [PATCH] dt-bindings: iio: Document common properties > > > > It's about time we standardize on common names for frequently used IIO > > properties. For starters, document "vref-supply" and "continuous". > > > > Suggested-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > --- > > Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt > > index 68d6f8c..c3e87e15 100644 > > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt > > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt > > @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. > > io-channels = <&adc 10>, <&adc 11>; > > io-channel-names = "adc1", "adc2"; > > }; > > + > > +==Common IIO properties== > > + > > +Reference voltage: > > +ADCs, DACs and several other IIO devices require a reference voltage. > > +By convention the property specifying this regulator is named "vref-supply". > > +If the chip lacks a dedicated Vref pin and instead uses its own power supply > > +as reference, the property specifying the regulator is commonly named > > +"vdd-supply" or "vcc-supply". > > + > > +Continuous mode: > > +Some sensors can be configured to perform continuous (versus one-shot) > > +measurements. Continuous mode may require more energy in return for faster > > +or more reliable measurements. A boolean property named "continuous" > > +signifies that the device is configured for this mode. > > -- > > 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 03, 2017 at 02:37:49PM +0100, Jonathan Cameron wrote: > On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: > > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > +Optional properties: > > + - microchip,continuous-conversion (boolean): > > + Only applicable to MCP3550/1/3: These ADCs have long > > + conversion times and therefore support "continuous > > + conversion mode" to allow retrieval of conversions > > + at any time without observing a delay. The mode is > > + enabled by permanently driving CS low, e.g. by wiring > > + it to ground. > > hmm. This is odd. We probably need to make the SPI subsystem aware > of this. It is possible to ask for exclusive use of an SPI bus and > I think we should be doing this here. It may be wired low on your > board, but it may be wired to a controllable chip select on other > boards and we can still force it low to trigger this mode if it makes > sense for the current application. > > So I'd argue what we actually need to represent here is that the CS line > is not controllable. What this means to the driver should be handled > in the driver - ideally also dealing with the case where it is controllable > appropriately (via exclusive bus usage). Spi devices have the SPI_NO_CS > bit in the mode member of the spi device but I'm not sure about bindings. [...] > The one case where we normally want to flip to continuous modes is when > we have a chardev access going on to the device. In IIO that reflects > the fact we are in a push mode rather than userspace polling for new data. It seems there is no DT binding so far to set SPI_NO_CS. Conceivably, continuous mode could be used even with multiple devices on the bus if CLK and MISO is AND-gated with the CS signal coming from the SPI master. (And the CS of the ADC is pulled low.) In that case, the notion that "continuous mode == CS not controllable" would be incorrect, hence the approach I've chosen. On the Revolution Pi we don't use continuous mode. I merely included it in the driver for completeness. If it is too controversial I'd be inclined to drop the feature. On-demand switching to continuous mode by keeping CS low would be possible by setting the cs_change bit of struct mcp320x ->transfer[1], but that might not work if there are other devices on the bus. > Personally I don't think we are in a position yet to make this a generic > property - this is the first device where it is actually to do with the > physical circuit (and arguably it isn't really - see above). Okay. > Reference voltages are an oddity as the supply naming typically should match > that on the datasheet. It's 'fairly' consistent but some devices > have a set of relatively obscure references to different parts of the > input circuitry. We can document it as a 'default' assuming nothing > strange is going on though. This is why we have the vagueness below > on VDD and VCC. That is new to me, I believe it's not documented or am I missing something? I'd be happy to respin the below patch without the "Continuous mode" portion if you want? (Amended with the info you gave above.) Do you think iio-bindings.txt is the right place to put this or would a separate common.txt be more appropriate? (See e.g. leds/common.txt) Thanks, Lukas > -- >8 -- > Subject: [PATCH] dt-bindings: iio: Document common properties > > It's about time we standardize on common names for frequently used IIO > properties. For starters, document "vref-supply" and "continuous". > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt > index 68d6f8c..c3e87e15 100644 > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt > @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. > io-channels = <&adc 10>, <&adc 11>; > io-channel-names = "adc1", "adc2"; > }; > + > +==Common IIO properties== > + > +Reference voltage: > +ADCs, DACs and several other IIO devices require a reference voltage. > +By convention the property specifying this regulator is named "vref-supply". > +If the chip lacks a dedicated Vref pin and instead uses its own power supply > +as reference, the property specifying the regulator is commonly named > +"vdd-supply" or "vcc-supply". > + > +Continuous mode: > +Some sensors can be configured to perform continuous (versus one-shot) > +measurements. Continuous mode may require more energy in return for faster > +or more reliable measurements. A boolean property named "continuous" > +signifies that the device is configured for this mode. > -- > 2.11.0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, 3 Sep 2017 20:20:46 +0200 Lukas Wunner <lukas@wunner.de> wrote: > On Sun, Sep 03, 2017 at 02:37:49PM +0100, Jonathan Cameron wrote: > > On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: > > > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt > > > +Optional properties: > > > + - microchip,continuous-conversion (boolean): > > > + Only applicable to MCP3550/1/3: These ADCs have long > > > + conversion times and therefore support "continuous > > > + conversion mode" to allow retrieval of conversions > > > + at any time without observing a delay. The mode is > > > + enabled by permanently driving CS low, e.g. by wiring > > > + it to ground. > > > > hmm. This is odd. We probably need to make the SPI subsystem aware > > of this. It is possible to ask for exclusive use of an SPI bus and > > I think we should be doing this here. It may be wired low on your > > board, but it may be wired to a controllable chip select on other > > boards and we can still force it low to trigger this mode if it makes > > sense for the current application. > > > > So I'd argue what we actually need to represent here is that the CS line > > is not controllable. What this means to the driver should be handled > > in the driver - ideally also dealing with the case where it is controllable > > appropriately (via exclusive bus usage). Spi devices have the SPI_NO_CS > > bit in the mode member of the spi device but I'm not sure about bindings. > [...] > > The one case where we normally want to flip to continuous modes is when > > we have a chardev access going on to the device. In IIO that reflects > > the fact we are in a push mode rather than userspace polling for new data. > > It seems there is no DT binding so far to set SPI_NO_CS. Time to add one perhaps ;) > > Conceivably, continuous mode could be used even with multiple devices > on the bus if CLK and MISO is AND-gated with the CS signal coming from > the SPI master. (And the CS of the ADC is pulled low.) In that case, > the notion that "continuous mode == CS not controllable" would be > incorrect, hence the approach I've chosen. > > On the Revolution Pi we don't use continuous mode. I merely included > it in the driver for completeness. If it is too controversial I'd be > inclined to drop the feature. > > On-demand switching to continuous mode by keeping CS low would be > possible by setting the cs_change bit of struct mcp320x ->transfer[1], > but that might not work if there are other devices on the bus. > spi has exclusive bus access functions. Doing this sort of thing isn't that unusual. Some devices require that the cs is held low whilst they take a single reading, but provide an interrupt for when they are done. For those we have to hold it for some time. > > > Personally I don't think we are in a position yet to make this a generic > > property - this is the first device where it is actually to do with the > > physical circuit (and arguably it isn't really - see above). > > Okay. > > > > Reference voltages are an oddity as the supply naming typically should match > > that on the datasheet. It's 'fairly' consistent but some devices > > have a set of relatively obscure references to different parts of the > > input circuitry. We can document it as a 'default' assuming nothing > > strange is going on though. This is why we have the vagueness below > > on VDD and VCC. > > That is new to me, I believe it's not documented or am I missing something? > I'd be happy to respin the below patch without the "Continuous mode" > portion if you want? (Amended with the info you gave above.) It might make sense to drop continuous mode with the intent to add it as a follow up patch. Lets see what Mark and co come back with on how to support the hard wired cs on the spi. > Do you think iio-bindings.txt is the right place to put this or would > a separate common.txt be more appropriate? (See e.g. leds/common.txt) Perhaps a question best directed to the devicetree binding maintainers. I can't say that I personally care either way ;) > > Thanks, > > Lukas > > > -- >8 -- > > Subject: [PATCH] dt-bindings: iio: Document common properties > > > > It's about time we standardize on common names for frequently used IIO > > properties. For starters, document "vref-supply" and "continuous". > > > > Suggested-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > --- > > Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt > > index 68d6f8c..c3e87e15 100644 > > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt > > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt > > @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. > > io-channels = <&adc 10>, <&adc 11>; > > io-channel-names = "adc1", "adc2"; > > }; > > + > > +==Common IIO properties== > > + > > +Reference voltage: > > +ADCs, DACs and several other IIO devices require a reference voltage. > > +By convention the property specifying this regulator is named "vref-supply". > > +If the chip lacks a dedicated Vref pin and instead uses its own power supply > > +as reference, the property specifying the regulator is commonly named > > +"vdd-supply" or "vcc-supply". > > + > > +Continuous mode: > > +Some sensors can be configured to perform continuous (versus one-shot) > > +measurements. Continuous mode may require more energy in return for faster > > +or more reliable measurements. A boolean property named "continuous" > > +signifies that the device is configured for this mode. > > -- > > 2.11.0 > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sun, Sep 03, 2017 at 02:37:49PM +0100, Jonathan Cameron wrote: > +cc Mark Brown and linux-spi to address question about how to represent > a hardwired chip select. See below for why I think that is what we should > be representing rather than the fact it puts it in 'continuous mode'. There's a good chance that if you just send me mail with a not obviously related subject line it might get discarded unread, people CC me on lots of things that are of questionable relevance. > > >> > + - microchip,continuous-conversion (boolean): > > >> > + Only applicable to MCP3550/1/3: These ADCs have long > > >> > + conversion times and therefore support "continuous > > >> > + conversion mode" to allow retrieval of conversions > > >> > + at any time without observing a delay. The mode is > > >> > + enabled by permanently driving CS low, e.g. by wiring > > >> > + it to ground. > hmm. This is odd. We probably need to make the SPI subsystem aware of this. > It is possible to ask for exclusive use of an SPI bus and I think we should > be doing this here. It may be wired low on your board, but it may be wired to This sounds like SPI_NO_CS, though it's vanishingly rare for it to be implemented as there's a lot of ways for it to go wrong electrically - it's a lot simpler to just have a chip select and minimise the number of times it gets asserted.
On Sun, Aug 27, 2017 at 10:34 AM, Lukas Wunner <lukas@wunner.de> wrote: > On Fri, Aug 25, 2017 at 02:59:41PM -0500, Rob Herring wrote: >> On Tue, Aug 22, 2017 at 03:33:00PM +0200, Lukas Wunner wrote: >> > --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt >> > +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt >> > +Optional properties: >> > + - microchip,continuous-conversion (boolean): >> > + Only applicable to MCP3550/1/3: These ADCs have long >> > + conversion times and therefore support "continuous >> > + conversion mode" to allow retrieval of conversions >> > + at any time without observing a delay. The mode is >> > + enabled by permanently driving CS low, e.g. by wiring >> > + it to ground. >> >> Second binding I have seen today with a continuous property. Make this >> common (or maybe we already have one). > > The other one was the TI LMP92001 ADC driver posted by Abhisit Sangjan > (cc), however looking at the datasheet of that chip reveals that > continuous versus one-shot mode is selected by flipping a bit in the > chip's register map. > > So it is configurable at run-time. It's not something that's hardwired. > (Which is the case with the MCP3550 in my patch.) Couldn't you have CS tied to a GPIO line and then it is a run-time decision? > My understanding was that run-time configurable options should not be > listed in the device-tree at all, only hardware features. If that is > correct then that device-tree property should be dropped from Abhisit > Sangjan's patch. Configuring the feature via sysfs is fine I guess. It depends. The question who decides the mode. If an end-user would want to, then yes sysfs is the right place. If the h/w setup dictates what the configuration should be, then in DT is fine. > However we do have another driver supporting continuous versus one-shot > mode and that is drivers/iio/light/us5182d.c by Adriana Reus (cc). > The feature was added with commit c3304c212326. I'm not sure if it's > hardwired or runtime-configurable, the datasheet is gone from the > manufacturer's website. > > I agree that a common "continuous" property makes sense. We haven't > defined any common IIO properties so far and that has already led to > inconsistencies. E.g. most ADC/DAC drivers name the reference regulator > "vref-supply", but e.g. drivers/iio/dac/ad7303.c calls it "REF-supply". > > What do you think of this: > > -- >8 -- > Subject: [PATCH] dt-bindings: iio: Document common properties > > It's about time we standardize on common names for frequently used IIO > properties. For starters, document "vref-supply" and "continuous". > > Suggested-by: Rob Herring <robh@kernel.org> > Signed-off-by: Lukas Wunner <lukas@wunner.de> > --- > Documentation/devicetree/bindings/iio/iio-bindings.txt | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/iio-bindings.txt b/Documentation/devicetree/bindings/iio/iio-bindings.txt > index 68d6f8c..c3e87e15 100644 > --- a/Documentation/devicetree/bindings/iio/iio-bindings.txt > +++ b/Documentation/devicetree/bindings/iio/iio-bindings.txt > @@ -95,3 +95,18 @@ vdd channel is connected to output 0 of the &ref device. > io-channels = <&adc 10>, <&adc 11>; > io-channel-names = "adc1", "adc2"; > }; > + > +==Common IIO properties== > + > +Reference voltage: > +ADCs, DACs and several other IIO devices require a reference voltage. > +By convention the property specifying this regulator is named "vref-supply". > +If the chip lacks a dedicated Vref pin and instead uses its own power supply > +as reference, the property specifying the regulator is commonly named > +"vdd-supply" or "vcc-supply". > + > +Continuous mode: > +Some sensors can be configured to perform continuous (versus one-shot) > +measurements. Continuous mode may require more energy in return for faster > +or more reliable measurements. A boolean property named "continuous" > +signifies that the device is configured for this mode. Seems file, but start with the property names rather than buried in the paragraph. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 4 Sep 2017 18:22:21 +0100 Mark Brown <broonie@kernel.org> wrote: > On Sun, Sep 03, 2017 at 02:37:49PM +0100, Jonathan Cameron wrote: > > > +cc Mark Brown and linux-spi to address question about how to represent > > a hardwired chip select. See below for why I think that is what we should > > be representing rather than the fact it puts it in 'continuous mode'. > > There's a good chance that if you just send me mail with a not obviously > related subject line it might get discarded unread, people CC me on lots > of things that are of questionable relevance. > > > > >> > + - microchip,continuous-conversion (boolean): > > > >> > + Only applicable to MCP3550/1/3: These ADCs have long > > > >> > + conversion times and therefore support "continuous > > > >> > + conversion mode" to allow retrieval of conversions > > > >> > + at any time without observing a delay. The mode is > > > >> > + enabled by permanently driving CS low, e.g. by wiring > > > >> > + it to ground. > > > hmm. This is odd. We probably need to make the SPI subsystem aware of this. > > It is possible to ask for exclusive use of an SPI bus and I think we should > > be doing this here. It may be wired low on your board, but it may be wired to > > This sounds like SPI_NO_CS, though it's vanishingly rare for it to be > implemented as there's a lot of ways for it to go wrong electrically - > it's a lot simpler to just have a chip select and minimise the number of > times it gets asserted. Thanks Mark. It sounds like we don't actually need it for the driver under consideration on the particular platform it is being used on. If it comes up later, then adding bindings for SPI_NO_CS and adding relevant support in the driver looks the way to go to me. Jonathan -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt index bcd3ac8e6e0c..cf28af9ec0ac 100644 --- a/Documentation/devicetree/bindings/iio/adc/mcp320x.txt +++ b/Documentation/devicetree/bindings/iio/adc/mcp320x.txt @@ -29,15 +29,38 @@ Required properties: "microchip,mcp3204" "microchip,mcp3208" "microchip,mcp3301" + "microchip,mcp3550-50" + "microchip,mcp3550-60" + "microchip,mcp3551" + "microchip,mcp3553" NOTE: The use of the compatibles with no vendor prefix is deprecated and only listed because old DT use them. + - spi-cpha, spi-cpol (boolean): + Either SPI mode (0,0) or (1,1) must be used, so specify + none or both of spi-cpha, spi-cpol. The MCP3550/1/3 + is more efficient in mode (1,1) as only 3 instead of + 4 bytes need to be read from the ADC, but not all SPI + masters support it. + + - vref-supply: Phandle to the external reference voltage supply. + +Optional properties: + - microchip,continuous-conversion (boolean): + Only applicable to MCP3550/1/3: These ADCs have long + conversion times and therefore support "continuous + conversion mode" to allow retrieval of conversions + at any time without observing a delay. The mode is + enabled by permanently driving CS low, e.g. by wiring + it to ground. + Examples: spi_controller { mcp3x0x@0 { compatible = "mcp3002"; reg = <0>; spi-max-frequency = <1000000>; + vref-supply = <&vref_reg>; }; };
All chips supported by this driver clock data out on the falling edge and latch data in on the rising edge, hence SPI mode (0,0) or (1,1) must be used. Furthermore, none of the chips has an internal reference voltage regulator, so an external supply is always required and needs to be specified in the device tree lest the IIO "scale" in sysfs cannot be calculated. Document these requirements in the device tree binding, together with the new "microchip,continuous-conversion" property added specifically for the mcp3550/1/3. Cc: Mathias Duckeck <m.duckeck@kunbus.de> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- .../devicetree/bindings/iio/adc/mcp320x.txt | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+)