Devicetree
 help / color / mirror / Atom feed
* Re: [PATCH] ARM: dts: am572x-idk: Add gpios property to control PCIE_RESETn
From: Tony Lindgren @ 2016-12-30 19:00 UTC (permalink / raw)
  To: Kishon Vijay Abraham I
  Cc: Benoît Cousson, linux-omap-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
	Russell King, nsekhar-l0cyMroinI0
In-Reply-To: <1483085240-16102-1-git-send-email-kishon-l0cyMroinI0@public.gmane.org>

* Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org> [161230 00:08]:
> Add 'gpios' property to pcie1 dt node and populate it with
> GPIO3_23 in order to drive PCIE_RESETn high.
> 
> This gets PCIe cards to be detected in AM572X IDK board.
> 
> Signed-off-by: Kishon Vijay Abraham I <kishon-l0cyMroinI0@public.gmane.org>
> ---
>  arch/arm/boot/dts/am572x-idk.dts |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/am572x-idk.dts b/arch/arm/boot/dts/am572x-idk.dts
> index 27d9149..1540f7a 100644
> --- a/arch/arm/boot/dts/am572x-idk.dts
> +++ b/arch/arm/boot/dts/am572x-idk.dts
> @@ -87,3 +87,7 @@
>  &sn65hvs882 {
>  	load-gpios = <&gpio3 19 GPIO_ACTIVE_LOW>;
>  };
> +
> +&pcie1 {
> +	gpios = <&gpio3 23 GPIO_ACTIVE_HIGH>;
> +};

I'll apply this into omap-for-v4.10/fixes thanks.

Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] iio: adc: Add Renesas GyroADC driver
From: Marek Vasut @ 2016-12-30 19:18 UTC (permalink / raw)
  To: linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Marek Vasut,
	Geert Uytterhoeven, Simon Horman

Add IIO driver for the Renesas RCar GyroADC block. This block is a
simple 4/8-channel ADC which samples 12/15/24 bits of data every
cycle from all channels.

Signed-off-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Cc: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
---
 .../bindings/iio/adc/renesas,gyroadc.txt           |  38 +++
 MAINTAINERS                                        |   6 +
 drivers/iio/adc/Kconfig                            |  10 +
 drivers/iio/adc/Makefile                           |   1 +
 drivers/iio/adc/rcar_gyro_adc.c                    | 379 +++++++++++++++++++++
 5 files changed, 434 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
 create mode 100644 drivers/iio/adc/rcar_gyro_adc.c

diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
new file mode 100644
index 0000000..3fd5f57
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
@@ -0,0 +1,38 @@
+* Renesas RCar GyroADC device driver
+
+Required properties:
+- compatible:	Should be "renesas,rcar-gyroadc" for regular GyroADC or
+		"renesas,rcar-gyroadc-r8a7792" for GyroADC without interrupt
+		block found in R8A7792.
+- reg:		Address and length of the register set for the device
+- clocks:	References to all the clocks specified in the clock-names
+		property as specified in
+		Documentation/devicetree/bindings/clock/clock-bindings.txt.
+- clock-names:	Shall contain "fck" and "if". The "fck" are the GyroADC block
+		clock, the "if" are the interface clock.
+		power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
+- power-domains: Must contain a reference to the PM domain, if available.
+- renesas,gyroadc-mode:	GyroADC mode of operation, must be either of:
+			1 - MB88101A mode, 12bit sampling, 4 channels
+			2 - ADCS7476 mode, 15bit sampling, 8 channels
+			3 - MAX1162 mode,  16bit sampling, 8 channels
+- renesas,gyroadc-vref:	Array of reference voltage values for each input to
+			the GyroADC, in uV. Array must have 4 elemenets for
+			mode 1 and 8 elements for mode 2 and 3.
+
+Example:
+	&adc {
+		compatible = "renesas,rcar-gyroadc";
+		reg = <0 0xe6e54000 0 64>;
+		clocks = <&mstp9_clks R8A7791_CLK_GYROADC>, <&clk_65m>;
+		clock-names = "fck", "if";
+		power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
+
+		pinctrl-0 = <&adc_pins>;
+		pinctrl-names = "default";
+
+		renesas,gyroadc-vref = <4096000 4096000 4096000 4096000
+					4096000 4096000 4096000 4096000>;
+		renesas,gyroadc-mode = <3>;
+		status = "okay";
+	};
diff --git a/MAINTAINERS b/MAINTAINERS
index 162d904..751e760 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10271,6 +10271,12 @@ L:	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
 F:	drivers/net/ethernet/renesas/
 F:	include/linux/sh_eth.h
 
+RENESAS RCAR GYROADC DRIVER
+M:	Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+S:	Supported
+F:	drivers/iio/adc/rcar_gyro_adc.c
+
 RENESAS USB2 PHY DRIVER
 M:	Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
 L:	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 99c0514..4a4cac7 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -408,6 +408,16 @@ config QCOM_SPMI_VADC
 	  To compile this driver as a module, choose M here: the module will
 	  be called qcom-spmi-vadc.
 
+config RCAR_GYRO_ADC
+	tristate "Renesas RCAR GyroADC driver"
+	depends on ARCH_RCAR_GEN2 || (ARM && COMPILE_TEST)
+	help
+	  Say yes here to build support for the GyroADC found in Renesas
+	  RCar Gen2 SoCs.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called rcar-gyroadc.
+
 config ROCKCHIP_SARADC
 	tristate "Rockchip SARADC driver"
 	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 7a40c04..253aeb2 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_NAU7802) += nau7802.o
 obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
 obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
 obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
+obj-$(CONFIG_RCAR_GYRO_ADC) += rcar_gyro_adc.o
 obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
 obj-$(CONFIG_STX104) += stx104.o
 obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
