Devicetree
 help / color / mirror / Atom feed
* [PATCH v3 2/4] iio: chemical: scd30: add I2C interface driver
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

Add I2C interface driver for the SCD30 sensor.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
 MAINTAINERS                      |   1 +
 drivers/iio/chemical/Kconfig     |  11 +++
 drivers/iio/chemical/Makefile    |   1 +
 drivers/iio/chemical/scd30_i2c.c | 134 +++++++++++++++++++++++++++++++
 4 files changed, 147 insertions(+)
 create mode 100644 drivers/iio/chemical/scd30_i2c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 41a509cca6f1..13aed3473b7e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15142,6 +15142,7 @@ M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
 S:	Maintained
 F:	drivers/iio/chemical/scd30.h
 F:	drivers/iio/chemical/scd30_core.c
+F:	drivers/iio/chemical/scd30_i2c.c
 
 SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 99e852b67e55..970d34888c2e 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -96,6 +96,17 @@ config SCD30_CORE
 	  To compile this driver as a module, choose M here: the module will
 	  be called scd30_core.
 
+config SCD30_I2C
+	tristate "SCD30 carbon dioxide sensor I2C driver"
+	depends on SCD30_CORE && I2C
+	select CRC8
+	help
+	  Say Y here to build support for the Sensirion SCD30 I2C interface
+	  driver.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called scd30_i2c.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index c9804b041ecd..0966ca34e34b 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SCD30_CORE) += scd30_core.o
+obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/scd30_i2c.c b/drivers/iio/chemical/scd30_i2c.c
new file mode 100644
index 000000000000..a6b532b83669
--- /dev/null
+++ b/drivers/iio/chemical/scd30_i2c.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor i2c driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ *
+ * I2C slave address: 0x61
+ */
+#include <linux/crc8.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#include "scd30.h"
+
+#define SCD30_I2C_MAX_BUF_SIZE 18
+#define SCD30_I2C_CRC8_POLYNOMIAL 0x31
+
+static u16 scd30_i2c_cmd_lookup_tbl[] = {
+	[CMD_START_MEAS] = 0x0010,
+	[CMD_STOP_MEAS] = 0x0104,
+	[CMD_MEAS_INTERVAL] = 0x4600,
+	[CMD_MEAS_READY] = 0x0202,
+	[CMD_READ_MEAS] = 0x0300,
+	[CMD_ASC] = 0x5306,
+	[CMD_FRC] = 0x5204,
+	[CMD_TEMP_OFFSET] = 0x5403,
+	[CMD_FW_VERSION] = 0xd100,
+	[CMD_RESET] = 0xd304,
+};
+
+DECLARE_CRC8_TABLE(scd30_i2c_crc8_tbl);
+
+static int scd30_i2c_xfer(struct scd30_state *state, char *txbuf, int txsize,
+			  char *rxbuf, int rxsize)
+{
+	struct i2c_client *client = to_i2c_client(state->dev);
+	int ret;
+
+	/*
+	 * repeated start is not supported hence instead of sending two i2c
+	 * messages in a row we send one by one
+	 */
+	ret = i2c_master_send(client, txbuf, txsize);
+	if (ret != txsize)
+		return ret < 0 ? ret : -EIO;
+
+	if (!rxbuf)
+		return 0;
+
+	ret = i2c_master_recv(client, rxbuf, rxsize);
+	if (ret != rxsize)
+		return ret < 0 ? ret : -EIO;
+
+	return 0;
+}
+
+static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd,
+			     u16 arg, void *response, int size)
+{
+	char crc, buf[SCD30_I2C_MAX_BUF_SIZE], *rsp = response;
+	int i, ret;
+
+	put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
+	i = 2;
+
+	if (rsp) {
+		/* each two bytes are followed by a crc8 */
+		size += size / 2;
+	} else {
+		put_unaligned_be16(arg, buf + i);
+		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+		i += 2;
+		buf[i] = crc;
+		i += 1;
+
+		/* commands below don't take an argument */
+		if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+			i -= 3;
+	}
+
+	ret = scd30_i2c_xfer(state, buf, i, buf, size);
+	if (ret)
+		return ret;
+
+	/* validate received data and strip off crc bytes */
+	for (i = 0; i < size; i += 3) {
+		crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
+		if (crc != buf[i + 2]) {
+			dev_err(state->dev, "data integrity check failed\n");
+			return -EIO;
+		}
+
+		*rsp++ = buf[i];
+		*rsp++ = buf[i + 1];
+	}
+
+	return 0;
+}
+
+static int scd30_i2c_probe(struct i2c_client *client)
+{
+	if (!i2c_check_functionality(client->adapter, I2C_FUNC_I2C))
+		return -EOPNOTSUPP;
+
+	crc8_populate_msb(scd30_i2c_crc8_tbl, SCD30_I2C_CRC8_POLYNOMIAL);
+
+	return scd30_probe(&client->dev, client->irq, client->name, NULL,
+			   scd30_i2c_command);
+}
+
+static const struct of_device_id scd30_i2c_of_match[] = {
+	{ .compatible = "sensirion,scd30" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, scd30_i2c_of_match);
+
+static struct i2c_driver scd30_i2c_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = scd30_i2c_of_match,
+		.pm = &scd30_pm_ops,
+	},
+	.probe_new = scd30_i2c_probe,
+};
+module_i2c_driver(scd30_i2c_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor i2c driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related

* [PATCH v3 4/4] dt-bindings: iio: scd30: add device binding file
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

Add SCD30 sensor binding file.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
 .../iio/chemical/sensirion,scd30.yaml         | 68 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml

diff --git a/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
new file mode 100644
index 000000000000..40d87346ff4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
@@ -0,0 +1,68 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/chemical/sensirion,scd30.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Sensirion SCD30 carbon dioxide sensor
+
+maintainers:
+  - Tomasz Duszynski <tomasz.duszynski@octakon.com>
+
+description: |
+  Air quality sensor capable of measuring co2 concentration, temperature
+  and relative humidity.
+
+properties:
+  compatible:
+    enum:
+      - sensirion,scd30
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vdd-supply: true
+
+  sensirion,sel-gpios:
+    description: GPIO connected to the SEL line
+    maxItems: 1
+
+  sensirion,pwm-gpios:
+    description: GPIO connected to the PWM line
+    maxItems: 1
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    # include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      co2-sensor@61 {
+        compatible = "sensirion,scd30";
+        reg = <0x61>;
+        vdd-supply = <&vdd>;
+        interrupt-parent = <&gpio0>;
+        interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
+  - |
+    # include <dt-bindings/interrupt-controller/irq.h>
+    serial {
+      co2-sensor {
+        compatible = "sensirion,scd30";
+        vdd-supply = <&vdd>;
+        interrupt-parent = <&gpio0>;
+        interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+      };
+    };
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 5db4b446c8ba..0ab9cf39e051 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15140,6 +15140,7 @@ F:	include/uapi/linux/phantom.h
 SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
 M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
 F:	drivers/iio/chemical/scd30.h
 F:	drivers/iio/chemical/scd30_core.c
 F:	drivers/iio/chemical/scd30_i2c.c
-- 
2.26.2


^ permalink raw reply related

* [PATCH v3 3/4] iio: chemical: scd30: add serial interface driver
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

Add serial interface driver for the SCD30 sensor.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
 MAINTAINERS                         |   1 +
 drivers/iio/chemical/Kconfig        |  11 ++
 drivers/iio/chemical/Makefile       |   1 +
 drivers/iio/chemical/scd30_serial.c | 266 ++++++++++++++++++++++++++++
 4 files changed, 279 insertions(+)
 create mode 100644 drivers/iio/chemical/scd30_serial.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 13aed3473b7e..5db4b446c8ba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15143,6 +15143,7 @@ S:	Maintained
 F:	drivers/iio/chemical/scd30.h
 F:	drivers/iio/chemical/scd30_core.c
 F:	drivers/iio/chemical/scd30_i2c.c
+F:	drivers/iio/chemical/scd30_serial.c
 
 SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 970d34888c2e..10bb431bc3ce 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -107,6 +107,17 @@ config SCD30_I2C
 	  To compile this driver as a module, choose M here: the module will
 	  be called scd30_i2c.
 
+config SCD30_SERIAL
+	tristate "SCD30 carbon dioxide sensor serial driver"
+	depends on SCD30_CORE && SERIAL_DEV_BUS
+	select CRC16
+	help
+	  Say Y here to build support for the Sensirion SCD30 serial interface
+	  driver.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called scd30_serial.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index 0966ca34e34b..fef63dd5bf92 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
 obj-$(CONFIG_PMS7003) += pms7003.o
 obj-$(CONFIG_SCD30_CORE) += scd30_core.o
 obj-$(CONFIG_SCD30_I2C) += scd30_i2c.o
+obj-$(CONFIG_SCD30_SERIAL) += scd30_serial.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/scd30_serial.c b/drivers/iio/chemical/scd30_serial.c
new file mode 100644
index 000000000000..07d7d3110fe0
--- /dev/null
+++ b/drivers/iio/chemical/scd30_serial.c
@@ -0,0 +1,266 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor serial driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ */
+#include <linux/crc16.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/iio/iio.h>
+#include <linux/jiffies.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/property.h>
+#include <linux/serdev.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <asm/unaligned.h>
+
+#include "scd30.h"
+
+#define SCD30_SERDEV_ADDR 0x61
+#define SCD30_SERDEV_WRITE 0x06
+#define SCD30_SERDEV_READ 0x03
+#define SCD30_SERDEV_MAX_BUF_SIZE 17
+#define SCD30_SERDEV_RX_HEADER_SIZE 3
+#define SCD30_SERDEV_CRC_SIZE 2
+#define SCD30_SERDEV_TIMEOUT msecs_to_jiffies(200)
+
+struct scd30_serdev_priv {
+	struct completion meas_ready;
+	char *buf;
+	int num_expected;
+	int num;
+};
+
+static u16 scd30_serdev_cmd_lookup_tbl[] = {
+	[CMD_START_MEAS] = 0x0036,
+	[CMD_STOP_MEAS] = 0x0037,
+	[CMD_MEAS_INTERVAL] = 0x0025,
+	[CMD_MEAS_READY] = 0x0027,
+	[CMD_READ_MEAS] = 0x0028,
+	[CMD_ASC] = 0x003a,
+	[CMD_FRC] = 0x0039,
+	[CMD_TEMP_OFFSET] = 0x003b,
+	[CMD_FW_VERSION] = 0x0020,
+	[CMD_RESET] = 0x0034,
+};
+
+static u16 scd30_serdev_calc_crc(const char *buf, int size)
+{
+	return crc16(0xffff, buf, size);
+}
+
+static int scd30_serdev_xfer(struct scd30_state *state, char *txbuf, int txsize,
+			     char *rxbuf, int rxsize)
+{
+	struct serdev_device *serdev = to_serdev_device(state->dev);
+	struct scd30_serdev_priv *priv = state->priv;
+	int ret;
+
+	priv->buf = rxbuf;
+	priv->num_expected = rxsize;
+	priv->num = 0;
+
+	ret = serdev_device_write(serdev, txbuf, txsize, SCD30_SERDEV_TIMEOUT);
+	if (ret < txsize)
+		return ret < 0 ? ret : -EIO;
+
+	ret = wait_for_completion_interruptible_timeout(&priv->meas_ready,
+							SCD30_SERDEV_TIMEOUT);
+	if (ret > 0)
+		ret = 0;
+	else if (!ret)
+		ret = -ETIMEDOUT;
+
+	return ret;
+}
+
+static int scd30_serdev_command(struct scd30_state *state, enum scd30_cmd cmd,
+				u16 arg, void *response, int size)
+{
+	/*
+	 * Communication over serial line is based on modbus protocol (or rather
+	 * its variation called modbus over serial to be precise). Upon
+	 * receiving a request device should reply with response.
+	 *
+	 * Frame below represents a request message. Each field takes
+	 * exactly one byte.
+	 *
+	 * +------+------+-----+-----+-------+-------+-----+-----+
+	 * | dev  | op   | reg | reg | byte1 | byte0 | crc | crc |
+	 * | addr | code | msb | lsb |       |       | lsb | msb |
+	 * +------+------+-----+-----+-------+-------+-----+-----+
+	 *
+	 * The message device replies with depends on the 'op code' field from
+	 * the request. In case it was set to SCD30_SERDEV_WRITE sensor should
+	 * reply with unchanged request. Otherwise 'op code' was set to
+	 * SCD30_SERDEV_READ and response looks like the one below. As with
+	 * request, each field takes one byte.
+	 *
+	 * +------+------+--------+-------+-----+-------+-----+-----+
+	 * | dev  | op   | num of | byte0 | ... | byteN | crc | crc |
+	 * | addr | code | bytes  |       |     |       | lsb | msb |
+	 * +------+------+--------+-------+-----+-------+-----+-----+
+	 */
+	char txbuf[SCD30_SERDEV_MAX_BUF_SIZE] = { SCD30_SERDEV_ADDR },
+	     rxbuf[SCD30_SERDEV_MAX_BUF_SIZE], *rsp = response;
+	int ret, rxsize, txsize = 2;
+	u16 crc;
+
+	put_unaligned_be16(scd30_serdev_cmd_lookup_tbl[cmd], txbuf + txsize);
+	txsize += 2;
+
+	if (rsp) {
+		txbuf[1] = SCD30_SERDEV_READ;
+		if (cmd == CMD_READ_MEAS)
+			/* number of u16 words to read */
+			put_unaligned_be16(size / 2, txbuf + txsize);
+		else
+			put_unaligned_be16(0x0001, txbuf + txsize);
+		txsize += 2;
+		crc = scd30_serdev_calc_crc(txbuf, txsize);
+		put_unaligned_le16(crc, txbuf + txsize);
+		txsize += 2;
+		rxsize = SCD30_SERDEV_RX_HEADER_SIZE + size +
+			 SCD30_SERDEV_CRC_SIZE;
+	} else {
+		if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
+			arg = 0x0001;
+
+		txbuf[1] = SCD30_SERDEV_WRITE;
+		put_unaligned_be16(arg, txbuf + txsize);
+		txsize += 2;
+		crc = scd30_serdev_calc_crc(txbuf, txsize);
+		put_unaligned_le16(crc, txbuf + txsize);
+		txsize += 2;
+		rxsize = txsize;
+	}
+
+	ret = scd30_serdev_xfer(state, txbuf, txsize, rxbuf, rxsize);
+	if (ret)
+		return ret;
+
+	switch (txbuf[1]) {
+	case SCD30_SERDEV_WRITE:
+		if (memcmp(txbuf, txbuf, txsize)) {
+			dev_err(state->dev, "wrong message received\n");
+			return -EIO;
+		}
+		break;
+	case SCD30_SERDEV_READ:
+		if (rxbuf[2] != (rxsize -
+				 SCD30_SERDEV_RX_HEADER_SIZE -
+				 SCD30_SERDEV_CRC_SIZE)) {
+			dev_err(state->dev,
+				"received data size does not match header\n");
+			return -EIO;
+		}
+
+		rxsize -= SCD30_SERDEV_CRC_SIZE;
+		crc = get_unaligned_le16(rxbuf + rxsize);
+		if (crc != scd30_serdev_calc_crc(rxbuf, rxsize)) {
+			dev_err(state->dev, "data integrity check failed\n");
+			return -EIO;
+		}
+
+		rxsize -= SCD30_SERDEV_RX_HEADER_SIZE;
+		memcpy(rsp, rxbuf + SCD30_SERDEV_RX_HEADER_SIZE, rxsize);
+		break;
+	default:
+		dev_err(state->dev, "received unknown op code\n");
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int scd30_serdev_receive_buf(struct serdev_device *serdev,
+				    const unsigned char *buf, size_t size)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(&serdev->dev);
+	struct scd30_serdev_priv *priv;
+	struct scd30_state *state;
+	int num;
+
+	if (!indio_dev)
+		return 0;
+
+	state = iio_priv(indio_dev);
+	priv = state->priv;
+
+	/* just in case sensor puts some unexpected bytes on the bus */
+	if (!priv->buf)
+		return 0;
+
+	if (priv->num + size >= priv->num_expected)
+		num = priv->num_expected - priv->num;
+	else
+		num = size;
+
+	memcpy(priv->buf + priv->num, buf, num);
+	priv->num += num;
+
+	if (priv->num == priv->num_expected) {
+		priv->buf = NULL;
+		complete(&priv->meas_ready);
+	}
+
+	return num;
+}
+
+static const struct serdev_device_ops scd30_serdev_ops = {
+	.receive_buf = scd30_serdev_receive_buf,
+	.write_wakeup = serdev_device_write_wakeup,
+};
+
+static int scd30_serdev_probe(struct serdev_device *serdev)
+{
+	struct device *dev = &serdev->dev;
+	struct scd30_serdev_priv *priv;
+	int irq, ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	init_completion(&priv->meas_ready);
+	serdev_device_set_client_ops(serdev, &scd30_serdev_ops);
+
+	ret = devm_serdev_device_open(dev, serdev);
+	if (ret)
+		return ret;
+
+	serdev_device_set_baudrate(serdev, 19200);
+	serdev_device_set_flow_control(serdev, false);
+
+	ret = serdev_device_set_parity(serdev, SERDEV_PARITY_NONE);
+	if (ret)
+		return ret;
+
+	irq = fwnode_irq_get(dev_fwnode(dev), 0);
+
+	return scd30_probe(dev, irq, KBUILD_MODNAME, priv,
+			   scd30_serdev_command);
+}
+
+static const struct of_device_id scd30_serdev_of_match[] = {
+	{ .compatible = "sensirion,scd30" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, scd30_serdev_of_match);
+
+static struct serdev_device_driver scd30_serdev_driver = {
+	.driver = {
+		.name = KBUILD_MODNAME,
+		.of_match_table = scd30_serdev_of_match,
+		.pm = &scd30_pm_ops,
+	},
+	.probe = scd30_serdev_probe,
+};
+module_serdev_device_driver(scd30_serdev_driver);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor serial driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related

* [PATCH v3 0/4] Add support for SCD30 sensor
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski

Following series adds support for Sensirion SCD30 sensor module capable of
measuring carbon dioxide, temperature and relative humidity. CO2 measurements
base on NDIR principle while temperature and relative humidity are measured by
the on board SHT31. As for sensor communication, both I2C and serial interfaces
are supported.

v3:
* simplify code by scaling temperature & humidity in _read_meas()
* update realbits in scan types
* s/adjecent/adjacent
* drop IIO_CHAN_INFO_RAW from _write_raw_get_fmt because there's no raw
  output channel
* rework locking in _read_raw
* fix endianess problem on BE machine
* align timestamp properly before pushing to buffers
* explain why interrupt gets disabled after registration
* add trigger validation
* drop SCALE for temperature and humidity channel as they are processed
* register action which stops measuring after starting measurements
* spit generic calibration attr into two doing specific things
* add comment explaining why priv in struct scd30_state is for
* rename node in binding example to co2-sensor

v2:
* move asm/byteorder.h towards the bottom of include list
* make channel address names in enum more specific
* add postfixes to defines and extra comments
* drop unneeded i2c include from scd30 header
* break generic command sending function into specialized options
* expose automatic calibration and forced calibration via the same attr
* use SAMP_FREQ to set frequency instead of meas_interval attr
* use CALISCALE to set pressure compensation instead of pressure_comp attr
* use CALIBBIAS to set temperature offset instead of temp_offset attr
* fix order in MAINTAINERS
* drop attribute allowing one to reset sensor
* as we have dt probing drop board file based probing (i2c_device_id)
* merge patches touching related files
* use fwnode API to retrieve interrupt from dt
* fix interrupt-parent spelling
* change binding license
* drop supply from required property

Tomasz Duszynski (4):
  iio: chemical: scd30: add core driver
  iio: chemical: scd30: add I2C interface driver
  iio: chemical: scd30: add serial interface driver
  dt-bindings: iio: scd30: add device binding file

 Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 +
 .../iio/chemical/sensirion,scd30.yaml         |  68 ++
 MAINTAINERS                                   |   9 +
 drivers/iio/chemical/Kconfig                  |  33 +
 drivers/iio/chemical/Makefile                 |   3 +
 drivers/iio/chemical/scd30.h                  |  80 ++
 drivers/iio/chemical/scd30_core.c             | 783 ++++++++++++++++++
 drivers/iio/chemical/scd30_i2c.c              | 134 +++
 drivers/iio/chemical/scd30_serial.c           | 266 ++++++
 9 files changed, 1410 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
 create mode 100644 Documentation/devicetree/bindings/iio/chemical/sensirion,scd30.yaml
 create mode 100644 drivers/iio/chemical/scd30.h
 create mode 100644 drivers/iio/chemical/scd30_core.c
 create mode 100644 drivers/iio/chemical/scd30_i2c.c
 create mode 100644 drivers/iio/chemical/scd30_serial.c

--
2.26.2


^ permalink raw reply

* [PATCH v3 1/4] iio: chemical: scd30: add core driver
From: Tomasz Duszynski @ 2020-06-02 16:47 UTC (permalink / raw)
  To: linux-iio
  Cc: linux-kernel, devicetree, robh+dt, jic23, andy.shevchenko, pmeerw,
	Tomasz Duszynski
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

Add Sensirion SCD30 carbon dioxide core driver.

Signed-off-by: Tomasz Duszynski <tomasz.duszynski@octakon.com>
---
 Documentation/ABI/testing/sysfs-bus-iio-scd30 |  34 +
 MAINTAINERS                                   |   6 +
 drivers/iio/chemical/Kconfig                  |  11 +
 drivers/iio/chemical/Makefile                 |   1 +
 drivers/iio/chemical/scd30.h                  |  80 ++
 drivers/iio/chemical/scd30_core.c             | 783 ++++++++++++++++++
 6 files changed, 915 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-scd30
 create mode 100644 drivers/iio/chemical/scd30.h
 create mode 100644 drivers/iio/chemical/scd30_core.c

diff --git a/Documentation/ABI/testing/sysfs-bus-iio-scd30 b/Documentation/ABI/testing/sysfs-bus-iio-scd30
new file mode 100644
index 000000000000..b9712f390bec
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-iio-scd30
@@ -0,0 +1,34 @@
+What:		/sys/bus/iio/devices/iio:deviceX/calibration_auto_enable
+Date:		June 2020
+KernelVersion:	5.8
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Contaminants build-up in the measurement chamber or optical
+		elements deterioration leads to sensor drift.
+
+		One can compensate for sensor drift by using automatic self
+		calibration procedure (asc).
+
+		Writing 1 or 0 to this attribute will respectively activate or
+		deactivate asc.
+
+		Upon reading current asc status is returned.
+
+What:		/sys/bus/iio/devices/iio:deviceX/calibration_forced_value
+Date:		June 2020
+KernelVersion:	5.8
+Contact:	linux-iio@vger.kernel.org
+Description:
+		Contaminants build-up in the measurement chamber or optical
+		elements deterioration leads to sensor drift.
+
+		One can compensate for sensor drift by using forced
+		recalibration (frc). This is useful in case there's known
+		co2 reference available nearby the sensor.
+
+		Picking value from the range [400 1 2000] and writing it to the
+		sensor will set frc.
+
+		Upon reading current frc value is returned. Note that after
+		power cycling default value (i.e 400) is returned even though
+		internally sensor had recalibrated itself.
diff --git a/MAINTAINERS b/MAINTAINERS
index 60ed2963efaa..41a509cca6f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15137,6 +15137,12 @@ S:	Maintained
 F:	drivers/misc/phantom.c
 F:	include/uapi/linux/phantom.h
 
+SENSIRION SCD30 CARBON DIOXIDE SENSOR DRIVER
+M:	Tomasz Duszynski <tomasz.duszynski@octakon.com>
+S:	Maintained
+F:	drivers/iio/chemical/scd30.h
+F:	drivers/iio/chemical/scd30_core.c
+
 SENSIRION SPS30 AIR POLLUTION SENSOR DRIVER
 M:	Tomasz Duszynski <tduszyns@gmail.com>
 S:	Maintained
diff --git a/drivers/iio/chemical/Kconfig b/drivers/iio/chemical/Kconfig
index 7f21afd73b1c..99e852b67e55 100644
--- a/drivers/iio/chemical/Kconfig
+++ b/drivers/iio/chemical/Kconfig
@@ -85,6 +85,17 @@ config PMS7003
 	  To compile this driver as a module, choose M here: the module will
 	  be called pms7003.
 
+config SCD30_CORE
+	tristate "SCD30 carbon dioxide sensor driver"
+	select IIO_BUFFER
+	select IIO_TRIGGERED_BUFFER
+	help
+	  Say Y here to build support for the Sensirion SCD30 sensor with carbon
+	  dioxide, relative humidity and temperature sensing capabilities.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called scd30_core.
+
 config SENSIRION_SGP30
 	tristate "Sensirion SGPxx gas sensors"
 	depends on I2C
diff --git a/drivers/iio/chemical/Makefile b/drivers/iio/chemical/Makefile
index aba4167db745..c9804b041ecd 100644
--- a/drivers/iio/chemical/Makefile
+++ b/drivers/iio/chemical/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_BME680_SPI) += bme680_spi.o
 obj-$(CONFIG_CCS811)		+= ccs811.o
 obj-$(CONFIG_IAQCORE)		+= ams-iaq-core.o
 obj-$(CONFIG_PMS7003) += pms7003.o
+obj-$(CONFIG_SCD30_CORE) += scd30_core.o
 obj-$(CONFIG_SENSIRION_SGP30)	+= sgp30.o
 obj-$(CONFIG_SPS30) += sps30.o
 obj-$(CONFIG_VZ89X)		+= vz89x.o
diff --git a/drivers/iio/chemical/scd30.h b/drivers/iio/chemical/scd30.h
new file mode 100644
index 000000000000..38dcf5647d7f
--- /dev/null
+++ b/drivers/iio/chemical/scd30.h
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _SCD30_H
+#define _SCD30_H
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/regulator/consumer.h>
+#include <linux/types.h>
+
+struct scd30_state;
+
+enum scd30_cmd {
+	/* start continuous measurement with pressure compensation */
+	CMD_START_MEAS,
+	/* stop continuous measurement */
+	CMD_STOP_MEAS,
+	/* set/get measurement interval */
+	CMD_MEAS_INTERVAL,
+	/* check whether new measurement is ready */
+	CMD_MEAS_READY,
+	/* get measurement */
+	CMD_READ_MEAS,
+	/* turn on/off automatic self calibration */
+	CMD_ASC,
+	/* set/get forced recalibration value */
+	CMD_FRC,
+	/* set/get temperature offset */
+	CMD_TEMP_OFFSET,
+	/* get firmware version */
+	CMD_FW_VERSION,
+	/* reset sensor */
+	CMD_RESET,
+	/*
+	 * Command for altitude compensation was omitted intentionally because
+	 * the same can be achieved by means of CMD_START_MEAS which takes
+	 * pressure above the sea level as an argument.
+	 */
+};
+
+#define SCD30_MEAS_COUNT 3
+
+typedef int (*scd30_command_t)(struct scd30_state *state, enum scd30_cmd cmd,
+			       u16 arg, void *response, int size);
+
+struct scd30_state {
+	/* serialize access to the device */
+	struct mutex lock;
+	struct device *dev;
+	struct regulator *vdd;
+	struct completion meas_ready;
+	/*
+	 * priv pointer is solely for serdev driver private data. We keep it
+	 * here because driver_data inside dev has been already used for iio and
+	 * struct serdev_device doesn't have one.
+	 */
+	void *priv;
+	int irq;
+	/*
+	 * no way to retrieve current ambient pressure compensation value from
+	 * the sensor so keep one around
+	 */
+	u16 pressure_comp;
+	u16 meas_interval;
+	int meas[SCD30_MEAS_COUNT];
+
+	scd30_command_t command;
+};
+
+int scd30_suspend(struct device *dev);
+int scd30_resume(struct device *dev);
+
+static __maybe_unused SIMPLE_DEV_PM_OPS(scd30_pm_ops, scd30_suspend,
+					scd30_resume);
+
+int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
+		scd30_command_t command);
+
+#endif
diff --git a/drivers/iio/chemical/scd30_core.c b/drivers/iio/chemical/scd30_core.c
new file mode 100644
index 000000000000..a41a09e7a513
--- /dev/null
+++ b/drivers/iio/chemical/scd30_core.c
@@ -0,0 +1,783 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Sensirion SCD30 carbon dioxide sensor core driver
+ *
+ * Copyright (c) 2020 Tomasz Duszynski <tomasz.duszynski@octakon.com>
+ */
+#include <linux/bits.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
+#include <linux/iio/types.h>
+#include <linux/interrupt.h>
+#include <linux/irqreturn.h>
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/regulator/consumer.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <asm/byteorder.h>
+
+#include "scd30.h"
+
+#define SCD30_PRESSURE_COMP_MIN_MBAR 700
+#define SCD30_PRESSURE_COMP_MAX_MBAR 1400
+#define SCD30_PRESSURE_COMP_DEFAULT 1013
+#define SCD30_MEAS_INTERVAL_MIN_S 2
+#define SCD30_MEAS_INTERVAL_MAX_S 1800
+#define SCD30_MEAS_INTERVAL_DEFAULT SCD30_MEAS_INTERVAL_MIN_S
+#define SCD30_FRC_MIN_PPM 400
+#define SCD30_FRC_MAX_PPM 2000
+#define SCD30_TEMP_OFFSET_MAX 655360
+#define SCD30_EXTRA_TIMEOUT_PER_S 250
+
+enum {
+	SCD30_CONC,
+	SCD30_TEMP,
+	SCD30_HR,
+};
+
+static int scd30_command_write(struct scd30_state *state, enum scd30_cmd cmd,
+			       u16 arg)
+{
+	return state->command(state, cmd, arg, NULL, 0);
+}
+
+static int scd30_command_read(struct scd30_state *state, enum scd30_cmd cmd,
+			      u16 *val)
+{
+	__be16 tmp;
+	int ret;
+
+	ret = state->command(state, cmd, 0, &tmp, sizeof(tmp));
+	*val = be16_to_cpup(&tmp);
+
+	return ret;
+}
+
+static int scd30_reset(struct scd30_state *state)
+{
+	int ret;
+	u16 val;
+
+	ret = scd30_command_write(state, CMD_RESET, 0);
+	if (ret)
+		return ret;
+
+	/* sensor boots up within 2 secs */
+	msleep(2000);
+	/*
+	 * Power-on-reset causes sensor to produce some glitch on i2c bus and
+	 * some controllers end up in error state. Try to recover by placing
+	 * any data on the bus.
+	 */
+	scd30_command_read(state, CMD_MEAS_READY, &val);
+
+	return 0;
+}
+
+/* simplified float to fixed point conversion with a scaling factor of 0.01 */
+static int scd30_float_to_fp(int float32)
+{
+	int fraction, shift,
+	    mantissa = float32 & GENMASK(22, 0),
+	    sign = float32 & BIT(31) ? -1 : 1,
+	    exp = (float32 & ~BIT(31)) >> 23;
+
+	/* special case 0 */
+	if (!exp && !mantissa)
+		return 0;
+
+	exp -= 127;
+	if (exp < 0) {
+		exp = -exp;
+		/* return values ranging from 1 to 99 */
+		return sign * ((((BIT(23) + mantissa) * 100) >> 23) >> exp);
+	}
+
+	/* return values starting at 100 */
+	shift = 23 - exp;
+	float32 = BIT(exp) + (mantissa >> shift);
+	fraction = mantissa & GENMASK(shift - 1, 0);
+
+	return sign * (float32 * 100 + ((fraction * 100) >> shift));
+}
+
+static int scd30_read_meas(struct scd30_state *state)
+{
+	int i, ret;
+
+	ret = state->command(state, CMD_READ_MEAS, 0, state->meas,
+			     sizeof(state->meas));
+	if (ret)
+		return ret;
+
+	be32_to_cpu_array(state->meas, state->meas, ARRAY_SIZE(state->meas));
+
+	for (i = 0; i < ARRAY_SIZE(state->meas); i++)
+		state->meas[i] = scd30_float_to_fp(state->meas[i]);
+
+	/*
+	 * co2 is left unprocessed while temperature and humidity are scaled
+	 * to milli deg C and milli percent respectively.
+	 */
+	state->meas[SCD30_TEMP] *= 10;
+	state->meas[SCD30_HR] *= 10;
+
+	return 0;
+}
+
+static int scd30_wait_meas_irq(struct scd30_state *state)
+{
+	int ret, timeout;
+
+	timeout = state->meas_interval * (1000 + SCD30_EXTRA_TIMEOUT_PER_S);
+	timeout = msecs_to_jiffies(timeout);
+	reinit_completion(&state->meas_ready);
+	enable_irq(state->irq);
+	ret = wait_for_completion_interruptible_timeout(&state->meas_ready,
+							timeout);
+	if (ret > 0)
+		ret = 0;
+	else if (!ret)
+		ret = -ETIMEDOUT;
+
+	disable_irq(state->irq);
+
+	return ret;
+}
+
+static int scd30_wait_meas_poll(struct scd30_state *state)
+{
+	int timeout = state->meas_interval * SCD30_EXTRA_TIMEOUT_PER_S;
+	int tries = 5;
+
+	do {
+		int ret;
+		u16 val;
+
+		ret = scd30_command_read(state, CMD_MEAS_READY, &val);
+		if (ret)
+			return -EIO;
+
+		/* new measurement available */
+		if (val)
+			break;
+
+		msleep_interruptible(timeout);
+	} while (--tries);
+
+	return tries ? 0 : -ETIMEDOUT;
+}
+
+static int scd30_read_poll(struct scd30_state *state)
+{
+	int ret;
+
+	ret = scd30_wait_meas_poll(state);
+	if (ret)
+		return ret;
+
+	return scd30_read_meas(state);
+}
+
+static int scd30_read(struct scd30_state *state)
+{
+	if (state->irq > 0)
+		return scd30_wait_meas_irq(state);
+
+	return scd30_read_poll(state);
+}
+
+static int scd30_read_raw(struct iio_dev *indio_dev,
+			  struct iio_chan_spec const *chan,
+			  int *val, int *val2, long mask)
+{
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret = -EINVAL;
+	u16 tmp;
+
+	mutex_lock(&state->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+	case IIO_CHAN_INFO_PROCESSED:
+		ret = iio_device_claim_direct_mode(indio_dev);
+		if (ret)
+			break;
+
+		ret = scd30_read(state);
+		if (ret) {
+			iio_device_release_direct_mode(indio_dev);
+			break;
+		}
+
+		*val = state->meas[chan->address];
+		iio_device_release_direct_mode(indio_dev);
+		ret = IIO_VAL_INT;
+		break;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = 1;
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		ret = scd30_command_read(state, CMD_MEAS_INTERVAL, &tmp);
+		if (ret)
+			break;
+
+		*val = 0;
+		*val2 = 1000000000 / tmp;
+		ret = IIO_VAL_INT_PLUS_NANO;
+		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*val = state->pressure_comp / 10;
+		*val2 = (state->pressure_comp % 10) * 100000;
+		ret = IIO_VAL_INT_PLUS_MICRO;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		ret = scd30_command_read(state, CMD_TEMP_OFFSET, &tmp);
+		if (ret)
+			break;
+
+		*val = tmp;
+		ret = IIO_VAL_INT;
+		break;
+	}
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static int scd30_write_raw(struct iio_dev *indio_dev,
+			   struct iio_chan_spec const *chan,
+			   int val, int val2, long mask)
+{
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret = -EINVAL;
+
+	mutex_lock(&state->lock);
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		if (val)
+			break;
+
+		val = 1000000000 / val2;
+		if (val < SCD30_MEAS_INTERVAL_MIN_S ||
+		    val > SCD30_MEAS_INTERVAL_MAX_S)
+			break;
+
+		ret = scd30_command_write(state, CMD_MEAS_INTERVAL, val);
+		if (ret)
+			break;
+
+		state->meas_interval = val;
+		break;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		val = (val * 1000000 + val2) / 100000;
+		if (val < SCD30_PRESSURE_COMP_MIN_MBAR ||
+		    val > SCD30_PRESSURE_COMP_MAX_MBAR)
+			break;
+
+		ret = scd30_command_write(state, CMD_START_MEAS, val);
+		if (ret)
+			break;
+
+		state->pressure_comp = val;
+		break;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		if (val < 0 || val > SCD30_TEMP_OFFSET_MAX)
+			break;
+		/*
+		 * Manufacturer does not explicitly specify min/max sensible
+		 * values hence check is omitted for simplicity.
+		 */
+		ret = scd30_command_write(state, CMD_TEMP_OFFSET / 10, val);
+	}
+	mutex_unlock(&state->lock);
+
+	return ret;
+}
+
+static int scd30_write_raw_get_fmt(struct iio_dev *indio_dev,
+				   struct iio_chan_spec const *chan, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_CALIBSCALE:
+		return IIO_VAL_INT_PLUS_MICRO;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		return IIO_VAL_INT;
+	}
+
+	return -EINVAL;
+}
+
+static const int scd30_pressure_calibscale_available[] = {
+	SCD30_PRESSURE_COMP_MIN_MBAR / 10, 0,
+	0, 100000,
+	SCD30_PRESSURE_COMP_MAX_MBAR / 10, 0,
+};
+
+static const int scd30_temp_calibbias_available[] = {
+	0, 10, SCD30_TEMP_OFFSET_MAX,
+};
+
+static int scd30_read_avail(struct iio_dev *indio_dev,
+			    struct iio_chan_spec const *chan, const int **vals,
+			    int *type, int *length, long mask)
+{
+	switch (mask) {
+	case IIO_CHAN_INFO_CALIBSCALE:
+		*vals = scd30_pressure_calibscale_available;
+		*type = IIO_VAL_INT_PLUS_MICRO;
+
+		return IIO_AVAIL_RANGE;
+	case IIO_CHAN_INFO_CALIBBIAS:
+		*vals = scd30_temp_calibbias_available;
+		*type = IIO_VAL_INT;
+
+		return IIO_AVAIL_RANGE;
+	}
+
+	return -EINVAL;
+}
+
+static ssize_t sampling_frequency_available_show(struct device *dev,
+						 struct device_attribute *attr,
+						 char *buf)
+{
+	int i = SCD30_MEAS_INTERVAL_MIN_S;
+	ssize_t len = 0;
+
+	do {
+		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%09u ",
+				 1000000000 / i);
+		/*
+		 * Not all values fit PAGE_SIZE buffer hence print every 6th
+		 * (each frequency differs by 6s in time domain from the
+		 * adjacent). Unlisted but valid ones are still accepted.
+		 */
+		i += 6;
+	} while (i <= SCD30_MEAS_INTERVAL_MAX_S);
+
+	buf[len - 1] = '\n';
+
+	return len;
+}
+
+static ssize_t calibration_auto_enable_show(struct device *dev,
+					    struct device_attribute *attr,
+					    char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+	u16 val;
+
+	mutex_lock(&state->lock);
+	ret = scd30_command_read(state, CMD_ASC, &val);
+	mutex_unlock(&state->lock);
+
+	return ret ?: sprintf(buf, "%d\n", val);
+}
+
+static ssize_t calibration_auto_enable_store(struct device *dev,
+					     struct device_attribute *attr,
+					     const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+	u16 val;
+
+	ret = kstrtou16(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	mutex_lock(&state->lock);
+	ret = scd30_command_write(state, CMD_ASC, !!val);
+	mutex_unlock(&state->lock);
+
+	return ret ?: len;
+}
+
+static ssize_t calibration_forced_value_show(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+	u16 val;
+
+	mutex_lock(&state->lock);
+	ret = scd30_command_read(state, CMD_FRC, &val);
+	mutex_unlock(&state->lock);
+
+	return ret ?: sprintf(buf, "%d\n", val);
+}
+
+static ssize_t calibration_forced_value_store(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf, size_t len)
+{
+	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+	u16 val;
+
+	ret = kstrtou16(buf, 0, &val);
+	if (ret)
+		return ret;
+
+	if (val < SCD30_FRC_MIN_PPM || val > SCD30_FRC_MAX_PPM)
+		return -EINVAL;
+
+	mutex_lock(&state->lock);
+	ret = scd30_command_write(state, CMD_FRC, val);
+	mutex_unlock(&state->lock);
+
+	return ret ?: len;
+}
+
+static IIO_DEVICE_ATTR_RO(sampling_frequency_available, 0);
+static IIO_DEVICE_ATTR_RW(calibration_auto_enable, 0);
+static IIO_DEVICE_ATTR_RW(calibration_forced_value, 0);
+
+static struct attribute *scd30_attrs[] = {
+	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
+	&iio_dev_attr_calibration_auto_enable.dev_attr.attr,
+	&iio_dev_attr_calibration_forced_value.dev_attr.attr,
+	NULL
+};
+
+static const struct attribute_group scd30_attr_group = {
+	.attrs = scd30_attrs,
+};
+
+static const struct iio_info scd30_info = {
+	.attrs = &scd30_attr_group,
+	.read_raw = scd30_read_raw,
+	.write_raw = scd30_write_raw,
+	.write_raw_get_fmt = scd30_write_raw_get_fmt,
+	.read_avail = scd30_read_avail,
+};
+
+#define SCD30_CHAN_SCAN_TYPE(_sign, _realbits) .scan_type = { \
+	.sign = _sign, \
+	.realbits = _realbits, \
+	.storagebits = 32, \
+	.endianness = IIO_CPU, \
+}
+
+static const struct iio_chan_spec scd30_channels[] = {
+	{
+		.type = IIO_PRESSURE,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_CALIBSCALE),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBSCALE),
+		.scan_index = -1,
+	},
+	{
+		.type = IIO_CONCENTRATION,
+		.channel2 = IIO_MOD_CO2,
+		.address = SCD30_CONC,
+		.scan_index = SCD30_CONC,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
+				      BIT(IIO_CHAN_INFO_SCALE),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+		.modified = 1,
+
+		SCD30_CHAN_SCAN_TYPE('u', 20),
+	},
+	{
+		.type = IIO_TEMP,
+		.address = SCD30_TEMP,
+		.scan_index = SCD30_TEMP,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED) |
+				      BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.info_mask_separate_available = BIT(IIO_CHAN_INFO_CALIBBIAS),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+
+		SCD30_CHAN_SCAN_TYPE('s', 18),
+	},
+	{
+		.type = IIO_HUMIDITYRELATIVE,
+		.address = SCD30_HR,
+		.scan_index = SCD30_HR,
+		.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+		.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
+
+		SCD30_CHAN_SCAN_TYPE('u', 17),
+	},
+	IIO_CHAN_SOFT_TIMESTAMP(3),
+};
+
+int __maybe_unused scd30_suspend(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct scd30_state *state  = iio_priv(indio_dev);
+	int ret;
+
+	ret = scd30_command_write(state, CMD_STOP_MEAS, 0);
+	if (ret)
+		return ret;
+
+	return regulator_disable(state->vdd);
+}
+EXPORT_SYMBOL(scd30_suspend);
+
+int __maybe_unused scd30_resume(struct device *dev)
+{
+	struct iio_dev *indio_dev = dev_get_drvdata(dev);
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+
+	ret = regulator_enable(state->vdd);
+	if (ret)
+		return ret;
+
+	return scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
+}
+EXPORT_SYMBOL(scd30_resume);
+
+static void scd30_stop_meas(void *data)
+{
+	struct scd30_state *state = data;
+
+	scd30_command_write(state, CMD_STOP_MEAS, 0);
+}
+
+static void scd30_disable_regulator(void *data)
+{
+	struct scd30_state *state = data;
+
+	regulator_disable(state->vdd);
+}
+
+static irqreturn_t scd30_irq_handler(int irq, void *priv)
+{
+	struct iio_dev *indio_dev = priv;
+
+	if (iio_buffer_enabled(indio_dev)) {
+		iio_trigger_poll(indio_dev->trig);
+
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t scd30_irq_thread_handler(int irq, void *priv)
+{
+	struct iio_dev *indio_dev = priv;
+	struct scd30_state *state = iio_priv(indio_dev);
+	int ret;
+
+	ret = scd30_read_meas(state);
+	if (ret)
+		goto out;
+
+	complete_all(&state->meas_ready);
+out:
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t scd30_trigger_handler(int irq, void *p)
+{
+	struct iio_poll_func *pf = p;
+	struct iio_dev *indio_dev = pf->indio_dev;
+	struct scd30_state *state = iio_priv(indio_dev);
+	struct {
+		int data[SCD30_MEAS_COUNT];
+		s64 ts __aligned(8);
+	} scan = { 0, };
+	int ret;
+
+	mutex_lock(&state->lock);
+	if (!iio_trigger_using_own(indio_dev))
+		ret = scd30_read_poll(state);
+	else
+		ret = scd30_read_meas(state);
+	memcpy(scan.data, state->meas, sizeof(state->meas));
+	mutex_unlock(&state->lock);
+	if (ret)
+		goto out;
+
+	iio_push_to_buffers_with_timestamp(indio_dev, &scan,
+					   iio_get_time_ns(indio_dev));
+out:
+	iio_trigger_notify_done(indio_dev->trig);
+	return IRQ_HANDLED;
+}
+
+static int scd30_set_trigger_state(struct iio_trigger *trig, bool state)
+{
+	struct iio_dev *indio_dev = iio_trigger_get_drvdata(trig);
+	struct scd30_state *st = iio_priv(indio_dev);
+
+	if (state)
+		enable_irq(st->irq);
+	else
+		disable_irq(st->irq);
+
+	return 0;
+}
+
+static const struct iio_trigger_ops scd30_trigger_ops = {
+	.set_trigger_state = scd30_set_trigger_state,
+	.validate_device = iio_trigger_validate_own_device,
+};
+
+static int scd30_setup_trigger(struct iio_dev *indio_dev)
+{
+	struct scd30_state *state = iio_priv(indio_dev);
+	struct device *dev = indio_dev->dev.parent;
+	struct iio_trigger *trig;
+	int ret;
+
+	trig = devm_iio_trigger_alloc(dev, "%s-dev%d", indio_dev->name,
+				      indio_dev->id);
+	if (!trig) {
+		dev_err(dev, "failed to allocate trigger\n");
+		return -ENOMEM;
+	}
+
+	trig->dev.parent = dev;
+	trig->ops = &scd30_trigger_ops;
+	iio_trigger_set_drvdata(trig, indio_dev);
+
+	ret = devm_iio_trigger_register(dev, trig);
+	if (ret)
+		return ret;
+
+	indio_dev->trig = iio_trigger_get(trig);
+
+	ret = devm_request_threaded_irq(dev, state->irq, scd30_irq_handler,
+					scd30_irq_thread_handler,
+					IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+					indio_dev->name, indio_dev);
+	if (ret)
+		dev_err(dev, "failed to request irq\n");
+
+	/*
+	 * Interrupt is enabled just before taking a fresh measurement
+	 * and disabled afterwards. This means we need to disable it here
+	 * to keep calls to enable/disable balanced.
+	 */
+	disable_irq(state->irq);
+
+	return ret;
+}
+
+int scd30_probe(struct device *dev, int irq, const char *name, void *priv,
+		scd30_command_t command)
+{
+	static const unsigned long scd30_scan_masks[] = { 0x07, 0x00 };
+	struct scd30_state *state;
+	struct iio_dev *indio_dev;
+	int ret;
+	u16 val;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	state = iio_priv(indio_dev);
+	state->dev = dev;
+	state->priv = priv;
+	state->irq = irq;
+	state->pressure_comp = SCD30_PRESSURE_COMP_DEFAULT;
+	state->meas_interval = SCD30_MEAS_INTERVAL_DEFAULT;
+	state->command = command;
+	mutex_init(&state->lock);
+	init_completion(&state->meas_ready);
+
+	dev_set_drvdata(dev, indio_dev);
+
+	indio_dev->dev.parent = dev;
+	indio_dev->info = &scd30_info;
+	indio_dev->name = name;
+	indio_dev->channels = scd30_channels;
+	indio_dev->num_channels = ARRAY_SIZE(scd30_channels);
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	indio_dev->available_scan_masks = scd30_scan_masks;
+
+	state->vdd = devm_regulator_get(dev, "vdd");
+	if (IS_ERR(state->vdd)) {
+		if (PTR_ERR(state->vdd) == -EPROBE_DEFER)
+			return -EPROBE_DEFER;
+
+		dev_err(dev, "failed to get regulator\n");
+		return PTR_ERR(state->vdd);
+	}
+
+	ret = regulator_enable(state->vdd);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(dev, scd30_disable_regulator, state);
+	if (ret)
+		return ret;
+
+	ret = scd30_reset(state);
+	if (ret) {
+		dev_err(dev, "failed to reset device: %d\n", ret);
+		return ret;
+	}
+
+	if (state->irq > 0) {
+		ret = scd30_setup_trigger(indio_dev);
+		if (ret) {
+			dev_err(dev, "failed to setup trigger: %d\n", ret);
+			return ret;
+		}
+	}
+
+	ret = devm_iio_triggered_buffer_setup(dev, indio_dev, NULL,
+					      scd30_trigger_handler, NULL);
+	if (ret)
+		return ret;
+
+	ret = scd30_command_read(state, CMD_FW_VERSION, &val);
+	if (ret) {
+		dev_err(dev, "failed to read firmware version: %d\n", ret);
+		return ret;
+	}
+	dev_info(dev, "firmware version: %d.%d\n", val >> 8, (char)val);
+
+	ret = scd30_command_write(state, CMD_MEAS_INTERVAL,
+				  state->meas_interval);
+	if (ret) {
+		dev_err(dev, "failed to set measurement interval: %d\n", ret);
+		return ret;
+	}
+
+	ret = scd30_command_write(state, CMD_START_MEAS, state->pressure_comp);
+	if (ret) {
+		dev_err(dev, "failed to start measurement: %d\n", ret);
+		return ret;
+	}
+
+	ret = devm_add_action_or_reset(dev, scd30_stop_meas, state);
+	if (ret)
+		return ret;
+
+	return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL(scd30_probe);
+
+MODULE_AUTHOR("Tomasz Duszynski <tomasz.duszynski@octakon.com>");
+MODULE_DESCRIPTION("Sensirion SCD30 carbon dioxide sensor core driver");
+MODULE_LICENSE("GPL v2");
-- 
2.26.2


^ permalink raw reply related

* Re: [PATCH v3 0/4] Add support for SCD30 sensor
From: Andy Shevchenko @ 2020-06-02 16:55 UTC (permalink / raw)
  To: Tomasz Duszynski
  Cc: linux-iio, Linux Kernel Mailing List, devicetree, Rob Herring,
	Jonathan Cameron, Peter Meerwald
In-Reply-To: <20200602164723.28858-1-tomasz.duszynski@octakon.com>

On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
<tomasz.duszynski@octakon.com> wrote:
>
> Following series adds support for Sensirion SCD30 sensor module capable of
> measuring carbon dioxide, temperature and relative humidity. CO2 measurements
> base on NDIR principle while temperature and relative humidity are measured by
> the on board SHT31. As for sensor communication, both I2C and serial interfaces
> are supported.

Btw, since we have relaxed 80 limit to 100, I recommend to reconsider
some lines to be joined.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 3/4] iio: chemical: scd30: add serial interface driver
From: Andy Shevchenko @ 2020-06-02 17:04 UTC (permalink / raw)
  To: Tomasz Duszynski
  Cc: linux-iio, Linux Kernel Mailing List, devicetree, Rob Herring,
	Jonathan Cameron, Peter Meerwald
In-Reply-To: <20200602164723.28858-4-tomasz.duszynski@octakon.com>

On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
<tomasz.duszynski@octakon.com> wrote:
>
> Add serial interface driver for the SCD30 sensor.

...

> +static u16 scd30_serdev_cmd_lookup_tbl[] = {
> +       [CMD_START_MEAS] = 0x0036,
> +       [CMD_STOP_MEAS] = 0x0037,
> +       [CMD_MEAS_INTERVAL] = 0x0025,
> +       [CMD_MEAS_READY] = 0x0027,
> +       [CMD_READ_MEAS] = 0x0028,
> +       [CMD_ASC] = 0x003a,
> +       [CMD_FRC] = 0x0039,
> +       [CMD_TEMP_OFFSET] = 0x003b,
> +       [CMD_FW_VERSION] = 0x0020,
> +       [CMD_RESET] = 0x0034,

Hmm... Can't we keep them ordered by value?

> +};

...

> +       ret = wait_for_completion_interruptible_timeout(&priv->meas_ready,
> +                                                       SCD30_SERDEV_TIMEOUT);
> +       if (ret > 0)
> +               ret = 0;
> +       else if (!ret)
> +               ret = -ETIMEDOUT;
> +
> +       return ret;

Perhaps

  if (ret < 0)
    return ret;
  if (ret == 0)
    return -ETIMEDOUT;
  return 0;

?

...

> +       char txbuf[SCD30_SERDEV_MAX_BUF_SIZE] = { SCD30_SERDEV_ADDR },
> +            rxbuf[SCD30_SERDEV_MAX_BUF_SIZE], *rsp = response;

Please, apply type to each variable separately.

...

> +       switch (txbuf[1]) {
> +       case SCD30_SERDEV_WRITE:

> +               if (memcmp(txbuf, txbuf, txsize)) {

I'm not sure I understand this.

> +                       dev_err(state->dev, "wrong message received\n");
> +                       return -EIO;
> +               }
> +               break;
> +       case SCD30_SERDEV_READ:

> +               if (rxbuf[2] != (rxsize -
> +                                SCD30_SERDEV_RX_HEADER_SIZE -
> +                                SCD30_SERDEV_CRC_SIZE)) {

Perhaps you can come up with better indentation/ line split?

> +                       dev_err(state->dev,
> +                               "received data size does not match header\n");
> +                       return -EIO;
> +               }
> +
> +               rxsize -= SCD30_SERDEV_CRC_SIZE;
> +               crc = get_unaligned_le16(rxbuf + rxsize);
> +               if (crc != scd30_serdev_calc_crc(rxbuf, rxsize)) {
> +                       dev_err(state->dev, "data integrity check failed\n");
> +                       return -EIO;
> +               }
> +
> +               rxsize -= SCD30_SERDEV_RX_HEADER_SIZE;
> +               memcpy(rsp, rxbuf + SCD30_SERDEV_RX_HEADER_SIZE, rxsize);
> +               break;
> +       default:
> +               dev_err(state->dev, "received unknown op code\n");
> +               return -EIO;
> +       }
> +
> +       return 0;
> +}

...

> +static struct serdev_device_driver scd30_serdev_driver = {
> +       .driver = {

> +               .name = KBUILD_MODNAME,

This is not the best what we can do. The name is an ABI and better if
it will be constant (independent on file name).

> +               .of_match_table = scd30_serdev_of_match,
> +               .pm = &scd30_pm_ops,
> +       },
> +       .probe = scd30_serdev_probe,
> +};

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* R: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
From: ansuelsmth @ 2020-06-02 17:07 UTC (permalink / raw)
  To: 'Bjorn Helgaas'
  Cc: 'Rob Herring', 'Sham Muthayyan',
	'Rob Herring', 'Andy Gross',
	'Bjorn Andersson', 'Bjorn Helgaas',
	'Mark Rutland', 'Stanimir Varbanov',
	'Lorenzo Pieralisi', 'Andrew Murray',
	'Philipp Zabel', linux-arm-msm, linux-pci, devicetree,
	linux-kernel
In-Reply-To: <20200602163255.GA821782@bjorn-Precision-5520>

> On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> > From: Sham Muthayyan <smuthayy@codeaurora.org>
> >
> > Add Force GEN1 support needed in some ipq8064 board that needs to
> limit
> > some PCIe line to gen1 for some hardware limitation. This is set by the
> > max-link-speed binding and needed by some soc based on ipq8064. (for
> > example Netgear R7800 router)
> >
> > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 259b627bf890..0ce15d53c46e 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -27,6 +27,7 @@
> >  #include <linux/slab.h>
> >  #include <linux/types.h>
> >
> > +#include "../../pci.h"
> >  #include "pcie-designware.h"
> >
> >  #define PCIE20_PARF_SYS_CTRL			0x00
> > @@ -99,6 +100,8 @@
> >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> >  #define SLV_ADDR_SPACE_SZ			0x10000000
> >
> > +#define PCIE20_LNK_CONTROL2_LINK_STATUS2	0xa0
> > +
> >  #define DEVICE_TYPE_RC				0x4
> >
> >  #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
> > @@ -195,6 +198,7 @@ struct qcom_pcie {
> >  	struct phy *phy;
> >  	struct gpio_desc *reset;
> >  	const struct qcom_pcie_ops *ops;
> > +	int gen;
> >  };
> >
> >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> > @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct
> qcom_pcie *pcie)
> >  	/* wait for clock acquisition */
> >  	usleep_range(1000, 1500);
> >
> > +	if (pcie->gen == 1) {
> > +		val = readl(pci->dbi_base +
> PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > +		val |= 1;
> 
> Is this the same bit that's visible in config space as
> PCI_EXP_LNKSTA_CLS_2_5GB?  Why not use that #define?
> 
> There are a bunch of other #defines in this file that look like
> redefinitions of standard things:
> 
>   #define PCIE20_COMMAND_STATUS                   0x04
>     Looks like PCI_COMMAND
> 
>   #define CMD_BME_VAL                             0x4
>     Looks like PCI_COMMAND_MASTER
> 
>   #define PCIE20_DEVICE_CONTROL2_STATUS2          0x98
>     Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)
> 
>   #define PCIE_CAP_CPL_TIMEOUT_DISABLE            0x10
>     Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS
> 
>   #define PCIE20_CAP                              0x70
>     This one is obviously device-specific
> 
>   #define PCIE20_CAP_LINK_CAPABILITIES            (PCIE20_CAP + 0xC)
>     Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)
> 
>   #define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) |
> BIT(11))
>     Looks like PCI_EXP_LNKCAP_ASPMS
> 
>   #define PCIE20_CAP_LINK_1                       (PCIE20_CAP + 0x14)
>   #define PCIE_CAP_LINK1_VAL                      0x2FD7F
>     This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
>     PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
>     Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
>     means.
> 

