[PATCH 0/2] iio: pressure: Add support for Honeywell HSC SPI sensors

[PATCH 0/2] iio: pressure: Add support for Honeywell HSC SPI sensors

[Carlos Iglesias]

Hi all,

The Honeywell HSC is a series of pressure sensors that provide different
pressure ranges and measurement types (absolute, differential, gauge). The
available output types are analog, I2C or SPI (see [1]).

This driver provides support for a limited set of the HSC sensor models,
namely the absolute pressure sensors with SPI interface.

References:
[1] Datasheet https://sensing.honeywell.com/honeywell-sensing-trustability-hsc-series-high-accuracy-board-mount-pressure-sensors-50099148-a-en.pdf
[2] SPI Communications with Honeywell Digital Output Pressure Sensors https://sensing.honeywell.com/spi-comms-digital-ouptu-pressure-sensors-tn-008202-3-en-final-30may12.pdf

Carlos Iglesias (2):
  dt-bindings: iio: pressure: Add support for Honeywell HSC SPI sensors
  iio: pressure: Add support for Honeywell HSC SPI sensors

 .../bindings/iio/pressure/hsc_spi.txt         |  85 +++
 MAINTAINERS                                   |   6 +
 drivers/iio/pressure/Kconfig                  |  10 +
 drivers/iio/pressure/Makefile                 |   2 +
 drivers/iio/pressure/hsc_spi.c                | 543 ++++++++++++++++++
 5 files changed, 646 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
 create mode 100644 drivers/iio/pressure/hsc_spi.c

-- 
2.19.1

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

[Carlos Iglesias]

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.
+
+Example:
+
+	hsc_spi0: hsc@0 {
+		compatible = "honeywell,hsc010baa";
+		reg = <0>;
+		spi-max-frequency = <400000>;
+	};
-- 
2.19.1

[PATCH 2/2] iio: pressure: Add support for Honeywell HSC SPI sensors

[Carlos Iglesias]

Add device driver for the HSC pressure sensors with SPI interface.
In addition to the main measurement -pressure- these sensors also
provide temperature measurement.

Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com>
---
 MAINTAINERS                    |   6 +
 drivers/iio/pressure/Kconfig   |  10 +
 drivers/iio/pressure/Makefile  |   2 +
 drivers/iio/pressure/hsc_spi.c | 543 +++++++++++++++++++++++++++++++++
 4 files changed, 561 insertions(+)
 create mode 100644 drivers/iio/pressure/hsc_spi.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 556f902b3766..a4af4cbd4b0f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6696,6 +6696,12 @@ W:	http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi
 S:	Maintained
 F:	fs/hpfs/
 
+HSC SERIES PRESSURE SENSORS
+M;	Carlos Iglesias <carlos.iglesias@emutex.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
+F:	drivers/iio/pressure/hsc_spi.c
+
 HSI SUBSYSTEM
 M:	Sebastian Reichel <sre@kernel.org>
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-hsi.git
diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
index eaa7cfcb4c2a..593663d4ffa8 100644
--- a/drivers/iio/pressure/Kconfig
+++ b/drivers/iio/pressure/Kconfig
@@ -227,4 +227,14 @@ config ZPA2326_SPI
 	tristate
 	select REGMAP_SPI
 
+config HSC_SPI
+	tristate "Honeywell HSC pressure sensors (SPI)"
+	depends on SPI_MASTER
+	help
+	  Say yes here to build support for the Honeywell HSC range of
+	  pressure sensors (SPI interface only).
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called hsc_spi.
+
 endmenu
diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
index c2058d7b2f93..b1db2116e9f6 100644
--- a/drivers/iio/pressure/Makefile
+++ b/drivers/iio/pressure/Makefile
@@ -31,3 +31,5 @@ obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o
 
 obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
 obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
