diff mbox series

[1/2] dt-bindings: iio: pressure: Add support for Honeywell HSC SPI sensors

Message ID 20181026181406.14545-2-carlos.iglesias@emutex.com (mailing list archive)
State New, archived
Headers show
Series iio: pressure: Add support for Honeywell HSC SPI sensors | expand

Commit Message

Carlos Iglesias Oct. 26, 2018, 6:14 p.m. UTC 
Add device tree bindings for the HSC pressure sensors.

Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com>
---
 .../bindings/iio/pressure/hsc_spi.txt         | 85 +++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt

Comments

Jonathan Cameron Oct. 28, 2018, 5:38 p.m. UTC | #1 
On Fri, 26 Oct 2018 18:14:36 +0000
Carlos Iglesias <carlos.iglesias@emutex.com> wrote:

> Add device tree bindings for the HSC pressure sensors.
> 
> Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com>
> ---
>  .../bindings/iio/pressure/hsc_spi.txt         | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> new file mode 100644
> index 000000000000..2302d6eef7c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> @@ -0,0 +1,85 @@
> +Honeywell HSC Series of Pressure Sensors
> +
> +Pressure sensors from Honeywell with analog, I2C and SPI interfaces
> +
> +Required properties:
> +- compatible: selects the sensor model; must be one of the following
> +	"honeywell,hsc001baa"
> +	"honeywell,hsc001bab"
> +	"honeywell,hsc001bac"
> +	"honeywell,hsc001baf"
> +	"honeywell,hsc1_6baa"
> +	"honeywell,hsc1_6bab"
> +	"honeywell,hsc1_6bac"
> +	"honeywell,hsc1_6baf"
> +	"honeywell,hsc2_5baa"
> +	"honeywell,hsc2_5bab"
> +	"honeywell,hsc2_5bac"
> +	"honeywell,hsc2_5baf"
> +	"honeywell,hsc004baa"
> +	"honeywell,hsc004bab"
> +	"honeywell,hsc004bac"
> +	"honeywell,hsc004baf"
> +	"honeywell,hsc006baa"
> +	"honeywell,hsc006bab"
> +	"honeywell,hsc006bac"
> +	"honeywell,hsc006baf"
> +	"honeywell,hsc010baa"
> +	"honeywell,hsc010bab"
> +	"honeywell,hsc010bac"
> +	"honeywell,hsc010baf"
> +	"honeywell,hsc100kaa"
> +	"honeywell,hsc100kab"
> +	"honeywell,hsc100kac"
> +	"honeywell,hsc100kaf"
> +	"honeywell,hsc160kaa"
> +	"honeywell,hsc160kab"
> +	"honeywell,hsc160kac"
> +	"honeywell,hsc160kaf"
> +	"honeywell,hsc250kaa"
> +	"honeywell,hsc250kab"
> +	"honeywell,hsc250kac"
> +	"honeywell,hsc250kaf"
> +	"honeywell,hsc400kaa"
> +	"honeywell,hsc400kab"
> +	"honeywell,hsc400kac"
> +	"honeywell,hsc400kaf"
> +	"honeywell,hsc600kaa"
> +	"honeywell,hsc600kab"
> +	"honeywell,hsc600kac"
> +	"honeywell,hsc600kaf"
> +	"honeywell,hsc001gaa"
> +	"honeywell,hsc001gab"
> +	"honeywell,hsc001gac"
> +	"honeywell,hsc001gaf"
> +	"honeywell,hsc015paa"
> +	"honeywell,hsc015pab"
> +	"honeywell,hsc015pac"
> +	"honeywell,hsc015paf"
> +	"honeywell,hsc030paa"
> +	"honeywell,hsc030pab"
> +	"honeywell,hsc030pac"
> +	"honeywell,hsc030paf"
> +	"honeywell,hsc060paa"
> +	"honeywell,hsc060pab"
> +	"honeywell,hsc060pac"
> +	"honeywell,hsc060paf"
> +	"honeywell,hsc100paa"
> +	"honeywell,hsc100pab"
> +	"honeywell,hsc100pac"
> +	"honeywell,hsc100paf"
> +	"honeywell,hsc150paa"
> +	"honeywell,hsc150pab"
> +	"honeywell,hsc150pac"
> +	"honeywell,hsc150paf"
> +- reg: the SPI chip select number used by the sensor.
> +- spi-max-frequency: maximum clock frequency (Hz) used for the SPI bus.
> +  The maximum value supported by the sensors is 400000.
As Rob pointed out in a few reviews this week, the devicetree binding
for this should only be applying a tighter bound than either the
device or the bus master. So something introduced by the board
layout for example, or a level convertor..