The define are used by ipq8074 and I really can't test the changes. Anyway
it shouldn't make a difference use the define instead of the custom value so
a patch should not harm anything... Problem is the last 2 define that we
really
don't know what they are about... How should I proceed? Change only the 
value related to PCI_EXP_LNKSTA_CLS_2_5GB or change all the other except the

last 2?


> > +		writel(val, pci->dbi_base +
> PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > +	}
> >
> >  	/* Set the Max TLP size to 2K, instead of using default of 4K */
> >  	writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
> > @@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct
> platform_device *pdev)
> >  		goto err_pm_runtime_put;
> >  	}
> >
> > +	pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
> > +	if (pcie->gen < 0)
> > +		pcie->gen = 2;
> > +
> >  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "parf");
> >  	pcie->parf = devm_ioremap_resource(dev, res);
> >  	if (IS_ERR(pcie->parf)) {
> > --
> > 2.25.1
> >


^ permalink raw reply

* Re: [PATCH v3 2/4] iio: chemical: scd30: add I2C interface driver
From: Andy Shevchenko @ 2020-06-02 17:14 UTC (permalink / raw)
  To: Tomasz Duszynski
  Cc: linux-iio, Linux Kernel Mailing List, devicetree, Rob Herring,
	Jonathan Cameron, Peter Meerwald
In-Reply-To: <20200602164723.28858-3-tomasz.duszynski@octakon.com>

On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
<tomasz.duszynski@octakon.com> wrote:
>
> Add I2C interface driver for the SCD30 sensor.

...

> +static u16 scd30_i2c_cmd_lookup_tbl[] = {
> +       [CMD_START_MEAS] = 0x0010,
> +       [CMD_STOP_MEAS] = 0x0104,
> +       [CMD_MEAS_INTERVAL] = 0x4600,
> +       [CMD_MEAS_READY] = 0x0202,
> +       [CMD_READ_MEAS] = 0x0300,
> +       [CMD_ASC] = 0x5306,
> +       [CMD_FRC] = 0x5204,
> +       [CMD_TEMP_OFFSET] = 0x5403,
> +       [CMD_FW_VERSION] = 0xd100,
> +       [CMD_RESET] = 0xd304,

Keep sorted by value?

> +};

...

> +       ret = i2c_master_send(client, txbuf, txsize);

> +       if (ret != txsize)
> +               return ret < 0 ? ret : -EIO;

Wouldn't be better

  if (ret < 0)
    return ret;
  if (ret != txsize)
    return -EIO;

?

> +       if (!rxbuf)
> +               return 0;
> +
> +       ret = i2c_master_recv(client, rxbuf, rxsize);

> +       if (ret != rxsize)
> +               return ret < 0 ? ret : -EIO;

Ditto.

...

> +static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd,
> +                            u16 arg, void *response, int size)
> +{
> +       char crc, buf[SCD30_I2C_MAX_BUF_SIZE], *rsp = response;
> +       int i, ret;

i -> offset ?

> +       put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
> +       i = 2;
> +
> +       if (rsp) {
> +               /* each two bytes are followed by a crc8 */
> +               size += size / 2;
> +       } else {
> +               put_unaligned_be16(arg, buf + i);
> +               crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
> +               i += 2;

> +               buf[i] = crc;
> +               i += 1;

buf[offset++] = crc; ?

> +               /* commands below don't take an argument */
> +               if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
> +                       i -= 3;
> +       }
> +
> +       ret = scd30_i2c_xfer(state, buf, i, buf, size);
> +       if (ret)
> +               return ret;
> +
> +       /* validate received data and strip off crc bytes */
> +       for (i = 0; i < size; i += 3) {
> +               crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
> +               if (crc != buf[i + 2]) {
> +                       dev_err(state->dev, "data integrity check failed\n");
> +                       return -EIO;
> +               }
> +

> +               *rsp++ = buf[i];

+ 0 (for the sake of consistency?

> +               *rsp++ = buf[i + 1];
> +       }
> +
> +       return 0;
> +}

...

> +static struct i2c_driver scd30_i2c_driver = {
> +       .driver = {

> +               .name = KBUILD_MODNAME,

Better to hard code.

> +               .of_match_table = scd30_i2c_of_match,
> +               .pm = &scd30_pm_ops,
> +       },
> +       .probe_new = scd30_i2c_probe,
> +};

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: R: [PATCH v5 11/11] PCI: qcom: Add Force GEN1 support
From: Bjorn Helgaas @ 2020-06-02 17:28 UTC (permalink / raw)
  To: ansuelsmth
  Cc: 'Rob Herring', 'Sham Muthayyan',
	'Rob Herring', 'Andy Gross',
	'Bjorn Andersson', 'Bjorn Helgaas',
	'Mark Rutland', 'Stanimir Varbanov',
	'Lorenzo Pieralisi', 'Andrew Murray',
	'Philipp Zabel', linux-arm-msm, linux-pci, devicetree,
	linux-kernel, Varadarajan Narayanan
In-Reply-To: <001101d63900$4c7aae60$e5700b20$@gmail.com>

[+cc Varada]

On Tue, Jun 02, 2020 at 07:07:27PM +0200, ansuelsmth@gmail.com wrote:
> > On Tue, Jun 02, 2020 at 01:53:52PM +0200, Ansuel Smith wrote:
> > > From: Sham Muthayyan <smuthayy@codeaurora.org>
> > >
> > > Add Force GEN1 support needed in some ipq8064 board that needs to
> > limit
> > > some PCIe line to gen1 for some hardware limitation. This is set by the
> > > max-link-speed binding and needed by some soc based on ipq8064. (for
> > > example Netgear R7800 router)
> > >
> > > Signed-off-by: Sham Muthayyan <smuthayy@codeaurora.org>
> > > Signed-off-by: Ansuel Smith <ansuelsmth@gmail.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > ---
> > >  drivers/pci/controller/dwc/pcie-qcom.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c
> > b/drivers/pci/controller/dwc/pcie-qcom.c
> > > index 259b627bf890..0ce15d53c46e 100644
> > > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > > @@ -27,6 +27,7 @@
> > >  #include <linux/slab.h>
> > >  #include <linux/types.h>
> > >
> > > +#include "../../pci.h"
> > >  #include "pcie-designware.h"
> > >
> > >  #define PCIE20_PARF_SYS_CTRL			0x00
> > > @@ -99,6 +100,8 @@
> > >  #define PCIE20_v3_PARF_SLV_ADDR_SPACE_SIZE	0x358
> > >  #define SLV_ADDR_SPACE_SZ			0x10000000
> > >
> > > +#define PCIE20_LNK_CONTROL2_LINK_STATUS2	0xa0
> > > +
> > >  #define DEVICE_TYPE_RC				0x4
> > >
> > >  #define QCOM_PCIE_2_1_0_MAX_SUPPLY	3
> > > @@ -195,6 +198,7 @@ struct qcom_pcie {
> > >  	struct phy *phy;
> > >  	struct gpio_desc *reset;
> > >  	const struct qcom_pcie_ops *ops;
> > > +	int gen;
> > >  };
> > >
> > >  #define to_qcom_pcie(x)		dev_get_drvdata((x)->dev)
> > > @@ -395,6 +399,11 @@ static int qcom_pcie_init_2_1_0(struct
> > qcom_pcie *pcie)
> > >  	/* wait for clock acquisition */
> > >  	usleep_range(1000, 1500);
> > >
> > > +	if (pcie->gen == 1) {
> > > +		val = readl(pci->dbi_base +
> > PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > > +		val |= 1;
> > 
> > Is this the same bit that's visible in config space as
> > PCI_EXP_LNKSTA_CLS_2_5GB?  Why not use that #define?
> > 
> > There are a bunch of other #defines in this file that look like
> > redefinitions of standard things:
> > 
> >   #define PCIE20_COMMAND_STATUS                   0x04
> >     Looks like PCI_COMMAND
> > 
> >   #define CMD_BME_VAL                             0x4
> >     Looks like PCI_COMMAND_MASTER
> > 
> >   #define PCIE20_DEVICE_CONTROL2_STATUS2          0x98
> >     Looks like (PCIE20_CAP + PCI_EXP_DEVCTL2)
> > 
> >   #define PCIE_CAP_CPL_TIMEOUT_DISABLE            0x10
> >     Looks like PCI_EXP_DEVCTL2_COMP_TMOUT_DIS
> > 
> >   #define PCIE20_CAP                              0x70
> >     This one is obviously device-specific
> > 
> >   #define PCIE20_CAP_LINK_CAPABILITIES            (PCIE20_CAP + 0xC)
> >     Looks like (PCIE20_CAP + PCI_EXP_LNKCAP)
> > 
> >   #define PCIE20_CAP_ACTIVE_STATE_LINK_PM_SUPPORT (BIT(10) |
> > BIT(11))
> >     Looks like PCI_EXP_LNKCAP_ASPMS
> > 
> >   #define PCIE20_CAP_LINK_1                       (PCIE20_CAP + 0x14)
> >   #define PCIE_CAP_LINK1_VAL                      0x2FD7F
> >     This looks like PCIE20_CAP_LINK_1 should be (PCIE20_CAP +
> >     PCI_EXP_SLTCAP), but "CAP_LINK_1" doesn't sound like the Slot
> >     Capabilities register, and I don't know what PCIE_CAP_LINK1_VAL
> >     means.
> 
> The define are used by ipq8074 and I really can't test the changes.
> Anyway it shouldn't make a difference use the define instead of the
> custom value so a patch should not harm anything... Problem is the
> last 2 define that we really don't know what they are about... How
> should I proceed? Change only the value related to
> PCI_EXP_LNKSTA_CLS_2_5GB or change all the other except the last 2?

I personally would change all the ones I mentioned above (in a
separate patch from the one that adds "max-link-speed" support).
Testing isn't a big deal because changing the #defines shouldn't
change the generated code at all.

PCIE20_CAP_LINK_1 / PCIE_CAP_LINK1_VAL looks like a potential bug or
at least a very misleading name.  I wouldn't touch it unless we can
figure out what's going on.

Looks like most of this was added by 5d76117f070d ("PCI: qcom: Add
support for IPQ8074 PCIe controller").  Shame on me for not asking
these questions at the time.

Sham, Varada, can you shed any light on PCIE20_CAP_LINK_1 and
PCIE_CAP_LINK1_VAL?

> > > +		writel(val, pci->dbi_base +
> > PCIE20_LNK_CONTROL2_LINK_STATUS2);
> > > +	}
> > >
> > >  	/* Set the Max TLP size to 2K, instead of using default of 4K */
> > >  	writel(CFG_REMOTE_RD_REQ_BRIDGE_SIZE_2K,
> > > @@ -1397,6 +1406,10 @@ static int qcom_pcie_probe(struct
> > platform_device *pdev)
> > >  		goto err_pm_runtime_put;
> > >  	}
> > >
> > > +	pcie->gen = of_pci_get_max_link_speed(pdev->dev.of_node);
> > > +	if (pcie->gen < 0)
> > > +		pcie->gen = 2;
> > > +
> > >  	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "parf");
> > >  	pcie->parf = devm_ioremap_resource(dev, res);
> > >  	if (IS_ERR(pcie->parf)) {
> > > --
> > > 2.25.1
> > >
> 

^ permalink raw reply

* [PATCH v3 04/10] arm64: dts: actions: limit address range for pinctrl node
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, manivannan.sadhasivam, robh+dt
  Cc: cristian.ciocaltea, linux-kernel, linux-arm-kernel, linux-actions,
	devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

After commit 7cdf8446ed1d ("arm64: dts: actions: Add pinctrl node for
Actions Semi S700") following error has been observed while booting
Linux on Cubieboard7-lite(based on S700 SoC).

[    0.257415] pinctrl-s700 e01b0000.pinctrl: can't request region for
resource [mem 0xe01b0000-0xe01b0fff]
[    0.266902] pinctrl-s700: probe of e01b0000.pinctrl failed with error -16

This is due to the fact that memory range for "sps" power domain controller
clashes with pinctrl.

One way to fix it, is to limit pinctrl address range which is safe
to do as current pinctrl driver uses address range only up to 0x100.

This commit limits the pinctrl address range to 0x100 so that it doesn't
conflict with sps range.

Fixes: 7cdf8446ed1d ("arm64: dts: actions: Add pinctrl node for Actions
Semi S700")

Suggested-by: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* this is no more don't merge and fixed
	  the broken S700 boot by limiting pinctrl
	  address range.
	* Modified the subject to reflect the changes.
Changes since v1:
        * No change.
Changes since RFC:
        * kept as do not merge.
---
 arch/arm64/boot/dts/actions/s700.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index 2006ad5424fa..f8eb72bb4125 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -231,7 +231,7 @@
 
 		pinctrl: pinctrl@e01b0000 {
 			compatible = "actions,s700-pinctrl";
-			reg = <0x0 0xe01b0000 0x0 0x1000>;
+			reg = <0x0 0xe01b0000 0x0 0x100>;
 			clocks = <&cmu CLK_GPIO>;
 			gpio-controller;
 			gpio-ranges = <&pinctrl 0 0 136>;
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 05/10] dt-bindings: dmaengine: convert Actions Semi Owl SoCs bindings to yaml
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, vkoul, manivannan.sadhasivam, robh+dt
  Cc: dan.j.williams, cristian.ciocaltea, linux-kernel,
	linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

Converts the device tree bindings for the Actions Semi Owl SoCs DMA
Controller over to YAML schemas.

It also adds new compatible string "actions,s700-dma".

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* Addressed Rob's comments:
           - removed unnecessary description. 
	   - added unevaluatedProperties
	   - added relevant information about
             dma-channels and dma-request
	* Added power-domain property.   	
Change since v1:
        * Updated the description field to reflect
          only the necessary information.
        * replaced the maxItems field with description for each
          controller attribute(except interrupts).
        * Replaced the clock macro with number to keep the example
          as independent as possible.

 New patch, was not there in RFC.
---
 Documentation/devicetree/bindings/dma/owl-dma.txt  | 47 -------------
 Documentation/devicetree/bindings/dma/owl-dma.yaml | 79 ++++++++++++++++++++++
 2 files changed, 79 insertions(+), 47 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/dma/owl-dma.txt
 create mode 100644 Documentation/devicetree/bindings/dma/owl-dma.yaml

diff --git a/Documentation/devicetree/bindings/dma/owl-dma.txt b/Documentation/devicetree/bindings/dma/owl-dma.txt
deleted file mode 100644
index 03e9bb12b75f..000000000000
--- a/Documentation/devicetree/bindings/dma/owl-dma.txt
+++ /dev/null
@@ -1,47 +0,0 @@
-* Actions Semi Owl SoCs DMA controller
-
-This binding follows the generic DMA bindings defined in dma.txt.
-
-Required properties:
-- compatible: Should be "actions,s900-dma".
-- reg: Should contain DMA registers location and length.
-- interrupts: Should contain 4 interrupts shared by all channel.
-- #dma-cells: Must be <1>. Used to represent the number of integer
-              cells in the dmas property of client device.
-- dma-channels: Physical channels supported.
-- dma-requests: Number of DMA request signals supported by the controller.
-                Refer to Documentation/devicetree/bindings/dma/dma.txt
-- clocks: Phandle and Specifier of the clock feeding the DMA controller.
-
-Example:
-
-Controller:
-                dma: dma-controller@e0260000 {
-                        compatible = "actions,s900-dma";
-                        reg = <0x0 0xe0260000 0x0 0x1000>;
-                        interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
-                                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
-                                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
-                                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
-                        #dma-cells = <1>;
-                        dma-channels = <12>;
-                        dma-requests = <46>;
-                        clocks = <&clock CLK_DMAC>;
-                };
-
-Client:
-
-DMA clients connected to the Actions Semi Owl SoCs DMA controller must
-use the format described in the dma.txt file, using a two-cell specifier
-for each channel.
-
-The two cells in order are:
-1. A phandle pointing to the DMA controller.
-2. The channel id.
-
-uart5: serial@e012a000 {
-        ...
-        dma-names = "tx", "rx";
-        dmas = <&dma 26>, <&dma 27>;
-        ...
-};
diff --git a/Documentation/devicetree/bindings/dma/owl-dma.yaml b/Documentation/devicetree/bindings/dma/owl-dma.yaml
new file mode 100644
index 000000000000..5577cd910781
--- /dev/null
+++ b/Documentation/devicetree/bindings/dma/owl-dma.yaml
@@ -0,0 +1,79 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/dma/owl-dma.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Actions Semi Owl SoCs DMA controller
+
+description: |
+  The OWL DMA is a general-purpose direct memory access controller capable of
+  supporting 10 and 12 independent DMA channels for S700 and S900 SoCs
+  respectively.
+
+maintainers:
+  - Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
+
+allOf:
+  - $ref: "dma-controller.yaml#"
+
+properties:
+  compatible:
+    enum:
+      - actions,s900-dma
+      - actions,s700-dma
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      controller supports 4 interrupts, which are freely assignable to the
+      DMA channels.
+    maxItems: 4
+
+  "#dma-cells":
+    const: 1
+
+  dma-channels:
+    maximum: 12
+
+  dma-requests:
+    maximum: 46
+
+  clocks:
+    maxItems: 1
+    description:
+      Phandle and Specifier of the clock feeding the DMA controller.
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - "#dma-cells"
+  - dma-channels
+  - dma-requests
+  - clocks
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    dma: dma-controller@e0260000 {
+        compatible = "actions,s900-dma";
+        reg = <0x0 0xe0260000 0x0 0x1000>;
+        interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+        #dma-cells = <1>;
+        dma-channels = <12>;
+        dma-requests = <46>;
+        clocks = <&clock 22>;
+    };
+
+...
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 06/10] arm64: dts: actions: Add DMA Controller for S700
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, vkoul, manivannan.sadhasivam, robh+dt
  Cc: dan.j.williams, cristian.ciocaltea, linux-kernel,
	linux-arm-kernel, linux-actions, devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

This commit adds DAM controller present on Actions S700, it differs from
S900 in terms of number of dma channels and requests.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* added power-domain property as sps
          is enabled now and DMA needs it.
Changes since v1:
        * No Change.
Changes since RFC:
        * No Change.
---
 arch/arm64/boot/dts/actions/s700.dtsi | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index f8eb72bb4125..605594dd7a0e 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -6,6 +6,7 @@
 #include <dt-bindings/clock/actions,s700-cmu.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/actions,s700-reset.h>
+#include <dt-bindings/power/owl-s700-powergate.h>
 
 / {
 	compatible = "actions,s700";
@@ -244,5 +245,19 @@
 				     <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>,
 				     <GIC_SPI 40 IRQ_TYPE_LEVEL_HIGH>;
 		};
+
+		dma: dma-controller@e0230000 {
+			compatible = "actions,s700-dma";
+			reg = <0x0 0xe0230000 0x0 0x1000>;
+			interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 58 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 59 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 60 IRQ_TYPE_LEVEL_HIGH>;
+			#dma-cells = <1>;
+			dma-channels = <10>;
+			dma-requests = <44>;
+			clocks = <&cmu CLK_DMAC>;
+			power-domains = <&sps S700_PD_DMA>;
+		};
 	};
 };
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 07/10] dt-bindings: reset: s700: Add binding constants for mmc
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, manivannan.sadhasivam, robh+dt
  Cc: cristian.ciocaltea, linux-kernel, linux-arm-kernel, linux-actions,
	devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