diff --git a/drivers/iio/adc/rcar_gyro_adc.c b/drivers/iio/adc/rcar_gyro_adc.c
new file mode 100644
index 0000000..a74b148
--- /dev/null
+++ b/drivers/iio/adc/rcar_gyro_adc.c
@@ -0,0 +1,379 @@
+/*
+ * Renesas RCar GyroADC driver
+ *
+ * Copyright 2016 Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/of.h>
+#include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_platform.h>
+#include <linux/err.h>
+
+#include <linux/iio/iio.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/sysfs.h>
+#include <linux/iio/trigger.h>
+
+/* GyroADC registers. */
+#define RCAR_GYROADC_MODE_SELECT		0x00
+#define RCAR_GYROADC_MODE_SELECT_1_MB88101A	0x0
+#define RCAR_GYROADC_MODE_SELECT_2_ADCS7476	0x1
+#define RCAR_GYROADC_MODE_SELECT_3_MAX1162	0x3
+
+#define RCAR_GYROADC_START_STOP			0x04
+#define RCAR_GYROADC_START_STOP_START		BIT(0)
+
+#define RCAR_GYROADC_CLOCK_LENGTH		0x08
+#define RCAR_GYROADC_1_25MS_LENGTH		0x0c
+
+#define RCAR_GYROADC_REALTIME_DATA(ch)		(0x10 + ((ch) * 4))
+#define RCAR_GYROADC_100MS_ADDED_DATA(ch)	(0x30 + ((ch) * 4))
+#define RCAR_GYROADC_10MS_AVG_DATA(ch)		(0x50 + ((ch) * 4))
+
+#define RCAR_GYROADC_FIFO_STATUS		0x70
+#define RCAR_GYROADC_FIFO_STATUS_EMPTY(ch)	BIT(0 + (4 * (ch)))
+#define RCAR_GYROADC_FIFO_STATUS_FULL(ch)	BIT(1 + (4 * (ch)))
+#define RCAR_GYROADC_FIFO_STATUS_ERROR(ch)	BIT(2 + (4 * (ch)))
+
+#define RCAR_GYROADC_INTR			0x74
+#define RCAR_GYROADC_INTR_INT			BIT(0)
+
+#define RCAR_GYROADC_INTENR			0x78
+#define RCAR_GYROADC_INTENR_INTEN		BIT(0)
+
+#define RCAR_GYROADC_SAMPLE_RATE		800	/* Hz */
+
+enum rcar_gyroadc_model {
+	RCAR_GYROADC_MODEL_DEFAULT,
+	RCAR_GYROADC_MODEL_R8A7792,
+};
+
+struct rcar_gyroadc {
+	struct device		*dev;
+	void __iomem		*regs;
+	struct clk		*fclk;
+	struct clk		*clk;
+	enum rcar_gyroadc_model	model;
+	unsigned int		mode;
+	u32			vref_uv[8];
+	u32			buffer[8];
+};
+
+static void rcar_gyroadc_hw_init(struct rcar_gyroadc *priv)
+{
+	unsigned long clk_mhz = clk_get_rate(priv->clk) / 1000000;
+
+	/* Stop the GyroADC. */
+	writel(0, priv->regs + RCAR_GYROADC_START_STOP);
+
+	/* Disable IRQ, except on V2H. */
+	if (priv->model != RCAR_GYROADC_MODEL_R8A7792)
+		writel(0, priv->regs + RCAR_GYROADC_INTENR);
+
+	/* Set mode and timing. */
+	writel(priv->mode, priv->regs + RCAR_GYROADC_MODE_SELECT);
+
+	if (priv->mode == RCAR_GYROADC_MODE_SELECT_1_MB88101A)
+		writel(clk_mhz * 10, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
+	else if (priv->mode == RCAR_GYROADC_MODE_SELECT_2_ADCS7476)
+		writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
+	else if (priv->mode == RCAR_GYROADC_MODE_SELECT_3_MAX1162)
+		writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
+	writel(clk_mhz * 1250, priv->regs + RCAR_GYROADC_1_25MS_LENGTH);
+
+	/*
+	 * We can possibly turn the sampling on/off on-demand to reduce power
+	 * consumption, but for the sake of quick availability of samples, we
+	 * don't do it now.
+	 */
+	writel(RCAR_GYROADC_START_STOP_START,
+	       priv->regs + RCAR_GYROADC_START_STOP);
+
+	/* Wait for the first conversion to complete. */
+	udelay(1250);
+}
+
+#define RCAR_GYROADC_CHAN(_idx, _chan_type, _realbits) {	\
+	.type			= (_chan_type),			\
+	.indexed		= 1,				\
+	.channel		= (_idx),			\
+	.info_mask_separate	= BIT(IIO_CHAN_INFO_RAW),	\
+	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
+				    BIT(IIO_CHAN_INFO_SAMP_FREQ), \
+	.scan_index		= (_idx),			\
+	.scan_type	= {					\
+		.sign		= 'u',				\
+		.realbits	= (_realbits),			\
+		.storagebits	= 16,				\
+	},							\
+}
+
+static const struct iio_chan_spec rcar_gyroadc_iio_channels_1[] = {
+	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 12),
+	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 12),
+	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 12),
+	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 12),
+};
+
+static const struct iio_chan_spec rcar_gyroadc_iio_channels_2[] = {
+	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 15),
+	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 15),
+};
+
+/*
+ * NOTE: The data we receive in mode 3 from MAX1162 have MSByte = 0,
+ *       therefore we only use 16bit realbits here instead of 24.
+ */
+static const struct iio_chan_spec rcar_gyroadc_iio_channels_3[] = {
+	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 16),
+	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 16),
+};
+
+static int rcar_gyroadc_read_raw(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan,
+				 int *val, int *val2, long mask)
+{
+	struct rcar_gyroadc *priv = iio_priv(indio_dev);
+	unsigned int datareg = RCAR_GYROADC_REALTIME_DATA(chan->channel);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		if (chan->type != IIO_VOLTAGE)
+			return -EINVAL;
+
+		if (iio_buffer_enabled(indio_dev))
+			return -EBUSY;
+
+		*val = readl(priv->regs + datareg);
+		*val &= BIT(chan->scan_type.realbits) - 1;
+
+		return IIO_VAL_INT;
+	case IIO_CHAN_INFO_SCALE:
+		*val = 0;
+		*val2 = (priv->vref_uv[chan->channel] * 1000) / 0x10000;
+		return IIO_VAL_INT_PLUS_NANO;
+	case IIO_CHAN_INFO_SAMP_FREQ:
+		*val = RCAR_GYROADC_SAMPLE_RATE;
+		*val2 = 0;
+		return IIO_VAL_INT;
+	default:
+		break;
+	}
+
+	return -EINVAL;
+}
+
+static int rcar_gyroadc_reg_access(struct iio_dev *indio_dev,
+				   unsigned int reg, unsigned int writeval,
+				   unsigned int *readval)
+{
+	struct rcar_gyroadc *priv = iio_priv(indio_dev);
+	unsigned int maxreg = RCAR_GYROADC_INTENR;
+
+	if (readval == NULL)
+		return -EINVAL;
+
+	if (reg % 4)
+		return -EINVAL;
+
+	/* Handle the V2H case with missing interrupt block. */
+	if (priv->model == RCAR_GYROADC_MODEL_R8A7792)
+		maxreg = RCAR_GYROADC_FIFO_STATUS;
+
+	if (reg > maxreg)
+		return -EINVAL;
+
+	*readval = readl(priv->regs + reg);
+
+	return 0;
+}
+
+static const struct iio_info rcar_gyroadc_iio_info = {
+	.driver_module		= THIS_MODULE,
+	.read_raw		= rcar_gyroadc_read_raw,
+	.debugfs_reg_access	= rcar_gyroadc_reg_access,
+};
+
+static const struct of_device_id rcar_gyroadc_match[] = {
+	{
+		/* RCar Gen2 compatible GyroADC */
+		.compatible	= "renesas,rcar-gyroadc",
+		.data		= (void *)RCAR_GYROADC_MODEL_DEFAULT,
+	}, {
+		/* RCar V2H specialty without interrupt registers. */
+		.compatible	= "renesas,rcar-gyroadc-r8a7792",
+		.data		= (void *)RCAR_GYROADC_MODEL_R8A7792,
+	}, {
+		/* sentinel */
+	}
+};
+
+MODULE_DEVICE_TABLE(of, rcar_gyroadc_match);
+
+static int rcar_gyroadc_probe(struct platform_device *pdev)
+{
+	const struct of_device_id *of_id =
+		of_match_device(rcar_gyroadc_match, &pdev->dev);
+	struct device *dev = &pdev->dev;
+	struct rcar_gyroadc *priv;
+	struct iio_dev *indio_dev;
+	struct resource *mem;
+	int ret, mode;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
+	if (!indio_dev) {
+		dev_err(dev, "Failed to allocate IIO device.\n");
+		return -ENOMEM;
+	}
+
+	priv = iio_priv(indio_dev);
+	priv->dev = dev;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->regs = devm_ioremap_resource(dev, mem);
+	if (IS_ERR(priv->regs))
+		return PTR_ERR(priv->regs);
+
+	priv->fclk = devm_clk_get(dev, "fck");
+	if (IS_ERR(priv->fclk)) {
+		ret = PTR_ERR(priv->fclk);
+		dev_err(dev, "Failed to get FCK clock (ret=%i)\n", ret);
+		return ret;
+	}
+
+	priv->clk = devm_clk_get(dev, "if");
+	if (IS_ERR(priv->clk)) {
+		ret = PTR_ERR(priv->clk);
+		dev_err(dev, "Failed to get IF clock (ret=%i)\n", ret);
+		return ret;
+	}
+
+	ret = of_property_read_u32(pdev->dev.of_node, "renesas,gyroadc-mode",
+				   &mode);
+	if (ret || (mode < 1) || (mode > 3)) {
+		dev_err(dev, "Failed to get GyroADC mode (ret=%i)\n", ret);
+		return ret;
+	}
+
+	if (mode == 1)
+		priv->mode = RCAR_GYROADC_MODE_SELECT_1_MB88101A;
+	else if (mode == 2)
+		priv->mode = RCAR_GYROADC_MODE_SELECT_2_ADCS7476;
+	else if (mode == 3)
+		priv->mode = RCAR_GYROADC_MODE_SELECT_3_MAX1162;
+
+	of_property_read_u32_array(pdev->dev.of_node, "renesas,gyroadc-vref",
+				   priv->vref_uv, (mode == 1) ? 4 : 8);
+
+	priv->model = (enum rcar_gyroadc_model)of_id->data;
+
+	platform_set_drvdata(pdev, indio_dev);
+
+	indio_dev->name = dev_name(dev);
+	indio_dev->dev.parent = dev;
+	indio_dev->dev.of_node = pdev->dev.of_node;
+	indio_dev->info = &rcar_gyroadc_iio_info;
+	indio_dev->modes = INDIO_DIRECT_MODE;
+	if (mode == 1) {
+		indio_dev->channels = rcar_gyroadc_iio_channels_1;
+		indio_dev->num_channels =
+			ARRAY_SIZE(rcar_gyroadc_iio_channels_1);
+	} else if (mode == 2) {
+		indio_dev->channels = rcar_gyroadc_iio_channels_2;
+		indio_dev->num_channels =
+			ARRAY_SIZE(rcar_gyroadc_iio_channels_2);
+	} else if (mode == 3) {
+		indio_dev->channels = rcar_gyroadc_iio_channels_3;
+		indio_dev->num_channels =
+			ARRAY_SIZE(rcar_gyroadc_iio_channels_3);
+	}
+
+	ret = clk_prepare_enable(priv->fclk);
+	if (ret) {
+		dev_err(dev, "Could not prepare or enable the FCK clock.\n");
+		return ret;
+	}
+
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(dev, "Could not prepare or enable the IF clock.\n");
+		goto error_clk_if_enable;
+	}
+
+	rcar_gyroadc_hw_init(priv);
+
+	ret = iio_device_register(indio_dev);
+	if (ret) {
+		dev_err(dev, "Couldn't register IIO device.\n");
+		goto error_iio_device_register;
+	}
+
+	return 0;
+
+error_iio_device_register:
+	clk_disable_unprepare(priv->clk);
+error_clk_if_enable:
+	clk_disable_unprepare(priv->fclk);
+	return ret;
+}
+
+static int rcar_gyroadc_remove(struct platform_device *pdev)
+{
+	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
+	struct rcar_gyroadc *priv = iio_priv(indio_dev);
+
+	/* Stop sampling */
+	writel(0, priv->regs + RCAR_GYROADC_START_STOP);
+
+	iio_device_unregister(indio_dev);
+	clk_disable_unprepare(priv->clk);
+	clk_disable_unprepare(priv->fclk);
+
+	return 0;
+}
+
+static struct platform_driver rcar_gyroadc_driver = {
+	.probe          = rcar_gyroadc_probe,
+	.remove         = rcar_gyroadc_remove,
+	.driver         = {
+		.name		= "rcar-gyroadc",
+		.of_match_table	= rcar_gyroadc_match,
+	},
+};
+
+module_platform_driver(rcar_gyroadc_driver);
+
+MODULE_AUTHOR("Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
+MODULE_DESCRIPTION("Renesas RCAR GyroADC driver");
+MODULE_LICENSE("GPL v2");
-- 
2.10.2

^ permalink raw reply related

* Re: [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
From: kbuild test robot @ 2016-12-30 19:18 UTC (permalink / raw)
  To: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w
  Cc: kbuild-all-JC7UmRfGjtg, linux-0h96xk9xTtrk1uMJSBkQmQ,
	jdelvare-IBi9RG/b67k, corbet-T1hC0tSOHrs,
	mark.rutland-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
	wsa-z923LK4zBo2bacvFa/9K2g, andrew-zrmu5oMJ5Fs,
	joel-U3u1mxZcP9KHXe+LvDLADg, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-hwmon-u79uwXL29TY76Z2rM5mHXA,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Edward A. James
In-Reply-To: <1483120568-21082-2-git-send-email-eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

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

Hi Edward,

[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on v4.10-rc1 next-20161224]
[cannot apply to linux/master]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/eajames-ibm-gmail-com/drivers-hwmon-Add-On-Chip-Controller-driver/20161231-021324
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 6.2.0
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=ia64 

All warnings (new ones prefixed by >>):

   drivers/hwmon/occ/occ.c: In function 'occ_get_all':
>> drivers/hwmon/occ/occ.c:390:44: warning: passing argument 5 of 'occ_send_cmd' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
     rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
                                               ^
   drivers/hwmon/occ/occ.c:281:11: note: expected 'u8 * {aka unsigned char *}' but argument is of type 'const u8 * {aka const unsigned char *}'
    static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length,
              ^~~~~~~~~~~~

vim +390 drivers/hwmon/occ/occ.c

   374		return rc;
   375	}
   376	
   377	static int occ_get_all(struct occ *driver)
   378	{
   379		int i = 0, rc;
   380		u8 *occ_data;
   381		u16 num_bytes;
   382		const u8 poll_cmd_data = OCC_POLL_STAT_SENSOR;
   383		struct device *dev = driver->dev;
   384		struct occ_response *resp = &driver->response;
   385	
   386		occ_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
   387		if (!occ_data)
   388			return -ENOMEM;
   389	
 > 390		rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
   391		if (rc) {
   392			dev_err(dev, "OCC poll failed: %d\n", rc);
   393			goto out;
   394		}
   395	
   396		num_bytes = get_unaligned((u16 *)&occ_data[RESP_DATA_LENGTH]);
   397		num_bytes = be16_to_cpu(num_bytes);
   398		dev_dbg(dev, "OCC data length: %d\n", num_bytes);

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45853 bytes --]

^ permalink raw reply

* Re: [PATCH linux 2/6] hwmon: occ: Add sysfs interface
From: Guenter Roeck @ 2016-12-30 19:34 UTC (permalink / raw)
  To: eajames.ibm
  Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
	devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
	Edward A. James
In-Reply-To: <1483120568-21082-3-git-send-email-eajames.ibm@gmail.com>

On Fri, Dec 30, 2016 at 11:56:04AM -0600, eajames.ibm@gmail.com wrote:
> From: "Edward A. James" <eajames@us.ibm.com>
> 
> Add a generic mechanism to expose the sensors provided by the OCC in
> sysfs.
> 
> Signed-off-by: Edward A. James <eajames@us.ibm.com>
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
> Reviewed-by: Andrew Jeffery <andrew@aj.id.au>
> ---
>  Documentation/hwmon/occ       |  48 +++++
>  drivers/hwmon/occ/Makefile    |   2 +-
>  drivers/hwmon/occ/occ_sysfs.c | 492 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/hwmon/occ/occ_sysfs.h |  52 +++++
>  4 files changed, 593 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/hwmon/occ/occ_sysfs.c
>  create mode 100644 drivers/hwmon/occ/occ_sysfs.h
> 
> diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
> index 79d1642..1ee8689 100644
> --- a/Documentation/hwmon/occ
> +++ b/Documentation/hwmon/occ
> @@ -25,6 +25,54 @@ Currently, all versions of the OCC support four types of sensor data: power,
>  temperature, frequency, and "caps," which indicate limits and thresholds used
>  internally on the OCC.
>  
> +sysfs Entries
> +-------------
> +
> +The OCC driver uses the hwmon sysfs framework to provide data to userspace.
> +
> +The driver exports two sysfs files for each frequency, temperature, and power
> +sensor. These are "input" and "label". The input file contains the value of the
> +sensor. The label file contains the sensor id. The sensor id is the unique
> +internal OCC identifier. Sensor ids may be provided by the OCC specification.
> +The names of these files will be in the following format:
> +	<sensor type><sensor index>_input
> +	<sensor type><sensor index>_label
> +Sensor types will be one of "temp", "freq", or "power". The sensor index is
> +an index to differentiate different sensor files. For example, a single
> +temperature sensor will have two sysfs files: temp1_input and temp1_label.
> +
> +Caps sensors are exported differently. For each caps sensor, the driver will
> +export 6 entries:
> +	curr_powercap - current power cap in watts
> +	curr_powerreading - current power output in watts
> +	norm_powercap - power cap without redundant power
> +	max_powercap - maximum power cap that can be set in watts
> +	min_powercap - minimum power cap that can be set in watts
> +	user_powerlimit - power limit specified by the user in watts
> +In addition, the OCC driver for P9 will export a 7th entry:
> +	user_powerlimit_source - can be one of two values depending on who set
> +		the user_powerlimit. 0x1 - out of band from BMC or host. 0x2 -
> +		in band from other source.
> +The format for these files is caps<sensor index>_<entry type>. For example,
> +caps1_curr_powercap.
> +
> +The driver also provides a number of sysfs entries through hwmon to better
> +control the driver and monitor the OCC.
> +	powercap - read or write the OCC user power limit in watts.
> +	name - read the name of the driver
> +	update_interval - read or write the minimum interval for polling the
> +		OCC.
> +
> +The driver also exports a single sysfs file through the communication protocol
> +device (see BMC - Host Communications). The filename is "online" and represents
> +the status of the OCC with respect to the driver. The OCC can be in one of two
> +states: OCC polling enabled or OCC polling disabled. The purpose of this file
> +is to control the behavior of the driver and it's hwmon sysfs entries, not to
> +infer any information about the state of the physical OCC. Reading the file
> +returns either a 0 (polling disabled) or 1 (polling enabled). Writing 1 to the
> +file enables OCC polling in the driver if communications can be established
> +with the OCC. Writing a 0 to the driver disables OCC polling.
> +
>  BMC - Host Communications
>  -------------------------
>  
> diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
> index 93cb52f..a6881f9 100644
> --- a/drivers/hwmon/occ/Makefile
> +++ b/drivers/hwmon/occ/Makefile
> @@ -1 +1 @@
> -obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
> +obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
> diff --git a/drivers/hwmon/occ/occ_sysfs.c b/drivers/hwmon/occ/occ_sysfs.c
> new file mode 100644
> index 0000000..b0e063da
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_sysfs.c
> @@ -0,0 +1,492 @@
> +/*
> + * occ_sysfs.c - OCC sysfs interface
> + *
> + * This file contains the methods and data structures for implementing the OCC
> + * hwmon sysfs entries.
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/jiffies.h>
> +#include <linux/hwmon.h>
> +#include <linux/hwmon-sysfs.h>
> +#include <linux/err.h>
> +#include <linux/mutex.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/device.h>
> +
> +#include "occ_sysfs.h"
> +
> +#define MAX_SENSOR_ATTR_LEN	32
> +
> +#define RESP_RETURN_CMD_INVAL	0x13
> +
> +struct sensor_attr_data {
> +	enum sensor_type type;
> +	u32 hwmon_index;
> +	u32 attr_id;
> +	char name[MAX_SENSOR_ATTR_LEN];
> +	struct device_attribute dev_attr;
> +};
> +
> +static ssize_t show_input(struct device *dev,
> +			  struct device_attribute *attr, char *buf)
> +{
> +	int val;
> +	struct sensor_attr_data *sdata = container_of(attr,
> +						      struct sensor_attr_data,
> +						      dev_attr);
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	val = occ_get_sensor_value(driver->occ, sdata->type,
> +				   sdata->hwmon_index - 1);
> +	if (sdata->type == TEMP)
> +		val *= 1000;	/* in millidegree Celsius */
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +/* show_label provides the OCC sensor id. The sensor id will be either a
> + * 2-byte (for P8) or 4-byte (for P9) value. The sensor id is a way to
> + * identify what each sensor represents, according to the OCC specification.
> + */
> +static ssize_t show_label(struct device *dev,
> +			  struct device_attribute *attr, char *buf)
> +{
> +	int val;
> +	struct sensor_attr_data *sdata = container_of(attr,
> +						      struct sensor_attr_data,
> +						      dev_attr);
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	val = occ_get_sensor_id(driver->occ, sdata->type,
> +				sdata->hwmon_index - 1);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +static ssize_t show_caps(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	int val;
> +	struct caps_sensor *sensor;
> +	struct sensor_attr_data *sdata = container_of(attr,
> +						      struct sensor_attr_data,
> +						      dev_attr);
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	sensor = occ_get_sensor(driver->occ, CAPS);
> +	if (!sensor) {
> +		val = -1;
> +		return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +	}
> +
> +	val = occ_get_caps_value(driver->occ, sensor, sdata->hwmon_index - 1,
> +				 sdata->attr_id);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%d\n", val);
> +}
> +
> +static ssize_t show_update_interval(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%lu\n", driver->update_interval);
> +}
> +
> +static ssize_t store_update_interval(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int rc;
> +
> +	rc = kstrtoul(buf, 10, &val);
> +	if (rc)
> +		return rc;
> +
> +	driver->update_interval = val;
> +	occ_set_update_interval(driver->occ, val);
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(update_interval, S_IWUSR | S_IRUGO, show_update_interval,
> +		   store_update_interval);
> +
> +static ssize_t show_name(struct device *dev,
> +			 struct device_attribute *attr, char *buf)
> +{
> +	return snprintf(buf, PAGE_SIZE - 1, "occ\n");
> +}
> +
> +static DEVICE_ATTR(name, S_IRUGO, show_name, NULL);
> +
> +static ssize_t show_user_powercap(struct device *dev,
> +				  struct device_attribute *attr, char *buf)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->user_powercap);
> +}
> +
> +static ssize_t store_user_powercap(struct device *dev,
> +				   struct device_attribute *attr,
> +				   const char *buf, size_t count)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +	u16 val;
> +	int rc;
> +
> +	rc = kstrtou16(buf, 10, &val);
> +	if (rc)
> +		return rc;
> +
> +	dev_dbg(dev, "set user powercap to: %d\n", val);
> +	rc = occ_set_user_powercap(driver->occ, val);
> +	if (rc) {
> +		dev_err(dev, "set user powercap failed: 0x%x\n", rc);
> +		if (rc == RESP_RETURN_CMD_INVAL) {
> +			dev_err(dev, "set invalid powercap value: %d\n", val);
> +			return -EINVAL;
> +		}
> +
> +		return rc;
> +	}
> +
> +	driver->user_powercap = val;
> +
> +	return count;
> +}
> +
> +static DEVICE_ATTR(user_powercap, S_IWUSR | S_IRUGO, show_user_powercap,
> +		   store_user_powercap);
> +
> +static void deinit_sensor_groups(struct device *dev,
> +				 struct sensor_group *sensor_groups)
> +{
> +	int i;
> +
> +	for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> +		if (sensor_groups[i].group.attrs)
> +			devm_kfree(dev, sensor_groups[i].group.attrs);
> +		if (sensor_groups[i].sattr)
> +			devm_kfree(dev, sensor_groups[i].sattr);
> +		sensor_groups[i].group.attrs = NULL;
> +		sensor_groups[i].sattr = NULL;
> +	}
> +}
> +
> +static void sensor_attr_init(struct sensor_attr_data *sdata,
> +			     char *sensor_group_name,
> +			     char *attr_name,
> +			     ssize_t (*show)(struct device *dev,
> +					     struct device_attribute *attr,
> +					     char *buf))
> +{
> +	sysfs_attr_init(&sdata->dev_attr.attr);
> +
> +	snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> +		 sensor_group_name, sdata->hwmon_index, attr_name);
> +	sdata->dev_attr.attr.name = sdata->name;
> +	sdata->dev_attr.attr.mode = S_IRUGO;
> +	sdata->dev_attr.show = show;
> +}
> +
> +static int create_sensor_group(struct occ_sysfs *driver,
> +			       enum sensor_type type, int sensor_num)
> +{
> +	struct device *dev = driver->dev;
> +	struct sensor_group *sensor_groups = driver->sensor_groups;
> +	struct sensor_attr_data *sdata;
> +	int rc, i;
> +
> +	/* each sensor has 'label' and 'input' attributes */
> +	sensor_groups[type].group.attrs =
> +		devm_kzalloc(dev, sizeof(struct attribute *) *
> +			     sensor_num * 2 + 1, GFP_KERNEL);
> +	if (!sensor_groups[type].group.attrs) {
> +		rc = -ENOMEM;
> +		goto err;
> +	}
> +
> +	sensor_groups[type].sattr =
> +		devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> +			     sensor_num * 2, GFP_KERNEL);
> +	if (!sensor_groups[type].sattr) {
> +		rc = -ENOMEM;
> +		goto err;
> +	}
> +
> +	for (i = 0; i < sensor_num; i++) {
> +		sdata = &sensor_groups[type].sattr[i];
> +		/* hwmon attributes index starts from 1 */
> +		sdata->hwmon_index = i + 1;
> +		sdata->type = type;
> +		sensor_attr_init(sdata, sensor_groups[type].name, "input",
> +				 show_input);
> +		sensor_groups[type].group.attrs[i] = &sdata->dev_attr.attr;
> +
> +		sdata = &sensor_groups[type].sattr[i + sensor_num];
> +		sdata->hwmon_index = i + 1;
> +		sdata->type = type;
> +		sensor_attr_init(sdata, sensor_groups[type].name, "label",
> +				 show_label);
> +		sensor_groups[type].group.attrs[i + sensor_num] =
> +			&sdata->dev_attr.attr;
> +	}
> +
> +	rc = sysfs_create_group(&dev->kobj, &sensor_groups[type].group);
> +	if (rc)
> +		goto err;
> +
> +	return 0;
> +err:
> +	deinit_sensor_groups(dev, sensor_groups);
> +	return rc;
> +}
> +
> +static void caps_sensor_attr_init(struct sensor_attr_data *sdata,
> +				  char *attr_name, uint32_t hwmon_index,
> +				  uint32_t attr_id)
> +{
> +	sdata->type = CAPS;
> +	sdata->hwmon_index = hwmon_index;
> +	sdata->attr_id = attr_id;
> +
> +	snprintf(sdata->name, MAX_SENSOR_ATTR_LEN, "%s%d_%s",
> +		 "caps", sdata->hwmon_index, attr_name);
> +
> +	sysfs_attr_init(&sdata->dev_attr.attr);
> +	sdata->dev_attr.attr.name = sdata->name;
> +	sdata->dev_attr.attr.mode = S_IRUGO;
> +	sdata->dev_attr.show = show_caps;
> +}
> +
> +static int create_caps_sensor_group(struct occ_sysfs *driver, int sensor_num)
> +{
> +	struct device *dev = driver->dev;
> +	struct sensor_group *sensor_groups = driver->sensor_groups;
> +	int field_num = driver->num_caps_fields;
> +	struct sensor_attr_data *sdata;
> +	int i, j, rc;
> +
> +	sensor_groups[CAPS].group.attrs =
> +		devm_kzalloc(dev, sizeof(struct attribute *) * sensor_num *
> +			     field_num + 1, GFP_KERNEL);
> +	if (!sensor_groups[CAPS].group.attrs) {
> +		rc = -ENOMEM;
> +		goto err;
> +	}
> +
> +	sensor_groups[CAPS].sattr =
> +		devm_kzalloc(dev, sizeof(struct sensor_attr_data) *
> +			     sensor_num * field_num, GFP_KERNEL);
> +	if (!sensor_groups[CAPS].sattr) {
> +		rc = -ENOMEM;
> +		goto err;
> +	}
> +
> +	for (j = 0; j < sensor_num; ++j) {
> +		for (i = 0; i < field_num; ++i) {
> +			sdata = &sensor_groups[CAPS].sattr[j * field_num + i];
> +			caps_sensor_attr_init(sdata,
> +					      driver->caps_names[i], j + 1, i);
> +			sensor_groups[CAPS].group.attrs[j * field_num + i] =
> +				&sdata->dev_attr.attr;
> +		}
> +	}
> +
> +	rc = sysfs_create_group(&dev->kobj, &sensor_groups[CAPS].group);
> +	if (rc)
> +		goto err;
> +
> +	return rc;
> +err:
> +	deinit_sensor_groups(dev, sensor_groups);
> +	return rc;
> +}
> +
> +static void occ_remove_hwmon_attrs(struct occ_sysfs *driver)
> +{
> +	struct device *dev = driver->dev;
> +
> +	device_remove_file(dev, &dev_attr_user_powercap);
> +	device_remove_file(dev, &dev_attr_update_interval);
> +	device_remove_file(dev, &dev_attr_name);
> +}
> +
> +static int occ_create_hwmon_attrs(struct occ_sysfs *driver)
> +{
> +	int i, rc, id, sensor_num;
> +	struct device *dev = driver->dev;
> +	struct sensor_group *sensor_groups = driver->sensor_groups;
> +	struct occ_blocks *resp = NULL;
> +
> +	occ_get_response_blocks(driver->occ, &resp);
> +
> +	for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
> +		resp->sensor_block_id[i] = -1;
> +
> +	/* read sensor data from occ */
> +	rc = occ_update_device(driver->occ);
> +	if (rc) {
> +		dev_err(dev, "cannot get occ sensor data: %d\n", rc);
> +		return rc;
> +	}
> +	if (!resp->blocks)
> +		return -ENOMEM;
> +
> +	rc = device_create_file(dev, &dev_attr_name);
> +	if (rc)
> +		goto error;
> +
> +	rc = device_create_file(dev, &dev_attr_update_interval);
> +	if (rc)
> +		goto error;
> +
> +	if (resp->sensor_block_id[CAPS] >= 0) {
> +		/* user powercap: only for master OCC */
> +		rc = device_create_file(dev, &dev_attr_user_powercap);
> +		if (rc)
> +			goto error;
> +	}
> +
> +	sensor_groups[FREQ].name = "freq";
> +	sensor_groups[TEMP].name = "temp";
> +	sensor_groups[POWER].name = "power";
> +	sensor_groups[CAPS].name = "caps";
> +
> +	for (i = 0; i < MAX_OCC_SENSOR_TYPE; i++) {
> +		id = resp->sensor_block_id[i];
> +		if (id < 0)
> +			continue;
> +
> +		sensor_num = resp->blocks[id].header.sensor_num;
> +		if (i == CAPS)
> +			rc = create_caps_sensor_group(driver, sensor_num);
> +		else
> +			rc = create_sensor_group(driver, i, sensor_num);
> +		if (rc)
> +			goto error;
> +	}
> +
> +	return 0;
> +
> +error:
> +	dev_err(dev, "cannot create hwmon attributes: %d\n", rc);
> +	occ_remove_hwmon_attrs(driver);
> +	return rc;
> +}
> +
> +static ssize_t show_occ_online(struct device *dev,
> +			       struct device_attribute *attr, char *buf)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +
> +	return snprintf(buf, PAGE_SIZE - 1, "%u\n", driver->occ_online);
> +}
> +
> +static ssize_t store_occ_online(struct device *dev,
> +				struct device_attribute *attr,
> +				const char *buf, size_t count)
> +{
> +	struct occ_sysfs *driver = dev_get_drvdata(dev);
> +	unsigned long val;
> +	int rc;
> +
> +	rc = kstrtoul(buf, 10, &val);
> +	if (rc)
> +		return rc;
> +
> +	if (val == 1) {
> +		if (driver->occ_online)
> +			return count;
> +
> +		driver->dev = hwmon_device_register(dev);

hwmon_device_register() is deprecated. Please consider using
devm_hwmon_device_register_with_info() or at least
hwmon_device_register_with_info().

Uuh ... and registering a hwmon device based on writing into a sysfs attribute
is completely out of the question.

Thanks,
Guenter

> +		if (IS_ERR(driver->dev))
> +			return PTR_ERR(driver->dev);
> +
> +		dev_set_drvdata(driver->dev, driver);
> +
> +		rc = occ_create_hwmon_attrs(driver);
> +		if (rc) {
> +			hwmon_device_unregister(driver->dev);
> +			driver->dev = NULL;
> +			return rc;
> +		}
> +	} else if (val == 0) {
> +		if (!driver->occ_online)
> +			return count;
> +
> +		occ_remove_hwmon_attrs(driver);
> +		hwmon_device_unregister(driver->dev);
> +		driver->dev = NULL;
> +	} else
> +		return -EINVAL;
> +
> +	driver->occ_online = val;
> +	return count;
> +}
> +
> +static DEVICE_ATTR(online, S_IWUSR | S_IRUGO, show_occ_online,
> +		   store_occ_online);
> +
> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> +				  struct occ_sysfs_config *config)
> +{
> +	struct occ_sysfs *hwmon = devm_kzalloc(dev, sizeof(struct occ_sysfs),
> +					       GFP_KERNEL);
> +	int rc;
> +
> +	if (!hwmon)
> +		return ERR_PTR(-ENOMEM);
> +
> +	hwmon->occ = occ;
> +	hwmon->num_caps_fields = config->num_caps_fields;
> +	hwmon->caps_names = config->caps_names;
> +
> +	dev_set_drvdata(dev, hwmon);
> +
> +	rc = device_create_file(dev, &dev_attr_online);
> +	if (rc)
> +		return ERR_PTR(rc);
> +
> +	return hwmon;
> +}
> +EXPORT_SYMBOL(occ_sysfs_start);
> +
> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver)
> +{
> +	if (driver->dev) {
> +		occ_remove_hwmon_attrs(driver);
> +		hwmon_device_unregister(driver->dev);
> +	}
> +
> +	device_remove_file(driver->dev, &dev_attr_online);
> +
> +	devm_kfree(dev, driver);

Thw point of using devm_ functions is not to require remove/free functions.
Something is completely wrong here if you need that call.

Overall, this is architectually completely wrong. One does not register
or instantiate drivers based on writing into sysfs attributes. Please
reconsider your approach.

> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(occ_sysfs_stop);
> +
> +MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
> +MODULE_DESCRIPTION("OCC sysfs driver");
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/hwmon/occ/occ_sysfs.h b/drivers/hwmon/occ/occ_sysfs.h
> new file mode 100644
> index 0000000..2a8044f
> --- /dev/null
> +++ b/drivers/hwmon/occ/occ_sysfs.h
> @@ -0,0 +1,52 @@
> +/*
> + * occ_sysfs.h - OCC sysfs interface
> + *
> + * This file contains the data structures and function prototypes for the OCC
> + * hwmon sysfs entries.
> + *
> + * Copyright 2016 IBM Corp.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#ifndef __OCC_SYSFS_H__
> +#define __OCC_SYSFS_H__
> +
> +#include "occ.h"
> +
> +struct sensor_group {
> +	char *name;
> +	struct sensor_attr_data *sattr;
> +	struct attribute_group group;
> +};
> +
> +struct occ_sysfs_config {
> +	unsigned int num_caps_fields;
> +	char **caps_names;
> +};
> +
> +struct occ_sysfs {
> +	struct device *dev;
> +	struct occ *occ;
> +
> +	u16 user_powercap;
> +	bool occ_online;
> +	struct sensor_group sensor_groups[MAX_OCC_SENSOR_TYPE];
> +	unsigned long update_interval;
> +	unsigned int num_caps_fields;
> +	char **caps_names;
> +};
> +
> +struct occ_sysfs *occ_sysfs_start(struct device *dev, struct occ *occ,
> +				  struct occ_sysfs_config *config);
> +int occ_sysfs_stop(struct device *dev, struct occ_sysfs *driver);
> +
> +#endif /* __OCC_SYSFS_H__ */
> -- 
> 1.9.1
> 

^ permalink raw reply

* Re: [PATCH] iio: adc: Add Renesas GyroADC driver
From: Peter Meerwald-Stadler @ 2016-12-30 19:50 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	Simon Horman
In-Reply-To: <20161230191800.2532-1-marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


> Add IIO driver for the Renesas RCar GyroADC block. This block is a
> simple 4/8-channel ADC which samples 12/15/24 bits of data every
> cycle from all channels.

comments below
 
> Signed-off-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
> Cc: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
> ---
>  .../bindings/iio/adc/renesas,gyroadc.txt           |  38 +++
>  MAINTAINERS                                        |   6 +
>  drivers/iio/adc/Kconfig                            |  10 +
>  drivers/iio/adc/Makefile                           |   1 +
>  drivers/iio/adc/rcar_gyro_adc.c                    | 379 +++++++++++++++++++++
>  5 files changed, 434 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>  create mode 100644 drivers/iio/adc/rcar_gyro_adc.c
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
> new file mode 100644
> index 0000000..3fd5f57
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
> @@ -0,0 +1,38 @@
> +* Renesas RCar GyroADC device driver
> +
> +Required properties:
> +- compatible:	Should be "renesas,rcar-gyroadc" for regular GyroADC or
> +		"renesas,rcar-gyroadc-r8a7792" for GyroADC without interrupt
> +		block found in R8A7792.
> +- reg:		Address and length of the register set for the device
> +- clocks:	References to all the clocks specified in the clock-names
> +		property as specified in
> +		Documentation/devicetree/bindings/clock/clock-bindings.txt.
> +- clock-names:	Shall contain "fck" and "if". The "fck" are the GyroADC block

"fck" is...

> +		clock, the "if" are the interface clock.

"if" is ...

> +		power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;

this is just an example and not appropriate here?

> +- power-domains: Must contain a reference to the PM domain, if available.
> +- renesas,gyroadc-mode:	GyroADC mode of operation, must be either of:
> +			1 - MB88101A mode, 12bit sampling, 4 channels
> +			2 - ADCS7476 mode, 15bit sampling, 8 channels
> +			3 - MAX1162 mode,  16bit sampling, 8 channels
> +- renesas,gyroadc-vref:	Array of reference voltage values for each input to
> +			the GyroADC, in uV. Array must have 4 elemenets for

elements

> +			mode 1 and 8 elements for mode 2 and 3.
> +
> +Example:
> +	&adc {
> +		compatible = "renesas,rcar-gyroadc";
> +		reg = <0 0xe6e54000 0 64>;
> +		clocks = <&mstp9_clks R8A7791_CLK_GYROADC>, <&clk_65m>;
> +		clock-names = "fck", "if";
> +		power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
> +
> +		pinctrl-0 = <&adc_pins>;
> +		pinctrl-names = "default";
> +
> +		renesas,gyroadc-vref = <4096000 4096000 4096000 4096000
> +					4096000 4096000 4096000 4096000>;
> +		renesas,gyroadc-mode = <3>;
> +		status = "okay";
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 162d904..751e760 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10271,6 +10271,12 @@ L:	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>  F:	drivers/net/ethernet/renesas/
>  F:	include/linux/sh_eth.h
>  
> +RENESAS RCAR GYROADC DRIVER
> +M:	Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> +L:	linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +S:	Supported
> +F:	drivers/iio/adc/rcar_gyro_adc.c
> +
>  RENESAS USB2 PHY DRIVER
>  M:	Yoshihiro Shimoda <yoshihiro.shimoda.uh-zM6kxYcvzFBBDgjK7y7TUQ@public.gmane.org>
>  L:	linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 99c0514..4a4cac7 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -408,6 +408,16 @@ config QCOM_SPMI_VADC
>  	  To compile this driver as a module, choose M here: the module will
>  	  be called qcom-spmi-vadc.
>  
> +config RCAR_GYRO_ADC
> +	tristate "Renesas RCAR GyroADC driver"
> +	depends on ARCH_RCAR_GEN2 || (ARM && COMPILE_TEST)
> +	help
> +	  Say yes here to build support for the GyroADC found in Renesas
> +	  RCar Gen2 SoCs.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called rcar-gyroadc.

called rcar_gyro_adc?

> +
>  config ROCKCHIP_SARADC
>  	tristate "Rockchip SARADC driver"
>  	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index 7a40c04..253aeb2 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -39,6 +39,7 @@ obj-$(CONFIG_NAU7802) += nau7802.o
>  obj-$(CONFIG_PALMAS_GPADC) += palmas_gpadc.o
>  obj-$(CONFIG_QCOM_SPMI_IADC) += qcom-spmi-iadc.o
>  obj-$(CONFIG_QCOM_SPMI_VADC) += qcom-spmi-vadc.o
> +obj-$(CONFIG_RCAR_GYRO_ADC) += rcar_gyro_adc.o
>  obj-$(CONFIG_ROCKCHIP_SARADC) += rockchip_saradc.o
>  obj-$(CONFIG_STX104) += stx104.o
>  obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> diff --git a/drivers/iio/adc/rcar_gyro_adc.c b/drivers/iio/adc/rcar_gyro_adc.c
> new file mode 100644
> index 0000000..a74b148
> --- /dev/null
> +++ b/drivers/iio/adc/rcar_gyro_adc.c
> @@ -0,0 +1,379 @@
> +/*
> + * Renesas RCar GyroADC driver
> + *
> + * Copyright 2016 Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/interrupt.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/slab.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/of.h>
> +#include <linux/of_irq.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/of_platform.h>
> +#include <linux/err.h>
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/trigger.h>
> +
> +/* GyroADC registers. */
> +#define RCAR_GYROADC_MODE_SELECT		0x00
> +#define RCAR_GYROADC_MODE_SELECT_1_MB88101A	0x0
> +#define RCAR_GYROADC_MODE_SELECT_2_ADCS7476	0x1
> +#define RCAR_GYROADC_MODE_SELECT_3_MAX1162	0x3
> +
> +#define RCAR_GYROADC_START_STOP			0x04
> +#define RCAR_GYROADC_START_STOP_START		BIT(0)
> +
> +#define RCAR_GYROADC_CLOCK_LENGTH		0x08
> +#define RCAR_GYROADC_1_25MS_LENGTH		0x0c
> +
> +#define RCAR_GYROADC_REALTIME_DATA(ch)		(0x10 + ((ch) * 4))
> +#define RCAR_GYROADC_100MS_ADDED_DATA(ch)	(0x30 + ((ch) * 4))
> +#define RCAR_GYROADC_10MS_AVG_DATA(ch)		(0x50 + ((ch) * 4))
> +
> +#define RCAR_GYROADC_FIFO_STATUS		0x70
> +#define RCAR_GYROADC_FIFO_STATUS_EMPTY(ch)	BIT(0 + (4 * (ch)))

FIFO_STATUS_... is not used (yet)
4*ch looks suspicious for ch==8??

> +#define RCAR_GYROADC_FIFO_STATUS_FULL(ch)	BIT(1 + (4 * (ch)))
> +#define RCAR_GYROADC_FIFO_STATUS_ERROR(ch)	BIT(2 + (4 * (ch)))
> +
> +#define RCAR_GYROADC_INTR			0x74
> +#define RCAR_GYROADC_INTR_INT			BIT(0)
> +
> +#define RCAR_GYROADC_INTENR			0x78
> +#define RCAR_GYROADC_INTENR_INTEN		BIT(0)
> +
> +#define RCAR_GYROADC_SAMPLE_RATE		800	/* Hz */
> +
> +enum rcar_gyroadc_model {
> +	RCAR_GYROADC_MODEL_DEFAULT,
> +	RCAR_GYROADC_MODEL_R8A7792,
> +};
> +
> +struct rcar_gyroadc {
> +	struct device		*dev;
> +	void __iomem		*regs;
> +	struct clk		*fclk;
> +	struct clk		*clk;
> +	enum rcar_gyroadc_model	model;
> +	unsigned int		mode;
> +	u32			vref_uv[8];
> +	u32			buffer[8];
> +};
> +
> +static void rcar_gyroadc_hw_init(struct rcar_gyroadc *priv)
> +{
> +	unsigned long clk_mhz = clk_get_rate(priv->clk) / 1000000;
> +
> +	/* Stop the GyroADC. */
> +	writel(0, priv->regs + RCAR_GYROADC_START_STOP);
> +
> +	/* Disable IRQ, except on V2H. */
> +	if (priv->model != RCAR_GYROADC_MODEL_R8A7792)
> +		writel(0, priv->regs + RCAR_GYROADC_INTENR);
> +
> +	/* Set mode and timing. */
> +	writel(priv->mode, priv->regs + RCAR_GYROADC_MODE_SELECT);
> +
> +	if (priv->mode == RCAR_GYROADC_MODE_SELECT_1_MB88101A)
> +		writel(clk_mhz * 10, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
> +	else if (priv->mode == RCAR_GYROADC_MODE_SELECT_2_ADCS7476)
> +		writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
> +	else if (priv->mode == RCAR_GYROADC_MODE_SELECT_3_MAX1162)
> +		writel(clk_mhz * 5, priv->regs + RCAR_GYROADC_CLOCK_LENGTH);
> +	writel(clk_mhz * 1250, priv->regs + RCAR_GYROADC_1_25MS_LENGTH);
> +
> +	/*
> +	 * We can possibly turn the sampling on/off on-demand to reduce power
> +	 * consumption, but for the sake of quick availability of samples, we
> +	 * don't do it now.
> +	 */
> +	writel(RCAR_GYROADC_START_STOP_START,
> +	       priv->regs + RCAR_GYROADC_START_STOP);
> +
> +	/* Wait for the first conversion to complete. */
> +	udelay(1250);
> +}
> +
> +#define RCAR_GYROADC_CHAN(_idx, _chan_type, _realbits) {	\
> +	.type			= (_chan_type),			\

_chan_type is IIO_VOLTAGE always?

> +	.indexed		= 1,				\
> +	.channel		= (_idx),			\
> +	.info_mask_separate	= BIT(IIO_CHAN_INFO_RAW),	\
> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
> +				    BIT(IIO_CHAN_INFO_SAMP_FREQ), \
> +	.scan_index		= (_idx),			\

no buffered mode yet, so strictly no need for a scan_index and scan_type

> +	.scan_type	= {					\
> +		.sign		= 'u',				\
> +		.realbits	= (_realbits),			\
> +		.storagebits	= 16,				\
> +	},							\
> +}
> +
> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_1[] = {
> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 12),
> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 12),
> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 12),
> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 12),
> +};
> +
> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_2[] = {
> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 15),
> +	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 15),
> +};
> +
> +/*
> + * NOTE: The data we receive in mode 3 from MAX1162 have MSByte = 0,
> + *       therefore we only use 16bit realbits here instead of 24.
> + */
> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_3[] = {
> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 16),
> +	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 16),
> +};
> +
> +static int rcar_gyroadc_read_raw(struct iio_dev *indio_dev,
> +				 struct iio_chan_spec const *chan,
> +				 int *val, int *val2, long mask)
> +{
> +	struct rcar_gyroadc *priv = iio_priv(indio_dev);
> +	unsigned int datareg = RCAR_GYROADC_REALTIME_DATA(chan->channel);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		if (chan->type != IIO_VOLTAGE)
> +			return -EINVAL;
> +
> +		if (iio_buffer_enabled(indio_dev))
> +			return -EBUSY;

use iio_device_claim_direct_mode()

> +
> +		*val = readl(priv->regs + datareg);
> +		*val &= BIT(chan->scan_type.realbits) - 1;
> +
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = (priv->vref_uv[chan->channel] * 1000) / 0x10000;
> +		return IIO_VAL_INT_PLUS_NANO;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = RCAR_GYROADC_SAMPLE_RATE;
> +		*val2 = 0;

*val2 = 0 not needed

> +		return IIO_VAL_INT;
> +	default:
> +		break;

return -EINVAL;
here directly

> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static int rcar_gyroadc_reg_access(struct iio_dev *indio_dev,
> +				   unsigned int reg, unsigned int writeval,
> +				   unsigned int *readval)
> +{
> +	struct rcar_gyroadc *priv = iio_priv(indio_dev);
> +	unsigned int maxreg = RCAR_GYROADC_INTENR;
> +
> +	if (readval == NULL)
> +		return -EINVAL;
> +
> +	if (reg % 4)
> +		return -EINVAL;
> +
> +	/* Handle the V2H case with missing interrupt block. */
> +	if (priv->model == RCAR_GYROADC_MODEL_R8A7792)
> +		maxreg = RCAR_GYROADC_FIFO_STATUS;
> +
> +	if (reg > maxreg)
> +		return -EINVAL;
> +
> +	*readval = readl(priv->regs + reg);
> +
> +	return 0;
> +}
> +
> +static const struct iio_info rcar_gyroadc_iio_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= rcar_gyroadc_read_raw,
> +	.debugfs_reg_access	= rcar_gyroadc_reg_access,
> +};
> +
> +static const struct of_device_id rcar_gyroadc_match[] = {
> +	{
> +		/* RCar Gen2 compatible GyroADC */
> +		.compatible	= "renesas,rcar-gyroadc",
> +		.data		= (void *)RCAR_GYROADC_MODEL_DEFAULT,
> +	}, {
> +		/* RCar V2H specialty without interrupt registers. */
> +		.compatible	= "renesas,rcar-gyroadc-r8a7792",
> +		.data		= (void *)RCAR_GYROADC_MODEL_R8A7792,
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +
> +MODULE_DEVICE_TABLE(of, rcar_gyroadc_match);
> +
> +static int rcar_gyroadc_probe(struct platform_device *pdev)
> +{
> +	const struct of_device_id *of_id =
> +		of_match_device(rcar_gyroadc_match, &pdev->dev);
> +	struct device *dev = &pdev->dev;
> +	struct rcar_gyroadc *priv;
> +	struct iio_dev *indio_dev;
> +	struct resource *mem;
> +	int ret, mode;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*priv));
> +	if (!indio_dev) {
> +		dev_err(dev, "Failed to allocate IIO device.\n");
> +		return -ENOMEM;
> +	}
> +
> +	priv = iio_priv(indio_dev);
> +	priv->dev = dev;
> +
> +	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	priv->regs = devm_ioremap_resource(dev, mem);
> +	if (IS_ERR(priv->regs))
> +		return PTR_ERR(priv->regs);
> +
> +	priv->fclk = devm_clk_get(dev, "fck");
> +	if (IS_ERR(priv->fclk)) {
> +		ret = PTR_ERR(priv->fclk);
> +		dev_err(dev, "Failed to get FCK clock (ret=%i)\n", ret);
> +		return ret;
> +	}
> +
> +	priv->clk = devm_clk_get(dev, "if");
> +	if (IS_ERR(priv->clk)) {
> +		ret = PTR_ERR(priv->clk);
> +		dev_err(dev, "Failed to get IF clock (ret=%i)\n", ret);
> +		return ret;
> +	}
> +
> +	ret = of_property_read_u32(pdev->dev.of_node, "renesas,gyroadc-mode",
> +				   &mode);
> +	if (ret || (mode < 1) || (mode > 3)) {

the mode check could be a simple 'else' below?

> +		dev_err(dev, "Failed to get GyroADC mode (ret=%i)\n", ret);
> +		return ret;
> +	}
> +
> +	if (mode == 1)
> +		priv->mode = RCAR_GYROADC_MODE_SELECT_1_MB88101A;
> +	else if (mode == 2)
> +		priv->mode = RCAR_GYROADC_MODE_SELECT_2_ADCS7476;
> +	else if (mode == 3)
> +		priv->mode = RCAR_GYROADC_MODE_SELECT_3_MAX1162;
> +
> +	of_property_read_u32_array(pdev->dev.of_node, "renesas,gyroadc-vref",
> +				   priv->vref_uv, (mode == 1) ? 4 : 8);
> +
> +	priv->model = (enum rcar_gyroadc_model)of_id->data;
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->dev.of_node = pdev->dev.of_node;
> +	indio_dev->info = &rcar_gyroadc_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	if (mode == 1) {

maybe do the mode differentiation only once, any with a switch?

> +		indio_dev->channels = rcar_gyroadc_iio_channels_1;
> +		indio_dev->num_channels =
> +			ARRAY_SIZE(rcar_gyroadc_iio_channels_1);
> +	} else if (mode == 2) {
> +		indio_dev->channels = rcar_gyroadc_iio_channels_2;
> +		indio_dev->num_channels =
> +			ARRAY_SIZE(rcar_gyroadc_iio_channels_2);
> +	} else if (mode == 3) {
> +		indio_dev->channels = rcar_gyroadc_iio_channels_3;
> +		indio_dev->num_channels =
> +			ARRAY_SIZE(rcar_gyroadc_iio_channels_3);
> +	}
> +
> +	ret = clk_prepare_enable(priv->fclk);
> +	if (ret) {
> +		dev_err(dev, "Could not prepare or enable the FCK clock.\n");
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(priv->clk);
> +	if (ret) {
> +		dev_err(dev, "Could not prepare or enable the IF clock.\n");
> +		goto error_clk_if_enable;
> +	}
> +
> +	rcar_gyroadc_hw_init(priv);
> +
> +	ret = iio_device_register(indio_dev);
> +	if (ret) {
> +		dev_err(dev, "Couldn't register IIO device.\n");
> +		goto error_iio_device_register;
> +	}
> +
> +	return 0;
> +
> +error_iio_device_register:
> +	clk_disable_unprepare(priv->clk);
> +error_clk_if_enable:
> +	clk_disable_unprepare(priv->fclk);
> +	return ret;
> +}
> +
> +static int rcar_gyroadc_remove(struct platform_device *pdev)
> +{
> +	struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> +	struct rcar_gyroadc *priv = iio_priv(indio_dev);
> +
> +	/* Stop sampling */
> +	writel(0, priv->regs + RCAR_GYROADC_START_STOP);
> +
> +	iio_device_unregister(indio_dev);
> +	clk_disable_unprepare(priv->clk);
> +	clk_disable_unprepare(priv->fclk);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver rcar_gyroadc_driver = {
> +	.probe          = rcar_gyroadc_probe,
> +	.remove         = rcar_gyroadc_remove,
> +	.driver         = {
> +		.name		= "rcar-gyroadc",
> +		.of_match_table	= rcar_gyroadc_match,
> +	},
> +};
> +
> +module_platform_driver(rcar_gyroadc_driver);
> +
> +MODULE_AUTHOR("Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>");
> +MODULE_DESCRIPTION("Renesas RCAR GyroADC driver");
> +MODULE_LICENSE("GPL v2");
> 

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v4 1/5] devicetree: mfd: Add binding for the TI LM3533
From: Jonathan Cameron @ 2016-12-30 19:50 UTC (permalink / raw)
  To: Bjorn Andersson, Rob Herring, Mark Rutland
  Cc: Lee Jones, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Richard Purdie, Jacek Anaszewski,
	Pavel Machek, Jingoo Han, devicetree, linux-kernel, linux-iio,
	linux-leds, Bjorn Andersson
In-Reply-To: <20161226181153.11271-1-bjorn.andersson@linaro.org>

On 26/12/16 18:11, Bjorn Andersson wrote:
> From: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> 
> Add the binding for the Texas Instruments LM3533 lighting power
> solution.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@sonymobile.com>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
> 
> I had Acks from Jonathan and Rob on v3, but dropped these due to the added
> compatible properties.
Looks sensible to me

Acked-by: Jonathan Cameron <jic23@kernel.org>
> 
> Changes since v3:
> - Added compatible to sub-nodes, per Lee's requested to treat them as separate
>   pieces.
> 
>  Documentation/devicetree/bindings/mfd/lm3533.txt | 205 +++++++++++++++++++++++
>  1 file changed, 205 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/lm3533.txt
> 
> diff --git a/Documentation/devicetree/bindings/mfd/lm3533.txt b/Documentation/devicetree/bindings/mfd/lm3533.txt
> new file mode 100644
> index 000000000000..909281096ba2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/lm3533.txt
> @@ -0,0 +1,205 @@
> +Texas Instruments LM3533 binding
> +
> +This binding describes the Texas Instruments LM3533, a lighting power solution
> +for smartphone handsets. The common properties are described directly in the
> +node, while each individual component are described in an optional subnode.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be:
> +		    "ti,lm3533"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: i2c address of the LM3533 chip
> +
> +- als-supply:
> +	Usage: optional
> +	Value type: <prop-encoded-array>
> +	Definition: reference to regulator powering the V_als input; as
> +		    specified in "../regulator/regulator.txt"
> +
> +- hwen-gpios:
> +	Usage: required
> +	Value type: <prop-encoded-array>
> +	Definition: reference to gpio pin connected to the HWEN input; as
> +		    specified in "../gpio/gpio.txt"
> +
> +- ti,boost-freq-hz:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: switch-frequency of the boost converter, must be either:
> +		    500000 or 1000000
> +
> +- ti,boost-ovp-mv:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: over-voltage protection limit, in mV. Must be one of:
> +		    16000, 24000, 32000 or 40000
> +
> +- #address-cells:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: must be 1
> +
> +- #size-cells:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: must be 0
> +
> += ALS SUBNODE
> +The ambient light sensor subnode carrying the light sensor related properties.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be:
> +		    "ti,lm3533-als"
> +
> +- ti,als-resistance-ohm:
> +	Usage: required (unless ti,pwm-mode is specified)
> +	Value type: <u32>
> +	Definition: specifies the resistor value (R_als), in Ohm. Valid values
> +		    ranges from 200000 to 1574 Ohm.
> +
> +- ti,pwm-mode:
> +	Usage: optional
> +	Value type: <empty>
> +	Definition: specifies, if present, that the als should operate in PWM
> +		    mode - rather than analog mode
> +
> += BACKLIGHT NODES
> +Backlight subnodes carrying the backlight related properties.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be:
> +		    "ti,lm3533-backlight"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: specifies which of the two backlights this node corresponds
> +		    to
> +
> +- default-brightness:
> +	Usage: optional
> +	Value type: <32>
> +	Definition: specifies the default brightness for the backlight, in
> +		    units of brightness [0-255]
> +
> +- label:
> +	Usage: required
> +	Value type: <string>
> +	Definition: specifies a name of this backlight
> +
> +- led-max-microamp:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: specifies the max current for this backlight, in uA, as
> +		    described in "../leds/common.txt"
> +
> +- ti,pwm-zones:
> +	Usage: optional
> +	Value type: <u32 list>
> +	Definition: lists the ALS zones to be PWM controlled for this backlight,
> +		    the values in the list are in the range [0 - 4]
> +
> += LED NODES
> +LED subnodes carrying the LED related properties.
> +
> +- compatible:
> +	Usage: required
> +	Value type: <stringlist>
> +	Definition: must be:
> +		    "ti,lm3533-led"
> +
> +- reg:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: specifies which of the four LEDs this node corresponds to
> +
> +- linux,default-trigger:
> +	Usage: optional
> +	Value type: <string>
> +	Definition: specifies the default trigger for the LED, as described in
> +		    "../leds/common.txt"
> +
> +- label:
> +	Usage: required
> +	Value type: <string>
> +	Definition: specifies a name of this LED, as described in
> +		    "../leds/common.txt"
> +
> +- led-max-microamp:
> +	Usage: required
> +	Value type: <u32>
> +	Definition: specifies the max current for this LED, in uA, as described
> +		    in "../leds/common.txt"
> +
> +- ti,pwm-zones:
> +	Usage: optional
> +	Value type: <u32 list>
> +	Definition: lists the ALS zones to be PWM controlled for this LED, the
> +		    values in the list are in the range [0 - 4]
> +
> += EXAMPLE
> +
> +i2c@12460000 {
> +	compatible = "qcom,i2c-qup-v1.1.1";
> +	...
> +
> +	lm3533@36 {
> +		compatible = "ti,lm3533";
> +		reg = <0x36>;
> +
> +		als-supply = <&pm8921_l11>;
> +		hwen-gpios = <&pm8921_gpio 26 GPIO_ACTIVE_HIGH>;
> +
> +		ti,boost-freq-hz = <500000>;
> +		ti,boost-ovp-mv = <24000>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		als {
> +			compatible = "ti,lm3533-als";
> +			ti,als-resistance-ohm = <200000>;
> +		};
> +
> +		backlight@0 {
> +			compatible = "ti,lm3533-backlight";
> +			reg = <0>;
> +			label = "backlight";
> +
> +			led-max-microamp = <20200>;
> +		};
> +
> +		led@0 {
> +			compatible = "ti,lm3533-led";
> +			reg = <0>;
> +			label = "red";
> +
> +			led-max-microamp = <5000>;
> +		};
> +
> +		led@1 {
> +			compatible = "ti,lm3533-led";
> +			reg = <1>;
> +			label = "green";
> +
> +			led-max-microamp = <5000>;
> +		};
> +
> +		led@2 {
> +			compatible = "ti,lm3533-led";
> +			reg = <2>;
> +			label = "blue";
> +
> +			led-max-microamp = <5000>;
> +		};
> +	};
> +
> 

^ permalink raw reply

* Re: [PATCH v8 3/8] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: Jonathan Cameron @ 2016-12-30 20:17 UTC (permalink / raw)
  To: H. Nikolaus Schaller, Dmitry Torokhov
  Cc: Jonathan Cameron, Sebastian Reichel, Mark Rutland,
	Benoît Cousson, Tony Lindgren, Russell King, Arnd Bergmann,
	Michael Welling, Mika Penttilä, Javier Martinez Canillas,
	Igor Grinberg, Andrew F. Davis, Mark Brown, Rob Herring,
	Alexander Stein, Eric Engestrom, Hans de Goede,
	Benjamin Tissoires
In-Reply-To: <FA2666F0-0C5B-458E-97B5-83243F26D0DE@goldelico.com>

On 28/12/16 14:52, H. Nikolaus Schaller wrote:
> Hi Dmitry,
> 
>> Am 27.12.2016 um 22:54 schrieb Dmitry Torokhov <dmitry.torokhov@gmail.com>:
>>
>> On Mon, Dec 12, 2016 at 10:21:25PM +0100, H. Nikolaus Schaller wrote:
>>> Hi,
>>>
>>>
>>>> Am 27.11.2016 um 16:47 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
>>>>
>>>> Hi Jonathan,
>>>>
>>>>> Am 27.11.2016 um 12:02 schrieb Jonathan Cameron <jic23@kernel.org>:
>>>>>
>>>>> On 24/11/16 18:05, H. Nikolaus Schaller wrote:
>>>>>>
>>>>>>> Am 24.11.2016 um 18:38 schrieb Jonathan Cameron <jic23@jic23.retrosnub.co.uk>:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> On 22 November 2016 14:02:30 GMT+00:00, "H. Nikolaus Schaller" <hns@goldelico.com> wrote:
>>>
>>>>
>>>>> - hence cc'd Yann and the Kbuild list
>>>>> to see if they can offer some advices.
>>>
>>> no response / advice so far.
>>
>> Since you are saying that IIO stuff is optional, add it to Kconfig
>> explicitly:
>>
>> config "TOUCHSCREEN_TSC2007_IIO"
>> 	bool "IIO interface for external ADC input and temperature"
>> 	depends on TOUCHSCREEN_TSC2007
>> 	depends on IIO=y || IIO=TOUCHSCREEN_TSC2007
>> 	help
>> 	  ...
>>
>> and use this symbols in makefile:
>>
>> and in Makefile:
>>
>> obj-$(CONFIG_TOUCHSCREEN_TSC2007) += tsc2007.o
>> tsc2007-y := tsc2007-core.o ...
>> tsc2007-$(CONFIG_TOUCHSCREEN_TSC2007_IIO) += tsc2007_iio.o
> 
> Ah, ok. Well, we tried to make it without an explicit config option
> but it does not hurt to have one.
> 
> Your proposal works fine for 3 out of the 4 possible combinations.
> 
> I think it is unlikely that the combination TOUCHSCREEN_TSC2007=y IIO=m
> is needed at all (we configure as much as possible as =m in our kernel).
Indeed - that shouldn't work and is blocked by the above which is good.
> 
> Patch v9 will come next (where I have also moved this iio driver
> patch to be 8/8).
Wise move ;)
> 
> BR and thanks,
> Nikolaus
> 


^ permalink raw reply

* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Arend van Spriel @ 2016-12-30 20:20 UTC (permalink / raw)
  To: Rafał Miłecki
  Cc: Kalle Valo,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Rafał Miłecki
In-Reply-To: <CACna6rwaUWEjpBdfXS6uJSxKXH_mCP7YMGd1KaJropNQgVS7PA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 29-12-16 10:43, Rafał Miłecki wrote:
> On 29 December 2016 at 09:57, Arend van Spriel
> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>> On 28-12-16 22:30, Rafał Miłecki wrote:
>>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>> On 28 December 2016 at 22:07, Arend van Spriel
>>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>
>>>>>> They allow specifying hardware limitations of supported channels. This
>>>>>> may be useful for specifying single band devices or devices that support
>>>>>> only some part of the whole band.
>>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>>> part of 5 GHz band.
>>>>>>
>>>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>> ---
>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>  1 file changed, 34 insertions(+)
>>>>>>
>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>> index 5dbac37..35ba5c7 100644
>>>>>> --- a/net/wireless/reg.c
>>>>>> +++ b/net/wireless/reg.c
>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>>  }
>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>
>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>> +{
>>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>>> +     u32 val;
>>>>>> +
>>>>>> +     if (!np)
>>>>>> +             return true;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>> +
>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>> +             return false;
>>>>>
>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>> !CONFIG_OF.
>>>>
>>>> I compiled it with
>>>> # CONFIG_OF is not set
>>>>
>>>> Can you test it and provide a non-working config if you see a
>>>> compilation error, please?
>>>
>>> include/linux/of.h provides a lot of dummy static inline functions if
>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>> code).
>>
>> of_propeirty_read_u32 is static inline in of.h calling
>> of_property_read_u32_array, which has a dummy variant in of.h returning
>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>> not allow the compiler to drop any code so probably better to do:
>>
>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>         return true;
> 
> Please verify that using a compiler. If there is a problem I'll be
> happy to work on it, but I need a proof it exists.