Jonathan
> +
> +Example:
> +
> +	hsc_spi0: hsc@0 {
> +		compatible = "honeywell,hsc010baa";
> +		reg = <0>;
> +		spi-max-frequency = <400000>;
> +	};
Jonathan Cameron Nov. 3, 2018, 10:36 a.m. UTC | #2 
Rob, just to draw your attention, some discussion around
spi-max-frequency that you might want to look at below.


On Tue, 30 Oct 2018 15:56:58 +0000
Carlos Iglesias <carlos.iglesias@emutex.com> wrote:

> Please see comments inline below.
Hi Carlos,

Please try to use an email client (or configure yours to do it)
that adds indentation to each level of reply.  It's somewhat
fiddly to separate the different parts of the conversation!

> 
> On 10/28/18 5:38 PM, Jonathan Cameron wrote:
> 
> On Fri, 26 Oct 2018 18:14:36 +0000
> Carlos Iglesias <carlos.iglesias@emutex.com><mailto:carlos.iglesias@emutex.com> wrote:
> 
> 
> 
> Add device tree bindings for the HSC pressure sensors.
> 
> Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com><mailto:carlos.iglesias@emutex.com>
> ---
>  .../bindings/iio/pressure/hsc_spi.txt         | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> new file mode 100644
> index 000000000000..2302d6eef7c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> @@ -0,0 +1,85 @@
> +Honeywell HSC Series of Pressure Sensors
> +
> +Pressure sensors from Honeywell with analog, I2C and SPI interfaces
> +
> +Required properties:
> +- compatible: selects the sensor model; must be one of the following
> +       "honeywell,hsc001baa"
> +       "honeywell,hsc001bab"
> +       "honeywell,hsc001bac"
> +       "honeywell,hsc001baf"
> +       "honeywell,hsc1_6baa"
> +       "honeywell,hsc1_6bab"
> +       "honeywell,hsc1_6bac"
> +       "honeywell,hsc1_6baf"
> +       "honeywell,hsc2_5baa"
> +       "honeywell,hsc2_5bab"
> +       "honeywell,hsc2_5bac"
> +       "honeywell,hsc2_5baf"
> +       "honeywell,hsc004baa"
> +       "honeywell,hsc004bab"
> +       "honeywell,hsc004bac"
> +       "honeywell,hsc004baf"
> +       "honeywell,hsc006baa"
> +       "honeywell,hsc006bab"
> +       "honeywell,hsc006bac"
> +       "honeywell,hsc006baf"
> +       "honeywell,hsc010baa"
> +       "honeywell,hsc010bab"
> +       "honeywell,hsc010bac"
> +       "honeywell,hsc010baf"
> +       "honeywell,hsc100kaa"
> +       "honeywell,hsc100kab"
> +       "honeywell,hsc100kac"
> +       "honeywell,hsc100kaf"
> +       "honeywell,hsc160kaa"
> +       "honeywell,hsc160kab"
> +       "honeywell,hsc160kac"
> +       "honeywell,hsc160kaf"
> +       "honeywell,hsc250kaa"
> +       "honeywell,hsc250kab"
> +       "honeywell,hsc250kac"
> +       "honeywell,hsc250kaf"
> +       "honeywell,hsc400kaa"
> +       "honeywell,hsc400kab"
> +       "honeywell,hsc400kac"
> +       "honeywell,hsc400kaf"
> +       "honeywell,hsc600kaa"
> +       "honeywell,hsc600kab"
> +       "honeywell,hsc600kac"
> +       "honeywell,hsc600kaf"
> +       "honeywell,hsc001gaa"
> +       "honeywell,hsc001gab"
> +       "honeywell,hsc001gac"
> +       "honeywell,hsc001gaf"
> +       "honeywell,hsc015paa"
> +       "honeywell,hsc015pab"
> +       "honeywell,hsc015pac"
> +       "honeywell,hsc015paf"
> +       "honeywell,hsc030paa"
> +       "honeywell,hsc030pab"
> +       "honeywell,hsc030pac"
> +       "honeywell,hsc030paf"
> +       "honeywell,hsc060paa"
> +       "honeywell,hsc060pab"
> +       "honeywell,hsc060pac"
> +       "honeywell,hsc060paf"
> +       "honeywell,hsc100paa"
> +       "honeywell,hsc100pab"
> +       "honeywell,hsc100pac"
> +       "honeywell,hsc100paf"
> +       "honeywell,hsc150paa"
> +       "honeywell,hsc150pab"
> +       "honeywell,hsc150pac"
> +       "honeywell,hsc150paf"
> +- reg: the SPI chip select number used by the sensor.
> +- spi-max-frequency: maximum clock frequency (Hz) used for the SPI bus.
> +  The maximum value supported by the sensors is 400000.
> 
> 
> As Rob pointed out in a few reviews this week, the devicetree binding
> for this should only be applying a tighter bound than either the
> device or the bus master. So something introduced by the board
> layout for example, or a level convertor..
> 
> Jonathan
> 
> Checking the devicetree bindings files of other spi sensors I see a frequent approach is to include a reference to Documentation/devicetree/bindings/spi/spi-bus.txt, for example:
> 
> Property rules described in Documentation/devicetree/bindings/spi/spi-bus.txt apply. In particular, "reg" and "spi-max-frequency" properties must be given.
The point is that spi-max-frequency doesn't actually need to be given.  If the
fine with which ever is lower of that supported by the device and that supported
by the master then it will use that without spi-max-frequency being provided.