This commit adds device tree binding reset constants for mmc controller
present on Actions S700 Soc.

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* No change.
Changes since v1:
        * No change.
Changes since RFC:
        * added Rob's acked-by tag
---
 include/dt-bindings/reset/actions,s700-reset.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/dt-bindings/reset/actions,s700-reset.h b/include/dt-bindings/reset/actions,s700-reset.h
index 5e3b16b8ef53..a3118de6d7aa 100644
--- a/include/dt-bindings/reset/actions,s700-reset.h
+++ b/include/dt-bindings/reset/actions,s700-reset.h
@@ -30,5 +30,8 @@
 #define RESET_UART4				20
 #define RESET_UART5				21
 #define RESET_UART6				22
+#define RESET_SD0				23
+#define RESET_SD1				24
+#define RESET_SD2				25
 
 #endif /* __DT_BINDINGS_ACTIONS_S700_RESET_H */
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 08/10] dt-bindings: mmc: owl: add compatible string actions,s700-mmc
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, manivannan.sadhasivam, robh+dt
  Cc: cristian.ciocaltea, linux-kernel, linux-arm-kernel, linux-actions,
	devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

The commit adds a new SoC specific compatible string "actions,s700-mmc"
in combination with more generic string "actions,owl-mmc".

Placement order of these strings should abide by the principle of
"from most specific to most general".

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* Added Rob's Reviewed-by tag