I am on vacation right now so not having much more than email and web
browser to use as review reference.

> If compilers sees a:
> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
> condition, it's pretty clear it can be dropped. With both conditional
> blocks dropped function always returns "true" and... can be dropped.
> 
> Let me see if I can convince you with the following test:

No need to convince me. I made a mistake reviewing the code. Thanks for
clarifying it.

> $ grep -m 1 CONFIG_OF .config
> # CONFIG_OF is not set
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 0
> 
> $ grep -m 1 CONFIG_OF .config
> CONFIG_OF=y
> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
> 1
> 
> 
>> So with this patch you change the channel to DISABLED. I am not very
>> familiar with reg.c so do you know if this is done before or after
>> calling regulatory notifier in the driver. brcmfmac will enable channels
>> querying the device upon regulatory notifier call, which may undo the
>> behavior introduced by your patch.
> 
> I'm not regulatory export, so I hope someone will review this patch.
> So far I can say it works for me after trying it on SR400ac with
> BCM43602.

But you probably do not have a mapping table for mapping country code
received in notifier to firmware regulatory code/revision. Only if you
have that, brcmfmac will update the channels in the bands.

Giving this some more consideration I am not sure if this is the proper
place to handle this. ieee80211-(min|max)-center-freq is platform
specific configuration allowing multiple cards to be used in different
(sub)bands. This has nothing to do with regulatory. So probably better
to move it to core.c or chan.c.

Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: add Garmin vendor prefix
From: Jonathan Cameron @ 2016-12-30 20:26 UTC (permalink / raw)
  To: Matt Ranostay, linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
  Cc: Rob Herring
In-Reply-To: <20161229051306.28547-1-matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>

On 29/12/16 05:13, Matt Ranostay wrote:
> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
Rob, just to check - is this the right way to go with prefixes?
It's awfully ugly in this case ;)

Jonathan
> ---
> Changes from v1:
> * switch to stock ticker for Garmin Limited
> 
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 16d3b5e7f5d1..5749bfc5fc5b 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -107,6 +107,7 @@ firefly	Firefly
>  focaltech	FocalTech Systems Co.,Ltd
>  friendlyarm	Guangzhou FriendlyARM Computer Tech Co., Ltd
>  fsl	Freescale Semiconductor
> +grmn	Garmin Limited
>  ge	General Electric Company
>  geekbuying	GeekBuying
>  gef	GE Fanuc Intelligent Platforms Embedded Systems, Inc.
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 3/4] ARM64: dts: exynos5433: use macros for pinctrl configuration on Exynos5433
From: Andi Shyti @ 2016-12-30 20:28 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andi Shyti, Chanwoo Choi, Tomasz Figa, Krzysztof Kozlowski,
	Sylwester Nawrocki, Rob Herring, Mark Rutland, Catalin Marinas,
	Will Deacon, Kukjin Kim, Javier Martinez Canillas,
	devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-samsung-soc, linux-kernel@vger.kernel.org, stable,
	Andi Shyti
In-Reply-To: <CACRpkdaz9AyFJs1Th_SVexqKaP-=iPF0G-1YXOCoSmrQH8ZHDA@mail.gmail.com>

Hi Linus,

> > Use the macros defined in include/dt-bindings/pinctrl/samsung.h
> > instead of hardcoded values.
> >
> > Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> 
> These look fine, but that this and the other DTS patch through ARM SoC.
> 
> If you also need the headerfile patch (patch 2) to go through ARM SoC
> that is fine,
> I can take it out of pinctrl if you want.

yes, sure... no problem from my side :)

Thanks,
Andi

^ permalink raw reply

* Re: [PATCH v9 8/8] drivers:input:tsc2007: add iio interface to read external ADC input and temperature
From: Jonathan Cameron @ 2016-12-30 20:36 UTC (permalink / raw)
  To: H. Nikolaus Schaller, Sebastian Reichel, Dmitry Torokhov,
	Mark Rutland, Benoît Cousson, Tony Lindgren, Russell King,
	Arnd Bergmann, Michael Welling, Mika Penttilä,
	Javier Martinez Canillas, Igor Grinberg, Andrew F. Davis,
	Mark Brown, Rob Herring, Alexander Stein, Eric Engestrom,
	Hans de Goede
  Cc: linux-input-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	letux-kernel-S0jZdbWzriLCfDggNXIi3w,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	kernel-Jl6IXVxNIMRxAtABVqVhTwC/G2K4zDHf
In-Reply-To: <17a94568ffb91abedc9d12896b602022abb5f7e8.1482936802.git.hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>

On 28/12/16 14:53, H. Nikolaus Schaller wrote:
> The tsc2007 chip not only has a resistive touch screen controller but
> also an external AUX adc imput which can be used for an ambient
> light sensor, battery voltage monitoring or any general purpose.
> 
> Additionally it can measure the chip temperature.
> 
> This extension provides an iio interface for these adc channels.
> 
> Since it is not wasting much resources and is very straightforward,
> we simply provide all other adc channels as optional iio interfaces
> as weel. This can be used for debugging or special applications.
> 
> This patch also splits the tsc2007 driver in several source files:
> tsc2007.h -- constants, structs and stubs
> tsc2007_core.c -- functional parts of the original driver
> tsc2007_iio.c -- the optional iio stuff
> 
> Makefile magic allows to conditionally link the iio stuff
> if CONFIG_IIO=y or =m in a way that it works with
> CONFIG_TOUCHSCREEN_TSC2007=m.
> 
> Signed-off-by: H. Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> Reviewed-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Just for the record, I'm happy with the few changes from earlier versions.