I agree this wasn't previously my understanding but it does make sense. No
obvious reason why this value would need providing by DT unless there is
a board issue that neither of the 'ends' can see.

We might want to look at cleaning this up in general through the bindings
(and making sure device drivers apply there own maximums).  Rob
is this a worthwhile cleanup?

> 
> Maybe it would be a better option to place such a note instead of the "reg" and "spi-max-frequency" points. What do you think?
> 
> Sorry, I was not aware of Rob Herring's previous comments on this topic, I am not subscribed to the LKML.

Sure, I wasn't aware of them until that day anyway ;)

Thanks,

Jonathan

> 
> 
> 
> 
> +
> +Example:
> +
> +       hsc_spi0: hsc@0 {
> +               compatible = "honeywell,hsc010baa";
> +               reg = <0>;
> +               spi-max-frequency = <400000>;
> +       };
> 
> 
> 
>
Rob Herring Nov. 6, 2018, 9:29 p.m. UTC | #3 
On Fri, Oct 26, 2018 at 06:14:36PM +0000, Carlos Iglesias wrote:
> Add device tree bindings for the HSC pressure sensors.
> 
> Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com>
> ---
>  .../bindings/iio/pressure/hsc_spi.txt         | 85 +++++++++++++++++++
>  1 file changed, 85 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> new file mode 100644
> index 000000000000..2302d6eef7c7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> @@ -0,0 +1,85 @@
> +Honeywell HSC Series of Pressure Sensors
> +
> +Pressure sensors from Honeywell with analog, I2C and SPI interfaces

But you named the file with 'spi'. You expect 
> +
> +Required properties:
> +- compatible: selects the sensor model; must be one of the following
> +	"honeywell,hsc001baa"
> +	"honeywell,hsc001bab"
> +	"honeywell,hsc001bac"
> +	"honeywell,hsc001baf"

I normally tell people to imply things from compatible strings, but this 
may be a good case where some properties make sense. The last letter 
appears to be 'transfer function'. Maybe even the pressure range should 
be a property. I think it depends if there's any other s/w visible 
differences.

Plus this list doesn't even include the differential or gage parts and 
those lists are longer.