* Newly added patch in v2.
---
 Documentation/devicetree/bindings/mmc/owl-mmc.yaml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/mmc/owl-mmc.yaml b/Documentation/devicetree/bindings/mmc/owl-mmc.yaml
index 12b40213426d..9604ef695585 100644
--- a/Documentation/devicetree/bindings/mmc/owl-mmc.yaml
+++ b/Documentation/devicetree/bindings/mmc/owl-mmc.yaml
@@ -14,7 +14,11 @@ maintainers:
 
 properties:
   compatible:
-    const: actions,owl-mmc
+    oneOf:
+      - const: actions,owl-mmc
+      - items:
+          - const: actions,s700-mmc
+          - const: actions,owl-mmc
 
   reg:
     maxItems: 1
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 09/10] arm64: dts: actions: Add MMC controller support for S700
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, manivannan.sadhasivam, robh+dt
  Cc: cristian.ciocaltea, linux-kernel, linux-arm-kernel, linux-actions,
	devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

This commits adds support for MMC controllers present on Actions S700 SoC,
there are 3 MMC controllers in this SoC which can be used for accessing
SD/EMMC/SDIO cards.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* No change.
Changes since v1:
        * Added SoC specific compatibe string.
Changes since RFC:
        * No change
---
 arch/arm64/boot/dts/actions/s700.dtsi | 33 +++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index 605594dd7a0e..b1a34f95d44c 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -259,5 +259,38 @@
 			clocks = <&cmu CLK_DMAC>;
 			power-domains = <&sps S700_PD_DMA>;
 		};