Jonathan
> ---
>  drivers/input/touchscreen/Kconfig                  |  10 ++
>  drivers/input/touchscreen/Makefile                 |   2 +
>  drivers/input/touchscreen/tsc2007.h                | 116 ++++++++++++++++
>  .../touchscreen/{tsc2007.c => tsc2007_core.c}      |  95 +++----------
>  drivers/input/touchscreen/tsc2007_iio.c            | 150 +++++++++++++++++++++
>  5 files changed, 299 insertions(+), 74 deletions(-)
>  create mode 100644 drivers/input/touchscreen/tsc2007.h
>  rename drivers/input/touchscreen/{tsc2007.c => tsc2007_core.c} (86%)
>  create mode 100644 drivers/input/touchscreen/tsc2007_iio.c
> 
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index efca013..1616a8d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -1035,6 +1035,16 @@ config TOUCHSCREEN_TSC2007
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called tsc2007.
>  
> +config TOUCHSCREEN_TSC2007_IIO
> +	bool "IIO interface for external ADC input and temperature"
> +	depends on TOUCHSCREEN_TSC2007
> +	depends on IIO=y || IIO=TOUCHSCREEN_TSC2007
> +	help
> +	  Saying Y here adds an iio interface to the tsc2007 which
> +	  provides values for the AUX input (used for e.g. battery
> +	  or ambient light monitoring), temperature and raw input
> +	  values.
> +
>  config TOUCHSCREEN_W90X900
>  	tristate "W90P910 touchscreen driver"
>  	depends on ARCH_W90X900
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 81b8645..05d1cc8 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -80,6 +80,8 @@ obj-$(CONFIG_TOUCHSCREEN_TSC_SERIO)	+= tsc40.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC200X_CORE)	+= tsc200x-core.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2004)	+= tsc2004.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2005)	+= tsc2005.o
> +tsc2007-y := tsc2007_core.o
> +tsc2007-$(CONFIG_TOUCHSCREEN_TSC2007_IIO)	+= tsc2007_iio.o
>  obj-$(CONFIG_TOUCHSCREEN_TSC2007)	+= tsc2007.o
>  obj-$(CONFIG_TOUCHSCREEN_UCB1400)	+= ucb1400_ts.o
>  obj-$(CONFIG_TOUCHSCREEN_WACOM_W8001)	+= wacom_w8001.o
> diff --git a/drivers/input/touchscreen/tsc2007.h b/drivers/input/touchscreen/tsc2007.h
> new file mode 100644
> index 0000000..16efb60
> --- /dev/null
> +++ b/drivers/input/touchscreen/tsc2007.h
> @@ -0,0 +1,116 @@
> +/*
> + * Copyright (c) 2008 MtekVision Co., Ltd.
> + *	Kwangwoo Lee <kwlee-ec7hoAtq5SbSUeElwK9/Pw@public.gmane.org>
> + *
> + * Using code from:
> + *  - ads7846.c
> + *	Copyright (c) 2005 David Brownell
> + *	Copyright (c) 2006 Nokia Corporation
> + *  - corgi_ts.c
> + *	Copyright (C) 2004-2005 Richard Purdie
> + *  - omap_ts.[hc], ads7846.h, ts_osk.c
> + *	Copyright (C) 2002 MontaVista Software
> + *	Copyright (C) 2004 Texas Instruments
> + *	Copyright (C) 2005 Dirk Behme
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/input/touchscreen.h>
> +
> +#define TSC2007_MEASURE_TEMP0		(0x0 << 4)
> +#define TSC2007_MEASURE_AUX		(0x2 << 4)
> +#define TSC2007_MEASURE_TEMP1		(0x4 << 4)
> +#define TSC2007_ACTIVATE_XN		(0x8 << 4)
> +#define TSC2007_ACTIVATE_YN		(0x9 << 4)
> +#define TSC2007_ACTIVATE_YP_XN		(0xa << 4)
> +#define TSC2007_SETUP			(0xb << 4)
> +#define TSC2007_MEASURE_X		(0xc << 4)
> +#define TSC2007_MEASURE_Y		(0xd << 4)
> +#define TSC2007_MEASURE_Z1		(0xe << 4)
> +#define TSC2007_MEASURE_Z2		(0xf << 4)
> +
> +#define TSC2007_POWER_OFF_IRQ_EN	(0x0 << 2)
> +#define TSC2007_ADC_ON_IRQ_DIS0		(0x1 << 2)
> +#define TSC2007_ADC_OFF_IRQ_EN		(0x2 << 2)
> +#define TSC2007_ADC_ON_IRQ_DIS1		(0x3 << 2)
> +
> +#define TSC2007_12BIT			(0x0 << 1)
> +#define TSC2007_8BIT			(0x1 << 1)
> +
> +#define	MAX_12BIT			((1 << 12) - 1)
> +
> +#define ADC_ON_12BIT	(TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
> +
> +#define READ_Y		(ADC_ON_12BIT | TSC2007_MEASURE_Y)
> +#define READ_Z1		(ADC_ON_12BIT | TSC2007_MEASURE_Z1)
> +#define READ_Z2		(ADC_ON_12BIT | TSC2007_MEASURE_Z2)
> +#define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
> +#define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
> +
> +struct ts_event {
> +	u16	x;
> +	u16	y;
> +	u16	z1, z2;
> +};
> +
> +struct tsc2007 {
> +	struct input_dev	*input;
> +	char			phys[32];
> +
> +	struct i2c_client	*client;
> +
> +	u16			model;
> +	u16			x_plate_ohms;
> +
> +	struct touchscreen_properties prop;
> +
> +	bool			report_resistance;
> +	u16			min_x;
> +	u16			min_y;
> +	u16			max_x;
> +	u16			max_y;
> +	u16			max_rt;
> +	unsigned long		poll_period; /* in jiffies */
> +	int			fuzzx;
> +	int			fuzzy;
> +	int			fuzzz;
> +
> +	unsigned int		gpio;
> +	int			irq;
> +
> +	wait_queue_head_t	wait;
> +	bool			stopped;
> +	bool			pendown;
> +
> +	int			(*get_pendown_state)(struct device *);
> +	void			(*clear_penirq)(void);
> +
> +	struct mutex		mlock;
> +	struct iio_dev		*iio_dev;	/* optional */
> +};
> +
> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd);
> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
> +					struct ts_event *tc);
> +bool tsc2007_is_pen_down(struct tsc2007 *ts);
> +
> +#if IS_ENABLED(CONFIG_TOUCHSCREEN_TSC2007_IIO)
> +
> +/* defined in tsc2007_iio.c */
> +int tsc2007_iio_configure(struct tsc2007 *ts);
> +void tsc2007_iio_unconfigure(struct tsc2007 *ts);
> +
> +#else /* CONFIG_TOUCHSCREEN_TSC2007_IIO */
> +
> +static inline int tsc2007_iio_configure(struct tsc2007 *ts)
> +{
> +	return 0;
> +}
> +static inline void tsc2007_iio_unconfigure(struct tsc2007 *ts)
> +{
> +}
> +
> +#endif /* CONFIG_TOUCHSCREEN_TSC2007_IIO */
> diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007_core.c
> similarity index 86%
> rename from drivers/input/touchscreen/tsc2007.c
> rename to drivers/input/touchscreen/tsc2007_core.c
> index 76b462b..812ded8 100644
> --- a/drivers/input/touchscreen/tsc2007.c
> +++ b/drivers/input/touchscreen/tsc2007_core.c
> @@ -27,79 +27,11 @@
>  #include <linux/i2c.h>
>  #include <linux/i2c/tsc2007.h>
>  #include <linux/of_device.h>
> -#include <linux/of.h>
>  #include <linux/of_gpio.h>
> -#include <linux/input/touchscreen.h>
> -
> -#define TSC2007_MEASURE_TEMP0		(0x0 << 4)
> -#define TSC2007_MEASURE_AUX		(0x2 << 4)
> -#define TSC2007_MEASURE_TEMP1		(0x4 << 4)
> -#define TSC2007_ACTIVATE_XN		(0x8 << 4)
> -#define TSC2007_ACTIVATE_YN		(0x9 << 4)
> -#define TSC2007_ACTIVATE_YP_XN		(0xa << 4)
> -#define TSC2007_SETUP			(0xb << 4)
> -#define TSC2007_MEASURE_X		(0xc << 4)
> -#define TSC2007_MEASURE_Y		(0xd << 4)
> -#define TSC2007_MEASURE_Z1		(0xe << 4)
> -#define TSC2007_MEASURE_Z2		(0xf << 4)
> -
> -#define TSC2007_POWER_OFF_IRQ_EN	(0x0 << 2)
> -#define TSC2007_ADC_ON_IRQ_DIS0		(0x1 << 2)
> -#define TSC2007_ADC_OFF_IRQ_EN		(0x2 << 2)
> -#define TSC2007_ADC_ON_IRQ_DIS1		(0x3 << 2)
> -
> -#define TSC2007_12BIT			(0x0 << 1)
> -#define TSC2007_8BIT			(0x1 << 1)
> -
> -#define	MAX_12BIT			((1 << 12) - 1)
> -
> -#define ADC_ON_12BIT	(TSC2007_12BIT | TSC2007_ADC_ON_IRQ_DIS0)
> -
> -#define READ_Y		(ADC_ON_12BIT | TSC2007_MEASURE_Y)
> -#define READ_Z1		(ADC_ON_12BIT | TSC2007_MEASURE_Z1)
> -#define READ_Z2		(ADC_ON_12BIT | TSC2007_MEASURE_Z2)
> -#define READ_X		(ADC_ON_12BIT | TSC2007_MEASURE_X)
> -#define PWRDOWN		(TSC2007_12BIT | TSC2007_POWER_OFF_IRQ_EN)
> -
> -struct ts_event {
> -	u16	x;
> -	u16	y;
> -	u16	z1, z2;
> -};
> -
> -struct tsc2007 {
> -	struct input_dev	*input;
> -	char			phys[32];
> -
> -	struct i2c_client	*client;
> -
> -	u16			model;
> -	u16			x_plate_ohms;
> -
> -	struct touchscreen_properties prop;
> -
> -	bool			report_resistance;
> -	u16			min_x;
> -	u16			min_y;
> -	u16			max_x;
> -	u16			max_y;
> -	u16			max_rt;
> -	unsigned long		poll_period; /* in jiffies */
> -	int			fuzzx;
> -	int			fuzzy;
> -	int			fuzzz;
> -
> -	unsigned		gpio;
> -	int			irq;
> -
> -	wait_queue_head_t	wait;
> -	bool			stopped;
> +#include "tsc2007.h"
>  
> -	int			(*get_pendown_state)(struct device *);
> -	void			(*clear_penirq)(void);
> -};
>  
> -static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
> +int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
>  {
>  	s32 data;
>  	u16 val;
> @@ -137,7 +69,7 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
>  	tsc2007_xfer(tsc, PWRDOWN);
>  }
>  
> -static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
> +u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>  					struct ts_event *tc)
>  {
>  	u32 rt = 0;
> @@ -158,7 +90,7 @@ static u32 tsc2007_calculate_resistance(struct tsc2007 *tsc,
>  	return rt;
>  }
>  
> -static bool tsc2007_is_pen_down(struct tsc2007 *ts)
> +bool tsc2007_is_pen_down(struct tsc2007 *ts)
>  {
>  	/*
>  	 * NOTE: We can't rely on the pressure to determine the pen down
> @@ -191,7 +123,10 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
>  	while (!ts->stopped && tsc2007_is_pen_down(ts)) {
>  
>  		/* pen is down, continue with the measurement */
> +
> +		mutex_lock(&ts->mlock);
>  		tsc2007_read_values(ts, &tc);
> +		mutex_unlock(&ts->mlock);
>  
>  		rt = tsc2007_calculate_resistance(ts, &tc);
>  
> @@ -441,7 +376,8 @@ static void tsc2007_call_exit_platform_hw(void *data)
>  static int tsc2007_probe(struct i2c_client *client,
>  			 const struct i2c_device_id *id)
>  {
> -	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
> +	const struct tsc2007_platform_data *pdata =
> +		dev_get_platdata(&client->dev);
>  	struct tsc2007 *ts;
>  	struct input_dev *input_dev;
>  	int err;
> @@ -463,7 +399,9 @@ static int tsc2007_probe(struct i2c_client *client,
>  	ts->client = client;
>  	ts->irq = client->irq;
>  	ts->input = input_dev;
> +
>  	init_waitqueue_head(&ts->wait);
> +	mutex_init(&ts->mlock);
>  
>  	snprintf(ts->phys, sizeof(ts->phys),
>  		 "%s/input0", dev_name(&client->dev));
> @@ -534,7 +472,7 @@ static int tsc2007_probe(struct i2c_client *client,
>  	if (err < 0) {
>  		dev_err(&client->dev,
>  			"Failed to setup chip: %d\n", err);
> -		return err;	/* usually, chip does not respond */
> +		return err;	/* chip does not respond */
>  	}
>  
>  	err = input_register_device(input_dev);
> @@ -544,6 +482,14 @@ static int tsc2007_probe(struct i2c_client *client,
>  		return err;
>  	}
>  
> +	return tsc2007_iio_configure(ts);
> +}
> +
> +static int tsc2007_remove(struct i2c_client *client)
> +{
> +	struct tsc2007 *ts = i2c_get_clientdata(client);
> +
> +	tsc2007_iio_unconfigure(ts);
>  	return 0;
>  }
>  
> @@ -569,6 +515,7 @@ static struct i2c_driver tsc2007_driver = {
>  	},
>  	.id_table	= tsc2007_idtable,
>  	.probe		= tsc2007_probe,
> +	.remove		= tsc2007_remove,
>  };
>  
>  module_i2c_driver(tsc2007_driver);
> diff --git a/drivers/input/touchscreen/tsc2007_iio.c b/drivers/input/touchscreen/tsc2007_iio.c
> new file mode 100644
> index 0000000..ed79944
> --- /dev/null
> +++ b/drivers/input/touchscreen/tsc2007_iio.c
> @@ -0,0 +1,150 @@
> +/*
> + * Copyright (c) 2016 Golden Delicious Comp. GmbH&Co. KG
> + *	Nikolaus Schaller <hns-xXXSsgcRVICgSpxsJD1C4w@public.gmane.org>
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License version 2 as
> + *  published by the Free Software Foundation.
> + */
> +
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include "tsc2007.h"
> +
> +struct tsc2007_iio {
> +	struct tsc2007 *ts;
> +};
> +
> +#define TSC2007_CHAN_IIO(_chan, _name, _type, _chan_info) \
> +{ \
> +	.datasheet_name = _name, \
> +	.type = _type, \
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |	\
> +			BIT(_chan_info), \
> +	.indexed = 1, \
> +	.channel = _chan, \
> +}
> +
> +static const struct iio_chan_spec tsc2007_iio_channel[] = {
> +	TSC2007_CHAN_IIO(0, "x", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(1, "y", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(2, "z1", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(3, "z2", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(4, "adc", IIO_VOLTAGE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(5, "rt", IIO_VOLTAGE, IIO_CHAN_INFO_RAW), /* Ohms? */
> +	TSC2007_CHAN_IIO(6, "pen", IIO_PRESSURE, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(7, "temp0", IIO_TEMP, IIO_CHAN_INFO_RAW),
> +	TSC2007_CHAN_IIO(8, "temp1", IIO_TEMP, IIO_CHAN_INFO_RAW),
> +};
> +
> +static int tsc2007_read_raw(struct iio_dev *indio_dev,
> +	struct iio_chan_spec const *chan, int *val, int *val2, long mask)
> +{
> +	struct tsc2007_iio *iio = iio_priv(indio_dev);
> +	struct tsc2007 *tsc = iio->ts;
> +	int adc_chan = chan->channel;
> +	int ret = 0;
> +
> +	if (adc_chan >= ARRAY_SIZE(tsc2007_iio_channel))
> +		return -EINVAL;
> +
> +	if (mask != IIO_CHAN_INFO_RAW)
> +		return -EINVAL;
> +
> +	mutex_lock(&tsc->mlock);
> +
> +	switch (chan->channel) {
> +	case 0:
> +		*val = tsc2007_xfer(tsc, READ_X);
> +		break;
> +	case 1:
> +		*val = tsc2007_xfer(tsc, READ_Y);
> +		break;
> +	case 2:
> +		*val = tsc2007_xfer(tsc, READ_Z1);
> +		break;
> +	case 3:
> +		*val = tsc2007_xfer(tsc, READ_Z2);
> +		break;
> +	case 4:
> +		*val = tsc2007_xfer(tsc, (ADC_ON_12BIT | TSC2007_MEASURE_AUX));
> +		break;
> +	case 5: {
> +		struct ts_event tc;
> +
> +		tc.x = tsc2007_xfer(tsc, READ_X);
> +		tc.z1 = tsc2007_xfer(tsc, READ_Z1);
> +		tc.z2 = tsc2007_xfer(tsc, READ_Z2);
> +		*val = tsc2007_calculate_resistance(tsc, &tc);
> +		break;
> +	}
> +	case 6:
> +		*val = tsc2007_is_pen_down(tsc);
> +		break;
> +	case 7:
> +		*val = tsc2007_xfer(tsc,
> +				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP0));
> +		break;
> +	case 8:
> +		*val = tsc2007_xfer(tsc,
> +				    (ADC_ON_12BIT | TSC2007_MEASURE_TEMP1));
> +		break;
> +	}
> +
> +	/* Prepare for next touch reading - power down ADC, enable PENIRQ */
> +	tsc2007_xfer(tsc, PWRDOWN);
> +
> +	mutex_unlock(&tsc->mlock);
> +
> +	ret = IIO_VAL_INT;
> +
> +	return ret;
> +}
> +
> +static const struct iio_info tsc2007_iio_info = {
> +	.read_raw = tsc2007_read_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +int tsc2007_iio_configure(struct tsc2007 *ts)
> +{
> +	int err;
> +	struct iio_dev *indio_dev;
> +	struct tsc2007_iio *iio;
> +
> +	indio_dev = devm_iio_device_alloc(&ts->client->dev,
> +		sizeof(struct tsc2007_iio));
> +	if (!indio_dev) {
> +		dev_err(&ts->client->dev, "iio_device_alloc failed\n");
> +		return -ENOMEM;
> +	}
> +
> +	iio = iio_priv(indio_dev);
> +	iio->ts = ts;
> +	ts->iio_dev = (void *) indio_dev;
> +
> +	indio_dev->name = "tsc2007";
> +	indio_dev->dev.parent = &ts->client->dev;
> +	indio_dev->info = &tsc2007_iio_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = tsc2007_iio_channel;
> +	indio_dev->num_channels = ARRAY_SIZE(tsc2007_iio_channel);
> +
> +	err = iio_device_register(indio_dev);
> +	if (err < 0) {
> +		dev_err(&ts->client->dev, "iio_device_register() failed: %d\n",
> +			err);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(tsc2007_iio_configure);
> +
> +void tsc2007_iio_unconfigure(struct tsc2007 *ts)
> +{
> +	struct iio_dev *indio_dev = ts->iio_dev;
> +
> +	iio_device_unregister(indio_dev);
> +}
> +EXPORT_SYMBOL(tsc2007_iio_unconfigure);
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 2/8] MFD: add STM32 Timers driver
From: Jonathan Cameron @ 2016-12-30 20:38 UTC (permalink / raw)
  To: Benjamin Gaignard, lee.jones, robh+dt, mark.rutland,
	alexandre.torgue, devicetree, linux-kernel, thierry.reding,
	linux-pwm, knaack.h, lars, pmeerw, linux-iio, linux-arm-kernel
  Cc: fabrice.gasnier, gerald.baeza, arnaud.pouliquen, linus.walleij,
	linaro-kernel, Benjamin Gaignard
In-Reply-To: <1481292919-26587-3-git-send-email-benjamin.gaignard@st.com>

On 09/12/16 14:15, Benjamin Gaignard wrote:
> This hardware block could at used at same time for PWM generation
> and IIO timers.
> PWM and IIO timer configuration are mixed in the same registers
> so we need a multi fonction driver to be able to share those registers.
fonction -> function
> 
> version 6:
> - rename files to stm32-timers
> - rename functions to stm32_timers_xxx
> 
> version 5:
> - fix Lee comments about detect function
> - add missing dependency on REGMAP_MMIO
> 
> version 4:
> - add a function to detect Auto Reload Register (ARR) size
> - rename the structure shared with other drivers
> 
> version 2:
> - rename driver "stm32-gptimer" to be align with SoC documentation
> - only keep one compatible
> - use of_platform_populate() instead of devm_mfd_add_devices()
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  drivers/mfd/Kconfig              | 11 ++++++
>  drivers/mfd/Makefile             |  2 +
>  drivers/mfd/stm32-timers.c       | 80 ++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/stm32-timers.h | 71 +++++++++++++++++++++++++++++++++++
>  4 files changed, 164 insertions(+)
>  create mode 100644 drivers/mfd/stm32-timers.c
>  create mode 100644 include/linux/mfd/stm32-timers.h
> 
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index c6df644..4ec1906 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -1607,6 +1607,17 @@ config MFD_STW481X
>  	  in various ST Microelectronics and ST-Ericsson embedded
>  	  Nomadik series.
>  
> +config MFD_STM32_TIMERS
> +	tristate "Support for STM32 Timers"
> +	depends on (ARCH_STM32 && OF) || COMPILE_TEST
> +	select MFD_CORE
> +	select REGMAP
> +	select REGMAP_MMIO
> +	help
> +	  Select this option to enable STM32 timers driver used
> +	  for PWM and IIO Timer. This driver allow to share the
> +	  registers between the others drivers.
> +
>  menu "Multimedia Capabilities Port drivers"
>  	depends on ARCH_SA1100
>  
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 9834e66..11a52f8 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -211,3 +211,5 @@ obj-$(CONFIG_INTEL_SOC_PMIC)	+= intel-soc-pmic.o
>  obj-$(CONFIG_MFD_MT6397)	+= mt6397-core.o
>  
>  obj-$(CONFIG_MFD_ALTERA_A10SR)	+= altera-a10sr.o
> +
> +obj-$(CONFIG_MFD_STM32_TIMERS) 	+= stm32-timers.o
> diff --git a/drivers/mfd/stm32-timers.c b/drivers/mfd/stm32-timers.c
> new file mode 100644
> index 0000000..68d115e
> --- /dev/null
> +++ b/drivers/mfd/stm32-timers.c
> @@ -0,0 +1,80 @@
> +/*
> + * Copyright (C) STMicroelectronics 2016
> + *
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/mfd/stm32-timers.h>
> +#include <linux/module.h>
> +#include <linux/of_platform.h>
> +#include <linux/reset.h>
> +
> +static const struct regmap_config stm32_timers_regmap_cfg = {
> +	.reg_bits = 32,
> +	.val_bits = 32,
> +	.reg_stride = sizeof(u32),
> +	.max_register = 0x400,
> +};
> +
> +static void stm32_timers_get_arr_size(struct stm32_timers *ddata)
> +{
> +	/*
> +	 * Only the available bits will be written so when readback
> +	 * we get the maximum value of auto reload register
> +	 */
> +	regmap_write(ddata->regmap, TIM_ARR, ~0L);
> +	regmap_read(ddata->regmap, TIM_ARR, &ddata->max_arr);
> +	regmap_write(ddata->regmap, TIM_ARR, 0x0);
> +}
> +
> +static int stm32_timers_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct stm32_timers *ddata;
> +	struct resource *res;
> +	void __iomem *mmio;
> +
> +	ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
> +	if (!ddata)
> +		return -ENOMEM;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	mmio = devm_ioremap_resource(dev, res);
> +	if (IS_ERR(mmio))
> +		return PTR_ERR(mmio);
> +
> +	ddata->regmap = devm_regmap_init_mmio_clk(dev, "clk_int", mmio,
> +						  &stm32_timers_regmap_cfg);
> +	if (IS_ERR(ddata->regmap))
> +		return PTR_ERR(ddata->regmap);
> +
> +	ddata->clk = devm_clk_get(dev, NULL);
> +	if (IS_ERR(ddata->clk))
> +		return PTR_ERR(ddata->clk);
> +
> +	stm32_timers_get_arr_size(ddata);
> +
> +	platform_set_drvdata(pdev, ddata);
> +
> +	return of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
> +}
> +
> +static const struct of_device_id stm32_timers_of_match[] = {
> +	{ .compatible = "st,stm32-timers", },
> +	{ /* end node */ },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_timers_of_match);
> +
> +static struct platform_driver stm32_timers_driver = {
> +	.probe = stm32_timers_probe,
> +	.driver	= {
> +		.name = "stm32-timers",
> +		.of_match_table = stm32_timers_of_match,
> +	},
> +};
> +module_platform_driver(stm32_timers_driver);
> +
> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timers");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mfd/stm32-timers.h b/include/linux/mfd/stm32-timers.h
> new file mode 100644
> index 0000000..d030004
> --- /dev/null
> +++ b/include/linux/mfd/stm32-timers.h
> @@ -0,0 +1,71 @@
> +/*
> + * Copyright (C) STMicroelectronics 2016
> + *
> + * Author: Benjamin Gaignard <benjamin.gaignard@st.com>
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#ifndef _LINUX_STM32_GPTIMER_H_
> +#define _LINUX_STM32_GPTIMER_H_
> +
> +#include <linux/clk.h>
> +#include <linux/regmap.h>
> +
> +#define TIM_CR1		0x00	/* Control Register 1      */
> +#define TIM_CR2		0x04	/* Control Register 2      */
> +#define TIM_SMCR	0x08	/* Slave mode control reg  */
> +#define TIM_DIER	0x0C	/* DMA/interrupt register  */
> +#define TIM_SR		0x10	/* Status register	   */
> +#define TIM_EGR		0x14	/* Event Generation Reg    */
> +#define TIM_CCMR1	0x18	/* Capt/Comp 1 Mode Reg    */
> +#define TIM_CCMR2	0x1C	/* Capt/Comp 2 Mode Reg    */
> +#define TIM_CCER	0x20	/* Capt/Comp Enable Reg    */
> +#define TIM_PSC		0x28	/* Prescaler               */
> +#define TIM_ARR		0x2c	/* Auto-Reload Register    */
> +#define TIM_CCR1	0x34	/* Capt/Comp Register 1    */
> +#define TIM_CCR2	0x38	/* Capt/Comp Register 2    */
> +#define TIM_CCR3	0x3C	/* Capt/Comp Register 3    */
> +#define TIM_CCR4	0x40	/* Capt/Comp Register 4    */
> +#define TIM_BDTR	0x44	/* Break and Dead-Time Reg */
> +
> +#define TIM_CR1_CEN	BIT(0)	/* Counter Enable	   */
> +#define TIM_CR1_ARPE	BIT(7)	/* Auto-reload Preload Ena */
> +#define TIM_CR2_MMS	(BIT(4) | BIT(5) | BIT(6)) /* Master mode selection */
> +#define TIM_SMCR_SMS	(BIT(0) | BIT(1) | BIT(2)) /* Slave mode selection */
> +#define TIM_SMCR_TS	(BIT(4) | BIT(5) | BIT(6)) /* Trigger selection */
> +#define TIM_DIER_UIE	BIT(0)	/* Update interrupt	   */
> +#define TIM_SR_UIF	BIT(0)	/* Update interrupt flag   */
> +#define TIM_EGR_UG	BIT(0)	/* Update Generation       */
> +#define TIM_CCMR_PE	BIT(3)	/* Channel Preload Enable  */
> +#define TIM_CCMR_M1	(BIT(6) | BIT(5))  /* Channel PWM Mode 1 */
> +#define TIM_CCER_CC1E	BIT(0)	/* Capt/Comp 1  out Ena    */
> +#define TIM_CCER_CC1P	BIT(1)	/* Capt/Comp 1  Polarity   */
> +#define TIM_CCER_CC1NE	BIT(2)	/* Capt/Comp 1N out Ena    */
> +#define TIM_CCER_CC1NP	BIT(3)	/* Capt/Comp 1N Polarity   */
> +#define TIM_CCER_CC2E	BIT(4)	/* Capt/Comp 2  out Ena    */
> +#define TIM_CCER_CC3E	BIT(8)	/* Capt/Comp 3  out Ena    */
> +#define TIM_CCER_CC4E	BIT(12)	/* Capt/Comp 4  out Ena    */
> +#define TIM_CCER_CCXE	(BIT(0) | BIT(4) | BIT(8) | BIT(12))
> +#define TIM_BDTR_BKE	BIT(12) /* Break input enable	   */
> +#define TIM_BDTR_BKP	BIT(13) /* Break input polarity	   */
> +#define TIM_BDTR_AOE	BIT(14)	/* Automatic Output Enable */
> +#define TIM_BDTR_MOE	BIT(15)	/* Main Output Enable      */
> +#define TIM_BDTR_BKF	(BIT(16) | BIT(17) | BIT(18) | BIT(19))
> +#define TIM_BDTR_BK2F	(BIT(20) | BIT(21) | BIT(22) | BIT(23))
> +#define TIM_BDTR_BK2E	BIT(24) /* Break 2 input enable	   */
> +#define TIM_BDTR_BK2P	BIT(25) /* Break 2 input polarity  */
> +
> +#define MAX_TIM_PSC		0xFFFF
> +#define TIM_CR2_MMS_SHIFT	4
> +#define TIM_SMCR_TS_SHIFT	4
> +#define TIM_BDTR_BKF_MASK	0xF
> +#define TIM_BDTR_BKF_SHIFT	16
> +#define TIM_BDTR_BK2F_SHIFT	20
> +
> +struct stm32_timers {
> +	struct clk *clk;
> +	struct regmap *regmap;
> +	u32 max_arr;
> +};
> +#endif
> 

^ permalink raw reply

* Re: [PATCH] iio: adc: Add Renesas GyroADC driver
From: Marek Vasut @ 2016-12-30 20:52 UTC (permalink / raw)
  To: Peter Meerwald-Stadler
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	Simon Horman
In-Reply-To: <alpine.DEB.2.02.1612302027480.3977-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>

On 12/30/2016 08:50 PM, Peter Meerwald-Stadler wrote:
> 
>> Add IIO driver for the Renesas RCar GyroADC block. This block is a
>> simple 4/8-channel ADC which samples 12/15/24 bits of data every
>> cycle from all channels.
> 
> comments below
>  
>> Signed-off-by: Marek Vasut <marek.vasut-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> Cc: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
>> Cc: Simon Horman <horms+renesas-/R6kz+dDXgpPR4JQBCEnsQ@public.gmane.org>
>> ---
>>  .../bindings/iio/adc/renesas,gyroadc.txt           |  38 +++
>>  MAINTAINERS                                        |   6 +
>>  drivers/iio/adc/Kconfig                            |  10 +
>>  drivers/iio/adc/Makefile                           |   1 +
>>  drivers/iio/adc/rcar_gyro_adc.c                    | 379 +++++++++++++++++++++
>>  5 files changed, 434 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>>  create mode 100644 drivers/iio/adc/rcar_gyro_adc.c
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>> new file mode 100644
>> index 0000000..3fd5f57
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/renesas,gyroadc.txt
>> @@ -0,0 +1,38 @@
>> +* Renesas RCar GyroADC device driver
>> +
>> +Required properties:
>> +- compatible:	Should be "renesas,rcar-gyroadc" for regular GyroADC or
>> +		"renesas,rcar-gyroadc-r8a7792" for GyroADC without interrupt
>> +		block found in R8A7792.
>> +- reg:		Address and length of the register set for the device
>> +- clocks:	References to all the clocks specified in the clock-names
>> +		property as specified in
>> +		Documentation/devicetree/bindings/clock/clock-bindings.txt.
>> +- clock-names:	Shall contain "fck" and "if". The "fck" are the GyroADC block
> 
> "fck" is...
> 
>> +		clock, the "if" are the interface clock.
> 
> "if" is ...
> 
>> +		power-domains = <&sysc R8A7791_PD_ALWAYS_ON>;
> 
> this is just an example and not appropriate here?

Oh, copy-paste error, thanks :)

>> +- power-domains: Must contain a reference to the PM domain, if available.
>> +- renesas,gyroadc-mode:	GyroADC mode of operation, must be either of:
>> +			1 - MB88101A mode, 12bit sampling, 4 channels
>> +			2 - ADCS7476 mode, 15bit sampling, 8 channels
>> +			3 - MAX1162 mode,  16bit sampling, 8 channels
>> +- renesas,gyroadc-vref:	Array of reference voltage values for each input to
>> +			the GyroADC, in uV. Array must have 4 elemenets for
> 
> elements

All spelling fixed.

[...]

>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 99c0514..4a4cac7 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -408,6 +408,16 @@ config QCOM_SPMI_VADC
>>  	  To compile this driver as a module, choose M here: the module will
>>  	  be called qcom-spmi-vadc.
>>  
>> +config RCAR_GYRO_ADC
>> +	tristate "Renesas RCAR GyroADC driver"
>> +	depends on ARCH_RCAR_GEN2 || (ARM && COMPILE_TEST)
>> +	help
>> +	  Say yes here to build support for the GyroADC found in Renesas
>> +	  RCar Gen2 SoCs.
>> +
>> +	  To compile this driver as a module, choose M here: the
>> +	  module will be called rcar-gyroadc.
> 
> called rcar_gyro_adc?

Why so ? The driver is really named rcar-gyroadc , I guess I should
rename either the file or the driver to keep things consistent. So
probably the file .

>> +
>>  config ROCKCHIP_SARADC
>>  	tristate "Rockchip SARADC driver"
>>  	depends on ARCH_ROCKCHIP || (ARM && COMPILE_TEST)
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile

[...]

>> +
>> +#define RCAR_GYROADC_CLOCK_LENGTH		0x08
>> +#define RCAR_GYROADC_1_25MS_LENGTH		0x0c
>> +
>> +#define RCAR_GYROADC_REALTIME_DATA(ch)		(0x10 + ((ch) * 4))
>> +#define RCAR_GYROADC_100MS_ADDED_DATA(ch)	(0x30 + ((ch) * 4))
>> +#define RCAR_GYROADC_10MS_AVG_DATA(ch)		(0x50 + ((ch) * 4))
>> +
>> +#define RCAR_GYROADC_FIFO_STATUS		0x70
>> +#define RCAR_GYROADC_FIFO_STATUS_EMPTY(ch)	BIT(0 + (4 * (ch)))
> 
> FIFO_STATUS_... is not used (yet)

Is it a problem to have a complete register layout here ?

> 4*ch looks suspicious for ch==8??

Well yes, channel is in range 0..7 :)

>> +#define RCAR_GYROADC_FIFO_STATUS_FULL(ch)	BIT(1 + (4 * (ch)))
>> +#define RCAR_GYROADC_FIFO_STATUS_ERROR(ch)	BIT(2 + (4 * (ch)))
>> +
>> +#define RCAR_GYROADC_INTR			0x74
>> +#define RCAR_GYROADC_INTR_INT			BIT(0)
>> +
>> +#define RCAR_GYROADC_INTENR			0x78
>> +#define RCAR_GYROADC_INTENR_INTEN		BIT(0)
>> +
>> +#define RCAR_GYROADC_SAMPLE_RATE		800	/* Hz */

[...]

>> +#define RCAR_GYROADC_CHAN(_idx, _chan_type, _realbits) {	\
>> +	.type			= (_chan_type),			\
> 
> _chan_type is IIO_VOLTAGE always?

Yep, fixed.

>> +	.indexed		= 1,				\
>> +	.channel		= (_idx),			\
>> +	.info_mask_separate	= BIT(IIO_CHAN_INFO_RAW),	\
>> +	.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE) |	\
>> +				    BIT(IIO_CHAN_INFO_SAMP_FREQ), \
>> +	.scan_index		= (_idx),			\
> 
> no buffered mode yet, so strictly no need for a scan_index and scan_type

OK

>> +	.scan_type	= {					\
>> +		.sign		= 'u',				\
>> +		.realbits	= (_realbits),			\
>> +		.storagebits	= 16,				\
>> +	},							\
>> +}
>> +
>> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_1[] = {
>> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 12),
>> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 12),
>> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 12),
>> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 12),
>> +};
>> +
>> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_2[] = {
>> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 15),
>> +	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 15),
>> +};
>> +
>> +/*
>> + * NOTE: The data we receive in mode 3 from MAX1162 have MSByte = 0,
>> + *       therefore we only use 16bit realbits here instead of 24.
>> + */
>> +static const struct iio_chan_spec rcar_gyroadc_iio_channels_3[] = {
>> +	RCAR_GYROADC_CHAN(0, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(1, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(2, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(3, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(4, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(5, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(6, IIO_VOLTAGE, 16),
>> +	RCAR_GYROADC_CHAN(7, IIO_VOLTAGE, 16),
>> +};
>> +
>> +static int rcar_gyroadc_read_raw(struct iio_dev *indio_dev,
>> +				 struct iio_chan_spec const *chan,
>> +				 int *val, int *val2, long mask)
>> +{
>> +	struct rcar_gyroadc *priv = iio_priv(indio_dev);
>> +	unsigned int datareg = RCAR_GYROADC_REALTIME_DATA(chan->channel);
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		if (chan->type != IIO_VOLTAGE)
>> +			return -EINVAL;
>> +
>> +		if (iio_buffer_enabled(indio_dev))
>> +			return -EBUSY;
> 
> use iio_device_claim_direct_mode()

Why ? Do I really need the mutex locking here ?

>> +
>> +		*val = readl(priv->regs + datareg);
>> +		*val &= BIT(chan->scan_type.realbits) - 1;
>> +
>> +		return IIO_VAL_INT;
>> +	case IIO_CHAN_INFO_SCALE:
>> +		*val = 0;
>> +		*val2 = (priv->vref_uv[chan->channel] * 1000) / 0x10000;
>> +		return IIO_VAL_INT_PLUS_NANO;
>> +	case IIO_CHAN_INFO_SAMP_FREQ:
>> +		*val = RCAR_GYROADC_SAMPLE_RATE;
>> +		*val2 = 0;
> 
> *val2 = 0 not needed

OK

[...]

>> +	indio_dev->name = dev_name(dev);
>> +	indio_dev->dev.parent = dev;
>> +	indio_dev->dev.of_node = pdev->dev.of_node;
>> +	indio_dev->info = &rcar_gyroadc_iio_info;
>> +	indio_dev->modes = INDIO_DIRECT_MODE;
>> +	if (mode == 1) {
> 
> maybe do the mode differentiation only once, any with a switch?

Done

[...]

-- 
Best regards,
Marek Vasut

^ permalink raw reply

* Re: [PATCH] iio: adc: Add Renesas GyroADC driver
From: Peter Meerwald-Stadler @ 2016-12-30 21:09 UTC (permalink / raw)
  To: Marek Vasut
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	Simon Horman
In-Reply-To: <442f8085-110c-659d-d097-add788b8f291-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>


> >> Add IIO driver for the Renesas RCar GyroADC block. This block is a
> >> simple 4/8-channel ADC which samples 12/15/24 bits of data every

> >> +		if (iio_buffer_enabled(indio_dev))
> >> +			return -EBUSY;
> > 
> > use iio_device_claim_direct_mode()
> 
> Why ? Do I really need the mutex locking here ?

no, not needed, since you do not support buffered mode yet; but neither do 
you need iio_buffer_enabled() 

p.

-- 

Peter Meerwald-Stadler
+43-664-2444418 (mobile)
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v6 6/8] IIO: add STM32 timer trigger driver
From: Jonathan Cameron @ 2016-12-30 21:12 UTC (permalink / raw)
  To: Benjamin Gaignard, lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	alexandre.torgue-qxv4g6HH51o, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	thierry.reding-Re5JQEeQqe8AvxtiuMwx3w,
	linux-pwm-u79uwXL29TY76Z2rM5mHXA, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
  Cc: fabrice.gasnier-qxv4g6HH51o, gerald.baeza-qxv4g6HH51o,
	arnaud.pouliquen-qxv4g6HH51o,
	linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
	linaro-kernel-cunTk1MwBs8s++Sfvej+rw, Benjamin Gaignard
In-Reply-To: <1481292919-26587-7-git-send-email-benjamin.gaignard-qxv4g6HH51o@public.gmane.org>

On 09/12/16 14:15, Benjamin Gaignard wrote:
> Timers IPs can be used to generate triggers for other IPs like
> DAC, ADC or other timers.
> Each trigger may result of timer internals signals like counter enable,
> reset or edge, this configuration could be done through "master_mode"
> device attribute.
> 
> A timer device could be triggered by other timers, we use the trigger
> name and is_stm32_iio_timer_trigger() function to distinguish them
> and configure IP input switch.
> 
> Timer may also decide on which event (edge, level) they could
> be activated by a trigger, this configuration is done by writing in
> "slave_mode" device attribute.
> 
> Since triggers could also be used by DAC or ADC their names are defined
> in include/ nux/iio/timer/stm32-timer-trigger.h so those IPs will be able
> to configure themselves in valid_trigger function
> 
> Trigger have a "sampling_frequency" attribute which allow to configure
> timer sampling frequency without using PWM interface
> 
> version 5:
> - simplify tables of triggers
> - only create an IIO device when needed
> 
> version 4:
> - get triggers configuration from "reg" in DT
> - add tables of triggers
> - sampling frequency is enable/disable when writing in trigger
>   sampling_frequency attribute
> - no more use of interruptions
> 
> version 3:
> - change compatible to "st,stm32-timer-trigger"
> - fix attributes access right
> - use string instead of int for master_mode and slave_mode
> - document device attributes in sysfs-bus-iio-timer-stm32
> 
> version 2:
> - keep only one compatible
> - use st,input-triggers-names and st,output-triggers-names
>   to know which triggers are accepted and/or create by the device
Firstly, sorry it has taken me so long to get back to this.

I'm still not keen on this use of iio_device elements just to act as
glue between triggers.  I think we need to work out a more light weight
way to do this.  As you are only using them for validation and to provide
somewhere to hang the control attibutes off, there is nothing stopping us
moving that over to the iio_trigger instead which would avoid the messy
duality going on here.

I might still be missing something though!

You would only I think need 3 attributes

parrent_trigger
and something like your master_mode and slave_mode attributes.

The parrent_trigger would need some validation etc, but if we keep it
within this driver initially that won't be hard to do. Checking the device
parent matches will do most of it.

Jonathan
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> ---
>  .../ABI/testing/sysfs-bus-iio-timer-stm32          |  55 +++
>  drivers/iio/Kconfig                                |   2 +-
>  drivers/iio/Makefile                               |   1 +
>  drivers/iio/timer/Kconfig                          |  13 +
>  drivers/iio/timer/Makefile                         |   1 +
>  drivers/iio/timer/stm32-timer-trigger.c            | 466 +++++++++++++++++++++
>  drivers/iio/trigger/Kconfig                        |   1 -
>  include/linux/iio/timer/stm32-timer-trigger.h      |  62 +++
>  8 files changed, 599 insertions(+), 2 deletions(-)
>  create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
>  create mode 100644 drivers/iio/timer/Kconfig
>  create mode 100644 drivers/iio/timer/Makefile
>  create mode 100644 drivers/iio/timer/stm32-timer-trigger.c
>  create mode 100644 include/linux/iio/timer/stm32-timer-trigger.h
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32 b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> new file mode 100644
> index 0000000..26583dd
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-iio-timer-stm32
> @@ -0,0 +1,55 @@
> +What:		/sys/bus/iio/devices/iio:deviceX/master_mode_available
> +KernelVersion:	4.10
> +Contact:	benjamin.gaignard-qxv4g6HH51o@public.gmane.org
> +Description:
> +		Reading returns the list possible master modes which are:
> +		- "reset"     :	The UG bit from the TIMx_EGR register is used as trigger output (TRGO).
> +		- "enable"    : The Counter Enable signal CNT_EN is used as trigger output.
> +		- "update"    : The update event is selected as trigger output.
> +				For instance a master timer can then be used as a prescaler for a slave timer.
> +		- "compare_pulse" : The trigger output send a positive pulse when the CC1IF flag is to be set.
> +		- "OC1REF"    : OC1REF signal is used as trigger output.
> +		- "OC2REF"    : OC2REF signal is used as trigger output.
> +		- "OC3REF"    : OC3REF signal is used as trigger output.
> +		- "OC4REF"    : OC4REF signal is used as trigger output.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/master_mode
> +KernelVersion:	4.10
> +Contact:	benjamin.gaignard-qxv4g6HH51o@public.gmane.org
> +Description:
> +		Reading returns the current master modes.
> +		Writing set the master mode
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/slave_mode_available
> +KernelVersion:	4.10
> +Contact:	benjamin.gaignard-qxv4g6HH51o@public.gmane.org
> +Description:
> +		Reading returns the list possible slave modes which are:
> +		- "disabled"  : The prescaler is clocked directly by the internal clock.
> +		- "encoder_1" : Counter counts up/down on TI2FP1 edge depending on TI1FP2 level.
> +		- "encoder_2" : Counter counts up/down on TI1FP2 edge depending on TI2FP1 level.
> +		- "encoder_3" : Counter counts up/down on both TI1FP1 and TI2FP2 edges depending
> +				on the level of the other input.
> +		- "reset"     : Rising edge of the selected trigger input reinitializes the counter
> +				and generates an update of the registers.
> +		- "gated"     : The counter clock is enabled when the trigger input is high.
> +				The counter stops (but is not reset) as soon as the trigger becomes low.
> +				Both start and stop of the counter are controlled.
> +		- "trigger"   : The counter starts at a rising edge of the trigger TRGI (but it is not
> +				reset). Only the start of the counter is controlled.
> +		- "external_clock": Rising edges of the selected trigger (TRGI) clock the counter.
> +
> +What:		/sys/bus/iio/devices/iio:deviceX/slave_mode
> +KernelVersion:	4.10
> +Contact:	benjamin.gaignard-qxv4g6HH51o@public.gmane.org
> +Description:
> +		Reading returns the current slave mode.
> +		Writing set the slave mode
> +
> +What:		/sys/bus/iio/devices/triggerX/sampling_frequency
> +KernelVersion:	4.10
> +Contact:	benjamin.gaignard-qxv4g6HH51o@public.gmane.org
> +Description:
> +		Reading returns the current sampling frequency.
> +		Writing an value different of 0 set and start sampling.
> +		Writing 0 stop sampling.
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 6743b18..2de2a80 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -90,5 +90,5 @@ source "drivers/iio/potentiometer/Kconfig"
>  source "drivers/iio/pressure/Kconfig"
>  source "drivers/iio/proximity/Kconfig"
>  source "drivers/iio/temperature/Kconfig"
> -
> +source "drivers/iio/timer/Kconfig"
>  endif # IIO
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 87e4c43..b797c08 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -32,4 +32,5 @@ obj-y += potentiometer/
>  obj-y += pressure/
>  obj-y += proximity/
>  obj-y += temperature/
> +obj-y += timer/
>  obj-y += trigger/
> diff --git a/drivers/iio/timer/Kconfig b/drivers/iio/timer/Kconfig
> new file mode 100644
> index 0000000..e3c21f2
> --- /dev/null
> +++ b/drivers/iio/timer/Kconfig
> @@ -0,0 +1,13 @@
> +#
> +# Timers drivers
> +
> +menu "Timers"
> +
> +config IIO_STM32_TIMER_TRIGGER
> +	tristate "STM32 Timer Trigger"
> +	depends on (ARCH_STM32 && OF && MFD_STM32_TIMERS) || COMPILE_TEST
> +	select IIO_TRIGGERED_EVENT
> +	help
> +	  Select this option to enable STM32 Timer Trigger
> +
> +endmenu
> diff --git a/drivers/iio/timer/Makefile b/drivers/iio/timer/Makefile
> new file mode 100644
> index 0000000..4ad95ec9
> --- /dev/null
> +++ b/drivers/iio/timer/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_IIO_STM32_TIMER_TRIGGER) += stm32-timer-trigger.o
> diff --git a/drivers/iio/timer/stm32-timer-trigger.c b/drivers/iio/timer/stm32-timer-trigger.c
> new file mode 100644
> index 0000000..8d16e8f
> --- /dev/null
> +++ b/drivers/iio/timer/stm32-timer-trigger.c
> @@ -0,0 +1,466 @@
> +/*
> + * Copyright (C) STMicroelectronics 2016
> + *
> + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/iio/timer/stm32-timer-trigger.h>
> +#include <linux/iio/trigger.h>
> +#include <linux/iio/triggered_event.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/stm32-timers.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +
> +#define MAX_TRIGGERS 6
> +#define MAX_VALIDS 5
> +
> +/* List the triggers created by each timer */
> +static const void *triggers_table[][MAX_TRIGGERS] = {
> +	{ TIM1_TRGO, TIM1_CH1, TIM1_CH2, TIM1_CH3, TIM1_CH4,},
> +	{ TIM2_TRGO, TIM2_CH1, TIM2_CH2, TIM2_CH3, TIM2_CH4,},
> +	{ TIM3_TRGO, TIM3_CH1, TIM3_CH2, TIM3_CH3, TIM3_CH4,},
> +	{ TIM4_TRGO, TIM4_CH1, TIM4_CH2, TIM4_CH3, TIM4_CH4,},
> +	{ TIM5_TRGO, TIM5_CH1, TIM5_CH2, TIM5_CH3, TIM5_CH4,},
> +	{ TIM6_TRGO,},
> +	{ TIM7_TRGO,},
> +	{ TIM8_TRGO, TIM8_CH1, TIM8_CH2, TIM8_CH3, TIM8_CH4,},
> +	{ TIM9_TRGO, TIM9_CH1, TIM9_CH2,},
> +	{ TIM12_TRGO, TIM12_CH1, TIM12_CH2,},
> +};
> +
> +/* List the triggers accepted by each timer */
> +static const void *valids_table[][MAX_VALIDS] = {
> +	{ TIM5_TRGO, TIM2_TRGO, TIM4_TRGO, TIM3_TRGO,},
> +	{ TIM1_TRGO, TIM8_TRGO, TIM3_TRGO, TIM4_TRGO,},
> +	{ TIM1_TRGO, TIM8_TRGO, TIM5_TRGO, TIM4_TRGO,},
> +	{ TIM1_TRGO, TIM2_TRGO, TIM3_TRGO, TIM8_TRGO,},
> +	{ TIM2_TRGO, TIM3_TRGO, TIM4_TRGO, TIM8_TRGO,},
> +	{ }, /* timer 6 */
> +	{ }, /* timer 7 */
> +	{ TIM1_TRGO, TIM2_TRGO, TIM4_TRGO, TIM5_TRGO,},
> +	{ TIM2_TRGO, TIM3_TRGO,},
> +	{ TIM4_TRGO, TIM5_TRGO,},
> +};
> +
> +struct stm32_timer_trigger {
> +	struct device *dev;
> +	struct regmap *regmap;
> +	struct clk *clk;
> +	u32 max_arr;
> +	const void *triggers;
> +	const void *valids;
> +};
> +
> +static int stm32_timer_start(struct stm32_timer_trigger *priv,
> +			     unsigned int frequency)
> +{
> +	unsigned long long prd, div;
> +	int prescaler = 0;
> +	u32 ccer, cr1;
> +
> +	/* Period and prescaler values depends of clock rate */
> +	div = (unsigned long long)clk_get_rate(priv->clk);
> +
> +	do_div(div, frequency);
> +
> +	prd = div;
> +
> +	/*
> +	 * Increase prescaler value until we get a result that fit
> +	 * with auto reload register maximum value.
> +	 */
> +	while (div > priv->max_arr) {
> +		prescaler++;
> +		div = prd;
> +		do_div(div, (prescaler + 1));
> +	}
> +	prd = div;
> +
> +	if (prescaler > MAX_TIM_PSC) {
> +		dev_err(priv->dev, "prescaler exceeds the maximum value\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check if nobody else use the timer */
> +	regmap_read(priv->regmap, TIM_CCER, &ccer);
> +	if (ccer & TIM_CCER_CCXE)
> +		return -EBUSY;
> +
> +	regmap_read(priv->regmap, TIM_CR1, &cr1);
> +	if (!(cr1 & TIM_CR1_CEN))
> +		clk_enable(priv->clk);
> +
> +	regmap_write(priv->regmap, TIM_PSC, prescaler);
> +	regmap_write(priv->regmap, TIM_ARR, prd - 1);
> +	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_ARPE, TIM_CR1_ARPE);
> +
> +	/* Force master mode to update mode */
> +	regmap_update_bits(priv->regmap, TIM_CR2, TIM_CR2_MMS, 0x20);
> +
> +	/* Make sure that registers are updated */
> +	regmap_update_bits(priv->regmap, TIM_EGR, TIM_EGR_UG, TIM_EGR_UG);
> +
> +	/* Enable controller */
> +	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, TIM_CR1_CEN);
> +
> +	return 0;
> +}
> +
> +static void stm32_timer_stop(struct stm32_timer_trigger *priv)
> +{
> +	u32 ccer, cr1;
> +
> +	regmap_read(priv->regmap, TIM_CCER, &ccer);
> +	if (ccer & TIM_CCER_CCXE)
> +		return;
> +
> +	regmap_read(priv->regmap, TIM_CR1, &cr1);
> +	if (cr1 & TIM_CR1_CEN)
> +		clk_disable(priv->clk);
> +
> +	/* Stop timer */
> +	regmap_update_bits(priv->regmap, TIM_CR1, TIM_CR1_CEN, 0);
> +	regmap_write(priv->regmap, TIM_PSC, 0);
> +	regmap_write(priv->regmap, TIM_ARR, 0);
> +}
> +
> +static ssize_t stm32_tt_store_frequency(struct device *dev,
> +					struct device_attribute *attr,
> +					const char *buf, size_t len)
> +{
> +	struct iio_trigger *trig = to_iio_trigger(dev);
> +	struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig);
> +	unsigned int freq;
> +	int ret;
> +
> +	ret = kstrtouint(buf, 10, &freq);
> +	if (ret)
> +		return ret;
> +
> +	if (freq == 0) {
> +		stm32_timer_stop(priv);
> +	} else {
> +		ret = stm32_timer_start(priv, freq);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return len;
> +}
> +
> +static ssize_t stm32_tt_read_frequency(struct device *dev,
> +				       struct device_attribute *attr, char *buf)
> +{
> +	struct iio_trigger *trig = to_iio_trigger(dev);
> +	struct stm32_timer_trigger *priv = iio_trigger_get_drvdata(trig);
> +	u32 psc, arr, cr1;
> +	unsigned long long freq = 0;
> +
> +	regmap_read(priv->regmap, TIM_CR1, &cr1);
> +	regmap_read(priv->regmap, TIM_PSC, &psc);
> +	regmap_read(priv->regmap, TIM_ARR, &arr);
> +
> +	if (psc && arr && (cr1 & TIM_CR1_CEN)) {
> +		freq = (unsigned long long)clk_get_rate(priv->clk);
> +		do_div(freq, psc);
> +		do_div(freq, arr);
> +	}
> +
> +	return sprintf(buf, "%d\n", (unsigned int)freq);
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ(0660,
> +			      stm32_tt_read_frequency,
> +			      stm32_tt_store_frequency);
> +
> +static struct attribute *stm32_trigger_attrs[] = {
> +	&iio_dev_attr_sampling_frequency.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group stm32_trigger_attr_group = {
> +	.attrs = stm32_trigger_attrs,
> +};
> +
> +static const struct attribute_group *stm32_trigger_attr_groups[] = {
> +	&stm32_trigger_attr_group,
> +	NULL,
> +};
> +
> +static char *master_mode_table[] = {
> +	"reset",
> +	"enable",
> +	"update",
> +	"compare_pulse",
> +	"OC1REF",
> +	"OC2REF",
> +	"OC3REF",
> +	"OC4REF"
> +};
> +
> +static ssize_t stm32_tt_show_master_mode(struct device *dev,
> +					 struct device_attribute *attr,
> +					 char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 cr2;
> +
> +	regmap_read(priv->regmap, TIM_CR2, &cr2);
> +	cr2 = (cr2 & TIM_CR2_MMS) >> TIM_CR2_MMS_SHIFT;
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", master_mode_table[cr2]);
> +}
> +
> +static ssize_t stm32_tt_store_master_mode(struct device *dev,
> +					  struct device_attribute *attr,
> +					  const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(master_mode_table); i++) {
> +		if (!strncmp(master_mode_table[i], buf,
> +			     strlen(master_mode_table[i]))) {
> +			regmap_update_bits(priv->regmap, TIM_CR2,
> +					   TIM_CR2_MMS, i << TIM_CR2_MMS_SHIFT);
> +			return len;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static IIO_CONST_ATTR(master_mode_available,
> +	"reset enable update compare_pulse OC1REF OC2REF OC3REF OC4REF");
> +
> +static IIO_DEVICE_ATTR(master_mode, 0660,
> +		       stm32_tt_show_master_mode,
> +		       stm32_tt_store_master_mode,
> +		       0);
> +
> +static char *slave_mode_table[] = {
> +	"disabled",
> +	"encoder_1",
> +	"encoder_2",
> +	"encoder_3",
> +	"reset",
> +	"gated",
> +	"trigger",
> +	"external_clock",
> +};
> +
> +static ssize_t stm32_tt_show_slave_mode(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	u32 smcr;
> +
> +	regmap_read(priv->regmap, TIM_SMCR, &smcr);
> +	smcr &= TIM_SMCR_SMS;
> +
> +	return snprintf(buf, PAGE_SIZE, "%s\n", slave_mode_table[smcr]);
> +}
> +
> +static ssize_t stm32_tt_store_slave_mode(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t len)
> +{
> +	struct iio_dev *indio_dev = dev_to_iio_dev(dev);
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(slave_mode_table); i++) {
> +		if (!strncmp(slave_mode_table[i], buf,
> +			     strlen(slave_mode_table[i]))) {
> +			regmap_update_bits(priv->regmap,
> +					   TIM_SMCR, TIM_SMCR_SMS, i);
> +			return len;
> +		}
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static IIO_CONST_ATTR(slave_mode_available,
> +"disabled encoder_1 encoder_2 encoder_3 reset gated trigger external_clock");
> +
> +static IIO_DEVICE_ATTR(slave_mode, 0660,
> +		       stm32_tt_show_slave_mode,
> +		       stm32_tt_store_slave_mode,
> +		       0);
> +
> +static struct attribute *stm32_timer_attrs[] = {
> +	&iio_dev_attr_master_mode.dev_attr.attr,
> +	&iio_const_attr_master_mode_available.dev_attr.attr,
> +	&iio_dev_attr_slave_mode.dev_attr.attr,
> +	&iio_const_attr_slave_mode_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group stm32_timer_attr_group = {
> +	.attrs = stm32_timer_attrs,
> +};
> +
> +static const struct iio_trigger_ops timer_trigger_ops = {
> +	.owner = THIS_MODULE,
> +};
> +
> +static int stm32_setup_iio_triggers(struct stm32_timer_trigger *priv)
> +{
> +	int ret;
> +	const char * const *cur = priv->triggers;
> +
> +	while (cur && *cur) {
> +		struct iio_trigger *trig;
> +
> +		trig = devm_iio_trigger_alloc(priv->dev, "%s", *cur);
> +		if  (!trig)
> +			return -ENOMEM;
> +
> +		trig->dev.parent = priv->dev->parent;
> +		trig->ops = &timer_trigger_ops;
> +		trig->dev.groups = stm32_trigger_attr_groups;
> +		iio_trigger_set_drvdata(trig, priv);
> +
> +		ret = devm_iio_trigger_register(priv->dev, trig);
> +		if (ret)
> +			return ret;
> +		cur++;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * is_stm32_timer_trigger
> + * @trig: trigger to be checked
> + *
> + * return true if the trigger is a valid stm32 iio timer trigger
> + * either return false
> + */
> +bool is_stm32_timer_trigger(struct iio_trigger *trig)
> +{
> +	return (trig->ops == &timer_trigger_ops);
> +}
> +EXPORT_SYMBOL(is_stm32_timer_trigger);
> +
> +static int stm32_validate_trigger(struct iio_dev *indio_dev,
> +				  struct iio_trigger *trig)
> +{
> +	struct stm32_timer_trigger *priv = iio_priv(indio_dev);
> +	const char * const *cur = priv->valids;
> +	unsigned int i = 0;
> +
> +	if (!is_stm32_timer_trigger(trig))
> +		return -EINVAL;
> +
> +	while (cur && *cur) {
> +		if (!strncmp(trig->name, *cur, strlen(trig->name))) {
> +			regmap_update_bits(priv->regmap,
> +					   TIM_SMCR, TIM_SMCR_TS,
> +					   i << TIM_SMCR_TS_SHIFT);
> +			return 0;
> +		}
> +		cur++;
> +		i++;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static const struct iio_info stm32_trigger_info = {
> +	.driver_module = THIS_MODULE,
> +	.validate_trigger = stm32_validate_trigger,
> +	.attrs = &stm32_timer_attr_group,
> +};
> +
> +static struct stm32_timer_trigger *stm32_setup_iio_device(struct device *dev)
> +{
> +	struct iio_dev *indio_dev;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev,
> +					  sizeof(struct stm32_timer_trigger));
> +	if (!indio_dev)
> +		return NULL;
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &stm32_trigger_info;
> +	indio_dev->modes = INDIO_EVENT_TRIGGERED;
> +	indio_dev->num_channels = 0;
> +	indio_dev->dev.of_node = dev->of_node;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return NULL;
> +
> +	return iio_priv(indio_dev);
> +}
> +
> +static int stm32_timer_trigger_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct stm32_timer_trigger *priv;
> +	struct stm32_timers *ddata = dev_get_drvdata(pdev->dev.parent);
> +	unsigned int index;
> +	int ret;
> +
> +	if (of_property_read_u32(dev->of_node, "reg", &index))
> +		return -EINVAL;
> +
> +	if (index >= ARRAY_SIZE(triggers_table))
> +		return -EINVAL;
> +
> +	/* Create an IIO device only if we have triggers to be validated */
> +	if (*valids_table[index])
> +		priv = stm32_setup_iio_device(dev);

I still don't like this. Really feels like we shouldn't be creating an
iio device with all the bagage that carries just to allow us to do the
trigger trees.  We ought to have a much more light weight solution for this
functionality - we aren't typically even using the interrupt tree stuff
that the triggers for devices are all really about.

A simpler approach of allowing each trigger the option of a parent seems like
it would be cleaner.  Could be done entirely within this driver in the first
instance.  Basically it would just look like your master and slave attributes
but have those under triggerX not iio:deviceX.

We can work out how to make it more generic later - including perhaps the
option to trigger from triggers outside this driver, using some parallel
infrastructure to the device triggering.


> +	else
> +		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> +
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	priv->dev = dev;
> +	priv->regmap = ddata->regmap;
> +	priv->clk = ddata->clk;
> +	priv->max_arr = ddata->max_arr;
> +	priv->triggers = triggers_table[index];
> +	priv->valids = valids_table[index];
> +
> +	ret = stm32_setup_iio_triggers(priv);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, priv);
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id stm32_trig_of_match[] = {
> +	{ .compatible = "st,stm32-timer-trigger", },
> +	{ /* end node */ },
> +};
> +MODULE_DEVICE_TABLE(of, stm32_trig_of_match);
> +
> +static struct platform_driver stm32_timer_trigger_driver = {
> +	.probe = stm32_timer_trigger_probe,
> +	.driver = {
> +		.name = "stm32-timer-trigger",
> +		.of_match_table = stm32_trig_of_match,
> +	},
> +};
> +module_platform_driver(stm32_timer_trigger_driver);
> +
> +MODULE_ALIAS("platform: stm32-timer-trigger");
> +MODULE_DESCRIPTION("STMicroelectronics STM32 Timer Trigger driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/trigger/Kconfig b/drivers/iio/trigger/Kconfig
> index 809b2e7..f2af4fe 100644
> --- a/drivers/iio/trigger/Kconfig
> +++ b/drivers/iio/trigger/Kconfig
> @@ -46,5 +46,4 @@ config IIO_SYSFS_TRIGGER
>  
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called iio-trig-sysfs.
> -
Clean this up.
>  endmenu
> diff --git a/include/linux/iio/timer/stm32-timer-trigger.h b/include/linux/iio/timer/stm32-timer-trigger.h
> new file mode 100644
> index 0000000..55535ae
> --- /dev/null
> +++ b/include/linux/iio/timer/stm32-timer-trigger.h
> @@ -0,0 +1,62 @@
> +/*
> + * Copyright (C) STMicroelectronics 2016
> + *
> + * Author: Benjamin Gaignard <benjamin.gaignard-qxv4g6HH51o@public.gmane.org>
> + *
> + * License terms:  GNU General Public License (GPL), version 2
> + */
> +
> +#ifndef _STM32_TIMER_TRIGGER_H_
> +#define _STM32_TIMER_TRIGGER_H_
> +
> +#define TIM1_TRGO	"tim1_trgo"
> +#define TIM1_CH1	"tim1_ch1"
> +#define TIM1_CH2	"tim1_ch2"
> +#define TIM1_CH3	"tim1_ch3"
> +#define TIM1_CH4	"tim1_ch4"
> +
> +#define TIM2_TRGO	"tim2_trgo"
> +#define TIM2_CH1	"tim2_ch1"
> +#define TIM2_CH2	"tim2_ch2"
> +#define TIM2_CH3	"tim2_ch3"
> +#define TIM2_CH4	"tim2_ch4"
> +
> +#define TIM3_TRGO	"tim3_trgo"
> +#define TIM3_CH1	"tim3_ch1"
> +#define TIM3_CH2	"tim3_ch2"
> +#define TIM3_CH3	"tim3_ch3"
> +#define TIM3_CH4	"tim3_ch4"
> +
> +#define TIM4_TRGO	"tim4_trgo"
> +#define TIM4_CH1	"tim4_ch1"
> +#define TIM4_CH2	"tim4_ch2"
> +#define TIM4_CH3	"tim4_ch3"
> +#define TIM4_CH4	"tim4_ch4"
> +
> +#define TIM5_TRGO	"tim5_trgo"
> +#define TIM5_CH1	"tim5_ch1"
> +#define TIM5_CH2	"tim5_ch2"
> +#define TIM5_CH3	"tim5_ch3"
> +#define TIM5_CH4	"tim5_ch4"
> +
> +#define TIM6_TRGO	"tim6_trgo"
> +
> +#define TIM7_TRGO	"tim7_trgo"
> +
> +#define TIM8_TRGO	"tim8_trgo"
> +#define TIM8_CH1	"tim8_ch1"
> +#define TIM8_CH2	"tim8_ch2"
> +#define TIM8_CH3	"tim8_ch3"
> +#define TIM8_CH4	"tim8_ch4"
> +
> +#define TIM9_TRGO	"tim9_trgo"
> +#define TIM9_CH1	"tim9_ch1"
> +#define TIM9_CH2	"tim9_ch2"
> +
> +#define TIM12_TRGO	"tim12_trgo"
> +#define TIM12_CH1	"tim12_ch1"
> +#define TIM12_CH2	"tim12_ch2"
> +
> +bool is_stm32_timer_trigger(struct iio_trigger *trig);
> +
> +#endif
> 

^ permalink raw reply

* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Rafał Miłecki @ 2016-12-30 21:37 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Kalle Valo,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Martin Blumenstingl, Felix Fietkau, Arnd Bergmann,
	devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Rafał Miłecki
In-Reply-To: <86a22b00-1a04-25e7-9d31-2c1fd9d04e48-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

On 30 December 2016 at 21:20, Arend van Spriel
<arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
> On 29-12-16 10:43, Rafał Miłecki wrote:
>> On 29 December 2016 at 09:57, Arend van Spriel
>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>> On 28-12-16 22:30, Rafał Miłecki wrote:
>>>> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5-Re5JQEeQqe8@public.gmane.orgm> wrote:
>>>>> On 28 December 2016 at 22:07, Arend van Spriel
>>>>> <arend.vanspriel-dY08KVG/lbpWk0Htik3J/w@public.gmane.org> wrote:
>>>>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>>>>> From: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>>
>>>>>>> They allow specifying hardware limitations of supported channels. This
>>>>>>> may be useful for specifying single band devices or devices that support
>>>>>>> only some part of the whole band.
>>>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>>>> part of 5 GHz band.
>>>>>>>
>>>>>>> Signed-off-by: Rafał Miłecki <rafal-g1n6cQUeyibVItvQsEIGlw@public.gmane.org>
>>>>>>> ---
>>>>>>>  net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>>>>  1 file changed, 34 insertions(+)
>>>>>>>
>>>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>>>> index 5dbac37..35ba5c7 100644
>>>>>>> --- a/net/wireless/reg.c
>>>>>>> +++ b/net/wireless/reg.c
>>>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>>>>>  }
>>>>>>>  EXPORT_SYMBOL(reg_initiator_name);
>>>>>>>
>>>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>>>> +                                  struct ieee80211_channel *chan)
>>>>>>> +{
>>>>>>> +     struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>>>>> +     u32 val;
>>>>>>> +
>>>>>>> +     if (!np)
>>>>>>> +             return true;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>>>>> +         chan->center_freq < KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>> +
>>>>>>> +     if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>>>>> +         chan->center_freq > KHZ_TO_MHZ(val))
>>>>>>> +             return false;
>>>>>>
>>>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>>>> !CONFIG_OF.
>>>>>
>>>>> I compiled it with
>>>>> # CONFIG_OF is not set
>>>>>
>>>>> Can you test it and provide a non-working config if you see a
>>>>> compilation error, please?
>>>>
>>>> include/linux/of.h provides a lot of dummy static inline functions if
>>>> CONFIG_OF is not used (they also allow compiler to drop most of the
>>>> code).
>>>
>>> of_propeirty_read_u32 is static inline in of.h calling
>>> of_property_read_u32_array, which has a dummy variant in of.h returning
>>> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
>>> not allow the compiler to drop any code so probably better to do:
>>>
>>> if (!IS_ENABLED(CONFIG_OF) || !np)
>>>         return true;
>>
>> Please verify that using a compiler. If there is a problem I'll be
>> happy to work on it, but I need a proof it exists.
>
> I am on vacation right now so not having much more than email and web
> browser to use as review reference.
>
>> If compilers sees a:
>> if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
>> condition, it's pretty clear it can be dropped. With both conditional
>> blocks dropped function always returns "true" and... can be dropped.
>>
>> Let me see if I can convince you with the following test:
>
> No need to convince me. I made a mistake reviewing the code. Thanks for
> clarifying it.
>
>> $ grep -m 1 CONFIG_OF .config
>> # CONFIG_OF is not set
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 0
>>
>> $ grep -m 1 CONFIG_OF .config
>> CONFIG_OF=y
>> $ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
>> 1
>>
>>
>>> So with this patch you change the channel to DISABLED. I am not very
>>> familiar with reg.c so do you know if this is done before or after
>>> calling regulatory notifier in the driver. brcmfmac will enable channels
>>> querying the device upon regulatory notifier call, which may undo the
>>> behavior introduced by your patch.
>>
>> I'm not regulatory export, so I hope someone will review this patch.
>> So far I can say it works for me after trying it on SR400ac with
>> BCM43602.
>
> But you probably do not have a mapping table for mapping country code
> received in notifier to firmware regulatory code/revision. Only if you
> have that, brcmfmac will update the channels in the bands.

Thanks, I'll redo my tests.


> Giving this some more consideration I am not sure if this is the proper
> place to handle this. ieee80211-(min|max)-center-freq is platform
> specific configuration allowing multiple cards to be used in different
> (sub)bands. This has nothing to do with regulatory. So probably better
> to move it to core.c or chan.c.

I can see point in this, I'll see if I can put this code in a more
proper place. Thanks for your comment!

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH] iio: adc: Add Renesas GyroADC driver
From: Marek Vasut @ 2016-12-30 21:49 UTC (permalink / raw)
  To: Peter Meerwald-Stadler
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Geert Uytterhoeven,
	Simon Horman
In-Reply-To: <alpine.DEB.2.02.1612302205210.3977-jW+XmwGofnusTnJN9+BGXg@public.gmane.org>

On 12/30/2016 10:09 PM, Peter Meerwald-Stadler wrote:
> 
>>>> Add IIO driver for the Renesas RCar GyroADC block. This block is a
>>>> simple 4/8-channel ADC which samples 12/15/24 bits of data every
> 
>>>> +		if (iio_buffer_enabled(indio_dev))
>>>> +			return -EBUSY;
>>>
>>> use iio_device_claim_direct_mode()
>>
>> Why ? Do I really need the mutex locking here ?
> 
> no, not needed, since you do not support buffered mode yet; but neither do 
> you need iio_buffer_enabled() 
> 
> p.
> 

So drop the whole thing then ?

-- 
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: add Garmin vendor prefix
From: Matt Ranostay @ 2016-12-31  5:55 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <144bd616-1b5a-970a-edcb-13ebf67178f1-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

On Fri, Dec 30, 2016 at 12:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On 29/12/16 05:13, Matt Ranostay wrote:
>> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
> Rob, just to check - is this the right way to go with prefixes?
> It's awfully ugly in this case ;)

Rob said the stock ticker (likely the US exchanges) is the way to go :)

>
> Jonathan
>> ---
>> Changes from v1:
>> * switch to stock ticker for Garmin Limited
>>
>>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 16d3b5e7f5d1..5749bfc5fc5b 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -107,6 +107,7 @@ firefly   Firefly
>>  focaltech    FocalTech Systems Co.,Ltd
>>  friendlyarm  Guangzhou FriendlyARM Computer Tech Co., Ltd
>>  fsl  Freescale Semiconductor
>> +grmn Garmin Limited
>>  ge   General Electric Company
>>  geekbuying   GeekBuying
>>  gef  GE Fanuc Intelligent Platforms Embedded Systems, Inc.
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v5 3/4] clk: rockchip: add new pll-type for rk3328
From: Heiko Stuebner @ 2016-12-31 12:53 UTC (permalink / raw)
  To: Elaine Zhang
  Cc: mturquette, sboyd, xf, robh+dt, mark.rutland, linux-clk, huangtao,
	xxx, cl, linux-rockchip, linux-kernel, devicetree,
	linux-arm-kernel
In-Reply-To: <1482979511-6847-4-git-send-email-zhangqing@rock-chips.com>

Am Donnerstag, 29. Dezember 2016, 10:45:10 CET schrieb Elaine Zhang:
> The rk3328's pll and clock are similar with rk3036's,
> it different with pll_mode_mask, the rk3328 soc
> pll mode only one bit(rk3036 soc have two bits)
> so these should be independent and separate from
> the series of rk3328s.
> 
> Changes in v4:
>   adjust the pacth 3 and 4 order.
>   move pll_rk3328 to patch 3.
> Changes in v3:
>   fix up the pll type pll_rk3328 description and use
> 
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>

applied to my clk-branch for 4.11

The clock controller itself also looks good now, I'll just give Rob or someone 
else a bit of time for eventual comments after new years :-)

Heiko

^ permalink raw reply

* Re: [PATCH v2 1/2] devicetree: add Garmin vendor prefix
From: Jonathan Cameron @ 2016-12-31 15:19 UTC (permalink / raw)
  To: Matt Ranostay
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <CAJ_EiSTQ6naDiRP6o1UOr33UyKDtXaan2j4UPFNMTRKReuR-PQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 31/12/16 05:55, Matt Ranostay wrote:
> On Fri, Dec 30, 2016 at 12:26 PM, Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> On 29/12/16 05:13, Matt Ranostay wrote:
>>> Cc: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>> Signed-off-by: Matt Ranostay <matt-sk+viVC6FLCDq+mSdOJa79kegs52MxvZ@public.gmane.org>
>> Rob, just to check - is this the right way to go with prefixes?
>> It's awfully ugly in this case ;)
> 
> Rob said the stock ticker (likely the US exchanges) is the way to go :)
Fair enough.  Both applied to the togreg branch of iio.git and pushed
out as testing.

Thanks,

Jonathan
> 
>>
>> Jonathan
>>> ---
>>> Changes from v1:
>>> * switch to stock ticker for Garmin Limited
>>>
>>>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>> index 16d3b5e7f5d1..5749bfc5fc5b 100644
>>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>>> @@ -107,6 +107,7 @@ firefly   Firefly
>>>  focaltech    FocalTech Systems Co.,Ltd
>>>  friendlyarm  Guangzhou FriendlyARM Computer Tech Co., Ltd
>>>  fsl  Freescale Semiconductor
>>> +grmn Garmin Limited
>>>  ge   General Electric Company
>>>  geekbuying   GeekBuying
>>>  gef  GE Fanuc Intelligent Platforms Embedded Systems, Inc.
>>>
>>

^ permalink raw reply

* Re: [PATCH v6 3/9] iio: inkern: api for manipulating ext_info of iio channels
From: Jonathan Cameron @ 2016-12-31 15:51 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1480493823-21462-4-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>

On 30/11/16 08:16, Peter Rosin wrote:
> Extend the inkern api with functions for reading and writing ext_info
> of iio channels.
> 
> Signed-off-by: Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
Acked-by: Jonathan Cameron <jic23-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

It may make more sense to take this particular patch separately via
IIO, but as the churn on this file is fairly low I think it is probably
going to be easier to take it with the rest of the series if / when that
heads upstream.

Jonathan
> ---
>  drivers/iio/inkern.c         | 60 ++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/iio/consumer.h | 37 +++++++++++++++++++++++++++
>  2 files changed, 97 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index b0f4630a163f..4848b8129e6c 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -863,3 +863,63 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(iio_write_channel_raw);
> +
> +unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +	unsigned int i = 0;
> +
> +	if (!chan->channel->ext_info)
> +		return i;
> +
> +	for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
> +		++i;
> +
> +	return i;
> +}
> +EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
> +
> +static const struct iio_chan_spec_ext_info *iio_lookup_ext_info(
> +						const struct iio_channel *chan,
> +						const char *attr)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +
> +	if (!chan->channel->ext_info)
> +		return NULL;
> +
> +	for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
> +		if (!strcmp(attr, ext_info->name))
> +			return ext_info;
> +	}
> +
> +	return NULL;
> +}
> +
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> +				  const char *attr, char *buf)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +
> +	ext_info = iio_lookup_ext_info(chan, attr);
> +	if (!ext_info)
> +		return -EINVAL;
> +
> +	return ext_info->read(chan->indio_dev, ext_info->private,
> +			      chan->channel, buf);
> +}
> +EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
> +
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> +				   const char *buf, size_t len)
> +{
> +	const struct iio_chan_spec_ext_info *ext_info;
> +
> +	ext_info = iio_lookup_ext_info(chan, attr);
> +	if (!ext_info)
> +		return -EINVAL;
> +
> +	return ext_info->write(chan->indio_dev, ext_info->private,
> +			       chan->channel, buf, len);
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 47eeec3218b5..5e347a9805fd 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -312,4 +312,41 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
>  int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
>  	int *processed, unsigned int scale);
>  
> +/**
> + * iio_get_channel_ext_info_count() - get number of ext_info attributes
> + *				      connected to the channel.
> + * @chan:		The channel being queried
> + *
> + * Returns the number of ext_info attributes
> + */
> +unsigned int iio_get_channel_ext_info_count(struct iio_channel *chan);
> +
> +/**
> + * iio_read_channel_ext_info() - read ext_info attribute from a given channel
> + * @chan:		The channel being queried.
> + * @attr:		The ext_info attribute to read.
> + * @buf:		Where to store the attribute value. Assumed to hold
> + *			at least PAGE_SIZE bytes.
> + *
> + * Returns the number of bytes written to buf (perhaps w/o zero termination;
> + * it need not even be a string), or an error code.
> + */
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> +				  const char *attr, char *buf);
> +
> +/**
> + * iio_write_channel_ext_info() - write ext_info attribute from a given channel
> + * @chan:		The channel being queried.
> + * @attr:		The ext_info attribute to read.
> + * @buf:		The new attribute value. Strings needs to be zero-
> + *			terminated, but the terminator should not be included
> + *			in the below len.
> + * @len:		The size of the new attribute value.
> + *
> + * Returns the number of accepted bytes, which should be the same as len.
> + * An error code can also be returned.
> + */
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> +				   const char *buf, size_t len);
> +
>  #endif
> 