+
+obj-$(CONFIG_HSC_SPI) += hsc_spi.o
diff --git a/drivers/iio/pressure/hsc_spi.c b/drivers/iio/pressure/hsc_spi.c
new file mode 100644
index 000000000000..efe45a09da8f
--- /dev/null
+++ b/drivers/iio/pressure/hsc_spi.c
@@ -0,0 +1,543 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * hsc_spi.c - Driver for Honeywell HSC pressure sensors with
+ *             SPI interface
+ *
+ * Copyright (c) 2018 Carlos Iglesias <carlos.iglesias@emutex.com>
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/iio/iio.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/spi/spi.h>
+
+#define HSC_MAX_SPI_FREQ_HZ	400000
+
+#define HSC_TEMP_BITS		11
+#define HSC_PRESS_BITS		14
+#define HSC_TEMP_MASK		(0x7FF)
+#define HSC_TEMP_SHIFT		(5)
+
+#define HSC_STATUS_S0		BIT(14)
+#define HSC_STATUS_S1		BIT(15)
+#define HSC_STATUS_MSK		((HSC_STATUS_S0) | (HSC_STATUS_S1))
+#define HSC_STATUS_CMD		HSC_STATUS_S0
+#define HSC_STATUS_STALE	HSC_STATUS_S1
+#define HSC_STATUS_DIAG		((HSC_STATUS_S0) | (HSC_STATUS_S1))
+
+static inline int hsc_status_error(struct device dev, int val)
+{
+	int st_check = val & HSC_STATUS_MSK;
+
+	if (st_check) {
+		switch (st_check) {
+		case HSC_STATUS_CMD:
+			dev_warn(&dev, "%s:Device in COMMAND MODE\n",
+				 __func__);
+			return -EIO;
+		case HSC_STATUS_STALE:
+			dev_warn(&dev, "%s:Stale data - sampling too fast?\n",
+				 __func__);
+			return -EAGAIN;
+		case HSC_STATUS_DIAG:
+			dev_warn(&dev, "%s:Calibration signature changed\n",
+				 __func__);
+			return -EIO;
+		default:
+			dev_err(&dev, "%s:Invalid status code (%d)\n",
+				__func__, st_check);
+			return -EIO;
+		}
+	}
+
+	return 0;
+}
+
+enum hsc_variant {
+	/* Note: Only the absolute range sensors are supported */
+	HSC001BAA, HSC001BAB, HSC001BAC, HSC001BAF,
+	HSC1_6BAA, HSC1_6BAB, HSC1_6BAC, HSC1_6BAF,
+	HSC2_5BAA, HSC2_5BAB, HSC2_5BAC, HSC2_5BAF,
+	HSC004BAA, HSC004BAB, HSC004BAC, HSC004BAF,
+	HSC006BAA, HSC006BAB, HSC006BAC, HSC006BAF,
+	HSC010BAA, HSC010BAB, HSC010BAC, HSC010BAF,
+
+	HSC100KAA, HSC100KAB, HSC100KAC, HSC100KAF,
+	HSC160KAA, HSC160KAB, HSC160KAC, HSC160KAF,
+	HSC250KAA, HSC250KAB, HSC250KAC, HSC250KAF,
+	HSC400KAA, HSC400KAB, HSC400KAC, HSC400KAF,
+	HSC600KAA, HSC600KAB, HSC600KAC, HSC600KAF,
+	HSC001GAA, HSC001GAB, HSC001GAC, HSC001GAF,
+
+	HSC015PAA, HSC015PAB, HSC015PAC, HSC015PAF,
+	HSC030PAA, HSC030PAB, HSC030PAC, HSC030PAF,
+	HSC060PAA, HSC060PAB, HSC060PAC, HSC060PAF,
+	HSC100PAA, HSC100PAB, HSC100PAC, HSC100PAF,
+	HSC150PAA, HSC150PAB, HSC150PAC, HSC150PAF,
+};
+
+enum hsc_meas_channel {
+	HSC_CH_PRESSURE,
+	HSC_CH_TEMPERATURE
+};
+
+struct hsc_config {
+	int pmin;	/* Lower pressure limit */
+	int pmax;	/* Upper pressure limit */
+	int rmin;	/* Lower transfer function limit (%) */
+	int rmax;	/* Upper transfer function limit (%) */
+	int knum;	/* Pressure kPa conversion factor (numerator) */
+	int kden;	/* Pressure kPa conversion factor /denominator) */
+};
+
+struct hsc_fract_val {
+	int num;	/* numerator */
+	int den;	/* denominator */
+};
+
+struct hsc_state {
+	struct device *dev;
+	struct spi_device *spi_dev;
+	struct spi_transfer spi_xfer;
+	struct spi_message spi_msg;
+	__be16 rx_buf[2];
+
+	/* Model-dependent values */
+	struct hsc_fract_val scale;
+	struct hsc_fract_val offset;
+};
+
+#define HSC_CONFIG(_pmin, _pmax, _rmin, _rmax, _knum, _kden) {	\
+		.pmin = (_pmin),				\
+		.pmax = (_pmax),				\
+		.rmin = (_rmin),				\
+		.rmax = (_rmax),				\
+		.knum = (_knum),				\
+		.kden = (_kden),				\
+	}
+
+static struct hsc_config hsc_cfg[] = {
+	/* Absolute range, mbar */
+	[HSC001BAA] = HSC_CONFIG(0, 1000, 10, 90, 1, 10),
+	[HSC001BAB] = HSC_CONFIG(0, 1000, 5, 95, 1, 10),
+	[HSC001BAC] = HSC_CONFIG(0, 1000, 5, 85, 1, 10),
+	[HSC001BAF] = HSC_CONFIG(0, 1000, 4, 94, 1, 10),
+	[HSC1_6BAA] = HSC_CONFIG(0, 1600, 10, 90, 1, 10),
+	[HSC1_6BAB] = HSC_CONFIG(0, 1600, 5, 95, 1, 10),
+	[HSC1_6BAC] = HSC_CONFIG(0, 1600, 5, 85, 1, 10),
+	[HSC1_6BAF] = HSC_CONFIG(0, 1600, 4, 94, 1, 10),
+	[HSC2_5BAA] = HSC_CONFIG(0, 2500, 10, 90, 1, 10),
+	[HSC2_5BAB] = HSC_CONFIG(0, 2500, 5, 95, 1, 10),
+	[HSC2_5BAC] = HSC_CONFIG(0, 2500, 5, 85, 1, 10),
+	[HSC2_5BAF] = HSC_CONFIG(0, 2500, 4, 94, 1, 10),
+	[HSC004BAA] = HSC_CONFIG(0, 4000, 10, 90, 1, 10),
+	[HSC004BAB] = HSC_CONFIG(0, 4000, 5, 95, 1, 10),
+	[HSC004BAC] = HSC_CONFIG(0, 4000, 5, 85, 1, 10),
+	[HSC004BAF] = HSC_CONFIG(0, 4000, 4, 94, 1, 10),
+	[HSC006BAA] = HSC_CONFIG(0, 6000, 10, 90, 1, 10),
+	[HSC006BAB] = HSC_CONFIG(0, 6000, 5, 95, 1, 10),
+	[HSC006BAC] = HSC_CONFIG(0, 6000, 5, 85, 1, 10),
+	[HSC006BAF] = HSC_CONFIG(0, 6000, 4, 94, 1, 10),
+	[HSC010BAA] = HSC_CONFIG(0, 10000, 10, 90, 1, 10),
+	[HSC010BAB] = HSC_CONFIG(0, 10000, 5, 95, 1, 10),
+	[HSC010BAC] = HSC_CONFIG(0, 10000, 5, 85, 1, 10),
+	[HSC010BAF] = HSC_CONFIG(0, 10000, 4, 94, 1, 10),
+	/* Absolute range, kPa */
+	[HSC100KAA] = HSC_CONFIG(0, 100, 10, 90, 1, 1),
+	[HSC100KAB] = HSC_CONFIG(0, 100, 5, 95, 1, 1),
+	[HSC100KAC] = HSC_CONFIG(0, 100, 5, 85, 1, 1),
+	[HSC100KAF] = HSC_CONFIG(0, 100, 4, 94, 1, 1),
+	[HSC160KAA] = HSC_CONFIG(0, 160, 10, 90, 1, 1),
+	[HSC160KAB] = HSC_CONFIG(0, 160, 5, 95, 1, 1),
+	[HSC160KAC] = HSC_CONFIG(0, 160, 5, 85, 1, 1),
+	[HSC160KAF] = HSC_CONFIG(0, 160, 4, 94, 1, 1),
+	[HSC250KAA] = HSC_CONFIG(0, 250, 10, 90, 1, 1),
+	[HSC250KAB] = HSC_CONFIG(0, 250, 5, 95, 1, 1),
+	[HSC250KAC] = HSC_CONFIG(0, 250, 5, 85, 1, 1),
+	[HSC250KAF] = HSC_CONFIG(0, 250, 4, 94, 1, 1),
+	[HSC400KAA] = HSC_CONFIG(0, 400, 10, 90, 1, 1),
+	[HSC400KAB] = HSC_CONFIG(0, 400, 5, 95, 1, 1),
+	[HSC400KAC] = HSC_CONFIG(0, 400, 5, 85, 1, 1),
+	[HSC400KAF] = HSC_CONFIG(0, 400, 4, 94, 1, 1),
+	[HSC600KAA] = HSC_CONFIG(0, 600, 10, 90, 1, 1),
+	[HSC600KAB] = HSC_CONFIG(0, 600, 5, 95, 1, 1),
+	[HSC600KAC] = HSC_CONFIG(0, 600, 5, 85, 1, 1),
+	[HSC600KAF] = HSC_CONFIG(0, 600, 4, 94, 1, 1),
+	[HSC001GAA] = HSC_CONFIG(0, 1000, 10, 90, 1, 1),
+	[HSC001GAB] = HSC_CONFIG(0, 1000, 5, 95, 1, 1),
+	[HSC001GAC] = HSC_CONFIG(0, 1000, 5, 85, 1, 1),
+	[HSC001GAF] = HSC_CONFIG(0, 1000, 4, 94, 1, 1),
+	/* Absolute range, psi */
+	[HSC015PAA] = HSC_CONFIG(0, 15, 10, 90, 6895, 1000),
+	[HSC015PAB] = HSC_CONFIG(0, 15, 5, 95, 6895, 1000),
+	[HSC015PAC] = HSC_CONFIG(0, 15, 5, 85, 6895, 1000),
+	[HSC015PAF] = HSC_CONFIG(0, 15, 4, 94, 6895, 1000),
+	[HSC030PAA] = HSC_CONFIG(0, 30, 10, 90, 6895, 1000),
+	[HSC030PAB] = HSC_CONFIG(0, 30, 5, 95, 6895, 1000),
+	[HSC030PAC] = HSC_CONFIG(0, 30, 5, 85, 6895, 1000),
+	[HSC030PAF] = HSC_CONFIG(0, 30, 4, 94, 6895, 1000),
+	[HSC060PAA] = HSC_CONFIG(0, 60, 10, 90, 6895, 1000),
+	[HSC060PAB] = HSC_CONFIG(0, 60, 5, 95, 6895, 1000),
+	[HSC060PAC] = HSC_CONFIG(0, 60, 5, 85, 6895, 1000),
+	[HSC060PAF] = HSC_CONFIG(0, 60, 4, 94, 6895, 1000),
+	[HSC100PAA] = HSC_CONFIG(0, 100, 10, 90, 6895, 1000),
+	[HSC100PAB] = HSC_CONFIG(0, 100, 5, 95, 6895, 1000),
+	[HSC100PAC] = HSC_CONFIG(0, 100, 5, 85, 6895, 1000),
+	[HSC100PAF] = HSC_CONFIG(0, 100, 4, 94, 6895, 1000),
+	[HSC150PAA] = HSC_CONFIG(0, 150, 10, 90, 6895, 1000),
+	[HSC150PAB] = HSC_CONFIG(0, 150, 5, 95, 6895, 1000),
+	[HSC150PAC] = HSC_CONFIG(0, 150, 5, 85, 6895, 1000),
+	[HSC150PAF] = HSC_CONFIG(0, 150, 4, 94, 6895, 1000),
+};
+
+static const struct iio_chan_spec hsc_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_OFFSET) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.channel = HSC_CH_PRESSURE,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = HSC_PRESS_BITS,
+			.storagebits = 16,
+			.shift = 0,
+			.endianness = IIO_BE,
+		}
+	},
+	{
+		.type = IIO_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_OFFSET) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.channel = HSC_CH_TEMPERATURE,
+		.scan_type = {
+			.sign = 'u',
+			.realbits = HSC_TEMP_BITS,
+			.storagebits = 16,
+			.shift = HSC_TEMP_SHIFT,
+			.endianness = IIO_BE,
+		}
+	}
+};
+
+static int hsc_get_pressure(struct hsc_state *state)
+{
+	int ret = 0;
+	int error;
+
+	state->spi_xfer.len = 2;
+	ret = spi_sync(state->spi_dev, &state->spi_msg);
+	if (ret)
+		return ret;
+
+	ret = be16_to_cpu(state->rx_buf[0]);
+
+	error = hsc_status_error(state->spi_dev->dev, ret);
+	if (error)
+		return error;
+
+	return ret;
+}
+
+static int hsc_get_temperature(struct hsc_state *state)
+{
+	int ret;
+	int error;
+
+	state->spi_xfer.len = 4;
+	ret = spi_sync(state->spi_dev, &state->spi_msg);
+	if (ret)
+		return ret;
+
+	ret = be16_to_cpu(state->rx_buf[0]);
+	error = hsc_status_error(state->spi_dev->dev, ret);
+	if (error)
+		return error;
+
+	ret = be16_to_cpu(state->rx_buf[1]);
+	ret = (ret >> HSC_TEMP_SHIFT) & HSC_TEMP_MASK;
+
+	return ret;
+}
+
+static int hsc_read_raw(struct iio_dev *indio_dev,
+			struct iio_chan_spec const *chan, int *val,
+			int *val2, long mask)
+{
+	struct hsc_state *state = iio_priv(indio_dev);
+	int ret;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		switch (chan->channel) {
+		case HSC_CH_PRESSURE:
+			ret = hsc_get_pressure(state);
+			if (ret < 0)
+				break;
+			*val = ret;
+			ret = IIO_VAL_INT;
+			break;
+		case HSC_CH_TEMPERATURE:
+			ret = hsc_get_temperature(state);
+			if (ret < 0)
+				break;
+			*val = ret;
+			ret = IIO_VAL_INT;
+			break;
+		default:
+			ret = -EINVAL;
+			dev_err(state->dev,
+				"%s - IIO_CHAN_INFO_RAW-bad channel (%d)\n",
+				__func__, chan->channel);
+			break;
+		}
+		break;
+	case IIO_CHAN_INFO_OFFSET:
+		switch (chan->channel) {
+		case HSC_CH_PRESSURE:
+			*val = state->offset.num;
+			*val2 = state->offset.den;
+			ret = IIO_VAL_FRACTIONAL;
+			break;
+		case HSC_CH_TEMPERATURE:
+			*val = BIT(HSC_TEMP_BITS) - 1;
+			*val2 = -200 / 50;
+			ret = IIO_VAL_FRACTIONAL;
+			break;
+		default:
+			dev_err(state->dev,
+				"%s - IIO_CHAN_INFO_OFFSET-bad channel (%d)\n",
+				__func__, chan->channel);
+			ret = -EINVAL;
+			break;
+		}
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		switch (chan->channel) {
+		case HSC_CH_PRESSURE:		/* output unit is kPa */
+			*val = state->scale.num;
+			*val2 = state->scale.den;
+			ret = IIO_VAL_FRACTIONAL;
+			break;
+		case HSC_CH_TEMPERATURE:  /* output unit is milli Celsius */
+			*val = 200 * 1000;
+			*val2 = BIT(HSC_TEMP_BITS) - 1;
+			ret = IIO_VAL_FRACTIONAL;
+			break;
+		default:
+			dev_err(state->dev,
+				"%s - IIO_CHAN_INFO_SCALE-bad channel (%d)\n",
+				__func__, chan->channel);
+			ret = -EINVAL;
+			break;
+		}
+		break;
+	default:
+		dev_err(state->dev, "%s - mask = %ld (INVALID)\n",
+			__func__, mask);
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+static const struct iio_info hsc_info = {
+	.read_raw = hsc_read_raw,
+};
+
+static void hsc_init_device(struct iio_dev *indio_dev)
+{
+	struct hsc_state *state = iio_priv(indio_dev);
+	const struct hsc_config *cfg = of_device_get_match_data(state->dev);
+
+	/* Pressure offset value */
+	state->offset.num = BIT(HSC_PRESS_BITS) *
+			    (cfg->pmin * cfg->rmax - cfg->pmax * cfg->rmin);
+	state->offset.den = 100 * (cfg->pmax - cfg->pmin);
+
+	/* Pressure scale value */
+	state->scale.num = 100 * cfg->knum * (cfg->pmax - cfg->pmin);
+	state->scale.den = BIT(HSC_PRESS_BITS) *
+			   cfg->kden * (cfg->rmax - cfg->rmin);
+}
+
+static int hsc_spi_probe(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct hsc_state *state;
+	struct iio_dev *indio_dev;
+	int ret;
+
+	if (spi->max_speed_hz > HSC_MAX_SPI_FREQ_HZ) {
+		dev_warn(dev, "SPI CLK, %d Hz exceeds %d Hz - changed to max\n",
+			spi->max_speed_hz,
+			HSC_MAX_SPI_FREQ_HZ);
+		spi->max_speed_hz = HSC_MAX_SPI_FREQ_HZ;
+	}
+
+	spi->bits_per_word = 8;
+	spi->mode = SPI_MODE_0;
+
+	ret = spi_setup(spi);
+	if (ret < 0) {
+		dev_err(dev, "%s - Error in spi_setup()\n", __func__);
+		return ret;
+	}
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+	if (!indio_dev) {
+		dev_err(dev, "%s - Error allocating iio_device\n", __func__);
+		return -ENOMEM;
+	}
+
+	state = iio_priv(indio_dev);
+	spi_set_drvdata(spi, state);
+	state->spi_dev = spi;
+	state->dev = dev;
+
+	indio_dev->dev.parent = dev;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->info = &hsc_info;
+	indio_dev->channels = hsc_channels;
+	indio_dev->num_channels = ARRAY_SIZE(hsc_channels);
+
+	state->spi_xfer.rx_buf = &state->rx_buf[0];
+	state->spi_xfer.tx_buf = NULL;
+	state->spi_xfer.cs_change = 0;
+	spi_message_init(&state->spi_msg);
+	spi_message_add_tail(&state->spi_xfer, &state->spi_msg);
+
+	hsc_init_device(indio_dev);
+
+	ret = iio_device_register(indio_dev);
+	if (ret < 0)
+		dev_err(dev, "iio_device_register failed: %d\n", ret);
+
+	dev_dbg(dev, "%s - scale = %d/%d, offset = %d/%d\n", __func__,
+		state->scale.num, state->scale.den,
+		state->offset.num, state->offset.den);
+
+	return ret;
+}
+
+static int hsc_spi_remove(struct spi_device *spi)
+{
+	struct device *dev = &spi->dev;
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+
+	iio_device_unregister(indio_dev);
+
+	return 0;
+}
+
+static const struct of_device_id hsc_of_match[] = {
+	{ .compatible = "honeywell,hsc001baa", .data = &hsc_cfg[HSC001BAA] },
+	{ .compatible = "honeywell,hsc001bab", .data = &hsc_cfg[HSC001BAB] },
+	{ .compatible = "honeywell,hsc001bac", .data = &hsc_cfg[HSC001BAC] },
+	{ .compatible = "honeywell,hsc001baf", .data = &hsc_cfg[HSC001BAF] },
+
+	{ .compatible = "honeywell,hsc1_6baa", .data = &hsc_cfg[HSC1_6BAA] },
+	{ .compatible = "honeywell,hsc1_6bab", .data = &hsc_cfg[HSC1_6BAB] },
+	{ .compatible = "honeywell,hsc1_6bac", .data = &hsc_cfg[HSC1_6BAC] },
+	{ .compatible = "honeywell,hsc1_6baf", .data = &hsc_cfg[HSC1_6BAF] },
+
+	{ .compatible = "honeywell,hsc2_5baa", .data = &hsc_cfg[HSC2_5BAA] },
+	{ .compatible = "honeywell,hsc2_5bab", .data = &hsc_cfg[HSC2_5BAB] },
+	{ .compatible = "honeywell,hsc2_5bac", .data = &hsc_cfg[HSC2_5BAC] },
+	{ .compatible = "honeywell,hsc2_5baf", .data = &hsc_cfg[HSC2_5BAF] },
+
+	{ .compatible = "honeywell,hsc004baa", .data = &hsc_cfg[HSC004BAA] },
+	{ .compatible = "honeywell,hsc004bab", .data = &hsc_cfg[HSC004BAB] },
+	{ .compatible = "honeywell,hsc004bac", .data = &hsc_cfg[HSC004BAC] },
+	{ .compatible = "honeywell,hsc004baf", .data = &hsc_cfg[HSC004BAF] },
+
+	{ .compatible = "honeywell,hsc006baa", .data = &hsc_cfg[HSC006BAA] },
+	{ .compatible = "honeywell,hsc006bab", .data = &hsc_cfg[HSC006BAB] },
+	{ .compatible = "honeywell,hsc006bac", .data = &hsc_cfg[HSC006BAC] },
+	{ .compatible = "honeywell,hsc006baf", .data = &hsc_cfg[HSC006BAF] },
+
+	{ .compatible = "honeywell,hsc010baa", .data = &hsc_cfg[HSC010BAA] },
+	{ .compatible = "honeywell,hsc010bab", .data = &hsc_cfg[HSC010BAB] },
+	{ .compatible = "honeywell,hsc010bac", .data = &hsc_cfg[HSC010BAC] },
+	{ .compatible = "honeywell,hsc010baf", .data = &hsc_cfg[HSC010BAF] },
+
+	{ .compatible = "honeywell,hsc100kaa", .data = &hsc_cfg[HSC100KAA] },
+	{ .compatible = "honeywell,hsc100kab", .data = &hsc_cfg[HSC100KAB] },
+	{ .compatible = "honeywell,hsc100kac", .data = &hsc_cfg[HSC100KAC] },
+	{ .compatible = "honeywell,hsc100kaf", .data = &hsc_cfg[HSC100KAF] },
+
+	{ .compatible = "honeywell,hsc160kaa", .data = &hsc_cfg[HSC160KAA] },
+	{ .compatible = "honeywell,hsc160kab", .data = &hsc_cfg[HSC160KAB] },
+	{ .compatible = "honeywell,hsc160kac", .data = &hsc_cfg[HSC160KAC] },
+	{ .compatible = "honeywell,hsc160kaf", .data = &hsc_cfg[HSC160KAF] },
+
+	{ .compatible = "honeywell,hsc250kaa", .data = &hsc_cfg[HSC250KAA] },
+	{ .compatible = "honeywell,hsc250kab", .data = &hsc_cfg[HSC250KAB] },
+	{ .compatible = "honeywell,hsc250kac", .data = &hsc_cfg[HSC250KAC] },
+	{ .compatible = "honeywell,hsc250kaf", .data = &hsc_cfg[HSC250KAF] },
+
+	{ .compatible = "honeywell,hsc400kaa", .data = &hsc_cfg[HSC400KAA] },
+	{ .compatible = "honeywell,hsc400kab", .data = &hsc_cfg[HSC400KAB] },
+	{ .compatible = "honeywell,hsc400kac", .data = &hsc_cfg[HSC400KAC] },
+	{ .compatible = "honeywell,hsc400kaf", .data = &hsc_cfg[HSC400KAF] },
+
+	{ .compatible = "honeywell,hsc600kaa", .data = &hsc_cfg[HSC600KAA] },
+	{ .compatible = "honeywell,hsc600kab", .data = &hsc_cfg[HSC600KAB] },
+	{ .compatible = "honeywell,hsc600kac", .data = &hsc_cfg[HSC600KAC] },
+	{ .compatible = "honeywell,hsc600kaf", .data = &hsc_cfg[HSC600KAF] },
+
+	{ .compatible = "honeywell,hsc001gaa", .data = &hsc_cfg[HSC001GAA] },
+	{ .compatible = "honeywell,hsc001gab", .data = &hsc_cfg[HSC001GAB] },
+	{ .compatible = "honeywell,hsc001gac", .data = &hsc_cfg[HSC001GAC] },
+	{ .compatible = "honeywell,hsc001gaf", .data = &hsc_cfg[HSC001GAF] },
+
+	{ .compatible = "honeywell,hsc015paa", .data = &hsc_cfg[HSC015PAA] },
+	{ .compatible = "honeywell,hsc015pab", .data = &hsc_cfg[HSC015PAB] },
+	{ .compatible = "honeywell,hsc015pac", .data = &hsc_cfg[HSC015PAC] },
+	{ .compatible = "honeywell,hsc015paf", .data = &hsc_cfg[HSC015PAF] },
+
+	{ .compatible = "honeywell,hsc030paa", .data = &hsc_cfg[HSC030PAA] },
+	{ .compatible = "honeywell,hsc030pab", .data = &hsc_cfg[HSC030PAB] },
+	{ .compatible = "honeywell,hsc030pac", .data = &hsc_cfg[HSC030PAC] },
+	{ .compatible = "honeywell,hsc030paf", .data = &hsc_cfg[HSC030PAF] },
+
+	{ .compatible = "honeywell,hsc060paa", .data = &hsc_cfg[HSC060PAA] },
+	{ .compatible = "honeywell,hsc060pab", .data = &hsc_cfg[HSC060PAB] },
+	{ .compatible = "honeywell,hsc060pac", .data = &hsc_cfg[HSC060PAC] },
+	{ .compatible = "honeywell,hsc060paf", .data = &hsc_cfg[HSC060PAF] },
+
+	{ .compatible = "honeywell,hsc100paa", .data = &hsc_cfg[HSC100PAA] },
+	{ .compatible = "honeywell,hsc100pab", .data = &hsc_cfg[HSC100PAB] },
+	{ .compatible = "honeywell,hsc100pac", .data = &hsc_cfg[HSC100PAC] },
+	{ .compatible = "honeywell,hsc100paf", .data = &hsc_cfg[HSC100PAF] },
+
+	{ .compatible = "honeywell,hsc150paa", .data = &hsc_cfg[HSC150PAA] },
+	{ .compatible = "honeywell,hsc150pab", .data = &hsc_cfg[HSC150PAB] },
+	{ .compatible = "honeywell,hsc150pac", .data = &hsc_cfg[HSC150PAC] },
+	{ .compatible = "honeywell,hsc150paf", .data = &hsc_cfg[HSC150PAF] },
+
+	{ },
+};
+MODULE_DEVICE_TABLE(of, hsc_of_match);
+
+static struct spi_driver hsc_spi_driver = {
+	.probe = hsc_spi_probe,
+	.remove = hsc_spi_remove,
+	.driver = {
+		.name = "hsc_spi_pressure_sensor",
+		.of_match_table = hsc_of_match,
+	},
+};
+
+module_spi_driver(hsc_spi_driver);
+
+MODULE_AUTHOR("Carlos Iglesias <carlosiglesias@emutex.com>");
+MODULE_DESCRIPTION("Honeywell HSC SPI pressure sensor driver");
+MODULE_LICENSE("GPL v2");
-- 
2.19.1

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

[Jonathan Cameron]

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>;
> +	};