+
+		mmc0: mmc@e0210000 {
+			compatible = "actions,s700-mmc", "actions,owl-mmc";
+			reg = <0x0 0xe0210000 0x0 0x4000>;
+			interrupts = <GIC_SPI 42 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cmu CLK_SD0>;
+			resets = <&cmu RESET_SD0>;
+			dmas = <&dma 2>;
+			dma-names = "mmc";
+			status = "disabled";
+		};
+
+		mmc1: mmc@e0214000 {
+			compatible = "actions,s700-mmc", "actions,owl-mmc";
+			reg = <0x0 0xe0214000 0x0 0x4000>;
+			interrupts = <GIC_SPI 43 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cmu CLK_SD1>;
+			resets = <&cmu RESET_SD1>;
+			dmas = <&dma 3>;
+			dma-names = "mmc";
+			status = "disabled";
+		};
+
+		mmc2: mmc@e0218000 {
+			compatible = "actions,s700-mmc", "actions,owl-mmc";
+			reg = <0x0 0xe0218000 0x0 0x4000>;
+			interrupts = <GIC_SPI 44 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cmu CLK_SD2>;
+			resets = <&cmu RESET_SD2>;
+			dmas = <&dma 4>;
+			dma-names = "mmc";
+			status = "disabled";
+		};
 	};
 };