> +	"honeywell,hsc1_6baa"
> +	"honeywell,hsc1_6bab"
> +	"honeywell,hsc1_6bac"
> +	"honeywell,hsc1_6baf"
> +	"honeywell,hsc2_5baa"
> +	"honeywell,hsc2_5bab"
> +	"honeywell,hsc2_5bac"
> +	"honeywell,hsc2_5baf"
> +	"honeywell,hsc004baa"
> +	"honeywell,hsc004bab"
> +	"honeywell,hsc004bac"
> +	"honeywell,hsc004baf"
> +	"honeywell,hsc006baa"
> +	"honeywell,hsc006bab"
> +	"honeywell,hsc006bac"
> +	"honeywell,hsc006baf"
> +	"honeywell,hsc010baa"
> +	"honeywell,hsc010bab"
> +	"honeywell,hsc010bac"
> +	"honeywell,hsc010baf"
> +	"honeywell,hsc100kaa"
> +	"honeywell,hsc100kab"
> +	"honeywell,hsc100kac"
> +	"honeywell,hsc100kaf"
> +	"honeywell,hsc160kaa"
> +	"honeywell,hsc160kab"
> +	"honeywell,hsc160kac"
> +	"honeywell,hsc160kaf"
> +	"honeywell,hsc250kaa"
> +	"honeywell,hsc250kab"
> +	"honeywell,hsc250kac"
> +	"honeywell,hsc250kaf"
> +	"honeywell,hsc400kaa"
> +	"honeywell,hsc400kab"
> +	"honeywell,hsc400kac"
> +	"honeywell,hsc400kaf"
> +	"honeywell,hsc600kaa"
> +	"honeywell,hsc600kab"
> +	"honeywell,hsc600kac"
> +	"honeywell,hsc600kaf"
> +	"honeywell,hsc001gaa"
> +	"honeywell,hsc001gab"
> +	"honeywell,hsc001gac"
> +	"honeywell,hsc001gaf"
> +	"honeywell,hsc015paa"
> +	"honeywell,hsc015pab"
> +	"honeywell,hsc015pac"
> +	"honeywell,hsc015paf"
> +	"honeywell,hsc030paa"
> +	"honeywell,hsc030pab"
> +	"honeywell,hsc030pac"
> +	"honeywell,hsc030paf"
> +	"honeywell,hsc060paa"
> +	"honeywell,hsc060pab"
> +	"honeywell,hsc060pac"
> +	"honeywell,hsc060paf"
> +	"honeywell,hsc100paa"
> +	"honeywell,hsc100pab"
> +	"honeywell,hsc100pac"
> +	"honeywell,hsc100paf"
> +	"honeywell,hsc150paa"
> +	"honeywell,hsc150pab"
> +	"honeywell,hsc150pac"
> +	"honeywell,hsc150paf"
> +- reg: the SPI chip select number used by the sensor.
> +- spi-max-frequency: maximum clock frequency (Hz) used for the SPI bus.
> +  The maximum value supported by the sensors is 400000.
> +
> +Example:
> +
> +	hsc_spi0: hsc@0 {

pressure-sensor@0

> +		compatible = "honeywell,hsc010baa";
> +		reg = <0>;
> +		spi-max-frequency = <400000>;
> +	};
> -- 
> 2.19.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
new file mode 100644
index 000000000000..2302d6eef7c7
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
@@ -0,0 +1,85 @@ 
+Honeywell HSC Series of Pressure Sensors
+
+Pressure sensors from Honeywell with analog, I2C and SPI interfaces
+
+Required properties:
+- compatible: selects the sensor model; must be one of the following
+	"honeywell,hsc001baa"
+	"honeywell,hsc001bab"
+	"honeywell,hsc001bac"
+	"honeywell,hsc001baf"
+	"honeywell,hsc1_6baa"
+	"honeywell,hsc1_6bab"
+	"honeywell,hsc1_6bac"
+	"honeywell,hsc1_6baf"
+	"honeywell,hsc2_5baa"
+	"honeywell,hsc2_5bab"
+	"honeywell,hsc2_5bac"
+	"honeywell,hsc2_5baf"
+	"honeywell,hsc004baa"
+	"honeywell,hsc004bab"
+	"honeywell,hsc004bac"
+	"honeywell,hsc004baf"
+	"honeywell,hsc006baa"
+	"honeywell,hsc006bab"
+	"honeywell,hsc006bac"
+	"honeywell,hsc006baf"
+	"honeywell,hsc010baa"
+	"honeywell,hsc010bab"
+	"honeywell,hsc010bac"
+	"honeywell,hsc010baf"
+	"honeywell,hsc100kaa"
+	"honeywell,hsc100kab"
+	"honeywell,hsc100kac"
+	"honeywell,hsc100kaf"
+	"honeywell,hsc160kaa"
+	"honeywell,hsc160kab"
+	"honeywell,hsc160kac"
+	"honeywell,hsc160kaf"
+	"honeywell,hsc250kaa"
+	"honeywell,hsc250kab"
+	"honeywell,hsc250kac"
+	"honeywell,hsc250kaf"
+	"honeywell,hsc400kaa"
+	"honeywell,hsc400kab"
+	"honeywell,hsc400kac"
+	"honeywell,hsc400kaf"
+	"honeywell,hsc600kaa"
+	"honeywell,hsc600kab"
+	"honeywell,hsc600kac"
+	"honeywell,hsc600kaf"
+	"honeywell,hsc001gaa"
+	"honeywell,hsc001gab"
+	"honeywell,hsc001gac"
+	"honeywell,hsc001gaf"
+	"honeywell,hsc015paa"
+	"honeywell,hsc015pab"
+	"honeywell,hsc015pac"
+	"honeywell,hsc015paf"
+	"honeywell,hsc030paa"
+	"honeywell,hsc030pab"
+	"honeywell,hsc030pac"
+	"honeywell,hsc030paf"
+	"honeywell,hsc060paa"
+	"honeywell,hsc060pab"
+	"honeywell,hsc060pac"
+	"honeywell,hsc060paf"
+	"honeywell,hsc100paa"
+	"honeywell,hsc100pab"
+	"honeywell,hsc100pac"
+	"honeywell,hsc100paf"
+	"honeywell,hsc150paa"
+	"honeywell,hsc150pab"
+	"honeywell,hsc150pac"
+	"honeywell,hsc150paf"
+- reg: the SPI chip select number used by the sensor.
+- spi-max-frequency: maximum clock frequency (Hz) used for the SPI bus.
+  The maximum value supported by the sensors is 400000.
+
+Example:
+
+	hsc_spi0: hsc@0 {
+		compatible = "honeywell,hsc010baa";
+		reg = <0>;
+		spi-max-frequency = <400000>;
+	};