Re: [PATCH 2/2] iio: pressure: Add support for Honeywell HSC SPI sensors

[Jonathan Cameron]

On Fri, 26 Oct 2018 18:14:37 +0000
Carlos Iglesias <carlos.iglesias@emutex.com> wrote:

> Add device driver for the HSC pressure sensors with SPI interface.
> In addition to the main measurement -pressure- these sensors also
> provide temperature measurement.
> 
> Signed-off-by: Carlos Iglesias <carlos.iglesias@emutex.com>

Hi Carlos,

A few minor things inline.  Note that the spi buffer isn't currently
dma safe which may result in some really hard to find bugs (I've chased
these in the past and they can be really rare and tricky to track down!)

Jonathan
> ---
>  MAINTAINERS                    |   6 +
>  drivers/iio/pressure/Kconfig   |  10 +
>  drivers/iio/pressure/Makefile  |   2 +
>  drivers/iio/pressure/hsc_spi.c | 543 +++++++++++++++++++++++++++++++++
>  4 files changed, 561 insertions(+)
>  create mode 100644 drivers/iio/pressure/hsc_spi.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 556f902b3766..a4af4cbd4b0f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6696,6 +6696,12 @@ W:	http://artax.karlin.mff.cuni.cz/~mikulas/vyplody/hpfs/index-e.cgi
>  S:	Maintained
>  F:	fs/hpfs/
>  
> +HSC SERIES PRESSURE SENSORS
> +M;	Carlos Iglesias <carlos.iglesias@emutex.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/iio/pressure/hsc_spi.txt
> +F:	drivers/iio/pressure/hsc_spi.c
> +
>  HSI SUBSYSTEM
>  M:	Sebastian Reichel <sre@kernel.org>
>  T:	git git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-hsi.git
> diff --git a/drivers/iio/pressure/Kconfig b/drivers/iio/pressure/Kconfig
> index eaa7cfcb4c2a..593663d4ffa8 100644
> --- a/drivers/iio/pressure/Kconfig
> +++ b/drivers/iio/pressure/Kconfig
> @@ -227,4 +227,14 @@ config ZPA2326_SPI
>  	tristate
>  	select REGMAP_SPI
>  
> +config HSC_SPI
> +	tristate "Honeywell HSC pressure sensors (SPI)"
> +	depends on SPI_MASTER
> +	help
> +	  Say yes here to build support for the Honeywell HSC range of
> +	  pressure sensors (SPI interface only).
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called hsc_spi.
> +
>  endmenu
> diff --git a/drivers/iio/pressure/Makefile b/drivers/iio/pressure/Makefile
> index c2058d7b2f93..b1db2116e9f6 100644
> --- a/drivers/iio/pressure/Makefile
> +++ b/drivers/iio/pressure/Makefile
> @@ -31,3 +31,5 @@ obj-$(CONFIG_ZPA2326_SPI) += zpa2326_spi.o
>  
>  obj-$(CONFIG_IIO_ST_PRESS_I2C) += st_pressure_i2c.o
>  obj-$(CONFIG_IIO_ST_PRESS_SPI) += st_pressure_spi.o
> +
> +obj-$(CONFIG_HSC_SPI) += hsc_spi.o
> diff --git a/drivers/iio/pressure/hsc_spi.c b/drivers/iio/pressure/hsc_spi.c
> new file mode 100644
> index 000000000000..efe45a09da8f
> --- /dev/null
> +++ b/drivers/iio/pressure/hsc_spi.c
> @@ -0,0 +1,543 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * hsc_spi.c - Driver for Honeywell HSC pressure sensors with
> + *             SPI interface
> + *
> + * Copyright (c) 2018 Carlos Iglesias <carlos.iglesias@emutex.com>
> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/iio/iio.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/spi/spi.h>
> +
> +#define HSC_MAX_SPI_FREQ_HZ	400000
> +
> +#define HSC_TEMP_BITS		11
> +#define HSC_PRESS_BITS		14
> +#define HSC_TEMP_MASK		(0x7FF)
> +#define HSC_TEMP_SHIFT		(5)
> +
> +#define HSC_STATUS_S0		BIT(14)
> +#define HSC_STATUS_S1		BIT(15)
> +#define HSC_STATUS_MSK		((HSC_STATUS_S0) | (HSC_STATUS_S1))
> +#define HSC_STATUS_CMD		HSC_STATUS_S0
> +#define HSC_STATUS_STALE	HSC_STATUS_S1
> +#define HSC_STATUS_DIAG		((HSC_STATUS_S0) | (HSC_STATUS_S1))
> +
> +static inline int hsc_status_error(struct device dev, int val)
> +{
> +	int st_check = val & HSC_STATUS_MSK;
> +
> +	if (st_check) {
> +		switch (st_check) {
> +		case HSC_STATUS_CMD:
> +			dev_warn(&dev, "%s:Device in COMMAND MODE\n",
> +				 __func__);
> +			return -EIO;
> +		case HSC_STATUS_STALE:
> +			dev_warn(&dev, "%s:Stale data - sampling too fast?\n",
> +				 __func__);
> +			return -EAGAIN;
> +		case HSC_STATUS_DIAG:
> +			dev_warn(&dev, "%s:Calibration signature changed\n",
> +				 __func__);
> +			return -EIO;
> +		default:
> +			dev_err(&dev, "%s:Invalid status code (%d)\n",
> +				__func__, st_check);
> +			return -EIO;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +enum hsc_variant {
> +	/* Note: Only the absolute range sensors are supported */
> +	HSC001BAA, HSC001BAB, HSC001BAC, HSC001BAF,
> +	HSC1_6BAA, HSC1_6BAB, HSC1_6BAC, HSC1_6BAF,
> +	HSC2_5BAA, HSC2_5BAB, HSC2_5BAC, HSC2_5BAF,
> +	HSC004BAA, HSC004BAB, HSC004BAC, HSC004BAF,
> +	HSC006BAA, HSC006BAB, HSC006BAC, HSC006BAF,
> +	HSC010BAA, HSC010BAB, HSC010BAC, HSC010BAF,
> +
> +	HSC100KAA, HSC100KAB, HSC100KAC, HSC100KAF,
> +	HSC160KAA, HSC160KAB, HSC160KAC, HSC160KAF,
> +	HSC250KAA, HSC250KAB, HSC250KAC, HSC250KAF,
> +	HSC400KAA, HSC400KAB, HSC400KAC, HSC400KAF,
> +	HSC600KAA, HSC600KAB, HSC600KAC, HSC600KAF,
> +	HSC001GAA, HSC001GAB, HSC001GAC, HSC001GAF,
> +
> +	HSC015PAA, HSC015PAB, HSC015PAC, HSC015PAF,
> +	HSC030PAA, HSC030PAB, HSC030PAC, HSC030PAF,
> +	HSC060PAA, HSC060PAB, HSC060PAC, HSC060PAF,
> +	HSC100PAA, HSC100PAB, HSC100PAC, HSC100PAF,
> +	HSC150PAA, HSC150PAB, HSC150PAC, HSC150PAF,
> +};
> +
> +enum hsc_meas_channel {
> +	HSC_CH_PRESSURE,
> +	HSC_CH_TEMPERATURE
> +};
> +
> +struct hsc_config {
> +	int pmin;	/* Lower pressure limit */
> +	int pmax;	/* Upper pressure limit */
> +	int rmin;	/* Lower transfer function limit (%) */
> +	int rmax;	/* Upper transfer function limit (%) */
> +	int knum;	/* Pressure kPa conversion factor (numerator) */
> +	int kden;	/* Pressure kPa conversion factor /denominator) */
> +};
> +
> +struct hsc_fract_val {
> +	int num;	/* numerator */
> +	int den;	/* denominator */
> +};
> +
> +struct hsc_state {
> +	struct device *dev;
> +	struct spi_device *spi_dev;
> +	struct spi_transfer spi_xfer;
> +	struct spi_message spi_msg;
> +	__be16 rx_buf[2];
This is not a safe buffer for DMA as it shares its cacheline with other data.
Easiest option is to put it at the end of this structure and mark it
__cacheline_aligned.

We make sure that the private data is always appropriately aligned to allow
this to work.
> +
> +	/* Model-dependent values */
> +	struct hsc_fract_val scale;
> +	struct hsc_fract_val offset;
> +};
> +
> +#define HSC_CONFIG(_pmin, _pmax, _rmin, _rmax, _knum, _kden) {	\
> +		.pmin = (_pmin),				\
> +		.pmax = (_pmax),				\
> +		.rmin = (_rmin),				\
> +		.rmax = (_rmax),				\
> +		.knum = (_knum),				\
> +		.kden = (_kden),				\
> +	}
> +
> +static struct hsc_config hsc_cfg[] = {
> +	/* Absolute range, mbar */
> +	[HSC001BAA] = HSC_CONFIG(0, 1000, 10, 90, 1, 10),
> +	[HSC001BAB] = HSC_CONFIG(0, 1000, 5, 95, 1, 10),
> +	[HSC001BAC] = HSC_CONFIG(0, 1000, 5, 85, 1, 10),
> +	[HSC001BAF] = HSC_CONFIG(0, 1000, 4, 94, 1, 10),
> +	[HSC1_6BAA] = HSC_CONFIG(0, 1600, 10, 90, 1, 10),
> +	[HSC1_6BAB] = HSC_CONFIG(0, 1600, 5, 95, 1, 10),
> +	[HSC1_6BAC] = HSC_CONFIG(0, 1600, 5, 85, 1, 10),
> +	[HSC1_6BAF] = HSC_CONFIG(0, 1600, 4, 94, 1, 10),
> +	[HSC2_5BAA] = HSC_CONFIG(0, 2500, 10, 90, 1, 10),
> +	[HSC2_5BAB] = HSC_CONFIG(0, 2500, 5, 95, 1, 10),
> +	[HSC2_5BAC] = HSC_CONFIG(0, 2500, 5, 85, 1, 10),
> +	[HSC2_5BAF] = HSC_CONFIG(0, 2500, 4, 94, 1, 10),
> +	[HSC004BAA] = HSC_CONFIG(0, 4000, 10, 90, 1, 10),
> +	[HSC004BAB] = HSC_CONFIG(0, 4000, 5, 95, 1, 10),
> +	[HSC004BAC] = HSC_CONFIG(0, 4000, 5, 85, 1, 10),
> +	[HSC004BAF] = HSC_CONFIG(0, 4000, 4, 94, 1, 10),
> +	[HSC006BAA] = HSC_CONFIG(0, 6000, 10, 90, 1, 10),
> +	[HSC006BAB] = HSC_CONFIG(0, 6000, 5, 95, 1, 10),
> +	[HSC006BAC] = HSC_CONFIG(0, 6000, 5, 85, 1, 10),
> +	[HSC006BAF] = HSC_CONFIG(0, 6000, 4, 94, 1, 10),
> +	[HSC010BAA] = HSC_CONFIG(0, 10000, 10, 90, 1, 10),
> +	[HSC010BAB] = HSC_CONFIG(0, 10000, 5, 95, 1, 10),
> +	[HSC010BAC] = HSC_CONFIG(0, 10000, 5, 85, 1, 10),
> +	[HSC010BAF] = HSC_CONFIG(0, 10000, 4, 94, 1, 10),
> +	/* Absolute range, kPa */
> +	[HSC100KAA] = HSC_CONFIG(0, 100, 10, 90, 1, 1),
> +	[HSC100KAB] = HSC_CONFIG(0, 100, 5, 95, 1, 1),
> +	[HSC100KAC] = HSC_CONFIG(0, 100, 5, 85, 1, 1),
> +	[HSC100KAF] = HSC_CONFIG(0, 100, 4, 94, 1, 1),
> +	[HSC160KAA] = HSC_CONFIG(0, 160, 10, 90, 1, 1),
> +	[HSC160KAB] = HSC_CONFIG(0, 160, 5, 95, 1, 1),
> +	[HSC160KAC] = HSC_CONFIG(0, 160, 5, 85, 1, 1),
> +	[HSC160KAF] = HSC_CONFIG(0, 160, 4, 94, 1, 1),
> +	[HSC250KAA] = HSC_CONFIG(0, 250, 10, 90, 1, 1),
> +	[HSC250KAB] = HSC_CONFIG(0, 250, 5, 95, 1, 1),
> +	[HSC250KAC] = HSC_CONFIG(0, 250, 5, 85, 1, 1),
> +	[HSC250KAF] = HSC_CONFIG(0, 250, 4, 94, 1, 1),
> +	[HSC400KAA] = HSC_CONFIG(0, 400, 10, 90, 1, 1),
> +	[HSC400KAB] = HSC_CONFIG(0, 400, 5, 95, 1, 1),
> +	[HSC400KAC] = HSC_CONFIG(0, 400, 5, 85, 1, 1),
> +	[HSC400KAF] = HSC_CONFIG(0, 400, 4, 94, 1, 1),
> +	[HSC600KAA] = HSC_CONFIG(0, 600, 10, 90, 1, 1),
> +	[HSC600KAB] = HSC_CONFIG(0, 600, 5, 95, 1, 1),
> +	[HSC600KAC] = HSC_CONFIG(0, 600, 5, 85, 1, 1),
> +	[HSC600KAF] = HSC_CONFIG(0, 600, 4, 94, 1, 1),
> +	[HSC001GAA] = HSC_CONFIG(0, 1000, 10, 90, 1, 1),
> +	[HSC001GAB] = HSC_CONFIG(0, 1000, 5, 95, 1, 1),
> +	[HSC001GAC] = HSC_CONFIG(0, 1000, 5, 85, 1, 1),
> +	[HSC001GAF] = HSC_CONFIG(0, 1000, 4, 94, 1, 1),
> +	/* Absolute range, psi */
> +	[HSC015PAA] = HSC_CONFIG(0, 15, 10, 90, 6895, 1000),
> +	[HSC015PAB] = HSC_CONFIG(0, 15, 5, 95, 6895, 1000),
> +	[HSC015PAC] = HSC_CONFIG(0, 15, 5, 85, 6895, 1000),
> +	[HSC015PAF] = HSC_CONFIG(0, 15, 4, 94, 6895, 1000),
> +	[HSC030PAA] = HSC_CONFIG(0, 30, 10, 90, 6895, 1000),
> +	[HSC030PAB] = HSC_CONFIG(0, 30, 5, 95, 6895, 1000),
> +	[HSC030PAC] = HSC_CONFIG(0, 30, 5, 85, 6895, 1000),
> +	[HSC030PAF] = HSC_CONFIG(0, 30, 4, 94, 6895, 1000),
> +	[HSC060PAA] = HSC_CONFIG(0, 60, 10, 90, 6895, 1000),
> +	[HSC060PAB] = HSC_CONFIG(0, 60, 5, 95, 6895, 1000),
> +	[HSC060PAC] = HSC_CONFIG(0, 60, 5, 85, 6895, 1000),
> +	[HSC060PAF] = HSC_CONFIG(0, 60, 4, 94, 6895, 1000),
> +	[HSC100PAA] = HSC_CONFIG(0, 100, 10, 90, 6895, 1000),
> +	[HSC100PAB] = HSC_CONFIG(0, 100, 5, 95, 6895, 1000),
> +	[HSC100PAC] = HSC_CONFIG(0, 100, 5, 85, 6895, 1000),
> +	[HSC100PAF] = HSC_CONFIG(0, 100, 4, 94, 6895, 1000),
> +	[HSC150PAA] = HSC_CONFIG(0, 150, 10, 90, 6895, 1000),
> +	[HSC150PAB] = HSC_CONFIG(0, 150, 5, 95, 6895, 1000),
> +	[HSC150PAC] = HSC_CONFIG(0, 150, 5, 85, 6895, 1000),
> +	[HSC150PAF] = HSC_CONFIG(0, 150, 4, 94, 6895, 1000),
> +};
> +
> +static const struct iio_chan_spec hsc_channels[] = {
> +	{
> +		.type = IIO_PRESSURE,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_OFFSET) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.channel = HSC_CH_PRESSURE,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = HSC_PRESS_BITS,
> +			.storagebits = 16,
> +			.shift = 0,
Shift default is 0 so no need to have this.

> +			.endianness = IIO_BE,
> +		}
> +	},
> +	{
> +		.type = IIO_TEMP,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> +				      BIT(IIO_CHAN_INFO_OFFSET) |
> +				      BIT(IIO_CHAN_INFO_SCALE),
> +		.channel = HSC_CH_TEMPERATURE,
> +		.scan_type = {
> +			.sign = 'u',
> +			.realbits = HSC_TEMP_BITS,
> +			.storagebits = 16,
> +			.shift = HSC_TEMP_SHIFT,
> +			.endianness = IIO_BE,
> +		}
> +	}
> +};
> +
> +static int hsc_get_pressure(struct hsc_state *state)
> +{
> +	int ret = 0;
Always set below, so should not init here.

> +	int error;
> +
> +	state->spi_xfer.len = 2;
> +	ret = spi_sync(state->spi_dev, &state->spi_msg);
> +	if (ret)
> +		return ret;
> +
> +	ret = be16_to_cpu(state->rx_buf[0]);
> +
> +	error = hsc_status_error(state->spi_dev->dev, ret);
> +	if (error)
> +		return error;
> +
> +	return ret;
> +}
> +
> +static int hsc_get_temperature(struct hsc_state *state)
> +{
> +	int ret;
> +	int error;
> +
> +	state->spi_xfer.len = 4;
> +	ret = spi_sync(state->spi_dev, &state->spi_msg);
> +	if (ret)
> +		return ret;
As mentioned below I'm not sure the complexity of the spi_sync
instead of spi_read is worthwhile here.
> +
> +	ret = be16_to_cpu(state->rx_buf[0]);
Perhaps just have this inline in the function call.
(there are a few other instances of this, so perhaps all of them)
> +	error = hsc_status_error(state->spi_dev->dev, ret);
> +	if (error)
> +		return error;
> +
> +	ret = be16_to_cpu(state->rx_buf[1]);
Perhaps just put this inline in the next statement?
> +	ret = (ret >> HSC_TEMP_SHIFT) & HSC_TEMP_MASK;
> +
> +	return ret;
return (ret >> HSC_TEMP_SHIFT) & HSC_TEMP_MASK;

> +}
> +
> +static int hsc_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan, int *val,
> +			int *val2, long mask)
> +{
> +	struct hsc_state *state = iio_priv(indio_dev);
> +	int ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		switch (chan->channel) {
> +		case HSC_CH_PRESSURE:
> +			ret = hsc_get_pressure(state);
> +			if (ret < 0)
> +				break;
> +			*val = ret;
> +			ret = IIO_VAL_INT;
return IIO_VAL_INT;
> +			break;
> +		case HSC_CH_TEMPERATURE:
> +			ret = hsc_get_temperature(state);
> +			if (ret < 0)
> +				break;
> +			*val = ret;
> +			ret = IIO_VAL_INT;
return IIO_VAL_INT;

> +			break;
> +		default:
> +			ret = -EINVAL;
> +			dev_err(state->dev,
> +				"%s - IIO_CHAN_INFO_RAW-bad channel (%d)\n",
> +				__func__, chan->channel);
> +			break;
return -EINVAL;

> +		}
> +		break;
> +	case IIO_CHAN_INFO_OFFSET:
> +		switch (chan->channel) {
> +		case HSC_CH_PRESSURE:
> +			*val = state->offset.num;
> +			*val2 = state->offset.den;
> +			ret = IIO_VAL_FRACTIONAL;
> +			break;
> +		case HSC_CH_TEMPERATURE:
> +			*val = BIT(HSC_TEMP_BITS) - 1;
> +			*val2 = -200 / 50;
> +			ret = IIO_VAL_FRACTIONAL;
> +			break;
> +		default:
> +			dev_err(state->dev,
> +				"%s - IIO_CHAN_INFO_OFFSET-bad channel (%d)\n",
> +				__func__, chan->channel);
> +			ret = -EINVAL;
> +			break;
> +		}
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		switch (chan->channel) {
> +		case HSC_CH_PRESSURE:		/* output unit is kPa */
> +			*val = state->scale.num;
> +			*val2 = state->scale.den;
> +			ret = IIO_VAL_FRACTIONAL;
> +			break;
> +		case HSC_CH_TEMPERATURE:  /* output unit is milli Celsius */
> +			*val = 200 * 1000;
> +			*val2 = BIT(HSC_TEMP_BITS) - 1;
> +			ret = IIO_VAL_FRACTIONAL;
> +			break;
> +		default:
> +			dev_err(state->dev,
> +				"%s - IIO_CHAN_INFO_SCALE-bad channel (%d)\n",
> +				__func__, chan->channel);
> +			ret = -EINVAL;
> +			break;
> +		}
> +		break;
> +	default:
> +		dev_err(state->dev, "%s - mask = %ld (INVALID)\n",
> +			__func__, mask);
> +		ret = -EINVAL;
return -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
With direct returns everywhere above, no need to return ret here as you can't
get here.

> +}
> +
> +static const struct iio_info hsc_info = {
> +	.read_raw = hsc_read_raw,
> +};
> +
> +static void hsc_init_device(struct iio_dev *indio_dev)
This function seems a little oddly named given it doesn't do anything
to the device at all. Perhaps hsc_init_device_params?
Or more likely you can come up with something better than that.