-- 
2.7.4


^ permalink raw reply related

* [PATCH v3 10/10] arm64: dts: actions: Add uSD support for Cubieboard7
From: Amit Singh Tomar @ 2020-06-02 17:33 UTC (permalink / raw)
  To: andre.przywara, afaerber, manivannan.sadhasivam, robh+dt
  Cc: cristian.ciocaltea, linux-kernel, linux-arm-kernel, linux-actions,
	devicetree
In-Reply-To: <1591119192-18538-1-git-send-email-amittomer25@gmail.com>

This commit adds uSD support for Cubieboard7 board based on Actions Semi
S700 SoC. SD0 is connected to uSD slot. Since there is no PMIC support
added yet, fixed regulator has been used as a regulator node.

Signed-off-by: Amit Singh Tomar <amittomer25@gmail.com>
---
Changes since v2:
	* No change.
Changes since v1:
        * No change.
Changes since RFC:
        * No change.
---
 arch/arm64/boot/dts/actions/s700-cubieboard7.dts | 41 ++++++++++++++++++++++++
 arch/arm64/boot/dts/actions/s700.dtsi            |  1 +
 2 files changed, 42 insertions(+)

diff --git a/arch/arm64/boot/dts/actions/s700-cubieboard7.dts b/arch/arm64/boot/dts/actions/s700-cubieboard7.dts
index 63e375cd9eb4..ec117eb12f3a 100644
--- a/arch/arm64/boot/dts/actions/s700-cubieboard7.dts
+++ b/arch/arm64/boot/dts/actions/s700-cubieboard7.dts
@@ -13,6 +13,7 @@
 
 	aliases {
 		serial3 = &uart3;
+		mmc0 = &mmc0;
 	};
 
 	chosen {
@@ -28,6 +29,23 @@
 		device_type = "memory";
 		reg = <0x1 0xe0000000 0x0 0x0>;
 	};