^ permalink raw reply

* Re: [PATCH v6 4/9] dt-bindings: iio: iio-mux: document iio-mux bindings
From: Jonathan Cameron @ 2016-12-31 16:01 UTC (permalink / raw)
  To: Peter Rosin, Rob Herring
  Cc: linux-kernel, Wolfram Sang, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc
In-Reply-To: <2275a7ff-6a21-dd99-ab7d-b213d7fcd6e5@axentia.se>

On 12/12/16 12:18, Peter Rosin wrote:
> On 2016-12-10 19:21, Jonathan Cameron wrote:
>> On 06/12/16 09:18, Peter Rosin wrote:
>>> On 2016-12-06 00:26, Rob Herring wrote:
>>>> On Wed, Nov 30, 2016 at 09:16:58AM +0100, Peter Rosin wrote:
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>> ---
>>>>>  .../bindings/iio/multiplexer/iio-mux.txt           | 40 ++++++++++++++++++++++
>>>>>  MAINTAINERS                                        |  6 ++++
>>>>>  2 files changed, 46 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>
>>>> I'm still not convinced about this binding, but don't really have more 
>>>> comments ATM. Sending 6 versions in 2 weeks or so doesn't really help 
>>>> either.
>>>
>>> Sorry about the noise, I'll try to be more careful going forward. On
>>> the flip side, I haven't touched the code since v6.
>>>
>>> I don't see how bindings that are as flexible as the current (and
>>> original) phandle link between the mux consumer and the mux controller
>>> would look, and at the same time be simpler to understand. You need
>>> to be able to refer to a mux controller from several mux consumers, and
>>> you need to support several mux controllers in one node (the ADG792A
>>> case). And, AFAICT, the complex case wasn't really the problem, it was
>>> that it is overly complex to describe the simple case of one mux
>>> consumer and one mux controller. But in your comment for v2 [1] you
>>> said that I was working around limitations with shared GPIO pins. But
>>> solving that in the GPIO subsystem would not solve all that the
>>> phandle approach is solving, since you would not have support for
>>> ADG792A (or other non-GPIO controlled muxes). So, I think listing
>>> the gpio pins inside the mux consumer node is a non-starter, the mux
>>> controller has to live in its own node with its own compatible.
>>>
>>> Would you be happier if I managed to marry the phandle approach with
>>> the option of having the mux controller as a child node of the mux
>>> consumer for the simple case?
>>>
>>> I added an example at the end of this message (the same as the first
>>> example in v4 [2], at least in principle) for easy comparison between
>>> the phandle and the controller-in-child-node approaches. I can't say
>>> that I personally find the difference all that significant, and do not
>>> think it is worth it. As I see it, the "simple option" would just muddy
>>> the waters...
>>>
>>> [1] http://marc.info/?l=linux-kernel&m=147948334204795&w=2
>>> [2] http://marc.info/?l=linux-kernel&m=148001364904240&w=2
>>>
>>>>> diff --git a/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>> new file mode 100644
>>>>> index 000000000000..8080cf790d82
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
>>>>> @@ -0,0 +1,40 @@
>>>>> +IIO multiplexer bindings
>>>>> +
>>>>> +If a multiplexer is used to select which hardware signal is fed to
>>>>> +e.g. an ADC channel, these bindings describe that situation.
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible : "iio-mux"
>>>>
>>>> This is a Linuxism. perhaps "adc-mux".
>>>
>>> No, that's not general enough, it could just as well be used to mux a
>>> temperature sensor. Or whatever. Hmmm, given that "iio-mux" is bad, perhaps
>>> "io-channel-mux" is better? That matches the io-channels property used to
>>> refer to the parent channel.
>> analog-mux maybe? Makes more sense out of context (though with io-channels defined on
>> the next line you have plenty of context here ;)
> 
> Not that I care all that much about the name, but that doesn't really
> fit if you take e.g. an IIO_INDEX channel. That sounds entirely non-analog
> to me, but what do I know? Maybe that example doesn't make sense for some
> reason, but I can't help but think that there will be some non-analog
> channel in the future, if there isn't one already.
> 
> So, my preference is io-channel-mux, as that matches the previous dt
> naming for what is muxed. But that's just my opinion, if I'm told that
> it should be something else, then that's ok.
io-channel-mux works fine for me. It's some sort of input / output channel
and we are muxing it ;)
> 
> I'm more worried about other aspects, such as how to get reviewers and who
> is going to take the core mux patches and what tree they are going to be
> merged into etc. That is, if this series is going anywhere at all or if
> someone is going to put up a road-block for some reason...
Whilst it is meeting some resistance, I'm not seeing any absolute blockers
(people tend to be rather explicit about that).  The binding is still causing
the most friction I think and it may be that it just needs some more time for
Rob to mull it over. It's a fiddly thing to describe, so was never going
to drop straight in!