> +{
> +	struct hsc_state *state = iio_priv(indio_dev);
> +	const struct hsc_config *cfg = of_device_get_match_data(state->dev);
> +
> +	/* Pressure offset value */
> +	state->offset.num = BIT(HSC_PRESS_BITS) *
> +			    (cfg->pmin * cfg->rmax - cfg->pmax * cfg->rmin);
> +	state->offset.den = 100 * (cfg->pmax - cfg->pmin);
> +
> +	/* Pressure scale value */
> +	state->scale.num = 100 * cfg->knum * (cfg->pmax - cfg->pmin);
> +	state->scale.den = BIT(HSC_PRESS_BITS) *
> +			   cfg->kden * (cfg->rmax - cfg->rmin);
> +}
> +
> +static int hsc_spi_probe(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct hsc_state *state;
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	if (spi->max_speed_hz > HSC_MAX_SPI_FREQ_HZ) {
> +		dev_warn(dev, "SPI CLK, %d Hz exceeds %d Hz - changed to max\n",
> +			spi->max_speed_hz,
> +			HSC_MAX_SPI_FREQ_HZ);
> +		spi->max_speed_hz = HSC_MAX_SPI_FREQ_HZ;
> +	}
> +
> +	spi->bits_per_word = 8;
> +	spi->mode = SPI_MODE_0;
> +
> +	ret = spi_setup(spi);
> +	if (ret < 0) {
> +		dev_err(dev, "%s - Error in spi_setup()\n", __func__);
> +		return ret;
> +	}
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> +	if (!indio_dev) {
> +		dev_err(dev, "%s - Error allocating iio_device\n", __func__);
> +		return -ENOMEM;
> +	}
> +
> +	state = iio_priv(indio_dev);
> +	spi_set_drvdata(spi, state);
> +	state->spi_dev = spi;
> +	state->dev = dev;
This seems a little excessive as you can get it from spi_dev->dev where ever it
is needed.

> +
> +	indio_dev->dev.parent = dev;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->info = &hsc_info;
> +	indio_dev->channels = hsc_channels;
> +	indio_dev->num_channels = ARRAY_SIZE(hsc_channels);
> +
> +	state->spi_xfer.rx_buf = &state->rx_buf[0];
> +	state->spi_xfer.tx_buf = NULL;
Null is the default so should be no need to set it.
(state is kzalloc)

> +	state->spi_xfer.cs_change = 0;

I would assume that was the default and doesn't need setting...

> +	spi_message_init(&state->spi_msg);
> +	spi_message_add_tail(&state->spi_xfer, &state->spi_msg);
Is this worthwhile doing over a simple spi_read without precreating
the message etc?

> +
> +	hsc_init_device(indio_dev);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret < 0)
> +		dev_err(dev, "iio_device_register failed: %d\n", ret);
> +
> +	dev_dbg(dev, "%s - scale = %d/%d, offset = %d/%d\n", __func__,
> +		state->scale.num, state->scale.den,
> +		state->offset.num, state->offset.den);
> +
> +	return ret;
> +}
> +
> +static int hsc_spi_remove(struct spi_device *spi)
> +{
> +	struct device *dev = &spi->dev;
> +	struct iio_dev *indio_dev = dev_get_drvdata(dev);
> +
> +	iio_device_unregister(indio_dev);
If all you have in a remove function is iio_device_unregister, then
it implies you could have used devm_iio_device_register and not had
a remove function at all.

It's fairly unusual that there isn't 'something' to do, but perhaps not
if the interface is very simple, or you are doing tight power control
around individual readings.

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id hsc_of_match[] = {
> +	{ .compatible = "honeywell,hsc001baa", .data = &hsc_cfg[HSC001BAA] },
> +	{ .compatible = "honeywell,hsc001bab", .data = &hsc_cfg[HSC001BAB] },
> +	{ .compatible = "honeywell,hsc001bac", .data = &hsc_cfg[HSC001BAC] },
> +	{ .compatible = "honeywell,hsc001baf", .data = &hsc_cfg[HSC001BAF] },
> +
> +	{ .compatible = "honeywell,hsc1_6baa", .data = &hsc_cfg[HSC1_6BAA] },
> +	{ .compatible = "honeywell,hsc1_6bab", .data = &hsc_cfg[HSC1_6BAB] },
> +	{ .compatible = "honeywell,hsc1_6bac", .data = &hsc_cfg[HSC1_6BAC] },
> +	{ .compatible = "honeywell,hsc1_6baf", .data = &hsc_cfg[HSC1_6BAF] },
> +
> +	{ .compatible = "honeywell,hsc2_5baa", .data = &hsc_cfg[HSC2_5BAA] },
> +	{ .compatible = "honeywell,hsc2_5bab", .data = &hsc_cfg[HSC2_5BAB] },
> +	{ .compatible = "honeywell,hsc2_5bac", .data = &hsc_cfg[HSC2_5BAC] },
> +	{ .compatible = "honeywell,hsc2_5baf", .data = &hsc_cfg[HSC2_5BAF] },
> +
> +	{ .compatible = "honeywell,hsc004baa", .data = &hsc_cfg[HSC004BAA] },
> +	{ .compatible = "honeywell,hsc004bab", .data = &hsc_cfg[HSC004BAB] },
> +	{ .compatible = "honeywell,hsc004bac", .data = &hsc_cfg[HSC004BAC] },
> +	{ .compatible = "honeywell,hsc004baf", .data = &hsc_cfg[HSC004BAF] },
> +
> +	{ .compatible = "honeywell,hsc006baa", .data = &hsc_cfg[HSC006BAA] },
> +	{ .compatible = "honeywell,hsc006bab", .data = &hsc_cfg[HSC006BAB] },
> +	{ .compatible = "honeywell,hsc006bac", .data = &hsc_cfg[HSC006BAC] },
> +	{ .compatible = "honeywell,hsc006baf", .data = &hsc_cfg[HSC006BAF] },
> +
> +	{ .compatible = "honeywell,hsc010baa", .data = &hsc_cfg[HSC010BAA] },
> +	{ .compatible = "honeywell,hsc010bab", .data = &hsc_cfg[HSC010BAB] },
> +	{ .compatible = "honeywell,hsc010bac", .data = &hsc_cfg[HSC010BAC] },
> +	{ .compatible = "honeywell,hsc010baf", .data = &hsc_cfg[HSC010BAF] },
> +
> +	{ .compatible = "honeywell,hsc100kaa", .data = &hsc_cfg[HSC100KAA] },
> +	{ .compatible = "honeywell,hsc100kab", .data = &hsc_cfg[HSC100KAB] },
> +	{ .compatible = "honeywell,hsc100kac", .data = &hsc_cfg[HSC100KAC] },
> +	{ .compatible = "honeywell,hsc100kaf", .data = &hsc_cfg[HSC100KAF] },
> +
> +	{ .compatible = "honeywell,hsc160kaa", .data = &hsc_cfg[HSC160KAA] },
> +	{ .compatible = "honeywell,hsc160kab", .data = &hsc_cfg[HSC160KAB] },
> +	{ .compatible = "honeywell,hsc160kac", .data = &hsc_cfg[HSC160KAC] },
> +	{ .compatible = "honeywell,hsc160kaf", .data = &hsc_cfg[HSC160KAF] },
> +
> +	{ .compatible = "honeywell,hsc250kaa", .data = &hsc_cfg[HSC250KAA] },
> +	{ .compatible = "honeywell,hsc250kab", .data = &hsc_cfg[HSC250KAB] },
> +	{ .compatible = "honeywell,hsc250kac", .data = &hsc_cfg[HSC250KAC] },
> +	{ .compatible = "honeywell,hsc250kaf", .data = &hsc_cfg[HSC250KAF] },
> +
> +	{ .compatible = "honeywell,hsc400kaa", .data = &hsc_cfg[HSC400KAA] },
> +	{ .compatible = "honeywell,hsc400kab", .data = &hsc_cfg[HSC400KAB] },
> +	{ .compatible = "honeywell,hsc400kac", .data = &hsc_cfg[HSC400KAC] },
> +	{ .compatible = "honeywell,hsc400kaf", .data = &hsc_cfg[HSC400KAF] },
> +
> +	{ .compatible = "honeywell,hsc600kaa", .data = &hsc_cfg[HSC600KAA] },
> +	{ .compatible = "honeywell,hsc600kab", .data = &hsc_cfg[HSC600KAB] },
> +	{ .compatible = "honeywell,hsc600kac", .data = &hsc_cfg[HSC600KAC] },
> +	{ .compatible = "honeywell,hsc600kaf", .data = &hsc_cfg[HSC600KAF] },
> +
> +	{ .compatible = "honeywell,hsc001gaa", .data = &hsc_cfg[HSC001GAA] },
> +	{ .compatible = "honeywell,hsc001gab", .data = &hsc_cfg[HSC001GAB] },
> +	{ .compatible = "honeywell,hsc001gac", .data = &hsc_cfg[HSC001GAC] },
> +	{ .compatible = "honeywell,hsc001gaf", .data = &hsc_cfg[HSC001GAF] },
> +
> +	{ .compatible = "honeywell,hsc015paa", .data = &hsc_cfg[HSC015PAA] },
> +	{ .compatible = "honeywell,hsc015pab", .data = &hsc_cfg[HSC015PAB] },
> +	{ .compatible = "honeywell,hsc015pac", .data = &hsc_cfg[HSC015PAC] },
> +	{ .compatible = "honeywell,hsc015paf", .data = &hsc_cfg[HSC015PAF] },
> +
> +	{ .compatible = "honeywell,hsc030paa", .data = &hsc_cfg[HSC030PAA] },
> +	{ .compatible = "honeywell,hsc030pab", .data = &hsc_cfg[HSC030PAB] },
> +	{ .compatible = "honeywell,hsc030pac", .data = &hsc_cfg[HSC030PAC] },
> +	{ .compatible = "honeywell,hsc030paf", .data = &hsc_cfg[HSC030PAF] },
> +
> +	{ .compatible = "honeywell,hsc060paa", .data = &hsc_cfg[HSC060PAA] },
> +	{ .compatible = "honeywell,hsc060pab", .data = &hsc_cfg[HSC060PAB] },
> +	{ .compatible = "honeywell,hsc060pac", .data = &hsc_cfg[HSC060PAC] },
> +	{ .compatible = "honeywell,hsc060paf", .data = &hsc_cfg[HSC060PAF] },
> +
> +	{ .compatible = "honeywell,hsc100paa", .data = &hsc_cfg[HSC100PAA] },
> +	{ .compatible = "honeywell,hsc100pab", .data = &hsc_cfg[HSC100PAB] },
> +	{ .compatible = "honeywell,hsc100pac", .data = &hsc_cfg[HSC100PAC] },
> +	{ .compatible = "honeywell,hsc100paf", .data = &hsc_cfg[HSC100PAF] },
> +
> +	{ .compatible = "honeywell,hsc150paa", .data = &hsc_cfg[HSC150PAA] },
> +	{ .compatible = "honeywell,hsc150pab", .data = &hsc_cfg[HSC150PAB] },
> +	{ .compatible = "honeywell,hsc150pac", .data = &hsc_cfg[HSC150PAC] },
> +	{ .compatible = "honeywell,hsc150paf", .data = &hsc_cfg[HSC150PAF] },
> +
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, hsc_of_match);
> +
> +static struct spi_driver hsc_spi_driver = {
> +	.probe = hsc_spi_probe,
> +	.remove = hsc_spi_remove,
> +	.driver = {
> +		.name = "hsc_spi_pressure_sensor",
> +		.of_match_table = hsc_of_match,
> +	},
> +};
> +
> +module_spi_driver(hsc_spi_driver);
> +
> +MODULE_AUTHOR("Carlos Iglesias <carlosiglesias@emutex.com>");
> +MODULE_DESCRIPTION("Honeywell HSC SPI pressure sensor driver");
> +MODULE_LICENSE("GPL v2");

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

[Jonathan Cameron]

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>;
> +       };
> 
> 
> 
> 

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

[Rob Herring]

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
>