+
+	/* Fixed regulator used in the absence of PMIC */
+	vcc_3v1: vcc-3v1 {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.1V";
+		regulator-min-microvolt = <3100000>;
+		regulator-max-microvolt = <3100000>;
+	};
+
+	/* Fixed regulator used in the absence of PMIC */
+	sd_vcc: sd-vcc {
+		compatible = "regulator-fixed";
+		regulator-name = "fixed-3.1V";
+		regulator-min-microvolt = <3100000>;
+		regulator-max-microvolt = <3100000>;
+		regulator-always-on;
+	};
 };
 
 &i2c0 {
@@ -81,6 +99,14 @@
 			bias-pull-up;
 		};
 	};
+
+	mmc0_default: mmc0_default {
+		pinmux {
+			groups = "sd0_d0_mfp", "sd0_d1_mfp", "sd0_d2_d3_mfp",
+				 "sd0_cmd_mfp", "sd0_clk_mfp";
+			function = "sd0";
+		};
+	};
 };
 
 &timer {
@@ -90,3 +116,18 @@
 &uart3 {
 	status = "okay";
 };
+
+/* uSD */
+&mmc0 {
+	status = "okay";
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc0_default>;
+	cd-gpios = <&pinctrl 120 GPIO_ACTIVE_LOW>;
+	no-sdio;
+	no-mmc;
+	no-1-8-v;
+	bus-width = <4>;
+	vmmc-supply = <&sd_vcc>;
+	vqmmc-supply = <&sd_vcc>;
+};
+
diff --git a/arch/arm64/boot/dts/actions/s700.dtsi b/arch/arm64/boot/dts/actions/s700.dtsi
index b1a34f95d44c..2bb29bc683ef 100644
--- a/arch/arm64/boot/dts/actions/s700.dtsi
+++ b/arch/arm64/boot/dts/actions/s700.dtsi
@@ -4,6 +4,7 @@
  */
 
 #include <dt-bindings/clock/actions,s700-cmu.h>
+#include <dt-bindings/gpio/gpio.h>
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/reset/actions,s700-reset.h>
 #include <dt-bindings/power/owl-s700-powergate.h>
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v3 2/4] iio: chemical: scd30: add I2C interface driver
From: Tomasz Duszynski @ 2020-06-02 17:48 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Tomasz Duszynski, linux-iio, Linux Kernel Mailing List,
	devicetree, Rob Herring, Jonathan Cameron, Peter Meerwald
In-Reply-To: <CAHp75Vc60q1PC9j6KR1-OJHxw=nBAHt9zJK=h9f27yJxMHpb8A@mail.gmail.com>

Hello Andy,

On Tue, Jun 02, 2020 at 08:14:13PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
> <tomasz.duszynski@octakon.com> wrote:
> >
> > Add I2C interface driver for the SCD30 sensor.
>
> ...
>
> > +static u16 scd30_i2c_cmd_lookup_tbl[] = {
> > +       [CMD_START_MEAS] = 0x0010,
> > +       [CMD_STOP_MEAS] = 0x0104,
> > +       [CMD_MEAS_INTERVAL] = 0x4600,
> > +       [CMD_MEAS_READY] = 0x0202,
> > +       [CMD_READ_MEAS] = 0x0300,
> > +       [CMD_ASC] = 0x5306,
> > +       [CMD_FRC] = 0x5204,
> > +       [CMD_TEMP_OFFSET] = 0x5403,
> > +       [CMD_FW_VERSION] = 0xd100,
> > +       [CMD_RESET] = 0xd304,
>
> Keep sorted by value?
>

I'd rather leave it as is simply because order here matches order in
sensor datasheet.

> > +};
>
> ...
>
> > +       ret = i2c_master_send(client, txbuf, txsize);
>
> > +       if (ret != txsize)
> > +               return ret < 0 ? ret : -EIO;
>
> Wouldn't be better
>
>   if (ret < 0)
>     return ret;
>   if (ret != txsize)
>     return -EIO;
>
> ?
>

Hmm, okay. Perhaps slightly easier to read.

> > +       if (!rxbuf)
> > +               return 0;
> > +
> > +       ret = i2c_master_recv(client, rxbuf, rxsize);
>
> > +       if (ret != rxsize)
> > +               return ret < 0 ? ret : -EIO;
>
> Ditto.
>
> ...
>
> > +static int scd30_i2c_command(struct scd30_state *state, enum scd30_cmd cmd,
> > +                            u16 arg, void *response, int size)
> > +{
> > +       char crc, buf[SCD30_I2C_MAX_BUF_SIZE], *rsp = response;
> > +       int i, ret;
>
> i -> offset ?
>

'i' is shorter and I am lazy :).

> > +       put_unaligned_be16(scd30_i2c_cmd_lookup_tbl[cmd], buf);
> > +       i = 2;
> > +
> > +       if (rsp) {
> > +               /* each two bytes are followed by a crc8 */
> > +               size += size / 2;
> > +       } else {
> > +               put_unaligned_be16(arg, buf + i);
> > +               crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
> > +               i += 2;
>
> > +               buf[i] = crc;
> > +               i += 1;
>
> buf[offset++] = crc; ?
>

I'd rather stick to what I have now. It looks more consistent.

> > +               /* commands below don't take an argument */
> > +               if ((cmd == CMD_STOP_MEAS) || (cmd == CMD_RESET))
> > +                       i -= 3;
> > +       }
> > +
> > +       ret = scd30_i2c_xfer(state, buf, i, buf, size);
> > +       if (ret)
> > +               return ret;
> > +
> > +       /* validate received data and strip off crc bytes */
> > +       for (i = 0; i < size; i += 3) {
> > +               crc = crc8(scd30_i2c_crc8_tbl, buf + i, 2, CRC8_INIT_VALUE);
> > +               if (crc != buf[i + 2]) {
> > +                       dev_err(state->dev, "data integrity check failed\n");
> > +                       return -EIO;
> > +               }
> > +
>
> > +               *rsp++ = buf[i];
>
> + 0 (for the sake of consistency?
>

Adding 0 is a little bit odd.

> > +               *rsp++ = buf[i + 1];
> > +       }
> > +
> > +       return 0;
> > +}
>
> ...
>
> > +static struct i2c_driver scd30_i2c_driver = {
> > +       .driver = {
>
> > +               .name = KBUILD_MODNAME,
>
> Better to hard code.
>

I seriously doubt anyone will ever want to change module name. What for?

> > +               .of_match_table = scd30_i2c_of_match,
> > +               .pm = &scd30_pm_ops,
> > +       },
> > +       .probe_new = scd30_i2c_probe,
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v3 3/4] iio: chemical: scd30: add serial interface driver
From: Tomasz Duszynski @ 2020-06-02 17:57 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Tomasz Duszynski, linux-iio, Linux Kernel Mailing List,
	devicetree, Rob Herring, Jonathan Cameron, Peter Meerwald
In-Reply-To: <CAHp75VfjWG_3XC5FJoaU7XXJc+04JTbEKdjZK=g6ffBPvJNhxA@mail.gmail.com>

On Tue, Jun 02, 2020 at 08:04:16PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
> <tomasz.duszynski@octakon.com> wrote:
> >
> > Add serial interface driver for the SCD30 sensor.
>
> ...
>
> > +static u16 scd30_serdev_cmd_lookup_tbl[] = {
> > +       [CMD_START_MEAS] = 0x0036,
> > +       [CMD_STOP_MEAS] = 0x0037,
> > +       [CMD_MEAS_INTERVAL] = 0x0025,
> > +       [CMD_MEAS_READY] = 0x0027,
> > +       [CMD_READ_MEAS] = 0x0028,
> > +       [CMD_ASC] = 0x003a,
> > +       [CMD_FRC] = 0x0039,
> > +       [CMD_TEMP_OFFSET] = 0x003b,
> > +       [CMD_FW_VERSION] = 0x0020,
> > +       [CMD_RESET] = 0x0034,
>
> Hmm... Can't we keep them ordered by value?
>
> > +};
>
> ...
>
> > +       ret = wait_for_completion_interruptible_timeout(&priv->meas_ready,
> > +                                                       SCD30_SERDEV_TIMEOUT);
> > +       if (ret > 0)
> > +               ret = 0;
> > +       else if (!ret)
> > +               ret = -ETIMEDOUT;
> > +
> > +       return ret;
>
> Perhaps
>
>   if (ret < 0)
>     return ret;
>   if (ret == 0)
>     return -ETIMEDOUT;
>   return 0;
>
> ?

Matter of style but since I will be doing other changes I can touch that
too.

>
> ...
>
> > +       char txbuf[SCD30_SERDEV_MAX_BUF_SIZE] = { SCD30_SERDEV_ADDR },
> > +            rxbuf[SCD30_SERDEV_MAX_BUF_SIZE], *rsp = response;
>
> Please, apply type to each variable separately.
>

Fine.

> ...
>
> > +       switch (txbuf[1]) {
> > +       case SCD30_SERDEV_WRITE:
>
> > +               if (memcmp(txbuf, txbuf, txsize)) {
>
> I'm not sure I understand this.
>

Ah, thanks for catching this. tx should be compared to rx.

> > +                       dev_err(state->dev, "wrong message received\n");
> > +                       return -EIO;
> > +               }
> > +               break;
> > +       case SCD30_SERDEV_READ:
>
> > +               if (rxbuf[2] != (rxsize -
> > +                                SCD30_SERDEV_RX_HEADER_SIZE -
> > +                                SCD30_SERDEV_CRC_SIZE)) {
>
> Perhaps you can come up with better indentation/ line split?
>

I'd rather leave it as is.

> > +                       dev_err(state->dev,
> > +                               "received data size does not match header\n");
> > +                       return -EIO;
> > +               }
> > +
> > +               rxsize -= SCD30_SERDEV_CRC_SIZE;
> > +               crc = get_unaligned_le16(rxbuf + rxsize);
> > +               if (crc != scd30_serdev_calc_crc(rxbuf, rxsize)) {
> > +                       dev_err(state->dev, "data integrity check failed\n");
> > +                       return -EIO;
> > +               }
> > +
> > +               rxsize -= SCD30_SERDEV_RX_HEADER_SIZE;
> > +               memcpy(rsp, rxbuf + SCD30_SERDEV_RX_HEADER_SIZE, rxsize);
> > +               break;
> > +       default:
> > +               dev_err(state->dev, "received unknown op code\n");
> > +               return -EIO;
> > +       }
> > +
> > +       return 0;
> > +}
>
> ...
>
> > +static struct serdev_device_driver scd30_serdev_driver = {
> > +       .driver = {
>
> > +               .name = KBUILD_MODNAME,
>
> This is not the best what we can do. The name is an ABI and better if
> it will be constant (independent on file name).
>
> > +               .of_match_table = scd30_serdev_of_match,
> > +               .pm = &scd30_pm_ops,
> > +       },
> > +       .probe = scd30_serdev_probe,
> > +};
>
> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply

* Re: [PATCHv1 00/19] Improve SBS battery support
From: Sebastian Reichel @ 2020-06-02 18:01 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Rob Herring, Greg Kroah-Hartman, Rafael J . Wysocki, linux-pm,
	devicetree, linux-kernel, kernel, 'Linux Samsung SOC'
In-Reply-To: <b3fd35de-1dd6-1ddc-7e57-2d9ab2860e81@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 5570 bytes --]

Hi,