The core mux patches probably need to go one of a few possible routes.

1. Directly as a pull to linus with a good collection of Acks.
2. Via Greg KH perhaps as generic driver infrastructure, or Andrew Morton
as being in the category no one else will take.
3. Via either me or Wolfram (as a separate immutable branch) on the basis it's
core stuff but the users currently are IIO and I2C.

In any case, this needs at the very least Acks from Rob, Wolfram and myself.
Others would be most welcome, Arnd and/or Greg might be persuaded to take
a look for example...

Happy New Year,

Jonathan
> 
> Cheers,
> peda
> 


^ permalink raw reply

* Re: [PATCH v6 1/9] dt-bindings: document devicetree bindings for mux-controllers and mux-gpio
From: Jonathan Cameron @ 2016-12-31 16:10 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc
In-Reply-To: <1480493823-21462-2-git-send-email-peda@axentia.se>

On 30/11/16 08:16, Peter Rosin wrote:
> Signed-off-by: Peter Rosin <peda@axentia.se>
Bindings are still a bit of a black art to me ;)

Other than the naming of the iio-mux as discussed in the other patch
I'm happy with this. It feels like it has struck the right balance
between flexibility and complexity.  Which probably means we'll
have an application it doesn't stretch to before the day is out...

Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
>  .../devicetree/bindings/misc/mux-controller.txt    | 127 +++++++++++++++++++++
>  .../devicetree/bindings/misc/mux-gpio.txt          |  68 +++++++++++
>  MAINTAINERS                                        |   5 +
>  3 files changed, 200 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/misc/mux-controller.txt
>  create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
> 
> diff --git a/Documentation/devicetree/bindings/misc/mux-controller.txt b/Documentation/devicetree/bindings/misc/mux-controller.txt
> new file mode 100644
> index 000000000000..19c36b73173e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mux-controller.txt
> @@ -0,0 +1,127 @@
> +Common multiplexer controller bindings
> +======================================
> +
> +A multiplexer (or mux) controller will have one, or several, consumer devices
> +that uses the mux controller. Thus, a mux controller can possibly control
> +several parallel multiplexers, presumably there will be at least one
> +multiplexer needed by each consumer..
> +
> +A mux controller provides a number of states to its consumers, and the state
> +space is a simple zero-based enumeration. I.e. 0-1 for a 2-way multiplexer,
> +0-7 for an 8-way multiplexer, etc.
> +
> +
> +Consumers
> +---------
> +
> +Mux controller consumers should specify a list of mux controllers that they
> +want to use with a property containing a 'mux-ctrl-list':
> +
> +	mux-ctrl-list ::= <single-mux-ctrl> [mux-ctrl-list]
> +	single-mux-ctrl ::= <mux-ctrl-phandle> [mux-ctrl-specifier]
> +	mux-ctrl-phandle : phandle to mux controller node
> +	mux-ctrl-specifier : array of #mux-control-cells specifying the
> +			     given mux controller (controller specific)
> +
> +Mux controller properties should be named "mux-controls". The exact meaning of
> +each mux controller property must be documented in the device tree binding for
> +each consumer. An optional property "mux-control-names" may contain a list of
> +strings to label each of the mux controllers listed in the "mux-controls"
> +property.
> +
> +Drivers for devices that use more than a single mux controller can use the
> +"mux-control-names" property to map the name of the mux controller requested by
> +the mux_control_get() call to an index into the list given by the
> +"mux-controls" property.
> +
> +mux-ctrl-specifier typically encodes the chip-relative mux controller number.
> +If the mux controller chip only provides a single mux controller, the
> +mux-ctrl-specifier can typically be left out.
> +
> +Example:
> +
> +	/* One consumer of a 2-way mux controller (one GPIO-line) */
> +	mux: mux-controller {
> +		compatible = "mux-gpio";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "iio-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +		mux-controls = <&mux>;
> +		mux-control-names = "adc";
> +
> +		channels = "sync", "in";
> +	};
> +
> +Note that in the example above, specifying the "mux-control-names" is redundant
> +because there is only one mux controller in the list.
> +
> +	/*
> +	 * Two consumers (one for an ADC line and one for an i2c bus) of
> +	 * parallel 4-way multiplexers controlled by the same two GPIO-lines.
> +	 */
> +	mux: mux-controller {
> +		compatible = "mux-gpio";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "iio-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +		mux-controls = <&mux>;
> +
> +		channels = "sync-1", "in", "out", "sync-2";
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux-simple,mux-locked";
> +		i2c-parent = <&i2c1>;
> +		mux-controls = <&mux>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ssd1307: oled@3c {
> +				/* ... */
> +			};
> +		};
> +
> +		i2c@3 {
> +			reg = <3>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			pca9555: pca9555@20 {
> +				/* ... */
> +			};
> +		};
> +	};
> +
> +
> +Mux controller nodes
> +--------------------
> +
> +Mux controller nodes must specify the number of cells used for the
> +specifier using the '#mux-control-cells' property.
> +
> +An example mux controller might look like this:
> +
> +	mux: adg792a@50 {
> +		compatible = "adi,adg792a";
> +		reg = <0x50>;
> +		#mux-control-cells = <1>;
> +	};
> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> new file mode 100644
> index 000000000000..2ff814f082c8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
> @@ -0,0 +1,68 @@
> +GPIO-based multiplexer controller bindings
> +
> +Define what GPIO pins are used to control a multiplexer. Or several
> +multiplexers, if the same pins control more than one multiplexer.
> +
> +Required properties:
> +- compatible : "mux-gpio"
> +- mux-gpios : list of gpios used to control the multiplexer, least
> +	      significant bit first.
> +- #mux-control-cells : <0>
> +* Standard mux-controller bindings as decribed in mux-controller.txt
> +
> +Optional properties:
> +- idle-state : if present, the state the mux will have when idle.
> +
> +The multiplexer state is defined as the number represented by the
> +multiplexer GPIO pins, where the first pin is the least significant
> +bit. An active pin is a binary 1, an inactive pin is a binary 0.
> +
> +Example:
> +
> +	mux: mux-controller {
> +		compatible = "mux-gpio";
> +		#mux-control-cells = <0>;
> +
> +		mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> +			    <&pioA 1 GPIO_ACTIVE_HIGH>;
> +	};
> +
> +	adc-mux {
> +		compatible = "iio-mux";
> +		io-channels = <&adc 0>;
> +		io-channel-names = "parent";
> +
> +		mux-controls = <&mux>;
> +
> +		channels = "sync-1", "in", "out", "sync-2";
> +	};
> +
> +	i2c-mux {
> +		compatible = "i2c-mux-simple,mux-locked";
> +		i2c-parent = <&i2c1>;
> +
> +		mux-controls = <&mux>;
> +
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		i2c@0 {
> +			reg = <0>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			ssd1307: oled@3c {
> +				/* ... */
> +			};
> +		};
> +
> +		i2c@3 {
> +			reg = <3>;
> +			#address-cells = <1>;
> +			#size-cells = <0>;
> +
> +			pca9555: pca9555@20 {
> +				/* ... */
> +			};
> +		};
> +	};
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d8eb3843dbd4..3d4d0efc2b64 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8403,6 +8403,11 @@ S:	Orphan
>  F:	drivers/mmc/host/mmc_spi.c
>  F:	include/linux/spi/mmc_spi.h
>  
> +MULTIPLEXER SUBSYSTEM
> +M:	Peter Rosin <peda@axentia.se>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/misc/mux-*
> +
>  MULTISOUND SOUND DRIVER
>  M:	Andrew Veliath <andrewtv@usa.net>
>  S:	Maintained
> 


^ permalink raw reply

* Re: [PATCH v6 2/9] misc: minimal mux subsystem and gpio-based mux controller
From: Jonathan Cameron @ 2016-12-31 16:19 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc
In-Reply-To: <1480493823-21462-3-git-send-email-peda@axentia.se>

On 30/11/16 08:16, Peter Rosin wrote:
> Add a new minimalistic subsystem that handles multiplexer controllers.
> When multiplexers are used in various places in the kernel, and the
> same multiplexer controller can be used for several independent things,
> there should be one place to implement support for said multiplexer
> controller.
> 
> A single multiplexer controller can also be used to control several
> parallel multiplexers, that are in turn used by different subsystems
> in the kernel, leading to a need to coordinate multiplexer accesses.
> The multiplexer subsystem handles this coordination.
> 
> This new mux controller subsystem initially comes with a single backend
> driver that controls gpio based multiplexers. Even though not needed by
> this initial driver, the mux controller subsystem is prepared to handle
> chips with multiple (independent) mux controllers.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Few trivial bits inline + question of whether misc is the right location..
It's small, but not totally trivial as subsystems go, so perhaps it needs it's
own directory.
> ---
>  Documentation/driver-model/devres.txt |   6 +-
>  MAINTAINERS                           |   2 +
>  drivers/misc/Kconfig                  |  30 ++++
>  drivers/misc/Makefile                 |   2 +
>  drivers/misc/mux-core.c               | 311 ++++++++++++++++++++++++++++++++++
>  drivers/misc/mux-gpio.c               | 138 +++++++++++++++
>  include/linux/mux.h                   | 197 +++++++++++++++++++++
>  7 files changed, 685 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/misc/mux-core.c
>  create mode 100644 drivers/misc/mux-gpio.c
>  create mode 100644 include/linux/mux.h
> 
> diff --git a/Documentation/driver-model/devres.txt b/Documentation/driver-model/devres.txt
> index ca9d1eb46bc0..d64ede85b61b 100644
> --- a/Documentation/driver-model/devres.txt
> +++ b/Documentation/driver-model/devres.txt
> @@ -330,7 +330,11 @@ MEM
>    devm_kzalloc()
>  
>  MFD
> - devm_mfd_add_devices()
Technically should be in a separate cleanup patch...
> +  devm_mfd_add_devices()
> +
> +MUX
> +  devm_mux_control_get()
> +  devm_mux_control_put()
>  
>  PER-CPU MEM
>    devm_alloc_percpu()
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3d4d0efc2b64..dc7498682752 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8407,6 +8407,8 @@ MULTIPLEXER SUBSYSTEM
>  M:	Peter Rosin <peda@axentia.se>
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/misc/mux-*
> +F:	include/linux/mux.h
> +F:	drivers/misc/mux-*
>  
>  MULTISOUND SOUND DRIVER
>  M:	Andrew Veliath <andrewtv@usa.net>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 64971baf11fa..2ce675e410c5 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -766,6 +766,36 @@ config PANEL_BOOT_MESSAGE
>  	  An empty message will only clear the display at driver init time. Any other
>  	  printf()-formatted message is valid with newline and escape codes.
>  
> +menuconfig MULTIPLEXER
> +	bool "Multiplexer subsystem"
> +	help
> +	  Multiplexer controller subsystem. Multiplexers are used in a
> +	  variety of settings, and this subsystem abstracts their use
> +	  so that the rest of the kernel sees a common interface. When
> +	  multiple parallel multiplexers are controlled by one single
> +	  multiplexer controller, this subsystem also coordinates the
> +	  multiplexer accesses.
> +
> +	  If unsure, say no.
> +
Fun question of the day. Is misc the place to put this or should it
have it's own directory. I'd go for own directory...

On the plus side, whilst it's in misc you get to pester Greg and Arnd
for review.
> +if MULTIPLEXER
> +
> +config MUX_GPIO
> +	tristate "GPIO-controlled Multiplexer"
> +	depends on OF && GPIOLIB
> +	help
> +	  GPIO-controlled Multiplexer controller.
> +
> +	  The driver builds a single multiplexer controller using a number
> +	  of gpio pins. For N pins, there will be 2^N possible multiplexer
> +	  states. The GPIO pins can be connected (by the hardware) to several
> +	  multiplexers, which in that case will be operated in parallel.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called mux-gpio.
> +
> +endif
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 31983366090a..0befa2bba762 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO)		+= echo/
>  obj-$(CONFIG_VEXPRESS_SYSCFG)	+= vexpress-syscfg.o
>  obj-$(CONFIG_CXL_BASE)		+= cxl/
>  obj-$(CONFIG_PANEL)             += panel.o
> +obj-$(CONFIG_MULTIPLEXER)      	+= mux-core.o
> +obj-$(CONFIG_MUX_GPIO)		+= mux-gpio.o
>  
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_core.o
>  lkdtm-$(CONFIG_LKDTM)		+= lkdtm_bugs.o
> diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
> new file mode 100644
> index 000000000000..cccaa7261a6e
> --- /dev/null
> +++ b/drivers/misc/mux-core.c
> @@ -0,0 +1,311 @@
> +/*
> + * Multiplexer subsystem
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "mux-core: " fmt
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/slab.h>
> +
> +static struct class mux_class = {
> +	.name = "mux",
> +	.owner = THIS_MODULE,
> +};
> +
> +static int __init mux_init(void)
> +{
> +	return class_register(&mux_class);
> +}
> +
> +static DEFINE_IDA(mux_ida);
> +
> +static void mux_chip_release(struct device *dev)
> +{
> +	struct mux_chip *mux_chip = to_mux_chip(dev);
> +
> +	ida_simple_remove(&mux_ida, mux_chip->id);
> +	kfree(mux_chip);
> +}
> +
> +static struct device_type mux_type = {
> +	.name = "mux-chip",
> +	.release = mux_chip_release,
> +};
> +
> +struct mux_chip *mux_chip_alloc(struct device *dev,
> +				unsigned int controllers, size_t sizeof_priv)
> +{
> +	struct mux_chip *mux_chip;
> +	int i;
> +
> +	if (!dev || !controllers)
> +		return NULL;
> +
> +	mux_chip = kzalloc(sizeof(*mux_chip) +
> +			   controllers * sizeof(*mux_chip->mux) +
> +			   sizeof_priv, GFP_KERNEL);
> +	if (!mux_chip)
> +		return NULL;
> +
> +	mux_chip->mux = (struct mux_control *)(mux_chip + 1);
> +	mux_chip->dev.class = &mux_class;
> +	mux_chip->dev.type = &mux_type;
> +	mux_chip->dev.parent = dev;
> +	mux_chip->dev.of_node = dev->of_node;
> +	dev_set_drvdata(&mux_chip->dev, mux_chip);
> +
> +	mux_chip->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
> +	if (mux_chip->id < 0) {
> +		pr_err("muxchipX failed to get a device id\n");
> +		kfree(mux_chip);
> +		return NULL;
> +	}
> +	dev_set_name(&mux_chip->dev, "muxchip%d", mux_chip->id);
> +
> +	mux_chip->controllers = controllers;
> +	for (i = 0; i < controllers; ++i) {
> +		struct mux_control *mux = &mux_chip->mux[i];
> +
> +		mux->chip = mux_chip;
> +		init_rwsem(&mux->lock);
> +		mux->cached_state = -1;
> +		mux->idle_state = -1;
> +	}
> +
> +	device_initialize(&mux_chip->dev);
> +
> +	return mux_chip;
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_alloc);
> +
> +static int mux_control_set(struct mux_control *mux, int state)
> +{
> +	int ret = mux->chip->ops->set(mux, state);
> +
> +	mux->cached_state = ret < 0 ? -1 : state;
> +
> +	return ret;
> +}
> +
> +int mux_chip_register(struct mux_chip *mux_chip)
> +{
> +	int i;
> +	int ret;
> +
> +	for (i = 0; i < mux_chip->controllers; ++i) {
> +		struct mux_control *mux = &mux_chip->mux[i];
> +
> +		if (mux->idle_state == mux->cached_state)
> +			continue;
> +
> +		ret = mux_control_set(mux, mux->idle_state);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return device_add(&mux_chip->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_register);
> +
> +void mux_chip_unregister(struct mux_chip *mux_chip)
> +{
> +	device_del(&mux_chip->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_unregister);
> +
> +void mux_chip_free(struct mux_chip *mux_chip)
> +{
> +	if (!mux_chip)
> +		return;
I'd drop a blank line in here. Slightly nicer to read.
> +	put_device(&mux_chip->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_chip_free);
> +
> +int mux_control_select(struct mux_control *mux, int state)
> +{
> +	int ret;
> +
> +	if (down_read_trylock(&mux->lock)) {
> +		if (mux->cached_state == state)
> +			return 0;
> +
> +		/* Sigh, the mux needs updating... */
> +		up_read(&mux->lock);
> +	}
> +
> +	/* ...or it's just contended. */
> +	down_write(&mux->lock);
> +
> +	if (mux->cached_state == state) {
> +		/*
> +		 * Hmmm, someone else changed the mux to my liking.
> +		 * That makes me wonder how long I waited for nothing?
> +		 */
> +		downgrade_write(&mux->lock);
> +		return 0;
> +	}
> +
> +	ret = mux_control_set(mux, state);
> +	if (ret < 0) {
> +		if (mux->idle_state != -1)
> +			mux_control_set(mux, mux->idle_state);
> +
> +		up_write(&mux->lock);
> +		return ret;
> +	}
> +
> +	downgrade_write(&mux->lock);
> +
> +	return 1;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_select);
> +
> +int mux_control_deselect(struct mux_control *mux)
> +{
> +	int ret = 0;
> +
> +	if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
> +		ret = mux_control_set(mux, mux->idle_state);
> +
> +	up_read(&mux->lock);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_deselect);
> +
> +static int of_dev_node_match(struct device *dev, const void *data)
> +{
> +	return dev->of_node == data;
> +}
> +
> +static struct mux_chip *of_find_mux_chip_by_node(struct device_node *np)
> +{
> +	struct device *dev;
> +
> +	dev = class_find_device(&mux_class, NULL, np, of_dev_node_match);
> +
> +	return dev ? to_mux_chip(dev) : NULL;
> +}
> +
> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +{
> +	struct device_node *np = dev->of_node;
> +	struct of_phandle_args args;
> +	struct mux_chip *mux_chip;
> +	unsigned int controller;
> +	int index = 0;
> +	int ret;
> +
> +	if (mux_name) {
> +		index = of_property_match_string(np, "mux-control-names",
> +						 mux_name);
> +		if (index < 0)
> +			return ERR_PTR(index);
> +	}
> +
> +	ret = of_parse_phandle_with_args(np,
> +					 "mux-controls", "#mux-control-cells",
> +					 index, &args);
> +	if (ret) {
> +		dev_err(dev, "%s: failed to get mux-control %s(%i)\n",
> +			np->full_name, mux_name ?: "", index);
> +		return ERR_PTR(ret);
> +	}
> +
> +	mux_chip = of_find_mux_chip_by_node(args.np);
> +	of_node_put(args.np);
> +	if (!mux_chip)
> +		return ERR_PTR(-EPROBE_DEFER);
> +
> +	if (args.args_count > 1 ||
> +	    (!args.args_count && (mux_chip->controllers > 1))) {
> +		dev_err(dev, "%s: wrong #mux-control-cells for %s\n",
> +			np->full_name, args.np->full_name);
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	controller = 0;
> +	if (args.args_count)
> +		controller = args.args[0];
> +
> +	if (controller >= mux_chip->controllers)
> +		return ERR_PTR(-EINVAL);
> +
> +	get_device(&mux_chip->dev);
> +	return &mux_chip->mux[controller];
> +}
> +EXPORT_SYMBOL_GPL(mux_control_get);
> +
> +void mux_control_put(struct mux_control *mux)
> +{
> +	put_device(&mux->chip->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_put);
> +
> +static void devm_mux_control_release(struct device *dev, void *res)
> +{
> +	struct mux_control *mux = *(struct mux_control **)res;
> +
> +	mux_control_put(mux);
> +}
> +
> +struct mux_control *devm_mux_control_get(struct device *dev,
> +					 const char *mux_name)
> +{
> +	struct mux_control **ptr, *mux;
> +
> +	ptr = devres_alloc(devm_mux_control_release, sizeof(*ptr), GFP_KERNEL);
> +	if (!ptr)
> +		return ERR_PTR(-ENOMEM);
> +
> +	mux = mux_control_get(dev, mux_name);
> +	if (IS_ERR(mux)) {
> +		devres_free(ptr);
> +		return mux;
> +	}
> +
> +	*ptr = mux;
> +	devres_add(dev, ptr);
> +
> +	return mux;
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_get);
> +
> +static int devm_mux_control_match(struct device *dev, void *res, void *data)
> +{
> +	struct mux_control **r = res;
> +
> +	if (!r || !*r) {
> +		WARN_ON(!r || !*r);
> +		return 0;
> +	}
> +
> +	return *r == data;
> +}
> +
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux)
> +{
> +	WARN_ON(devres_release(dev, devm_mux_control_release,
> +			       devm_mux_control_match, mux));
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_put);
> +
> +subsys_initcall(mux_init);
> +
> +MODULE_DESCRIPTION("Multiplexer subsystem");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
> new file mode 100644
> index 000000000000..a107a9b96854
> --- /dev/null
> +++ b/drivers/misc/mux-gpio.c
> @@ -0,0 +1,138 @@
> +/*
> + * GPIO-controlled multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_gpio {
> +	struct gpio_descs *gpios;
> +	int *val;
> +};
> +
> +static int mux_gpio_set(struct mux_control *mux, int state)
> +{
> +	struct mux_gpio *mux_gpio = mux_chip_priv(mux->chip);
> +	int i;
> +
> +	for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> +		mux_gpio->val[i] = (state >> i) & 1;
> +
> +	gpiod_set_array_value_cansleep(mux_gpio->gpios->ndescs,
> +				       mux_gpio->gpios->desc,
> +				       mux_gpio->val);
> +
> +	return 0;
> +}
> +
> +static const struct mux_control_ops mux_gpio_ops = {
> +	.set = mux_gpio_set,
> +};
> +
> +static const struct of_device_id mux_gpio_dt_ids[] = {
> +	{ .compatible = "mux-gpio", },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
> +
> +static int mux_gpio_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct mux_chip *mux_chip;
> +	struct mux_gpio *mux_gpio;
> +	int pins;
> +	u32 idle_state;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	pins = gpiod_count(dev, "mux");
> +	if (pins < 0)
> +		return pins;
> +
> +	mux_chip = mux_chip_alloc(dev, 1, sizeof(*mux_gpio) +
> +				  pins * sizeof(*mux_gpio->val));
> +	if (!mux_chip)
> +		return -ENOMEM;
Rather feels like mux_chip_alloc is a good candidate for a managed
allocator. Might be worth doing the register as well then the remove
below would go away.  I guess perhaps not that worthwhile as not many
mux types are likely to ever exist...
> +
> +	mux_gpio = mux_chip_priv(mux_chip);
> +	mux_gpio->val = (int *)(mux_gpio + 1);
> +	mux_chip->ops = &mux_gpio_ops;
> +
> +	platform_set_drvdata(pdev, mux_chip);
> +
> +	mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> +	if (IS_ERR(mux_gpio->gpios)) {
> +		ret = PTR_ERR(mux_gpio->gpios);
> +		if (ret != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get gpios\n");
> +		goto free_mux_chip;
> +	}
> +	WARN_ON(pins != mux_gpio->gpios->ndescs);
> +	mux_chip->mux->states = 1 << pins;
> +
> +	ret = of_property_read_u32(np, "idle-state", &idle_state);
> +	if (ret >= 0) {
> +		if (idle_state >= mux_chip->mux->states) {
> +			dev_err(dev, "invalid idle-state %u\n", idle_state);
> +			ret = -EINVAL;
> +			goto free_mux_chip;
> +		}
> +
> +		mux_chip->mux->idle_state = idle_state;
> +	}
> +
> +	ret = mux_chip_register(mux_chip);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to register mux-chip\n");
> +		goto free_mux_chip;
> +	}
> +
> +	dev_info(dev, "%u-way mux-controller registered\n",
> +		 mux_chip->mux->states);
> +
> +	return 0;
> +
> +free_mux_chip:
> +	mux_chip_free(mux_chip);
> +	return ret;
> +}
> +
> +static int mux_gpio_remove(struct platform_device *pdev)
> +{
> +	struct mux_chip *mux_chip = platform_get_drvdata(pdev);
> +
> +	mux_chip_unregister(mux_chip);
> +	mux_chip_free(mux_chip);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver mux_gpio_driver = {
> +	.driver = {
> +		.name = "mux-gpio",
> +		.of_match_table	= of_match_ptr(mux_gpio_dt_ids),
> +	},
> +	.probe = mux_gpio_probe,
> +	.remove = mux_gpio_remove,
> +};
> +module_platform_driver(mux_gpio_driver);
> +
> +MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mux.h b/include/linux/mux.h
> new file mode 100644
> index 000000000000..88d607b7fde7
> --- /dev/null
> +++ b/include/linux/mux.h
> @@ -0,0 +1,197 @@
> +/*
> + * mux.h - definitions for the multiplexer interface
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_MUX_H
> +#define _LINUX_MUX_H
> +
> +#include <linux/device.h>
> +#include <linux/rwsem.h>
> +
> +struct mux_chip;
> +struct mux_control;
> +struct platform_device;
> +
> +struct mux_control_ops {
> +	int (*set)(struct mux_control *mux, int state);
> +};
> +
> +/**
> + * struct mux_control - Represents a mux controller.
> + * @lock:		Protects the mux controller state.
> + * @chip:		The mux chip that is handling this mux controller.
> + * @states:		The number of mux controller states.
> + * @cached_state:	The current mux controller state, or -1 if none.
> + * @idle_state:		The mux controller state to use when inactive, or -1
> + *			for none.
> + */
> +struct mux_control {
> +	struct rw_semaphore lock; /* protects the state of the mux */
> +
> +	struct mux_chip *chip;
> +
> +	unsigned int states;
> +	int cached_state;
> +	int idle_state;
> +};
> +
> +/**
> + * struct mux_chip -	Represents a chip holding mux controllers.
> + * @controllers:	Number of mux controllers handled by the chip.
> + * @mux:		Array of mux controllers that is handled.
> + * @dev:		Device structure.
> + * @id:			Used to identify the device internally.
> + * @ops:		Mux controller operations.
> + */
> +struct mux_chip {
> +	unsigned int controllers;
> +	struct mux_control *mux;
> +	struct device dev;
> +	int id;
> +
> +	const struct mux_control_ops *ops;
> +};
> +
> +#define to_mux_chip(x) container_of((x), struct mux_chip, dev)
> +
> +/**
> + * mux_chip_priv() - Get the extra memory reserved by mux_chip_alloc().
> + * @mux_chip: The mux-chip to get the private memory from.
> + *
> + * Return: Pointer to the private memory reserved by the allocator.
> + */
> +static inline void *mux_chip_priv(struct mux_chip *mux_chip)
> +{
> +	return &mux_chip->mux[mux_chip->controllers];
> +}
> +
> +/**
> + * mux_chip_alloc() - Allocate a mux-chip.
> + * @dev: The parent device implementing the mux interface.
> + * @controllers: The number of mux controllers to allocate for this chip.
> + * @sizeof_priv: Size of extra memory area for private use by the caller.
> + *
> + * Return: A pointer to the new mux-chip, NULL on failure.
> + */
> +struct mux_chip *mux_chip_alloc(struct device *dev,
> +				unsigned int controllers, size_t sizeof_priv);
> +
> +/**
> + * mux_chip_register() - Register a mux-chip, thus readying the controllers
> + *			 for use.
> + * @mux_chip: The mux-chip to register.
> + *
> + * Do not retry registration of the same mux-chip on failure. You should
> + * instead put it away with mux_chip_free() and allocate a new one, if you
> + * for some reason would like to retry registration.
> + *
> + * Return: Zero on success or a negative errno on error.
> + */
> +int mux_chip_register(struct mux_chip *mux_chip);
> +
> +/**
> + * mux_chip_unregister() - Take the mux-chip off-line.
> + * @mux_chip: The mux-chip to unregister.
> + *
> + * mux_chip_unregister() reverses the effects of mux_chip_register().
> + * But not completely, you should not try to call mux_chip_register()
> + * on a mux-chip that has been registered before.
> + */
> +void mux_chip_unregister(struct mux_chip *mux_chip);
> +
> +/**
> + * mux_chip_free() - Free the mux-chip for good.
> + * @mux_chip: The mux-chip to free.
> + *
> + * mux_chip_free() reverses the effects of mux_chip_alloc().
> + */
> +void mux_chip_free(struct mux_chip *mux_chip);
> +
> +/**
> + * mux_control_select() - Select the given multiplexer state.
> + * @mux: The mux-control to request a change of state from.
> + * @state: The new requested state.
> + *
> + * Make sure to call mux_control_deselect() when the operation is complete and
> + * the mux-control is free for others to use, but do not call
> + * mux_control_deselect() if mux_control_select() fails.
> + *
> + * Return: 0 if the requested state was already active, or 1 it the
> + * mux-control state was changed to the requested state. Or a negavive
> + * errno on error.
> + *
> + * Note that the difference in return value of zero or one is of
> + * questionable value; especially if the mux-control has several independent
> + * consumers, which is something the consumers should perhaps not be making
> + * assumptions about.
> + */
> +int mux_control_select(struct mux_control *mux, int state);
> +
> +/**
> + * mux_control_deselect() - Deselect the previously selected multiplexer state.
> + * @mux: The mux-control to deselect.
> + *
> + * Return: 0 on success and a negative errno on error. An error can only
> + * occur if the mux has an idle state. Note that even if an error occurs, the
> + * mux-control is unlocked for others to access.
> + */
> +int mux_control_deselect(struct mux_control *mux);
> +
> +/**
> + * mux_control_get_index() - Get the index of the given mux controller
> + * @mux: The mux-control to the the index for.
> + *
> + * Return: The index of the mux controller within the mux chip the mux
> + * controller is a part of.
> + */
> +static inline unsigned int mux_control_get_index(struct mux_control *mux)
> +{
> +	return mux - mux->chip->mux;
> +}
> +
> +/**
> + * mux_control_get() - Get the mux-control for a device.
> + * @dev: The device that needs a mux-control.
> + * @mux_name: The name identifying the mux-control.
> + *
> + * Return: A pointer to the mux-control, or an ERR_PTR with a negative errno.
> + */
> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name);
> +
> +/**
> + * mux_control_put() - Put away the mux-control for good.
> + * @mux: The mux-control to put away.
> + *
> + * mux_control_put() reverses the effects of mux_control_get().
> + */
> +void mux_control_put(struct mux_control *mux);
> +
> +/**
> + * devm_mux_control_get() - Get the mux-control for a device, with resource
> + *			    management.
> + * @dev: The device that needs a mux-control.
> + * @mux_name: The name identifying the mux-control.
> + *
> + * Return: Pointer to the mux-control, or an ERR_PTR with a negative errno.
> + */
> +struct mux_control *devm_mux_control_get(struct device *dev,
> +					 const char *mux_name);
> +
> +/**
> + * devm_mux_control_put() - Resource-managed version mux_control_put().
> + * @dev: The device that originally got the mux-control.
> + * @mux: The mux-control to put away.
> + *
> + * Note that you do not normally need to call this function.
> + */
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux);
> +
> +#endif /* _LINUX_MUX_H */
> 

^ permalink raw reply

* Re: [PATCH v6 5/9] iio: multiplexer: new iio category and iio-mux driver
From: Jonathan Cameron @ 2016-12-31 16:21 UTC (permalink / raw)
  To: Peter Rosin, linux-kernel
  Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Jonathan Corbet,
	Arnd Bergmann, Greg Kroah-Hartman, linux-i2c, devicetree,
	linux-iio, linux-doc
In-Reply-To: <1480493823-21462-6-git-send-email-peda@axentia.se>

On 30/11/16 08:16, Peter Rosin wrote:
> When a multiplexer changes how an iio device behaves (for example
> by feeding different signals to an ADC), this driver can be used
> to create one virtual iio channel for each multiplexer state.
> 
> Depends on the generic multiplexer subsystem.
> 
> Cache any ext_info values from the parent iio channel, creating a private
> copy of the ext_info attributes for each multiplexer state/channel.
> 
> Signed-off-by: Peter Rosin <peda@axentia.se>
Other than binding naming, I'm happy with this - so pending that changing...

Reviewed-by: Jonathan Cameron <jic23@kernel.org>

If it makes sense I can obviously take this via IIO once the core is in
place.

Jonathan
> ---
>  MAINTAINERS                       |   1 +
>  drivers/iio/Kconfig               |   1 +
>  drivers/iio/Makefile              |   1 +
>  drivers/iio/multiplexer/Kconfig   |  18 ++
>  drivers/iio/multiplexer/Makefile  |   6 +
>  drivers/iio/multiplexer/iio-mux.c | 456 ++++++++++++++++++++++++++++++++++++++
>  6 files changed, 483 insertions(+)
>  create mode 100644 drivers/iio/multiplexer/Kconfig
>  create mode 100644 drivers/iio/multiplexer/Makefile
>  create mode 100644 drivers/iio/multiplexer/iio-mux.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 77045ae15865..16490fbd1721 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6239,6 +6239,7 @@ M:	Peter Rosin <peda@axentia.se>
>  L:	linux-iio@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/iio/multiplexer/iio-mux.txt
> +F:	drivers/iio/multiplexer/iio-mux.c
>  
>  IIO SUBSYSTEM AND DRIVERS
>  M:	Jonathan Cameron <jic23@kernel.org>
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index a918270d6f54..b3c8c6ef0dff 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -83,6 +83,7 @@ source "drivers/iio/humidity/Kconfig"
>  source "drivers/iio/imu/Kconfig"
>  source "drivers/iio/light/Kconfig"
>  source "drivers/iio/magnetometer/Kconfig"
> +source "drivers/iio/multiplexer/Kconfig"
>  source "drivers/iio/orientation/Kconfig"
>  if IIO_TRIGGER
>     source "drivers/iio/trigger/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 33fa4026f92c..93c769cd99bf 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -28,6 +28,7 @@ obj-y += humidity/
>  obj-y += imu/
>  obj-y += light/
>  obj-y += magnetometer/
> +obj-y += multiplexer/
>  obj-y += orientation/
>  obj-y += potentiometer/
>  obj-y += potentiostat/
> diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
> new file mode 100644
> index 000000000000..70a044510686
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Kconfig
> @@ -0,0 +1,18 @@
> +#
> +# Multiplexer drivers
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Multiplexers"
> +
> +config IIO_MUX
> +	tristate "IIO multiplexer driver"
> +	select MULTIPLEXER
> +	depends on OF
> +	help
> +	  Say yes here to build support for the IIO multiplexer.
> +
> +	  To compile this driver as a module, choose M here: the
> +	  module will be called iio-mux.
> +
> +endmenu
> diff --git a/drivers/iio/multiplexer/Makefile b/drivers/iio/multiplexer/Makefile
> new file mode 100644
> index 000000000000..68be3c4abd07
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for industrial I/O multiplexer drivers
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_IIO_MUX) += iio-mux.o
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> new file mode 100644
> index 000000000000..92dfee2dfed1
> --- /dev/null
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -0,0 +1,456 @@
> +/*
> + * IIO multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_ext_info_cache {
> +	char *data;
> +	size_t size;
> +};
> +
> +struct mux_child {
> +	struct mux_ext_info_cache *ext_info_cache;
> +};
> +
> +struct mux {
> +	int cached_state;
> +	struct mux_control *control;
> +	struct iio_channel *parent;
> +	struct iio_dev *indio_dev;
> +	struct iio_chan_spec *chan;
> +	struct iio_chan_spec_ext_info *ext_info;
> +	struct mux_child *child;
> +};
> +
> +static int iio_mux_select(struct mux *mux, int idx)
> +{
> +	struct mux_child *child = &mux->child[idx];
> +	struct iio_chan_spec const *chan = &mux->chan[idx];
> +	int ret;
> +	int i;
> +
> +	ret = mux_control_select(mux->control, chan->channel);
> +	if (ret < 0) {
> +		mux->cached_state = -1;
> +		return ret;
> +	}
> +
> +	if (mux->cached_state == chan->channel)
> +		return 0;
> +
> +	if (chan->ext_info) {
> +		for (i = 0; chan->ext_info[i].name; ++i) {
> +			const char *attr = chan->ext_info[i].name;
> +			struct mux_ext_info_cache *cache;
> +
> +			cache = &child->ext_info_cache[i];
> +
> +			if (cache->size < 0)
> +				continue;
> +
> +			ret = iio_write_channel_ext_info(mux->parent, attr,
> +							 cache->data,
> +							 cache->size);
> +
> +			if (ret < 0) {
> +				mux_control_deselect(mux->control);
> +				mux->cached_state = -1;
> +				return ret;
> +			}
> +		}
> +	}
> +	mux->cached_state = chan->channel;
> +
> +	return 0;
> +}
> +
> +static void iio_mux_deselect(struct mux *mux)
> +{
> +	mux_control_deselect(mux->control);
> +}
> +
> +static int mux_read_raw(struct iio_dev *indio_dev,
> +			struct iio_chan_spec const *chan,
> +			int *val, int *val2, long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->chan;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_read_channel_raw(mux->parent, val);
> +		break;
> +
> +	case IIO_CHAN_INFO_SCALE:
> +		ret = iio_read_channel_scale(mux->parent, val, val2);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_read_avail(struct iio_dev *indio_dev,
> +			  struct iio_chan_spec const *chan,
> +			  const int **vals, int *type, int *length,
> +			  long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->chan;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		*type = IIO_VAL_INT;
> +		ret = iio_read_avail_channel_raw(mux->parent, vals, length);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_write_raw(struct iio_dev *indio_dev,
> +			 struct iio_chan_spec const *chan,
> +			 int val, int val2, long mask)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->chan;
> +	int ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = iio_write_channel_raw(mux->parent, val);
> +		break;
> +
> +	default:
> +		ret = -EINVAL;
> +	}
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static const struct iio_info mux_info = {
> +	.read_raw = mux_read_raw,
> +	.read_avail = mux_read_avail,
> +	.write_raw = mux_write_raw,
> +	.driver_module = THIS_MODULE,
> +};
> +
> +static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> +				 struct iio_chan_spec const *chan, char *buf)
> +{
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->chan;
> +	ssize_t ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = iio_read_channel_ext_info(mux->parent,
> +					mux->ext_info[private].name,
> +					buf);
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> +				  struct iio_chan_spec const *chan,
> +				  const char *buf, size_t len)
> +{
> +	struct device *dev = indio_dev->dev.parent;
> +	struct mux *mux = iio_priv(indio_dev);
> +	int idx = chan - mux->chan;
> +	char *new;
> +	ssize_t ret;
> +
> +	ret = iio_mux_select(mux, idx);
> +	if (ret < 0)
> +		return ret;
> +
> +	new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
> +	if (!new) {
> +		iio_mux_deselect(mux);
> +		return -ENOMEM;
> +	}
> +
> +	new[len] = 0;
> +
> +	ret = iio_write_channel_ext_info(mux->parent,
> +					 mux->ext_info[private].name,
> +					 buf, len);
> +	if (ret < 0) {
> +		iio_mux_deselect(mux);
> +		devm_kfree(dev, new);
> +		return ret;
> +	}
> +
> +	devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
> +	mux->child[idx].ext_info_cache[private].data = new;
> +	mux->child[idx].ext_info_cache[private].size = len;
> +
> +	iio_mux_deselect(mux);
> +
> +	return ret;
> +}
> +
> +static int mux_configure_channel(struct device *dev, struct mux *mux,
> +				 u32 state, const char *label, int idx)
> +{
> +	struct mux_child *child = &mux->child[idx];
> +	struct iio_chan_spec *chan = &mux->chan[idx];
> +	struct iio_chan_spec const *pchan = mux->parent->channel;
> +	char *page = NULL;
> +	int num_ext_info;
> +	int i;
> +	int ret;
> +
> +	chan->indexed = 1;
> +	chan->output = pchan->output;
> +	chan->datasheet_name = label;
> +	chan->ext_info = mux->ext_info;
> +
> +	ret = iio_get_channel_type(mux->parent, &chan->type);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get parent channel type\n");
> +		return ret;
> +	}
> +
> +	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_RAW))
> +		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
> +	if (iio_channel_has_info(pchan, IIO_CHAN_INFO_SCALE))
> +		chan->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
> +
> +	if (iio_channel_has_available(pchan, IIO_CHAN_INFO_RAW))
> +		chan->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> +
> +	if (state >= mux->control->states) {
> +		dev_err(dev, "too many channels\n");
> +		return -EINVAL;
> +	}
> +
> +	chan->channel = state;
> +
> +	num_ext_info = iio_get_channel_ext_info_count(mux->parent);
> +	if (num_ext_info) {
> +		page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
> +		if (!page)
> +			return -ENOMEM;
> +	}
> +	child->ext_info_cache = devm_kzalloc(dev,
> +					     sizeof(*child->ext_info_cache) *
> +					     num_ext_info, GFP_KERNEL);
> +	for (i = 0; i < num_ext_info; ++i) {
> +		child->ext_info_cache[i].size = -1;
> +
> +		if (!pchan->ext_info[i].write)
> +			continue;
> +		if (!pchan->ext_info[i].read)
> +			continue;
> +
> +		ret = iio_read_channel_ext_info(mux->parent,
> +						mux->ext_info[i].name,
> +						page);
> +		if (ret < 0) {
> +			dev_err(dev, "failed to get ext_info '%s'\n",
> +				pchan->ext_info[i].name);
> +			return ret;
> +		}
> +		if (ret >= PAGE_SIZE) {
> +			dev_err(dev, "too large ext_info '%s'\n",
> +				pchan->ext_info[i].name);
> +			return -EINVAL;
> +		}
> +
> +		child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
> +							     GFP_KERNEL);
> +		child->ext_info_cache[i].data[ret] = 0;
> +		child->ext_info_cache[i].size = ret;
> +	}
> +
> +	if (page)
> +		devm_kfree(dev, page);
> +
> +	return 0;
> +}
> +
> +/*
> + * Same as of_property_for_each_string(), but also keeps track of the
> + * index of each string.
> + */
> +#define of_property_for_each_string_index(np, propname, prop, s, i)	\
> +	for (prop = of_find_property(np, propname, NULL),		\
> +	     s = of_prop_next_string(prop, NULL),			\
> +	     i = 0;							\
> +	     s;								\
> +	     s = of_prop_next_string(prop, s),				\
> +	     i++)
> +
> +static int mux_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = pdev->dev.of_node;
> +	struct iio_dev *indio_dev;
> +	struct iio_channel *parent;
> +	struct mux *mux;
> +	struct property *prop;
> +	const char *label;
> +	u32 state;
> +	int sizeof_ext_info;
> +	int children;
> +	int sizeof_priv;
> +	int i;
> +	int ret;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	parent = devm_iio_channel_get(dev, "parent");
> +	if (IS_ERR(parent)) {
> +		if (PTR_ERR(parent) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get parent channel\n");
> +		return PTR_ERR(parent);
> +	}
> +
> +	sizeof_ext_info = iio_get_channel_ext_info_count(parent);
> +	if (sizeof_ext_info) {
> +		sizeof_ext_info += 1; /* one extra entry for the sentinel */
> +		sizeof_ext_info *= sizeof(*mux->ext_info);
> +	}
> +
> +	children = 0;
> +	of_property_for_each_string(np, "channels", prop, label) {
> +		if (*label)
> +			children++;
> +	}
> +	if (children <= 0) {
> +		dev_err(dev, "not even a single child\n");
> +		return -EINVAL;
> +	}
> +
> +	sizeof_priv = sizeof(*mux);
> +	sizeof_priv += sizeof(*mux->child) * children;
> +	sizeof_priv += sizeof(*mux->chan) * children;
> +	sizeof_priv += sizeof_ext_info;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof_priv);
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	mux = iio_priv(indio_dev);
> +	mux->child = (struct mux_child *)(mux + 1);
> +	mux->chan = (struct iio_chan_spec *)(mux->child + children);
> +
> +	platform_set_drvdata(pdev, indio_dev);
> +
> +	mux->parent = parent;
> +	mux->cached_state = -1;
> +
> +	indio_dev->name = dev_name(dev);
> +	indio_dev->dev.parent = dev;
> +	indio_dev->info = &mux_info;
> +	indio_dev->modes = INDIO_DIRECT_MODE;
> +	indio_dev->channels = mux->chan;
> +	indio_dev->num_channels = children;
> +	if (sizeof_ext_info) {
> +		mux->ext_info = devm_kmemdup(dev,
> +					     parent->channel->ext_info,
> +					     sizeof_ext_info, GFP_KERNEL);
> +		if (!mux->ext_info)
> +			return -ENOMEM;
> +
> +		for (i = 0; mux->ext_info[i].name; ++i) {
> +			if (parent->channel->ext_info[i].read)
> +				mux->ext_info[i].read = mux_read_ext_info;
> +			if (parent->channel->ext_info[i].write)
> +				mux->ext_info[i].write = mux_write_ext_info;
> +			mux->ext_info[i].private = i;
> +		}
> +	}
> +
> +	mux->control = devm_mux_control_get(dev, NULL);
> +	if (IS_ERR(mux->control)) {
> +		if (PTR_ERR(mux->control) != -EPROBE_DEFER)
> +			dev_err(dev, "failed to get control-mux\n");
> +		return PTR_ERR(mux->control);
> +	}
> +
> +	i = 0;
> +	of_property_for_each_string_index(np, "channels", prop, label, state) {
> +		if (!*label)
> +			continue;
> +
> +		ret = mux_configure_channel(dev, mux, state, label, i++);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret) {
> +		dev_err(dev, "failed to register iio device\n");
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id mux_match[] = {
> +	{ .compatible = "iio-mux" },
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_match);
> +
> +static struct platform_driver mux_driver = {
> +	.probe = mux_probe,
> +	.driver = {
> +		.name = "iio-mux",
> +		.of_match_table = mux_match,
> +	},
> +};
> +module_platform_driver(mux_driver);
> +
> +MODULE_DESCRIPTION("IIO multiplexer driver");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
> 


^ 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