On Tue, Jun 02, 2020 at 09:17:09AM +0200, Marek Szyprowski wrote:
> Hi Sebastian,
> 
> On 01.06.2020 19:05, Sebastian Reichel wrote:
> > On Mon, Jun 01, 2020 at 12:40:27PM +0200, Marek Szyprowski wrote:
> >> On 13.05.2020 20:55, Sebastian Reichel wrote:
> >>> This patchset improves support for SBS compliant batteries. Due to
> >>> the changes, the battery now exposes 32 power supply properties and
> >>> (un)plugging it generates a backtrace containing the following message
> >>> without the first patch in this series:
> >>>
> >>> ---------------------------
> >>> WARNING: CPU: 0 PID: 20 at lib/kobject_uevent.c:659 add_uevent_var+0xd4/0x104
> >>> add_uevent_var: too many keys
> >>> ---------------------------
> >>>
> >>> For references this is what an SBS battery status looks like after
> >>> the patch series has been applied:
> >>>
> >>> cat /sys/class/power_supply/sbs-0-000b/uevent
> >>> POWER_SUPPLY_NAME=sbs-0-000b
> >>> POWER_SUPPLY_TYPE=Battery
> >>> POWER_SUPPLY_STATUS=Discharging
> >>> POWER_SUPPLY_CAPACITY_LEVEL=Normal
> >>> POWER_SUPPLY_HEALTH=Good
> >>> POWER_SUPPLY_PRESENT=1
> >>> POWER_SUPPLY_TECHNOLOGY=Li-ion
> >>> POWER_SUPPLY_CYCLE_COUNT=12
> >>> POWER_SUPPLY_VOLTAGE_NOW=11441000
> >>> POWER_SUPPLY_CURRENT_NOW=-26000
> >>> POWER_SUPPLY_CURRENT_AVG=-24000
> >>> POWER_SUPPLY_CAPACITY=76
> >>> POWER_SUPPLY_CAPACITY_ERROR_MARGIN=1
> >>> POWER_SUPPLY_TEMP=198
> >>> POWER_SUPPLY_TIME_TO_EMPTY_AVG=438600
> >>> POWER_SUPPLY_TIME_TO_FULL_AVG=3932100
> >>> POWER_SUPPLY_SERIAL_NUMBER=0000
> >>> POWER_SUPPLY_VOLTAGE_MIN_DESIGN=10800000
> >>> POWER_SUPPLY_VOLTAGE_MAX_DESIGN=10800000
> >>> POWER_SUPPLY_ENERGY_NOW=31090000
> >>> POWER_SUPPLY_ENERGY_FULL=42450000
> >>> POWER_SUPPLY_ENERGY_FULL_DESIGN=41040000
> >>> POWER_SUPPLY_CHARGE_NOW=2924000
> >>> POWER_SUPPLY_CHARGE_FULL=3898000
> >>> POWER_SUPPLY_CHARGE_FULL_DESIGN=3800000
> >>> POWER_SUPPLY_CONSTANT_CHARGE_CURRENT_MAX=3000000
> >>> POWER_SUPPLY_CONSTANT_CHARGE_VOLTAGE_MAX=12300000
> >>> POWER_SUPPLY_MANUFACTURE_YEAR=2017
> >>> POWER_SUPPLY_MANUFACTURE_MONTH=7
> >>> POWER_SUPPLY_MANUFACTURE_DAY=3
> >>> POWER_SUPPLY_MANUFACTURER=UR18650A
> >>> POWER_SUPPLY_MODEL_NAME=GEHC
> >> This patch landed in linux-next dated 20200529. Sadly it causes a
> >> regression on Samsung Exynos-based Chromebooks (Exynos5250 Snow,
> >> Exynos5420 Peach-Pi and Exynos5800 Peach-Pit). System boots to
> >> userspace, but then, when udev populates /dev, booting hangs:
> >>
> >> [    4.435167] VFS: Mounted root (ext4 filesystem) readonly on device
> >> 179:51.
> >> [    4.457477] devtmpfs: mounted
> >> [    4.460235] Freeing unused kernel memory: 1024K
> >> [    4.464022] Run /sbin/init as init process
> >> INIT: version 2.88 booting
> >> [info] Using makefile-style concurrent boot in runlevel S.
> >> [    5.102096] random: crng init done
> >> [....] Starting the hotplug events dispatcher: systemd-udevdstarting
> >> version 236
> >> [ ok .
> >> [....] Synthesizing the initial hotplug events...[ ok done.
> >> [....] Waiting for /dev to be fully populated...[   34.409914]
> >> TPS65090_RAILSDCDC1: disabling
> >> [   34.412977] TPS65090_RAILSDCDC2: disabling
> >> [   34.417021] TPS65090_RAILSDCDC3: disabling
> >> [   34.423848] TPS65090_RAILSLDO1: disabling
> >> [   34.429068] TPS65090_RAILSLDO2: disabling
> > :(
> >
> > log does not look useful either.
> >
> >> Bisect between v5.7-rc1 and next-20200529 pointed me to the first bad
> >> commit: [c4b12a2f3f3de670f6be5e96092a2cab0b877f1a] power: supply:
> >> sbs-battery: simplify read_read_string_data.
> > ok. I tested this on an to-be-upstreamed i.MX6 based system
> > and arch/arm/boot/dts/imx53-ppd.dts. I think the difference
> > is, that i2c-exynos5 does not expose I2C_FUNC_SMBUS_READ_BLOCK_DATA.
> > I hoped all systems using SBS battery support this, but now
> > I see I2C_FUNC_SMBUS_EMUL only supports writing block data.
> > Looks like I need to add another patch implementing that
> > using the old code with added PEC support.
> >
> > In any case that should only return -ENODEV for the property
> > (and uevent), but not break boot. So something fishy is going
> > on.
> >
> >> However reverting it in linux-next doesn't fix the issue, so the
> >> next commits are also relevant to this issue.
> > The next patch, which adds PEC support depends on the simplification
> > of sbs_read_string_data. The old, open coded variant will result in
> > PEC failure for string properties (which should not stop boot either
> > of course). Can you try reverting both?
> Indeed, reverting both (and fixing the conflict) restores proper boot.

Ok, I pushed out a revert of those two patches. They should land in
tomorrows linux-next release. Please test it.

> > If that helps I will revert those two instead of dropping the whole
> > series for this merge window.
> >
> >> Let me know how can I help debugging it.
> > I suspect, that this is userspace endlessly retrying reading the
> > battery uevent when an error is returned. Could you check this?
> > Should be easy to see by adding some printfs.
> I've added some debug messages in sbs_get_property() and it read the 
> same properties many times. However I've noticed that if I wait long 
> enough booting finally continues.

So basically userspace slows down itself massively by trying to
re-read uevent over and over when an error occurs. Does not seem
like a sensible thing to do. I will have a look at this when I find
some time.

-- Sebastian

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 0/4] Add support for SCD30 sensor
From: Tomasz Duszynski @ 2020-06-02 18:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Tomasz Duszynski, linux-iio, Linux Kernel Mailing List,
	devicetree, Rob Herring, Jonathan Cameron, Peter Meerwald
In-Reply-To: <CAHp75VdPF=LzDn0f_Ljb=S+8L58DS4ofC6fD=Nzu1afR_nD8vQ@mail.gmail.com>

On Tue, Jun 02, 2020 at 07:55:55PM +0300, Andy Shevchenko wrote:
> On Tue, Jun 2, 2020 at 7:49 PM Tomasz Duszynski
> <tomasz.duszynski@octakon.com> wrote:
> >
> > Following series adds support for Sensirion SCD30 sensor module capable of
> > measuring carbon dioxide, temperature and relative humidity. CO2 measurements
> > base on NDIR principle while temperature and relative humidity are measured by
> > the on board SHT31. As for sensor communication, both I2C and serial interfaces
> > are supported.
>
> Btw, since we have relaxed 80 limit to 100, I recommend to reconsider
> some lines to be joined.
>

Heh, that particular change is becoming more viral that corona itself these days.

> --
> With Best Regards,
> Andy Shevchenko

^ permalink raw reply

* Re: Fwd: [PATCH v3 2/2] media: cec: i2c: ch7322: Add ch7322 CEC controller driver
From: Jeff Chase @ 2020-06-02 18:17 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, mchehab, robh+dt, devicetree
In-Reply-To: <d7e759d1-9bdf-0ab3-143a-f0e374667f04@xs4all.nl>

> > I haven't looked into the cec notifier mechanism yet but would it be
> > better to try to use that instead if possible and just ignore this
> > device's physical address detection? Then I could do more of a proper
> > reset in this enable op. But I'm not sure if I can properly associate
> > the device with an HDMI port on my platform unless I make some changes
> > to coreboot.
>
> I don't think this is useful: it's nice to have the CEC adapter do the
> physical address detection.
>
> One question about that though: if there is no physical address, can this
> driver still transmit CEC messages? Specifically Image View On. This is
> important to wake up displays that pull down the HPD when in standby.

Yes it can. I just verified this with the kwikwai analyzer.

Thanks,
Jeff

^ permalink raw reply

* Re: [PATCH] ARM: dts: AM33xx-l4: add gpio-ranges
From: Drew Fustini @ 2020-06-02 18:24 UTC (permalink / raw)
  To: Grygorii Strashko
  Cc: Tony Lindgren, linux-omap, linux-kernel, Benoît Cousson,
	Rob Herring, devicetree, Santosh Shilimkar, Suman Anna,
	Haojian Zhuang, Linus Walleij, linux-gpio, jkridner,
	robertcnelson
In-Reply-To: <803e2d78-28ba-0816-dbb5-d441d7659a91@ti.com>

On Tue, Jun 02, 2020 at 04:44:03PM +0300, Grygorii Strashko wrote:
> 
> 
> On 02/06/2020 16:14, Drew Fustini wrote:
> > Add gpio-ranges properties to the gpio controller nodes.
> > 
> > These gpio-ranges were created based on "Table 9-10. CONTROL_MODULE
> > REGISTERS" in the  "AM335x Technical Reference Manual" [0] and "Table
> > 4-2. Pin Attributes" in the "AM335x Sitara Processor datasheet" [1].
> > A csv file with this data is available for reference [2].
> 
> It will be good if you can explain not only "what" is changed, but
> also "why" it's needed in commit message.

That is a good point.

One reason for this patch is I think it makes sense to add gpio-ranges
properties as they describe the relationship between a gpio line and
pin control register that exists in the hardware.  For example, GPMC_A0
pin has mode 7 which is labeled gpio1_16. The conf_gpmc_a0 register is
at offset 840h which makes it pin 16.

The other reason is that I would like to patch omap_gpio_set_config()
to call gpiochip_generic_config() for PIN_CONFIG_PULL_{UP,DOWN}. It
currently fails at pinctrl_get_device_gpio_range():

  omap_gpio_set_config()  
  |- gpiochip_generic_config()
     |- pinctrl_gpio_set_config()
        |- gpio_to_pin()
           |- pinctrl_get_device_gpio_range() [no gpio-ranges so fails]


Thank you,
Drew

^ permalink raw reply

* [PATCH v2 1/1] dt-bindings: MIPS: Document Ingenic SoCs binding.
From: 周琰杰 (Zhou Yanjie) @ 2020-06-02 18:33 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, devicetree, tsbogend, robh+dt, paul, dongsheng.qiu,
	aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin
In-Reply-To: <20200602183354.39707-1-zhouyanjie@wanyeetech.com>

Document the available properties for the SoC root node and the
CPU nodes of the devicetree for the Ingenic XBurst SoCs.

Tested-by: H. Nikolaus Schaller <hns@goldelico.com>
Tested-by: Paul Boddie <paul@boddie.org.uk>
Signed-off-by: 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
---

Notes:
    v1->v2:
    1.Remove unnecessary "items".
    2.Add "clocks" as suggested by Paul Cercueil.

 .../bindings/mips/ingenic/ingenic,cpu.yaml         | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

diff --git a/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
new file mode 100644
index 000000000000..171503e08505
--- /dev/null
+++ b/Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml
@@ -0,0 +1,67 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mips/ingenic/ingenic,cpu.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bindings for Ingenic XBurst family CPUs
+
+maintainers:
+  - 周琰杰 (Zhou Yanjie) <zhouyanjie@wanyeetech.com>
+
+description:
+  Ingenic XBurst family CPUs shall have the following properties.
+
+properties:
+  compatible:
+    oneOf:
+
+      - description: Ingenic XBurst®1 CPU Cores
+        enum:
+          - ingenic,xburst-mxu1.0
+          - ingenic,xburst-fpu1.0-mxu1.1
+          - ingenic,xburst-fpu2.0-mxu2.0
+
+      - description: Ingenic XBurst®2 CPU Cores
+        enum:
+          - ingenic,xburst2-fpu2.1-mxu2.1-smt
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - device_type
+  - compatible
+  - reg
+  - clocks
+
+examples:
+  - |
+    #include <dt-bindings/clock/jz4780-cgu.h>
+
+    cpus {
+    	#address-cells = <1>;
+    	#size-cells = <0>;
+
+    	cpu0: cpu@0 {
+    		device_type = "cpu";
+    		compatible = "ingenic,xburst-fpu1.0-mxu1.1";
+    		reg = <0>;
+
+    		clocks = <&cgu JZ4780_CLK_CPU>;
+    		clock-names = "cpu";
+    	};
+
+    	cpu1: cpu@1 {
+    		device_type = "cpu";
+    		compatible = "ingenic,xburst-fpu1.0-mxu1.1";
+    		reg = <1>;
+
+    		clocks = <&cgu JZ4780_CLK_CORE1>;
+    		clock-names = "cpu";
+    	};
+    };
+...
-- 
2.11.0


^ permalink raw reply related

* [PATCH v2 0/1] Document Ingenic SoCs binding.
From: 周琰杰 (Zhou Yanjie) @ 2020-06-02 18:33 UTC (permalink / raw)
  To: linux-mips
  Cc: linux-kernel, devicetree, tsbogend, robh+dt, paul, dongsheng.qiu,
	aric.pzqi, rick.tyliu, yanfei.li, sernia.zhou, zhenwenjin

Document the available properties for the SoC root node and the
CPU nodes of the devicetree for the Ingenic XBurst SoCs.

v1->v2:
1.Remove unnecessary "items".
2.Add "clocks" as suggested by Paul Cercueil.

周琰杰 (Zhou Yanjie) (1):
  dt-bindings: MIPS: Document Ingenic SoCs binding.

 .../bindings/mips/ingenic/ingenic,cpu.yaml         | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mips/ingenic/ingenic,cpu.yaml

-- 
2.11.0


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox