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] ARM: dts: Add dra7 iodelay configuration and use it for MMC
From: Tony Lindgren @ 2016-12-30 18:49 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Gary Bisson, Grygorii Strashko, Mark Rutland, Nishanth Menon,
	Rob Herring, devicetree, linux-gpio, linux-omap, Lokesh Vutla
In-Reply-To: <20161230183732.5595-3-tony@atomide.com>

Add dra7 iodelay configuration and use it for MMC.

Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi | 21 ++++++++++++++++++++-
 arch/arm/boot/dts/dra7.dtsi                     |  8 ++++++++
 include/dt-bindings/pinctrl/dra.h               |  4 ++++
 3 files changed, 32 insertions(+), 1 deletion(-)
---

And ere are the related dts changes.

diff --git a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
--- a/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
+++ b/arch/arm/boot/dts/am57xx-beagle-x15-common.dtsi
@@ -190,6 +190,25 @@
 		>;
 	};
 };
+
+&dra7_iodelay_core
+{
+	mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf {
+		pinctrl-pin-array = <
+			0x18c A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A19_IN */
+			0x1a4 A_DELAY_PS(265) G_DELAY_PS(360)	/* CFG_GPMC_A20_IN */
+			0x1b0 A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A21_IN */
+			0x1bc A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A22_IN */
+			0x1c8 A_DELAY_PS(287) G_DELAY_PS(420)	/* CFG_GPMC_A23_IN */
+			0x1d4 A_DELAY_PS(144) G_DELAY_PS(240)	/* CFG_GPMC_A24_IN */
+			0x1e0 A_DELAY_PS(0) G_DELAY_PS(0)	/* CFG_GPMC_A25_IN */
+			0x1ec A_DELAY_PS(120) G_DELAY_PS(0)	/* CFG_GPMC_A26_IN */
+			0x1f8 A_DELAY_PS(120) G_DELAY_PS(180)	/* CFG_GPMC_A27_IN */
+			0x360 A_DELAY_PS(0) G_DELAY_PS(0)	/* CFG_GPMC_CS1_IN */
+		>;
+	};
+};
+
 &i2c1 {
 	status = "okay";
 	clock-frequency = <400000>;
@@ -452,7 +471,7 @@
 	status = "okay";
 
 	pinctrl-names = "default";
-	pinctrl-0 = <&mmc2_pins_default>;
+	pinctrl-0 = <&mmc2_pins_default &mmc2_iodelay_3v3_conf>;
 
 	vmmc-supply = <&vdd_3v3>;
 	bus-width = <8>;
diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -401,6 +401,14 @@
 			reg = <0x40d00000 0x100>;
 		};
 
+		dra7_iodelay_core: padconf@4844a000 {
+			compatible = "ti,dra7-iodelay";
+			reg = <0x4844a000 0x0d1c>;
+			#address-cells = <1>;
+			#size-cells = <0>;
+			#pinctrl-cells = <2>;
+		};
+
 		sdma: dma-controller@4a056000 {
 			compatible = "ti,omap4430-sdma";
 			reg = <0x4a056000 0x1000>;
diff --git a/include/dt-bindings/pinctrl/dra.h b/include/dt-bindings/pinctrl/dra.h
--- a/include/dt-bindings/pinctrl/dra.h
+++ b/include/dt-bindings/pinctrl/dra.h
@@ -73,5 +73,9 @@
  */
 #define DRA7XX_CORE_IOPAD(pa, val)	(((pa) & 0xffff) - 0x3400) (val)
 
+/* DRA7 IODELAY configuration parameters */
+#define A_DELAY_PS(val)		((val) & 0xffff)
+#define G_DELAY_PS(val)		((val) & 0xffff)
+
 #endif
 
-- 
2.11.0

^ permalink raw reply

* [PATCH 2/2] pinctrl: Introduce TI IOdelay configuration driver
From: Tony Lindgren @ 2016-12-30 18:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Gary Bisson, Grygorii Strashko, Mark Rutland, Nishanth Menon,
	Rob Herring, devicetree, linux-gpio, linux-omap, Lokesh Vutla
In-Reply-To: <20161230183732.5595-1-tony@atomide.com>

From: Nishanth Menon <nm@ti.com>

SoC family such as DRA7 family of processors have, in addition
to the regular muxing of pins (as done by pinctrl-single), a separate
hardware module called IODelay which is also expected to be configured.
The "IODelay" module has it's own register space that is independent
of the control module and the padconf register area.

With recent changes to the pinctrl framework, we can now support
this hardware with a reasonably minimal driver by using #pinctrl-cells,
GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUNCTIONS.

It is advocated strongly in TI's official documentation considering
the existing design of the DRA7 family of processors during mux or
IODelay reconfiguration, there is a potential for a significant glitch
which may cause functional impairment to certain hardware. It is
hence recommended to do as little of muxing as absolutely necessary
without I/O isolation (which can only be done in initial stages of
bootloader).

NOTE: with the system wide I/O isolation scheme present in DRA7 SoC
family, it is not reasonable to do stop all I/O operations for every
such pad configuration scheme. So, we will let it glitch when used in
this mode.

Even with the above limitation, certain functionality such as MMC has
mandatory need for IODelay reconfiguration requirements, depending on
speed of transfer. In these cases, with careful examination of usecase
involved, the expected glitch can be controlled such that it does not
impact functionality.

In short, IODelay module support as a padconf driver being introduced
here is not expected to do SoC wide I/O Isolation and is meant for
a limited subset of IODelay configuration requirements that need to
be dynamic and whose glitchy behavior will not cause functionality
failure for that interface.

IMPORTANT NOTE: we take the approach of keeping LOCK_BITs cleared
to 0x0 at all times, even when configuring Manual IO Timing Modes.
This is done by eliminating the LOCK_BIT=1 setting from Step
of the Manual IO timing Mode configuration procedure. This option
leaves the CFG_* registers unprotected from unintended writes to the
CTRL_CORE_PAD_* registers while Manual IO Timing Modes are configured.

This approach is taken to allow for a generic driver to exist in kernel
world that has to be used carefully in required usecases.

Signed-off-by: Nishanth Menon <nm@ti.com>
Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
[tony@atomide.com: updated to use generic pinctrl functions, added
 binding documentation, updated comments]
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
 .../devicetree/bindings/pinctrl/ti,iodelay.txt     |  47 +
 drivers/pinctrl/Kconfig                            |   1 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/ti/Kconfig                         |  10 +
 drivers/pinctrl/ti/Makefile                        |   1 +
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c            | 945 +++++++++++++++++++++
 6 files changed, 1005 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,iodelay.txt
 create mode 100644 drivers/pinctrl/ti/Kconfig
 create mode 100644 drivers/pinctrl/ti/Makefile
 create mode 100644 drivers/pinctrl/ti/pinctrl-ti-iodelay.c

diff --git a/Documentation/devicetree/bindings/pinctrl/ti,iodelay.txt b/Documentation/devicetree/bindings/pinctrl/ti,iodelay.txt
new file mode 100644
--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/ti,iodelay.txt
@@ -0,0 +1,47 @@
+* Pin configuration for TI IODELAY controller
+
+TI dra7 based SoCs such as am57xx have a controller for setting the IO delay
+for each pin. For most part the IO delay values are programmed by the bootloader,
+but some pins need to be configured dynamically by the kernel such as the
+MMC pins.
+
+Required Properties:
+
+  - compatible: Must be "ti,dra7-iodelay"
+  - reg: Base address and length of the memory resource used
+  - #address-cells: Number of address cells
+  - #size-cells: Size of cells
+  - #pinctrl-cells: Number of pinctrl cells, must be 2. See also
+    Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+
+Example
+-------
+
+In the SoC specific dtsi file:
+
+	dra7_iodelay_core: padconf@4844a000 {
+		compatible = "ti,dra7-iodelay";
+		reg = <0x4844a000 0x0d1c>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		#pinctrl-cells = <2>;
+	};
+
+In board-specific file:
+
+&dra7_iodelay_core {
+	mmc2_iodelay_3v3_conf: mmc2_iodelay_3v3_conf {
+		pinctrl-pin-array = <
+		0x18c A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A19_IN */
+		0x1a4 A_DELAY_PS(265) G_DELAY_PS(360)	/* CFG_GPMC_A20_IN */
+		0x1b0 A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A21_IN */
+		0x1bc A_DELAY_PS(0) G_DELAY_PS(120)	/* CFG_GPMC_A22_IN */
+		0x1c8 A_DELAY_PS(287) G_DELAY_PS(420)	/* CFG_GPMC_A23_IN */
+		0x1d4 A_DELAY_PS(144) G_DELAY_PS(240)	/* CFG_GPMC_A24_IN */
+		0x1e0 A_DELAY_PS(0) G_DELAY_PS(0)	/* CFG_GPMC_A25_IN */
+		0x1ec A_DELAY_PS(120) G_DELAY_PS(0)	/* CFG_GPMC_A26_IN */
+		0x1f8 A_DELAY_PS(120) G_DELAY_PS(180)	/* CFG_GPMC_A27_IN */
+		0x360 A_DELAY_PS(0) G_DELAY_PS(0)	/* CFG_GPMC_CS1_IN */
+		>;
+	};
+};
diff --git a/drivers/pinctrl/Kconfig b/drivers/pinctrl/Kconfig
--- a/drivers/pinctrl/Kconfig
+++ b/drivers/pinctrl/Kconfig
@@ -300,6 +300,7 @@ source "drivers/pinctrl/spear/Kconfig"
 source "drivers/pinctrl/stm32/Kconfig"
 source "drivers/pinctrl/sunxi/Kconfig"
 source "drivers/pinctrl/tegra/Kconfig"
+source "drivers/pinctrl/ti/Kconfig"
 source "drivers/pinctrl/uniphier/Kconfig"
 source "drivers/pinctrl/vt8500/Kconfig"
 source "drivers/pinctrl/mediatek/Kconfig"
diff --git a/drivers/pinctrl/Makefile b/drivers/pinctrl/Makefile
--- a/drivers/pinctrl/Makefile
+++ b/drivers/pinctrl/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_PINCTRL_SH_PFC)	+= sh-pfc/
 obj-$(CONFIG_PINCTRL_SPEAR)	+= spear/
 obj-$(CONFIG_PINCTRL_STM32)	+= stm32/
 obj-$(CONFIG_PINCTRL_SUNXI)	+= sunxi/
+obj-y				+= ti/
 obj-$(CONFIG_PINCTRL_UNIPHIER)	+= uniphier/
 obj-$(CONFIG_ARCH_VT8500)	+= vt8500/
 obj-$(CONFIG_PINCTRL_MTK)	+= mediatek/
diff --git a/drivers/pinctrl/ti/Kconfig b/drivers/pinctrl/ti/Kconfig
new file mode 100644
--- /dev/null
+++ b/drivers/pinctrl/ti/Kconfig
@@ -0,0 +1,10 @@
+config PINCTRL_TI_IODELAY
+	tristate "TI IODelay Module pinconf driver"
+	depends on OF
+	select GENERIC_PINCTRL_GROUPS
+	select GENERIC_PINMUX_FUNCTIONS
+	select GENERIC_PINCONF
+	select REGMAP_MMIO
+	help
+	  Say Y here to support Texas Instruments' IO delay pinconf driver.
+	  IO delay module is used for the DRA7 SoC family.
diff --git a/drivers/pinctrl/ti/Makefile b/drivers/pinctrl/ti/Makefile
new file mode 100644
--- /dev/null
+++ b/drivers/pinctrl/ti/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_PINCTRL_TI_IODELAY)	+= pinctrl-ti-iodelay.o
diff --git a/drivers/pinctrl/ti/pinctrl-ti-iodelay.c b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
new file mode 100644
--- /dev/null
+++ b/drivers/pinctrl/ti/pinctrl-ti-iodelay.c
@@ -0,0 +1,945 @@
+/*
+ * Support for configuration of IO Delay module found on Texas Instruments SoCs
+ * such as DRA7
+ *
+ * Copyright (C) 2015 Texas Instruments, Inc.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/err.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/pinctrl/pinconf.h>
+#include <linux/pinctrl/pinconf-generic.h>
+#include <linux/pinctrl/pinctrl.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+#include "../core.h"
+#include "../devicetree.h"
+
+#define DRIVER_NAME	"ti-iodelay"
+
+/**
+ * struct ti_iodelay_reg_data - Describes the registers for the iodelay instance
+ * @signature_mask: CONFIG_REG mask for the signature bits (see TRM)
+ * @signature_value: CONFIG_REG signature value to be written (see TRM)
+ * @lock_mask: CONFIG_REG mask for the lock bits (see TRM)
+ * @lock_val: CONFIG_REG lock value for the lock bits (see TRM)
+ * @unlock_val:CONFIG_REG unlock value for the lock bits (see TRM)
+ * @binary_data_coarse_mask: CONFIG_REG coarse mask (see TRM)
+ * @binary_data_fine_mask: CONFIG_REG fine mask (see TRM)
+ * @reg_refclk_offset: Refclk register offset
+ * @refclk_period_mask: Refclk mask
+ * @reg_coarse_offset: Coarse register configuration offset
+ * @coarse_delay_count_mask: Coarse delay count mask
+ * @coarse_ref_count_mask: Coarse ref count mask
+ * @reg_fine_offset: Fine register configuration offset
+ * @fine_delay_count_mask: Fine delay count mask
+ * @fine_ref_count_mask: Fine ref count mask
+ * @reg_global_lock_offset: Global iodelay module lock register offset
+ * @global_lock_mask: Lock mask
+ * @global_unlock_val: Unlock value
+ * @global_lock_val: Lock value
+ * @reg_start_offset: Offset to iodelay registers after the CONFIG_REG_0 to 8
+ * @reg_nr_per_pin: Number of iodelay registers for each pin
+ * @regmap_config: Regmap configuration for the IODelay region
+ */
+struct ti_iodelay_reg_data {
+	u32 signature_mask;
+	u32 signature_value;
+	u32 lock_mask;
+	u32 lock_val;
+	u32 unlock_val;
+	u32 binary_data_coarse_mask;
+	u32 binary_data_fine_mask;
+
+	u32 reg_refclk_offset;
+	u32 refclk_period_mask;
+
+	u32 reg_coarse_offset;
+	u32 coarse_delay_count_mask;
+	u32 coarse_ref_count_mask;
+
+	u32 reg_fine_offset;
+	u32 fine_delay_count_mask;
+	u32 fine_ref_count_mask;
+
+	u32 reg_global_lock_offset;
+	u32 global_lock_mask;
+	u32 global_unlock_val;
+	u32 global_lock_val;
+
+	u32 reg_start_offset;
+	u32 reg_nr_per_pin;
+
+	struct regmap_config *regmap_config;
+};
+
+/**
+ * struct ti_iodelay_reg_values - Computed io_reg configuration values (see TRM)
+ * @coarse_ref_count: Coarse reference count
+ * @coarse_delay_count: Coarse delay count
+ * @fine_ref_count: Fine reference count
+ * @fine_delay_count: Fine Delay count
+ * @ref_clk_period: Reference Clock period
+ * @cdpe: Coarse delay parameter
+ * @fdpe: Fine delay parameter
+ */
+struct ti_iodelay_reg_values {
+	u16 coarse_ref_count;
+	u16 coarse_delay_count;
+
+	u16 fine_ref_count;
+	u16 fine_delay_count;
+
+	u16 ref_clk_period;
+
+	u32 cdpe;
+	u32 fdpe;
+};
+
+/**
+ * struct ti_iodelay_cfg - Description of each configuration parameters
+ * @offset: Configuration register offset
+ * @a_delay: Agnostic Delay (in ps)
+ * @g_delay: Gnostic Delay (in ps)
+ */
+struct ti_iodelay_cfg {
+	u16 offset;
+	u16 a_delay;
+	u16 g_delay;
+};
+
+/**
+ * struct ti_iodelay_pingroup - Structure that describes one group
+ * @name: Name of the group
+ * @map: pinctrl map allocated for the group
+ * @cfg: configuration array for the pin (from dt)
+ * @ncfg: number of configuration values allocated
+ * @config: pinconf "Config" - currently a dummy value
+ */
+struct ti_iodelay_pingroup {
+	struct ti_iodelay_cfg *cfg;
+	int ncfg;
+	unsigned long config;
+};
+
+/**
+ * struct ti_iodelay_device - Represents information for a iodelay instance
+ * @dev: Device pointer
+ * @phys_base: Physical address base of the iodelay device
+ * @reg_base: Virtual address base of the iodelay device
+ * @regmap: Regmap for this iodelay instance
+ * @pctl: Pinctrl device
+ * @desc: pinctrl descriptor for pctl
+ * @pa: pinctrl pin wise description
+ * @names: names of the pins
+ * @reg_data: Register definition data for the IODelay instance
+ * @reg_init_conf_values: Initial configuration values.
+ */
+struct ti_iodelay_device {
+	struct device *dev;
+	unsigned long phys_base;
+	void __iomem *reg_base;
+	struct regmap *regmap;
+
+	struct pinctrl_dev *pctl;
+	struct pinctrl_desc desc;
+	struct pinctrl_pin_desc *pa;
+
+	const struct ti_iodelay_reg_data *reg_data;
+	struct ti_iodelay_reg_values reg_init_conf_values;
+};
+
+/**
+ * ti_iodelay_extract() - extract bits for a field
+ * @val: Register value
+ * @mask: Mask
+ *
+ * Return: extracted value which is appropriately shifted
+ */
+static inline u32 ti_iodelay_extract(u32 val, u32 mask)
+{
+	return (val & mask) >> __ffs(mask);
+}
+
+/**
+ * ti_iodelay_compute_dpe() - Compute equation for delay parameter
+ * @period: Period to use
+ * @ref: Reference Count
+ * @delay: Delay count
+ * @delay_m: Delay multiplier
+ *
+ * Return: Computed delay parameter
+ */
+static inline u32 ti_iodelay_compute_dpe(u16 period, u16 ref, u16 delay,
+					 u16 delay_m)
+{
+	u64 m, d;
+
+	/* Handle overflow conditions */
+	m = 10 * (u64)period * (u64)ref;
+	d = 2 * (u64)delay * (u64)delay_m;
+
+	/* Truncate result back to 32 bits */
+	return div64_u64(m, d);
+}
+
+/**
+ * ti_iodelay_pinconf_set() - Configure the pin configuration
+ * @iod: iodelay device
+ * @val: Configuration value
+ *
+ * Update the configuration register as per TRM and lockup once done.
+ * *IMPORTANT NOTE* SoC TRM does recommend doing iodelay programmation only
+ * while in Isolation. But, then, isolation also implies that every pin
+ * on the SoC (including DDR) will be isolated out. The only benefit being
+ * a glitchless configuration, However, the intent of this driver is purely
+ * to support a "glitchy" configuration where applicable.
+ *
+ * Return: 0 in case of success, else appropriate error value
+ */
+static int ti_iodelay_pinconf_set(struct ti_iodelay_device *iod,
+				  struct ti_iodelay_cfg *cfg)
+{
+	const struct ti_iodelay_reg_data *reg = iod->reg_data;
+	struct ti_iodelay_reg_values *ival = &iod->reg_init_conf_values;
+	struct device *dev = iod->dev;
+	u32 g_delay_coarse, g_delay_fine;
+	u32 a_delay_coarse, a_delay_fine;
+	u32 c_elements, f_elements;
+	u32 total_delay;
+	u32 reg_mask, reg_val, tmp_val;
+	int r;
+
+	/* NOTE: Truncation is expected in all division below */
+	g_delay_coarse = cfg->g_delay / 920;
+	g_delay_fine = ((cfg->g_delay % 920) * 10) / 60;
+
+	a_delay_coarse = cfg->a_delay / ival->cdpe;
+	a_delay_fine = ((cfg->a_delay % ival->cdpe) * 10) / ival->fdpe;
+
+	c_elements = g_delay_coarse + a_delay_coarse;
+	f_elements = (g_delay_fine + a_delay_fine) / 10;
+
+	if (f_elements > 22) {
+		total_delay = c_elements * ival->cdpe + f_elements * ival->fdpe;
+		c_elements = total_delay / ival->cdpe;
+		f_elements = (total_delay % ival->cdpe) / ival->fdpe;
+	}
+
+	reg_mask = reg->signature_mask;
+	reg_val = reg->signature_value << __ffs(reg->signature_mask);
+
+	reg_mask |= reg->binary_data_coarse_mask;
+	tmp_val = c_elements << __ffs(reg->binary_data_coarse_mask);
+	if (tmp_val & ~reg->binary_data_coarse_mask) {
+		dev_err(dev, "Masking overflow of coarse elements %08x\n",
+			tmp_val);
+		tmp_val &= reg->binary_data_coarse_mask;
+	}
+	reg_val |= tmp_val;
+
+	reg_mask |= reg->binary_data_fine_mask;
+	tmp_val = f_elements << __ffs(reg->binary_data_fine_mask);
+	if (tmp_val & ~reg->binary_data_fine_mask) {
+		dev_err(dev, "Masking overflow of fine elements %08x\n",
+			tmp_val);
+		tmp_val &= reg->binary_data_fine_mask;
+	}
+	reg_val |= tmp_val;
+
+	/*
+	 * NOTE: we leave the iodelay values unlocked - this is to work around
+	 * situations such as those found with mmc mode change.
+	 * However, this leaves open any unwarranted changes to padconf register
+	 * impacting iodelay configuration. Use with care!
+	 */
+	reg_mask |= reg->lock_mask;
+	reg_val |= reg->unlock_val << __ffs(reg->lock_mask);
+	r = regmap_update_bits(iod->regmap, cfg->offset, reg_mask, reg_val);
+
+	dev_info(dev, "Set reg 0x%x Delay(a: %d g: %d), Elements(C=%d F=%d)0x%x\n",
+		 cfg->offset, cfg->a_delay, cfg->g_delay, c_elements,
+		 f_elements, reg_val);
+
+	return r;
+}
+
+/**
+ * ti_iodelay_pinconf_init_dev() - Initialize IODelay device
+ * @iod: iodelay device
+ *
+ * Unlocks the iodelay region, computes the common parameters
+ *
+ * Return: 0 in case of success, else appropriate error value
+ */
+static int ti_iodelay_pinconf_init_dev(struct ti_iodelay_device *iod)
+{
+	const struct ti_iodelay_reg_data *reg = iod->reg_data;
+	struct device *dev = iod->dev;
+	struct ti_iodelay_reg_values *ival = &iod->reg_init_conf_values;
+	u32 val;
+	int r;
+
+	/* unlock the iodelay region */
+	r = regmap_update_bits(iod->regmap, reg->reg_global_lock_offset,
+			       reg->global_lock_mask, reg->global_unlock_val);
+	if (r)
+		return r;
+
+	/* Read up Recalibration sequence done by bootloader */
+	r = regmap_read(iod->regmap, reg->reg_refclk_offset, &val);
+	if (r)
+		return r;
+	ival->ref_clk_period = ti_iodelay_extract(val, reg->refclk_period_mask);
+	dev_dbg(dev, "refclk_period=0x%04x\n", ival->ref_clk_period);
+
+	r = regmap_read(iod->regmap, reg->reg_coarse_offset, &val);
+	if (r)
+		return r;
+	ival->coarse_ref_count =
+	    ti_iodelay_extract(val, reg->coarse_ref_count_mask);
+	ival->coarse_delay_count =
+	    ti_iodelay_extract(val, reg->coarse_delay_count_mask);
+	if (!ival->coarse_delay_count) {
+		dev_err(dev, "Invalid Coarse delay count (0) (reg=0x%08x)\n",
+			val);
+		return -EINVAL;
+	}
+	ival->cdpe = ti_iodelay_compute_dpe(ival->ref_clk_period,
+					    ival->coarse_ref_count,
+					    ival->coarse_delay_count, 88);
+	if (!ival->cdpe) {
+		dev_err(dev, "Invalid cdpe computed params = %d %d %d\n",
+			ival->ref_clk_period, ival->coarse_ref_count,
+			ival->coarse_delay_count);
+		return -EINVAL;
+	}
+	dev_dbg(iod->dev, "coarse: ref=0x%04x delay=0x%04x cdpe=0x%08x\n",
+		ival->coarse_ref_count, ival->coarse_delay_count, ival->cdpe);
+
+	r = regmap_read(iod->regmap, reg->reg_fine_offset, &val);
+	if (r)
+		return r;
+	ival->fine_ref_count =
+	    ti_iodelay_extract(val, reg->fine_ref_count_mask);
+	ival->fine_delay_count =
+	    ti_iodelay_extract(val, reg->fine_delay_count_mask);
+	if (!ival->fine_delay_count) {
+		dev_err(dev, "Invalid Fine delay count (0) (reg=0x%08x)\n",
+			val);
+		return -EINVAL;
+	}
+	ival->fdpe = ti_iodelay_compute_dpe(ival->ref_clk_period,
+					    ival->fine_ref_count,
+					    ival->fine_delay_count, 264);
+	if (!ival->fdpe) {
+		dev_err(dev, "Invalid fdpe(0) computed params = %d %d %d\n",
+			ival->ref_clk_period, ival->fine_ref_count,
+			ival->fine_delay_count);
+		return -EINVAL;
+	}
+	dev_dbg(iod->dev, "fine: ref=0x%04x delay=0x%04x fdpe=0x%08x\n",
+		ival->fine_ref_count, ival->fine_delay_count, ival->fdpe);
+
+	return 0;
+}
+
+/**
+ * ti_iodelay_pinconf_deinit_dev() - deinit the iodelay device
+ * @iod:	IODelay device
+ *
+ * Deinitialize the IODelay device (basically just lock the region back up.
+ */
+static void ti_iodelay_pinconf_deinit_dev(struct ti_iodelay_device *iod)
+{
+	const struct ti_iodelay_reg_data *reg = iod->reg_data;
+
+	/* lock the iodelay region back again */
+	regmap_update_bits(iod->regmap, reg->reg_global_lock_offset,
+			   reg->global_lock_mask, reg->global_lock_val);
+}
+
+/**
+ * ti_iodelay_get_pingroup() - Find the group mapped by a group selector
+ * @iod: iodelay device
+ * @selector: Group Selector
+ *
+ * Return: Corresponding group representing group selector
+ */
+static struct ti_iodelay_pingroup *
+ti_iodelay_get_pingroup(struct ti_iodelay_device *iod, unsigned int selector)
+{
+	struct group_desc *g;
+
+	g = pinctrl_generic_get_group(iod->pctl, selector);
+	if (!g) {
+		dev_err(iod->dev, "%s could not find pingroup %i\n", __func__,
+			selector);
+
+		return NULL;
+	}
+
+	return g->data;
+}
+
+/**
+ * ti_iodelay_pin_to_offset() - get pin register offset based on the pin index
+ * @iod: iodelay driver instance
+ * @selector: Pin index
+ */
+static unsigned int ti_iodelay_pin_to_offset(struct ti_iodelay_device *iod,
+					     unsigned int selector)
+{
+	const struct ti_iodelay_reg_data *r = iod->reg_data;
+	unsigned int offset;
+
+	offset = selector * r->regmap_config->reg_stride;
+	offset *= r->reg_nr_per_pin;
+	offset += r->reg_start_offset;
+
+	return offset;
+}
+
+/**
+ * ti_iodelay_offset_to_pin() - get a pin index based on the register offset
+ * @iod: iodelay driver instance
+ * @offset: register offset from the base
+ */
+static int ti_iodelay_offset_to_pin(struct ti_iodelay_device *iod,
+				    unsigned int offset)
+{
+	const struct ti_iodelay_reg_data *r = iod->reg_data;
+	unsigned int index;
+
+	if (offset > r->regmap_config->max_register) {
+		dev_err(iod->dev, "mux offset out of range: 0x%x (0x%x)\n",
+			offset, r->regmap_config->max_register);
+		return -EINVAL;
+	}
+
+	index = (offset - r->reg_start_offset) / r->regmap_config->reg_stride;
+	index /= r->reg_nr_per_pin;
+
+	return index;
+}
+
+/**
+ *
+ * @pctldev: Pin controller driver
+ * @np: Device node
+ * @pinctrl_spec: Parsed arguments from device tree
+ * @pins: Array of pins in the pin group
+ * @pin_index: Pin index in the pin array
+ * @data: Pin controller driver specific data
+ *
+ */
+static int ti_iodelay_node_iterator(struct pinctrl_dev *pctldev,
+				    struct device_node *np,
+				    const struct of_phandle_args *pinctrl_spec,
+				    int *pins, int pin_index, void *data)
+{
+	struct ti_iodelay_device *iod;
+	struct ti_iodelay_cfg *cfg = data;
+	const struct ti_iodelay_reg_data *r;
+	struct pinctrl_pin_desc *pd;
+	int pin;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	if (!iod)
+		return -EINVAL;
+
+	r = iod->reg_data;
+
+	if (pinctrl_spec->args_count < r->reg_nr_per_pin) {
+		dev_err(iod->dev, "invalid args_count for spec: %i\n",
+			pinctrl_spec->args_count);
+
+		return -EINVAL;
+	}
+
+	/* Index plus two value cells */
+	cfg[pin_index].offset = pinctrl_spec->args[0];
+	cfg[pin_index].a_delay = pinctrl_spec->args[1] & 0xffff;
+	cfg[pin_index].g_delay = pinctrl_spec->args[2] & 0xffff;
+
+	pin = ti_iodelay_offset_to_pin(iod, cfg[pin_index].offset);
+	if (pin < 0) {
+		dev_err(iod->dev, "could not add functions for %s %ux\n",
+			np->name, cfg[pin_index].offset);
+		return -ENODEV;
+	}
+	pins[pin_index] = pin;
+
+	pd = &iod->pa[pin];
+	pd->drv_data = &cfg[pin_index];
+
+	dev_dbg(iod->dev, "%s offset=%x a_delay = %d g_delay = %d\n",
+		np->name, cfg[pin_index].offset, cfg[pin_index].a_delay,
+		cfg[pin_index].g_delay);
+
+	return 0;
+}
+
+/**
+ * ti_iodelay_dt_node_to_map() - Map a device tree node to appropriate group
+ * @pctldev: pinctrl device representing IODelay device
+ * @np: Node Pointer (device tree)
+ * @map: Pinctrl Map returned back to pinctrl framework
+ * @num_maps: Number of maps (1)
+ *
+ * Maps the device tree description into a group of configuration parameters
+ * for iodelay block entry.
+ *
+ * Return: 0 in case of success, else appropriate error value
+ */
+static int ti_iodelay_dt_node_to_map(struct pinctrl_dev *pctldev,
+				     struct device_node *np,
+				     struct pinctrl_map **map,
+				     unsigned int *num_maps)
+{
+	struct ti_iodelay_device *iod;
+	struct ti_iodelay_cfg *cfg;
+	struct ti_iodelay_pingroup *g;
+	const char *name = "pinctrl-pin-array";
+	int rows, *pins, error = -EINVAL, i;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	if (!iod)
+		return -EINVAL;
+
+	rows = pinctrl_count_index_with_args(np, name);
+	if (rows == -EINVAL)
+		return rows;
+
+	*map = devm_kzalloc(iod->dev, sizeof(**map), GFP_KERNEL);
+	if (!*map)
+		return -ENOMEM;
+	*num_maps = 0;
+
+	g = devm_kzalloc(iod->dev, sizeof(*g), GFP_KERNEL);
+	if (!g) {
+		error = -ENOMEM;
+		goto free_map;
+	}
+
+	pins = devm_kzalloc(iod->dev, sizeof(*pins) * rows, GFP_KERNEL);
+	if (!pins)
+		goto free_group;
+
+	cfg = devm_kzalloc(iod->dev, sizeof(*cfg) * rows, GFP_KERNEL);
+	if (!cfg) {
+		error = -ENOMEM;
+		goto free_pins;
+	}
+
+	for (i = 0; i < rows; i++) {
+		struct of_phandle_args pinctrl_spec;
+
+		error = pinctrl_parse_index_with_args(np, name, i,
+						      &pinctrl_spec);
+		if (error)
+			goto free_data;
+
+		error = ti_iodelay_node_iterator(pctldev, np, &pinctrl_spec,
+						 pins, i, cfg);
+		if (error)
+			goto free_data;
+	}
+
+	g->cfg = cfg;
+	g->ncfg = i;
+	g->config = PIN_CONFIG_END;
+
+	error = pinctrl_generic_add_group(iod->pctl, np->name, pins, i, g);
+	if (error < 0)
+		goto free_data;
+
+	(*map)->type = PIN_MAP_TYPE_CONFIGS_GROUP;
+	(*map)->data.configs.group_or_pin = np->name;
+	(*map)->data.configs.configs = &g->config;
+	(*map)->data.configs.num_configs = 1;
+	*num_maps = 1;
+
+	return 0;
+
+free_data:
+	devm_kfree(iod->dev, cfg);
+free_pins:
+	devm_kfree(iod->dev, pins);
+free_group:
+	devm_kfree(iod->dev, g);
+free_map:
+	devm_kfree(iod->dev, *map);
+
+	return error;
+}
+
+/**
+ * ti_iodelay_pinconf_group_get() - Get the group configuration
+ * @pctldev: pinctrl device representing IODelay device
+ * @selector: Group selector
+ * @config: Configuration returned
+ *
+ * Return: The configuration if the group is valid, else returns -EINVAL
+ */
+static int ti_iodelay_pinconf_group_get(struct pinctrl_dev *pctldev,
+					unsigned int selector,
+					unsigned long *config)
+{
+	struct ti_iodelay_device *iod;
+	struct device *dev;
+	struct ti_iodelay_pingroup *group;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	dev = iod->dev;
+	group = ti_iodelay_get_pingroup(iod, selector);
+
+	if (!group)
+		return -EINVAL;
+
+	*config = group->config;
+	return 0;
+}
+
+/**
+ * ti_iodelay_pinconf_group_set() - Configure the groups of pins
+ * @pctldev: pinctrl device representing IODelay device
+ * @selector: Group selector
+ * @configs: Configurations
+ * @num_configs: Number of configurations
+ *
+ * Return: 0 if all went fine, else appropriate error value.
+ */
+static int ti_iodelay_pinconf_group_set(struct pinctrl_dev *pctldev,
+					unsigned int selector,
+					unsigned long *configs,
+					unsigned int num_configs)
+{
+	struct ti_iodelay_device *iod;
+	struct device *dev;
+	struct ti_iodelay_pingroup *group;
+	int i;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	dev = iod->dev;
+	group = ti_iodelay_get_pingroup(iod, selector);
+
+	if (num_configs != 1) {
+		dev_err(dev, "Unsupported number of configurations %d\n",
+			num_configs);
+		return -EINVAL;
+	}
+
+	if (*configs != PIN_CONFIG_END) {
+		dev_err(dev, "Unsupported configuration\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < group->ncfg; i++) {
+		if (ti_iodelay_pinconf_set(iod, &group->cfg[i]))
+			return -ENOTSUPP;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_DEBUG_FS
+static void ti_iodelay_pin_dbg_show(struct pinctrl_dev *pctldev,
+				    struct seq_file *s,
+				    unsigned int pin)
+{
+	struct ti_iodelay_device *iod;
+	struct pinctrl_pin_desc *pd;
+	struct ti_iodelay_cfg *cfg;
+	const struct ti_iodelay_reg_data *r;
+	unsigned long offset;
+	u32 in, oen, out;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	r = iod->reg_data;
+
+	offset = ti_iodelay_pin_to_offset(iod, pin);
+	if (pin < 0) {
+		dev_err(iod->dev, "invalid pin offset for pin%i\n", pin);
+
+		return;
+	}
+
+	pd = &iod->pa[pin];
+	cfg = pd->drv_data;
+
+	regmap_read(iod->regmap, offset, &in);
+	regmap_read(iod->regmap, offset + r->regmap_config->reg_stride, &oen);
+	regmap_read(iod->regmap, offset + r->regmap_config->reg_stride * 2,
+		    &out);
+
+	seq_printf(s, "%lx a: %i g: %i (%08x %08x %08x) %s ",
+		   iod->phys_base + offset,
+		   cfg ? cfg->a_delay : -1,
+		   cfg ? cfg->g_delay : -1,
+		   in, oen, out, DRIVER_NAME);
+}
+
+/**
+ * ti_iodelay_pinconf_group_dbg_show() - show the group information
+ * @pctldev: Show the group information
+ * @s: Sequence file
+ * @selector: Group selector
+ *
+ * Provide the configuration information of the selected group
+ */
+static void ti_iodelay_pinconf_group_dbg_show(struct pinctrl_dev *pctldev,
+					      struct seq_file *s,
+					      unsigned int selector)
+{
+	struct ti_iodelay_device *iod;
+	struct device *dev;
+	struct ti_iodelay_pingroup *group;
+	int i;
+
+	iod = pinctrl_dev_get_drvdata(pctldev);
+	dev = iod->dev;
+	group = ti_iodelay_get_pingroup(iod, selector);
+	if (!group)
+		return;
+
+	for (i = 0; i < group->ncfg; i++) {
+		struct ti_iodelay_cfg *cfg;
+		u32 reg = 0;
+
+		cfg = &group->cfg[i];
+		regmap_read(iod->regmap, cfg->offset, &reg),
+			seq_printf(s, "\n\t0x%08x = 0x%08x (%3d, %3d)",
+				   cfg->offset, reg, cfg->a_delay,
+				   cfg->g_delay);
+	}
+}
+#endif
+
+static struct pinctrl_ops ti_iodelay_pinctrl_ops = {
+	.get_groups_count = pinctrl_generic_get_group_count,
+	.get_group_name = pinctrl_generic_get_group_name,
+	.get_group_pins = pinctrl_generic_get_group_pins,
+	.pin_dbg_show = ti_iodelay_pin_dbg_show,
+	.dt_node_to_map = ti_iodelay_dt_node_to_map,
+};
+
+static struct pinconf_ops ti_iodelay_pinctrl_pinconf_ops = {
+	.pin_config_group_get = ti_iodelay_pinconf_group_get,
+	.pin_config_group_set = ti_iodelay_pinconf_group_set,
+#ifdef CONFIG_DEBUG_FS
+	.pin_config_group_dbg_show = ti_iodelay_pinconf_group_dbg_show,
+#endif
+};
+
+/**
+ * ti_iodelay_alloc_pins() - Allocate structures needed for pins for iodelay
+ * @dev: Device pointer
+ * @iod: iodelay device
+ * @base_phy: Base Physical Address
+ *
+ * Return: 0 if all went fine, else appropriate error value.
+ */
+static int ti_iodelay_alloc_pins(struct device *dev,
+				 struct ti_iodelay_device *iod, u32 base_phy)
+{
+	const struct ti_iodelay_reg_data *r = iod->reg_data;
+	struct pinctrl_pin_desc *pin;
+	u32 phy_reg;
+	int nr_pins, i;
+
+	nr_pins = ti_iodelay_offset_to_pin(iod, r->regmap_config->max_register);
+	dev_dbg(dev, "Allocating %i pins\n", nr_pins);
+
+	iod->pa = devm_kzalloc(dev, sizeof(*iod->pa) * nr_pins, GFP_KERNEL);
+	if (!iod->pa)
+		return -ENOMEM;
+
+	iod->desc.pins = iod->pa;
+	iod->desc.npins = nr_pins;
+
+	phy_reg = r->reg_start_offset + base_phy;
+
+	for (i = 0; i < nr_pins; i++, phy_reg += 4) {
+		pin = &iod->pa[i];
+		pin->number = i;
+	}
+
+	return 0;
+}
+
+static struct regmap_config dra7_iodelay_regmap_config = {
+	.reg_bits = 32,
+	.reg_stride = 4,
+	.val_bits = 32,
+	.max_register = 0xd1c,
+};
+
+static struct ti_iodelay_reg_data dra7_iodelay_data = {
+	.signature_mask = 0x0003f000,
+	.signature_value = 0x29,
+	.lock_mask = 0x00000400,
+	.lock_val = 1,
+	.unlock_val = 0,
+	.binary_data_coarse_mask = 0x000003e0,
+	.binary_data_fine_mask = 0x0000001f,
+
+	.reg_refclk_offset = 0x14,
+	.refclk_period_mask = 0xffff,
+
+	.reg_coarse_offset = 0x18,
+	.coarse_delay_count_mask = 0xffff0000,
+	.coarse_ref_count_mask = 0x0000ffff,
+
+	.reg_fine_offset = 0x1C,
+	.fine_delay_count_mask = 0xffff0000,
+	.fine_ref_count_mask = 0x0000ffff,
+
+	.reg_global_lock_offset = 0x2c,
+	.global_lock_mask = 0x0000ffff,
+	.global_unlock_val = 0x0000aaaa,
+	.global_lock_val = 0x0000aaab,
+
+	.reg_start_offset = 0x30,
+	.reg_nr_per_pin = 3,
+	.regmap_config = &dra7_iodelay_regmap_config,
+};
+
+static const struct of_device_id ti_iodelay_of_match[] = {
+	{.compatible = "ti,dra7-iodelay", .data = &dra7_iodelay_data},
+	{ /* Hopefully no more.. */ },
+};
+MODULE_DEVICE_TABLE(of, ti_iodelay_of_match);
+
+/**
+ * ti_iodelay_probe() - Standard probe
+ * @pdev: platform device
+ *
+ * Return: 0 if all went fine, else appropriate error value.
+ */
+static int ti_iodelay_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = of_node_get(dev->of_node);
+	const struct of_device_id *match;
+	struct resource *res;
+	struct ti_iodelay_device *iod;
+	int ret = 0;
+
+	if (!np) {
+		ret = -EINVAL;
+		dev_err(dev, "No OF node\n");
+		goto exit_out;
+	}
+
+	match = of_match_device(ti_iodelay_of_match, dev);
+	if (!match) {
+		ret = -EINVAL;
+		dev_err(dev, "No DATA match\n");
+		goto exit_out;
+	}
+
+	iod = devm_kzalloc(dev, sizeof(*iod), GFP_KERNEL);
+	if (!iod) {
+		ret = -ENOMEM;
+		goto exit_out;
+	}
+	iod->dev = dev;
+	iod->reg_data = match->data;
+
+	/* So far We can assume there is only 1 bank of registers */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res) {
+		dev_err(dev, "Missing MEM resource\n");
+		ret = -ENODEV;
+		goto exit_out;
+	}
+
+	iod->phys_base = res->start;
+	iod->reg_base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(iod->reg_base)) {
+		ret = PTR_ERR(iod->reg_base);
+		goto exit_out;
+	}
+
+	iod->regmap = devm_regmap_init_mmio(dev, iod->reg_base,
+					    iod->reg_data->regmap_config);
+	if (IS_ERR(iod->regmap)) {
+		dev_err(dev, "Regmap MMIO init failed.\n");
+		ret = PTR_ERR(iod->regmap);
+		goto exit_out;
+	}
+
+	if (ti_iodelay_pinconf_init_dev(iod))
+		goto exit_out;
+
+	ret = ti_iodelay_alloc_pins(dev, iod, res->start);
+	if (ret)
+		goto exit_out;
+
+	iod->desc.pctlops = &ti_iodelay_pinctrl_ops;
+	/* no pinmux ops - we are pinconf */
+	iod->desc.confops = &ti_iodelay_pinctrl_pinconf_ops;
+	iod->desc.name = dev_name(dev);
+	iod->desc.owner = THIS_MODULE;
+
+	iod->pctl = pinctrl_register(&iod->desc, dev, iod);
+	if (!iod->pctl) {
+		dev_err(dev, "Failed to register pinctrl\n");
+		ret = -ENODEV;
+		goto exit_out;
+	}
+
+	platform_set_drvdata(pdev, iod);
+
+exit_out:
+	of_node_put(np);
+	return ret;
+}
+
+/**
+ * ti_iodelay_remove() - standard remove
+ * @pdev: platform device
+ *
+ * Return: 0 if all went fine, else appropriate error value.
+ */
+static int ti_iodelay_remove(struct platform_device *pdev)
+{
+	struct ti_iodelay_device *iod = platform_get_drvdata(pdev);
+
+	if (!iod)
+		return 0;
+
+	if (iod->pctl)
+		pinctrl_unregister(iod->pctl);
+
+	ti_iodelay_pinconf_deinit_dev(iod);
+
+	/* Expect other allocations to be freed by devm */
+
+	return 0;
+}
+
+static struct platform_driver ti_iodelay_driver = {
+	.probe = ti_iodelay_probe,
+	.remove = ti_iodelay_remove,
+	.driver = {
+		   .owner = THIS_MODULE,
+		   .name = DRIVER_NAME,
+		   .of_match_table = ti_iodelay_of_match,
+	},
+};
+module_platform_driver(ti_iodelay_driver);
+
+MODULE_AUTHOR("Texas Instruments, Inc.");
+MODULE_DESCRIPTION("Pinconf driver for TI's IO Delay module");
+MODULE_LICENSE("GPL v2");
-- 
2.11.0

^ permalink raw reply

* [PATCH 1/2] pinctrl: core: Make dt_free_map optional
From: Tony Lindgren @ 2016-12-30 18:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Gary Bisson, Grygorii Strashko, Mark Rutland, Nishanth Menon,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20161230183732.5595-1-tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>

If the pin controller driver is using devm_kzalloc, there may not be
anything to do for dt_free_map. Let's make it optional to avoid
unncessary boilerplate code.

Signed-off-by: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
---
 drivers/pinctrl/core.c       | 3 ---
 drivers/pinctrl/devicetree.c | 3 ++-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/pinctrl/core.c b/drivers/pinctrl/core.c
--- a/drivers/pinctrl/core.c
+++ b/drivers/pinctrl/core.c
@@ -1913,9 +1913,6 @@ static int pinctrl_check_ops(struct pinctrl_dev *pctldev)
 	    !ops->get_group_name)
 		return -EINVAL;
 
-	if (ops->dt_node_to_map && !ops->dt_free_map)
-		return -EINVAL;
-
 	return 0;
 }
 
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -42,7 +42,8 @@ static void dt_free_map(struct pinctrl_dev *pctldev,
 {
 	if (pctldev) {
 		const struct pinctrl_ops *ops = pctldev->desc->pctlops;
-		ops->dt_free_map(pctldev, map, num_maps);
+		if (ops->dt_free_map)
+			ops->dt_free_map(pctldev, map, num_maps);
 	} else {
 		/* There is no pctldev for PIN_MAP_TYPE_DUMMY_STATE */
 		kfree(map);
-- 
2.11.0
--
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 0/2] Add TI iodelay driver using #pinctrl-cells
From: Tony Lindgren @ 2016-12-30 18:37 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Gary Bisson, Grygorii Strashko, Mark Rutland, Nishanth Menon,
	Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

Hi all,

Here's a respin of Nishanth's iodelay driver that I've updated to
use #pinctrl-cells and generic pinctrl group and function code.

Gary, note that this one has an iterator function that you may
be able to use too, and hopefully we can simplify things further
eventually by introducing a generic iterator.

Note that these patches are against the current pinctrl devel
branch at commit b61d1546906d ("pinctrl: single: Use generic
pinmux helpers for managing functions").

Regards,

Tony

Nishanth Menon (1):
  pinctrl: Introduce TI IOdelay configuration driver

Tony Lindgren (1):
  pinctrl: core: Make dt_free_map optional

 .../devicetree/bindings/pinctrl/ti,iodelay.txt     |  47 +
 drivers/pinctrl/Kconfig                            |   1 +
 drivers/pinctrl/Makefile                           |   1 +
 drivers/pinctrl/core.c                             |   3 -
 drivers/pinctrl/devicetree.c                       |   3 +-
 drivers/pinctrl/ti/Kconfig                         |  10 +
 drivers/pinctrl/ti/Makefile                        |   1 +
 drivers/pinctrl/ti/pinctrl-ti-iodelay.c            | 945 +++++++++++++++++++++
 8 files changed, 1007 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/pinctrl/ti,iodelay.txt
 create mode 100644 drivers/pinctrl/ti/Kconfig
 create mode 100644 drivers/pinctrl/ti/Makefile
 create mode 100644 drivers/pinctrl/ti/pinctrl-ti-iodelay.c

-- 
2.11.0
--
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 10/20] gpio: pca953x: Add optional reset gpio control
From: Steve Longerbeam @ 2016-12-30 18:03 UTC (permalink / raw)
  To: Linus Walleij, LW
  Cc: Mark Rutland, Alexandre Courbot, devel@driverdev.osuosl.org,
	Philipp Zabel, devicetree@vger.kernel.org, Greg KH, Russell King,
	linux-kernel@vger.kernel.org, linux-gpio@vger.kernel.org,
	Rob Herring, Sascha Hauer, Fabio Estevam, Mauro Carvalho Chehab,
	Shawn Guo, linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org
In-Reply-To: <CACRpkdZG2j4_LwP8KUVBTOtXma75YvYtC6CW1QqYwOm-MOZgHg@mail.gmail.com>

Hi Linus, Lothar,


On 12/30/2016 05:17 AM, Linus Walleij wrote:
> On Thu, Dec 29, 2016 at 11:27 PM, Steve Longerbeam
> <slongerbeam@gmail.com> wrote:
>
>> Add optional reset-gpios pin control. If present, de-assert the
>> specified reset gpio pin to bring the chip out of reset.
>>
>> Signed-off-by: Steve Longerbeam <steve_longerbeam@mentor.com>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Alexandre Courbot <gnurou@gmail.com>
>> Cc: linux-gpio@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
> This seems like a separate patch from the other 19 patches so please send it
> separately so I can handle logistics easier in the future.

Ok, I'll send the next version separately.

>
>
>> @@ -133,6 +134,7 @@ struct pca953x_chip {
>>          const char *const *names;
>>          unsigned long driver_data;
>>          struct regulator *regulator;
>> +       struct gpio_desc *reset_gpio;
> Why do you even keep this around in the device state container?
>
> As you only use it in the probe() function, use a local variable.
>
> The descriptor will be free():ed by the devm infrastructure anyways.

I think my reasoning for putting the gpio handle into the device
struct was for possibly using it for run-time reset control at some
point. But that could be done later if needed, so I'll go ahead and
make it local.

>> +               /* see if we need to de-assert a reset pin */
>> +               chip->reset_gpio = devm_gpiod_get_optional(&client->dev,
>> +                                                          "reset",
>> +                                                          GPIOD_OUT_LOW);
>> +               if (IS_ERR(chip->reset_gpio)) {
>> +                       dev_err(&client->dev, "request for reset pin failed\n");
>> +                       return PTR_ERR(chip->reset_gpio);
>> +               }
> Nice.
>
>> +               if (chip->reset_gpio) {
>> +                       /* bring chip out of reset */
>> +                       dev_info(&client->dev, "releasing reset\n");
>> +                       gpiod_set_value(chip->reset_gpio, 0);
>> +               }
> Is this really needed given that you set it low in the
> devm_gpiod_get_optional()?

Yep, this is left over from a previous iteration, but it isn't needed now.
I'll remove it.

Steve

^ permalink raw reply

* [PATCH linux 6/6] hwmon: occ: Add callbacks for parsing P9 OCC datastructures
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux
  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-1-git-send-email-eajames.ibm@gmail.com>

From: "Edward A. James" <eajames@us.ibm.com>

Add functions to parse the data structures that are specific to the OCC on
the POWER9 processor. These are the sensor data structures, including
temperature, frequency, power, and "caps."

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    |   3 +
 drivers/hwmon/occ/p9_occ.c | 243 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/occ/p9_occ.h |  30 ++++++
 3 files changed, 276 insertions(+)
 create mode 100644 drivers/hwmon/occ/p9_occ.c
 create mode 100644 drivers/hwmon/occ/p9_occ.h

diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index a6b3dd6..0f17c2f 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -34,6 +34,9 @@ number of data structures, such as command format, response headers, and the
 like, are also defined in this specification, and are common to both POWER8 and
 POWER9 OCCs.
 
+There is currently no public P9 OCC specification, and the data structures
+defined in the POWER9 OCC driver are subject to change.
+
 sysfs Entries
 -------------
 
diff --git a/drivers/hwmon/occ/p9_occ.c b/drivers/hwmon/occ/p9_occ.c
new file mode 100644
index 0000000..f69b469
--- /dev/null
+++ b/drivers/hwmon/occ/p9_occ.c
@@ -0,0 +1,243 @@
+/*
+ * p9.c - OCC hwmon driver
+ *
+ * This file contains the Power9-specific methods and data structures for
+ * the OCC hwmon driver.
+ *
+ * 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/i2c.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+#include "occ_p9.h"
+
+/* P9 OCC sensor data format */
+struct p9_temp_sensor {
+	u32 sensor_id;
+	u8 fru_type;
+	u8 value;
+};
+
+struct p9_freq_sensor {
+	u32 sensor_id;
+	u16 value;
+};
+
+struct p9_power_sensor {
+	u32 sensor_id;
+	u8 function_id;
+	u8 apss_channel;
+	u16 reserved;
+	u32 update_tag;
+	u64 accumulator;
+	u16 value;
+};
+
+struct p9_caps_sensor {
+	u16 curr_powercap;
+	u16 curr_powerreading;
+	u16 norm_powercap;
+	u16 max_powercap;
+	u16 min_powercap;
+	u16 user_powerlimit;
+	u8 user_powerlimit_source;
+};
+
+void p9_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+		     int snum)
+{
+	switch (sensor_type) {
+	case FREQ:
+	{
+		struct p9_freq_sensor *fs =
+			&(((struct p9_freq_sensor *)sensor)[snum]);
+
+		fs->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+		fs->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+	}
+		break;
+	case TEMP:
+	{
+		struct p9_temp_sensor *ts =
+			&(((struct p9_temp_sensor *)sensor)[snum]);
+
+		ts->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+		fs->fru_type = data[off + 4];
+		fs->value = data[off + 5];
+	}
+		break;
+	case POWER:
+	{
+		struct p9_power_sensor *ps =
+			&(((struct p9_power_sensor *)sensor)[snum]);
+
+		ps->sensor_id = be32_to_cpu(get_unaligned((u32 *)&data[off]));
+		ps->function_id = data[off + 4];
+		ps->apss_channel = data[off + 5];
+		ps->update_tag =
+			be32_to_cpu(get_unaligned((u32 *)&data[off + 8]));
+		ps->accumulator =
+			be64_to_cpu(get_unaligned((u64 *)&data[off + 12]));
+		ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 20]));
+	}
+		break;
+	case CAPS:
+	{
+		struct p9_caps_sensor *cs =
+			&(((struct p9_caps_sensor *)sensor)[snum]);
+
+		cs->curr_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off]));
+		cs->curr_powerreading =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+		cs->norm_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+		cs->max_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 6]));
+		cs->min_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 8]));
+		cs->user_powerlimit =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+		cs->user_powerlimit_source = data[off + 12];
+	}
+		break;
+	};
+}
+
+void *p9_alloc_sensor(int sensor_type, int num_sensors)
+{
+	switch (sensor_type) {
+	case FREQ:
+		return kcalloc(num_sensors, sizeof(struct p9_freq_sensor),
+			       GFP_KERNEL);
+	case TEMP:
+		return kcalloc(num_sensors, sizeof(struct p9_temp_sensor),
+			       GFP_KERNEL);
+	case POWER:
+		return kcalloc(num_sensors, sizeof(struct p9_power_sensor),
+			       GFP_KERNEL);
+	case CAPS:
+		return kcalloc(num_sensors, sizeof(struct p9_caps_sensor),
+			       GFP_KERNEL);
+	default:
+		return NULL;
+	}
+}
+
+int p9_get_sensor_value(struct occ *driver, int sensor_type, int snum)
+{
+	void *sensor;
+
+	if (sensor_type == CAPS)
+		return -EINVAL;
+
+	sensor = occ_get_sensor(driver, sensor_type);
+	if (!sensor)
+		return -ENODEV;
+
+	switch (sensor_type) {
+	case FREQ:
+		return ((struct p9_freq_sensor *)sensor)[snum].value;
+	case TEMP:
+		return ((struct p9_temp_sensor *)sensor)[snum].value;
+	case POWER:
+		return ((struct p9_power_sensor *)sensor)[snum].value;
+	default:
+		return -EINVAL;
+	}
+}
+
+int p9_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+	void *sensor;
+
+	if (sensor_type == CAPS)
+		return -EINVAL;
+
+	sensor = occ_get_sensor(driver, sensor_type);
+	if (!sensor)
+		return -ENODEV;
+
+	switch (sensor_type) {
+	case FREQ:
+		return ((struct p9_freq_sensor *)sensor)[snum].sensor_id;
+	case TEMP:
+		return ((struct p9_temp_sensor *)sensor)[snum].sensor_id;
+	case POWER:
+		return ((struct p9_power_sensor *)sensor)[snum].sensor_id;
+	default:
+		return -EINVAL;
+	}
+}
+
+int p9_get_caps_value(void *sensor, int snum, int caps_field)
+{
+	struct p9_caps_sensor *caps_sensor = sensor;
+
+	switch (caps_field) {
+	case 0:
+		return caps_sensor[snum].curr_powercap;
+	case 1:
+		return caps_sensor[snum].curr_powerreading;
+	case 2:
+		return caps_sensor[snum].norm_powercap;
+	case 3:
+		return caps_sensor[snum].max_powercap;
+	case 4:
+		return caps_sensor[snum].min_powercap;
+	case 5:
+		return caps_sensor[snum].user_powerlimit;
+	case 6:
+		return caps_sensor[snum].user_powerlimit_source;
+	default:
+		return -EINVAL;
+	}
+}
+static const struct occ_ops p9_ops = {
+	.parse_sensor = p9_parse_sensor,
+	.alloc_sensor = p9_alloc_sensor,
+	.get_sensor_value = p9_get_sensor_value,
+	.get_sensor_id = p9_get_sensor_id,
+	.get_caps_value = p9_get_caps_value,
+};
+
+static const struct occ_config p9_config = {
+	.command_addr = 0xFFFBE000,
+	.response_addr = 0xFFFBF000,
+};
+
+struct occ *p9_occ_start(struct device *dev, void *bus,
+			 struct occ_bus_ops *bus_ops)
+{
+	return occ_start(dev, bus, bus_ops, &p9_ops, &p9_config);
+}
+EXPORT_SYMBOL(occ_p9_start);
+
+int p9_occ_stop(struct occ *occ)
+{
+	return occ_stop(occ);
+}
+EXPORT_SYMBOL(occ_p9_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("P9 OCC sensors");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/p9_occ.h b/drivers/hwmon/occ/p9_occ.h
new file mode 100644
index 0000000..8130873
--- /dev/null
+++ b/drivers/hwmon/occ/p9_occ.h
@@ -0,0 +1,30 @@
+/*
+ * occ_p9.h - OCC hwmon driver
+ *
+ * This file contains Power9-specific function prototypes
+ *
+ * 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_P9_H__
+#define __OCC_P9_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *p9_occ_start(struct device *dev, void *bus,
+			 struct occ_bus_ops *bus_ops);
+int p9_occ_stop(struct occ *occ);
+
+#endif /* __OCC_P9_H__ */
-- 
1.9.1


^ permalink raw reply related

* [PATCH linux 5/6] hwmon: occ: Add hwmon implementation for the P8 OCC
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: 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-1-git-send-email-eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Add code to tie the hwmon sysfs code and the POWER8 OCC code together, as
well as probe the entire driver from the I2C bus. I2C is the communication
method between the BMC and the P8 OCC.

Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Reviewed-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-ibm-occ.txt        |  13 ++
 drivers/hwmon/occ/Kconfig                          |  14 ++
 drivers/hwmon/occ/Makefile                         |   1 +
 drivers/hwmon/occ/p8_occ_i2c.c                     | 141 +++++++++++++++++++++
 4 files changed, 169 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
 create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c

diff --git a/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
new file mode 100644
index 0000000..b0d2b36
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
@@ -0,0 +1,13 @@
+HWMON I2C driver for IBM POWER CPU OCC (On Chip Controller)
+
+Required properties:
+ - compatible: must be "ibm,p8-occ-i2c"
+ - reg: physical address
+
+Example:
+i2c3: i2c-bus@100 {
+	occ@50 {
+		compatible = "ibm,p8-occ-i2c";
+		reg = <0x50>;
+	};
+};
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
index cdb64a7..3a5188f 100644
--- a/drivers/hwmon/occ/Kconfig
+++ b/drivers/hwmon/occ/Kconfig
@@ -13,3 +13,17 @@ menuconfig SENSORS_PPC_OCC
 
 	  This driver can also be built as a module. If so, the module
 	  will be called occ.
+
+if SENSORS_PPC_OCC
+
+config SENSORS_PPC_OCC_P8_I2C
+	tristate "POWER8 OCC hwmon support"
+	depends on I2C
+	help
+	 Provide a hwmon sysfs interface for the POWER8 On-Chip Controller,
+	 exposing temperature, frequency and power measurements.
+
+	 This driver can also be built as a module. If so, the module will be
+	 called p8-occ-i2c.
+
+endif
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
index a6881f9..9294b58 100644
--- a/drivers/hwmon/occ/Makefile
+++ b/drivers/hwmon/occ/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o occ_sysfs.o
+obj-$(CONFIG_SENSORS_PPC_OCC_P8_I2C) += occ_scom_i2c.o occ_p8.o p8_occ_i2c.o
diff --git a/drivers/hwmon/occ/p8_occ_i2c.c b/drivers/hwmon/occ/p8_occ_i2c.c
new file mode 100644
index 0000000..0c65894
--- /dev/null
+++ b/drivers/hwmon/occ/p8_occ_i2c.c
@@ -0,0 +1,141 @@
+/*
+ * p8_occ_i2c.c - hwmon OCC driver
+ *
+ * This file contains the i2c layer for accessing the P8 OCC over i2c bus.
+ *
+ * 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/err.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+#include "occ_p8.h"
+#include "occ_sysfs.h"
+
+#define P8_OCC_I2C_NAME	"p8-occ-i2c"
+
+static char *caps_sensor_names[] = {
+	"curr_powercap",
+	"curr_powerreading",
+	"norm_powercap",
+	"max_powercap",
+	"min_powercap",
+	"user_powerlimit"
+};
+
+int p8_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+	/* P8 i2c slave requires address to be shifted by 1 */
+	address = address << 1;
+
+	return occ_i2c_getscom(bus, address, data);
+}
+
+int p8_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+	/* P8 i2c slave requires address to be shifted by 1 */
+	address = address << 1;
+
+	return occ_i2c_putscom(bus, address, data0, data1);
+}
+
+static struct occ_bus_ops p8_bus_ops = {
+	.getscom = p8_i2c_getscom,
+	.putscom = p8_i2c_putscom,
+};
+
+static struct occ_sysfs_config p8_sysfs_config = {
+	.num_caps_fields = ARRAY_SIZE(caps_sensor_names),
+	.caps_names = caps_sensor_names,
+};
+
+static int p8_occ_probe(struct i2c_client *client,
+			const struct i2c_device_id *id)
+{
+	struct occ *occ;
+	struct occ_sysfs *hwmon;
+
+	occ = p8_occ_start(&client->dev, client, &p8_bus_ops);
+	if (IS_ERR(occ))
+		return PTR_ERR(occ);
+
+	hwmon = occ_sysfs_start(&client->dev, occ, &p8_sysfs_config);
+	if (IS_ERR(hwmon))
+		return PTR_ERR(hwmon);
+
+	i2c_set_clientdata(client, hwmon);
+
+	return 0;
+}
+
+static int p8_occ_remove(struct i2c_client *client)
+{
+	int rc;
+	struct occ_sysfs *hwmon = i2c_get_clientdata(client);
+	struct occ *occ = hwmon->occ;
+
+	rc = occ_sysfs_stop(&client->dev, hwmon);
+
+	rc = rc || p8_occ_stop(occ);
+
+	return rc;
+}
+
+/* used by old-style board info. */
+static const struct i2c_device_id occ_ids[] = {
+	{ P8_OCC_I2C_NAME, 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(i2c, occ_ids);
+
+/* used by device table */
+static const struct of_device_id occ_of_match[] = {
+	{ .compatible = "ibm,p8-occ-i2c" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, occ_of_match);
+
+/*
+ * i2c-core uses i2c-detect() to detect device in below address list.
+ * If exists, address will be assigned to client.
+ * It is also possible to read address from device table.
+ */
+static const unsigned short normal_i2c[] = {0x50, 0x51, I2C_CLIENT_END };
+
+static struct i2c_driver p8_occ_driver = {
+	.class = I2C_CLASS_HWMON,
+	.driver = {
+		.name = P8_OCC_I2C_NAME,
+		.pm = NULL,
+		.of_match_table = occ_of_match,
+	},
+	.probe = p8_occ_probe,
+	.remove = p8_occ_remove,
+	.id_table = occ_ids,
+	.address_list = normal_i2c,
+};
+
+module_i2c_driver(p8_occ_driver);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("BMC P8 OCC hwmon driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

--
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 related

* [PATCH linux 4/6] hwmon: occ: Add callbacks for parsing P8 OCC datastructures
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: 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-1-git-send-email-eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Add functions to parse the data structures that are specific to the OCC on
the POWER8 processor. These are the sensor data structures, including
temperature, frequency, power, and "caps."

Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
Reviewed-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
 Documentation/hwmon/occ    |   9 ++
 drivers/hwmon/occ/occ_p8.c | 217 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/occ/occ_p8.h |  30 +++++++
 3 files changed, 256 insertions(+)
 create mode 100644 drivers/hwmon/occ/occ_p8.c
 create mode 100644 drivers/hwmon/occ/occ_p8.h

diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
index 1ee8689..a6b3dd6 100644
--- a/Documentation/hwmon/occ
+++ b/Documentation/hwmon/occ
@@ -25,6 +25,15 @@ 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.
 
+The format for the POWER8 OCC sensor data can be found in the P8 OCC
+specification:
+github.com/open-power/docs/blob/master/occ/OCC_OpenPwr_FW_Interfaces.pdf
+This document provides the details of the OCC sensors: power, frequency,
+temperature, and caps. These sensor formats are specific to the POWER8 OCC. A
+number of data structures, such as command format, response headers, and the
+like, are also defined in this specification, and are common to both POWER8 and
+POWER9 OCCs.
+
 sysfs Entries
 -------------
 
diff --git a/drivers/hwmon/occ/occ_p8.c b/drivers/hwmon/occ/occ_p8.c
new file mode 100644
index 0000000..812df16
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.c
@@ -0,0 +1,217 @@
+/*
+ * occ_p8.c - OCC hwmon driver
+ *
+ * This file contains the Power8-specific methods and data structures for
+ * the OCC hwmon driver.
+ *
+ * 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/err.h>
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <asm/unaligned.h>
+
+#include "occ.h"
+#include "occ_p8.h"
+
+/* P8 OCC sensor data format */
+struct p8_occ_sensor {
+	u16 sensor_id;
+	u16 value;
+};
+
+struct p8_power_sensor {
+	u16 sensor_id;
+	u32 update_tag;
+	u32 accumulator;
+	u16 value;
+};
+
+struct p8_caps_sensor {
+	u16 curr_powercap;
+	u16 curr_powerreading;
+	u16 norm_powercap;
+	u16 max_powercap;
+	u16 min_powercap;
+	u16 user_powerlimit;
+};
+
+void p8_parse_sensor(u8 *data, void *sensor, int sensor_type, int off,
+		     int snum)
+{
+	switch (sensor_type) {
+	case FREQ:
+	case TEMP:
+	{
+		struct p8_occ_sensor *os =
+			&(((struct p8_occ_sensor *)sensor)[snum]);
+
+		os->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+		os->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+	}
+		break;
+	case POWER:
+	{
+		struct p8_power_sensor *ps =
+			&(((struct p8_power_sensor *)sensor)[snum]);
+
+		ps->sensor_id = be16_to_cpu(get_unaligned((u16 *)&data[off]));
+		ps->update_tag =
+			be32_to_cpu(get_unaligned((u32 *)&data[off + 2]));
+		ps->accumulator =
+			be32_to_cpu(get_unaligned((u32 *)&data[off + 6]));
+		ps->value = be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+	}
+		break;
+	case CAPS:
+	{
+		struct p8_caps_sensor *cs =
+			&(((struct p8_caps_sensor *)sensor)[snum]);
+
+		cs->curr_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off]));
+		cs->curr_powerreading =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 2]));
+		cs->norm_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 4]));
+		cs->max_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 6]));
+		cs->min_powercap =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 8]));
+		cs->user_powerlimit =
+			be16_to_cpu(get_unaligned((u16 *)&data[off + 10]));
+	}
+		break;
+	};
+}
+
+void *p8_alloc_sensor(int sensor_type, int num_sensors)
+{
+	switch (sensor_type) {
+	case FREQ:
+	case TEMP:
+		return kcalloc(num_sensors, sizeof(struct p8_occ_sensor),
+			       GFP_KERNEL);
+	case POWER:
+		return kcalloc(num_sensors, sizeof(struct p8_power_sensor),
+			       GFP_KERNEL);
+	case CAPS:
+		return kcalloc(num_sensors, sizeof(struct p8_caps_sensor),
+			       GFP_KERNEL);
+	default:
+		return NULL;
+	}
+}
+
+int p8_get_sensor_value(struct occ *driver, int sensor_type, int snum)
+{
+	void *sensor;
+
+	if (sensor_type == CAPS)
+		return -EINVAL;
+
+	sensor = occ_get_sensor(driver, sensor_type);
+	if (!sensor)
+		return -ENODEV;
+
+	switch (sensor_type) {
+	case FREQ:
+	case TEMP:
+		return ((struct p8_occ_sensor *)sensor)[snum].value;
+	case POWER:
+		return ((struct p8_power_sensor *)sensor)[snum].value;
+	default:
+		return -EINVAL;
+	}
+}
+
+int p8_get_sensor_id(struct occ *driver, int sensor_type, int snum)
+{
+	void *sensor;
+	int i = snum;
+
+	if (sensor_type == CAPS)
+		return -EINVAL;
+
+	sensor = occ_get_sensor(driver, sensor_type);
+	if (!sensor)
+		return -ENODEV;
+
+	switch (sensor_type) {
+	case FREQ:
+	case TEMP:
+		return ((struct p8_occ_sensor *)sensor)[i].sensor_id;
+	case POWER:
+		return ((struct p8_power_sensor *)sensor)[i].sensor_id;
+	default:
+		return -EINVAL;
+	}
+}
+
+int p8_get_caps_value(void *sensor, int snum, int caps_field)
+{
+	struct p8_caps_sensor *caps_sensor = sensor;
+
+	switch (caps_field) {
+	case 0:
+		return caps_sensor[snum].curr_powercap;
+	case 1:
+		return caps_sensor[snum].curr_powerreading;
+	case 2:
+		return caps_sensor[snum].norm_powercap;
+	case 3:
+		return caps_sensor[snum].max_powercap;
+	case 4:
+		return caps_sensor[snum].min_powercap;
+	case 5:
+		return caps_sensor[snum].user_powerlimit;
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct occ_ops p8_ops = {
+	.parse_sensor = p8_parse_sensor,
+	.alloc_sensor = p8_alloc_sensor,
+	.get_sensor_value = p8_get_sensor_value,
+	.get_sensor_id = p8_get_sensor_id,
+	.get_caps_value = p8_get_caps_value,
+};
+
+static const struct occ_config p8_config = {
+	.command_addr = 0xFFFF6000,
+	.response_addr = 0xFFFF7000,
+};
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+			 struct occ_bus_ops *bus_ops)
+{
+	return occ_start(dev, bus, bus_ops, &p8_ops, &p8_config);
+}
+EXPORT_SYMBOL(p8_occ_start);
+
+int p8_occ_stop(struct occ *occ)
+{
+	return occ_stop(occ);
+}
+EXPORT_SYMBOL(p8_occ_stop);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("P8 OCC sensors");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_p8.h b/drivers/hwmon/occ/occ_p8.h
new file mode 100644
index 0000000..b44cdd4
--- /dev/null
+++ b/drivers/hwmon/occ/occ_p8.h
@@ -0,0 +1,30 @@
+/*
+ * occ_p8.h - OCC hwmon driver
+ *
+ * This file contains Power8-specific function prototypes
+ *
+ * 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_P8_H__
+#define __OCC_P8_H__
+
+#include "scom.h"
+
+struct device;
+
+struct occ *p8_occ_start(struct device *dev, void *bus,
+			 struct occ_bus_ops *bus_ops);
+int p8_occ_stop(struct occ *occ);
+
+#endif /* __OCC_P8_H__ */
-- 
1.9.1

--
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 related

* [PATCH linux 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations
From: eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux-0h96xk9xTtrk1uMJSBkQmQ
  Cc: 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-1-git-send-email-eajames.ibm-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

From: "Edward A. James" <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>

Add functions to send SCOM operations over I2C bus. The BMC can
communicate with the Power8 host processor over I2C, but needs to use SCOM
operations in order to access the OCC register space.

Signed-off-by: Edward A. James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
Signed-off-by: Andrew Jeffery <andrew-zrmu5oMJ5Fs@public.gmane.org>
---
 drivers/hwmon/occ/occ_scom_i2c.c | 73 ++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/occ/occ_scom_i2c.h | 26 ++++++++++++++
 2 files changed, 99 insertions(+)
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h

diff --git a/drivers/hwmon/occ/occ_scom_i2c.c b/drivers/hwmon/occ/occ_scom_i2c.c
new file mode 100644
index 0000000..a922f83
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.c
@@ -0,0 +1,73 @@
+/*
+ * occ_scom_i2c.c - hwmon OCC driver
+ *
+ * This file contains the functions for performing SCOM operations over I2C bus
+ * to access the OCC.
+ *
+ * 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/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#include "scom.h"
+#include "occ_scom_i2c.h"
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data)
+{
+	ssize_t rc;
+	u64 buf;
+	struct i2c_client *client = bus;
+
+	rc = i2c_master_send(client, (const char *)&address, sizeof(u32));
+	if (rc < 0)
+		return rc;
+	else if (rc != sizeof(u32))
+		return -EIO;
+
+	rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
+	if (rc < 0)
+		return rc;
+	else if (rc != sizeof(u64))
+		return -EIO;
+
+	*data = be64_to_cpu(buf);
+
+	return 0;
+}
+EXPORT_SYMBOL(occ_i2c_getscom);
+
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1)
+{
+	u32 buf[3];
+	ssize_t rc;
+	struct i2c_client *client = bus;
+
+	buf[0] = address;
+	buf[1] = data1;
+	buf[2] = data0;
+
+	rc = i2c_master_send(client, (const char *)buf, sizeof(u32) * 3);
+	if (rc < 0)
+		return rc;
+	else if (rc != sizeof(u32) * 3)
+		return -EIO;
+
+	return 0;
+}
+EXPORT_SYMBOL(occ_i2c_putscom);
+
+MODULE_AUTHOR("Eddie James <eajames-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>");
+MODULE_DESCRIPTION("I2C OCC SCOM transport");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ_scom_i2c.h b/drivers/hwmon/occ/occ_scom_i2c.h
new file mode 100644
index 0000000..945739c
--- /dev/null
+++ b/drivers/hwmon/occ/occ_scom_i2c.h
@@ -0,0 +1,26 @@
+/*
+ * occ_scom_i2c.h - hwmon OCC driver
+ *
+ * This file contains function protoypes for peforming SCOM operations over I2C
+ * bus to access the OCC.
+ *
+ * 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_SCOM_I2C_H__
+#define __OCC_SCOM_I2C_H__
+
+int occ_i2c_getscom(void *bus, u32 address, u64 *data);
+int occ_i2c_putscom(void *bus, u32 address, u32 data0, u32 data1);
+
+#endif /* __OCC_SCOM_I2C_H__ */
-- 
1.9.1

--
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 related

* [PATCH linux 2/6] hwmon: occ: Add sysfs interface
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux
  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-1-git-send-email-eajames.ibm@gmail.com>

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);
+		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);
+
+	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 related

* [PATCH linux 1/6] hwmon: Add core On-Chip Controller support for POWER CPUs
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux
  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-1-git-send-email-eajames.ibm@gmail.com>

From: "Edward A. James" <eajames@us.ibm.com>

Add core support for polling the OCC for it's sensor data and parsing that
data into sensor-specific information.

Signed-off-by: Edward A. James <eajames@us.ibm.com>
Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
---
 Documentation/hwmon/occ    |  40 ++++
 drivers/hwmon/Kconfig      |   2 +
 drivers/hwmon/Makefile     |   1 +
 drivers/hwmon/occ/Kconfig  |  15 ++
 drivers/hwmon/occ/Makefile |   1 +
 drivers/hwmon/occ/occ.c    | 544 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/hwmon/occ/occ.h    |  86 +++++++
 drivers/hwmon/occ/scom.h   |  47 ++++
 8 files changed, 736 insertions(+)
 create mode 100644 Documentation/hwmon/occ
 create mode 100644 drivers/hwmon/occ/Kconfig
 create mode 100644 drivers/hwmon/occ/Makefile
 create mode 100644 drivers/hwmon/occ/occ.c
 create mode 100644 drivers/hwmon/occ/occ.h
 create mode 100644 drivers/hwmon/occ/scom.h

diff --git a/Documentation/hwmon/occ b/Documentation/hwmon/occ
new file mode 100644
index 0000000..79d1642
--- /dev/null
+++ b/Documentation/hwmon/occ
@@ -0,0 +1,40 @@
+Kernel driver occ
+=================
+
+Supported chips:
+ * ASPEED AST2400
+ * ASPEED AST2500
+
+Please note that the chip must be connected to a POWER8 or POWER9 processor
+(see the BMC - Host Communications section).
+
+Author: Eddie James <eajames@us.ibm.com>
+
+Description
+-----------
+
+This driver implements support for the OCC (On-Chip Controller) on the IBM
+POWER8 and POWER9 processors, from a BMC (Baseboard Management Controller). The
+OCC is an embedded processor that provides real time power and thermal
+monitoring.
+
+This driver provides an interface on a BMC to poll OCC sensor data, set user
+power caps, and perform some basic OCC error handling.
+
+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.
+
+BMC - Host Communications
+-------------------------
+
+For the POWER8 application, the BMC can communicate with the P8 over I2C bus.
+However, to access the OCC register space, any data transfer must use a SCOM
+operation. SCOM is a procedure to initiate a data transfer, typically of 8
+bytes. SCOMs consist of writing a 32-bit command register and then
+reading/writing two 32-bit data registers. This driver implements these
+SCOM operations over I2C bus in order to communicate with the OCC.
+
+For the POWER9 application, the BMC can communicate with the P9 over FSI bus
+and SBE engine. Once again, SCOM operations are required. This driver will
+implement SCOM ops over FSI/SBE. This will require the FSI driver.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 190d270..e80ca81 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1240,6 +1240,8 @@ config SENSORS_NSA320
 	  This driver can also be built as a module. If so, the module
 	  will be called nsa320-hwmon.
 
+source drivers/hwmon/occ/Kconfig
+
 config SENSORS_PCF8591
 	tristate "Philips PCF8591 ADC/DAC"
 	depends on I2C
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index d2cb7e8..c7ec5d4 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_SENSORS_WM831X)	+= wm831x-hwmon.o
 obj-$(CONFIG_SENSORS_WM8350)	+= wm8350-hwmon.o
 obj-$(CONFIG_SENSORS_XGENE)	+= xgene-hwmon.o
 
+obj-$(CONFIG_SENSORS_PPC_OCC)	+= occ/
 obj-$(CONFIG_PMBUS)		+= pmbus/
 
 ccflags-$(CONFIG_HWMON_DEBUG_CHIP) := -DDEBUG
diff --git a/drivers/hwmon/occ/Kconfig b/drivers/hwmon/occ/Kconfig
new file mode 100644
index 0000000..cdb64a7
--- /dev/null
+++ b/drivers/hwmon/occ/Kconfig
@@ -0,0 +1,15 @@
+#
+# On Chip Controller configuration
+#
+
+menuconfig SENSORS_PPC_OCC
+	bool "PPC On-Chip Controller"
+	help
+	  If you say yes here you get support to monitor Power CPU
+	  sensors via the On-Chip Controller (OCC).
+
+	  Generally this is used by management controllers such as a BMC
+	  on an OpenPower system.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called occ.
diff --git a/drivers/hwmon/occ/Makefile b/drivers/hwmon/occ/Makefile
new file mode 100644
index 0000000..93cb52f
--- /dev/null
+++ b/drivers/hwmon/occ/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_SENSORS_PPC_OCC) += occ.o
diff --git a/drivers/hwmon/occ/occ.c b/drivers/hwmon/occ/occ.c
new file mode 100644
index 0000000..9884ddd
--- /dev/null
+++ b/drivers/hwmon/occ/occ.c
@@ -0,0 +1,544 @@
+/*
+ * occ.c - OCC hwmon driver
+ *
+ * This file contains the methods and data structures for the OCC hwmon driver.
+ *
+ * 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 <asm/unaligned.h>
+
+#include "occ.h"
+
+#define OCC_DATA_MAX		4096
+#define OCC_BMC_TIMEOUT_MS	20000
+
+/* To generate attn to OCC */
+#define ATTN_DATA		0x0006B035
+
+/* For BMC to read/write SRAM */
+#define OCB_ADDRESS		0x0006B070
+#define OCB_DATA		0x0006B075
+#define OCB_STATUS_CONTROL_AND	0x0006B072
+#define OCB_STATUS_CONTROL_OR	0x0006B073
+
+/* To init OCB */
+#define OCB_AND_INIT0		0xFBFFFFFF
+#define OCB_AND_INIT1		0xFFFFFFFF
+#define OCB_OR_INIT0		0x08000000
+#define OCB_OR_INIT1		0x00000000
+
+/* To generate attention on OCC */
+#define ATTN0			0x01010000
+#define ATTN1			0x00000000
+
+/* OCC return status */
+#define RESP_RETURN_CMD_IN_PRG	0xFF
+#define RESP_RETURN_SUCCESS	0
+#define RESP_RETURN_CMD_INVAL	0x11
+#define RESP_RETURN_CMD_LEN	0x12
+#define RESP_RETURN_DATA_INVAL	0x13
+#define RESP_RETURN_CHKSUM	0x14
+#define RESP_RETURN_OCC_ERR	0x15
+#define RESP_RETURN_STATE	0x16
+
+/* time interval to retry on "command in progress" return status */
+#define CMD_IN_PRG_INT_MS	100
+#define CMD_IN_PRG_RETRIES	(OCC_BMC_TIMEOUT_MS / CMD_IN_PRG_INT_MS)
+
+/* OCC command definitions */
+#define OCC_POLL		0
+#define OCC_SET_USER_POWR_CAP	0x22
+
+/* OCC poll command data */
+#define OCC_POLL_STAT_SENSOR	0x10
+
+/* OCC response data offsets */
+#define RESP_RETURN_STATUS	2
+#define RESP_DATA_LENGTH	3
+#define RESP_HEADER_OFFSET	5
+#define SENSOR_STR_OFFSET	37
+#define SENSOR_BLOCK_NUM_OFFSET	43
+#define SENSOR_BLOCK_OFFSET	45
+
+/* occ_poll_header
+ * structure to match the raw occ poll response data
+ */
+struct occ_poll_header {
+	u8 status;
+	u8 ext_status;
+	u8 occs_present;
+	u8 config;
+	u8 occ_state;
+	u8 mode;
+	u8 ips_status;
+	u8 error_log_id;
+	u32 error_log_addr_start;
+	u16 error_log_length;
+	u8 reserved2;
+	u8 reserved3;
+	u8 occ_code_level[16];
+	u8 sensor_eye_catcher[6];
+	u8 sensor_block_num;
+	u8 sensor_data_version;
+} __attribute__((packed, aligned(4)));
+
+struct occ_response {
+	struct occ_poll_header header;
+	struct occ_blocks data;
+};
+
+struct occ {
+	struct device *dev;
+	void *bus;
+	struct occ_bus_ops bus_ops;
+	struct occ_ops ops;
+	struct occ_config config;
+	unsigned long update_interval;
+	unsigned long last_updated;
+	struct mutex update_lock;
+	struct occ_response response;
+	bool valid;
+};
+
+static void deinit_occ_resp_buf(struct occ_response *resp)
+{
+	int i;
+
+	if (!resp)
+		return;
+
+	if (!resp->data.blocks)
+		return;
+
+	for (i = 0; i < resp->header.sensor_block_num; ++i)
+		kfree(resp->data.blocks[i].sensors);
+
+	kfree(resp->data.blocks);
+
+	memset(resp, 0, sizeof(struct occ_response));
+
+	for (i = 0; i < MAX_OCC_SENSOR_TYPE; ++i)
+		resp->data.sensor_block_id[i] = -1;
+}
+
+static void *occ_get_sensor_by_type(struct occ_response *resp,
+				    enum sensor_type t)
+{
+	if (!resp->data.blocks)
+		return NULL;
+
+	if (resp->data.sensor_block_id[t] == -1)
+		return NULL;
+
+	return resp->data.blocks[resp->data.sensor_block_id[t]].sensors;
+}
+
+static int occ_check_sensor(struct occ *driver, u8 sensor_length,
+			    u8 sensor_num, enum sensor_type t, int block)
+{
+	void *sensor;
+	int type_block_id;
+	struct occ_response *resp = &driver->response;
+
+	sensor = occ_get_sensor_by_type(resp, t);
+
+	/* empty sensor block, release older sensor data */
+	if (sensor_num == 0 || sensor_length == 0) {
+		kfree(sensor);
+		dev_err(driver->dev, "no sensor blocks available\n");
+		return -ENODATA;
+	}
+
+	type_block_id = resp->data.sensor_block_id[t];
+	if (!sensor || sensor_num !=
+	    resp->data.blocks[type_block_id].header.sensor_num) {
+		kfree(sensor);
+		resp->data.blocks[block].sensors =
+			driver->ops.alloc_sensor(t, sensor_num);
+		if (!resp->data.blocks[block].sensors)
+			return -ENOMEM;
+	}
+
+	return 0;
+}
+
+static int parse_occ_response(struct occ *driver, u8 *data,
+			      struct occ_response *resp)
+{
+	int b;
+	int s;
+	int rc;
+	int offset = SENSOR_BLOCK_OFFSET;
+	int sensor_type;
+	u8 sensor_block_num;
+	char sensor_type_string[5] = { 0 };
+	struct sensor_data_block_header *block;
+	struct device *dev = driver->dev;
+
+	/* check if the data is valid */
+	if (strncmp(&data[SENSOR_STR_OFFSET], "SENSOR", 6) != 0) {
+		dev_err(dev, "no SENSOR string in response\n");
+		rc = -ENODATA;
+		goto err;
+	}
+
+	sensor_block_num = data[SENSOR_BLOCK_NUM_OFFSET];
+	if (sensor_block_num == 0) {
+		dev_err(dev, "no sensor blocks available\n");
+		rc = -ENODATA;
+		goto err;
+	}
+
+	/* if number of sensor block has changed, re-malloc */
+	if (sensor_block_num != resp->header.sensor_block_num) {
+		deinit_occ_resp_buf(resp);
+		resp->data.blocks = kcalloc(sensor_block_num,
+					    sizeof(struct sensor_data_block),
+					    GFP_KERNEL);
+		if (!resp->data.blocks)
+			return -ENOMEM;
+	}
+
+	memcpy(&resp->header, &data[RESP_HEADER_OFFSET],
+	       sizeof(struct occ_poll_header));
+	resp->header.error_log_addr_start =
+		be32_to_cpu(resp->header.error_log_addr_start);
+	resp->header.error_log_length =
+		be16_to_cpu(resp->header.error_log_length);
+
+	dev_dbg(dev, "Reading %d sensor blocks\n",
+		resp->header.sensor_block_num);
+	for (b = 0; b < sensor_block_num; b++) {
+		block = (struct sensor_data_block_header *)&data[offset];
+		/* copy to a null terminated string */
+		strncpy(sensor_type_string, block->sensor_type, 4);
+		offset += 8;
+
+		dev_dbg(dev, "sensor block[%d]: type: %s, sensor_num: %d\n", b,
+			sensor_type_string, block->sensor_num);
+
+		if (strncmp(block->sensor_type, "FREQ", 4) == 0)
+			sensor_type = FREQ;
+		else if (strncmp(block->sensor_type, "TEMP", 4) == 0)
+			sensor_type = TEMP;
+		else if (strncmp(block->sensor_type, "POWR", 4) == 0)
+			sensor_type = POWER;
+		else if (strncmp(block->sensor_type, "CAPS", 4) == 0)
+			sensor_type = CAPS;
+		else {
+			dev_err(dev, "sensor type not supported %s\n",
+				sensor_type_string);
+			continue;
+		}
+
+		rc = occ_check_sensor(driver, block->sensor_length,
+				      block->sensor_num, sensor_type, b);
+		if (rc == -ENOMEM)
+			goto err;
+		else if (rc)
+			continue;
+
+		resp->data.sensor_block_id[sensor_type] = b;
+		for (s = 0; s < block->sensor_num; s++) {
+			driver->ops.parse_sensor(data,
+						 resp->data.blocks[b].sensors,
+						 sensor_type, offset, s);
+			offset += block->sensor_length;
+		}
+
+		/* copy block data over to response pointer */
+		resp->data.blocks[b].header = *block;
+	}
+
+	return 0;
+err:
+	deinit_occ_resp_buf(resp);
+	return rc;
+}
+
+static u8 occ_send_cmd(struct occ *driver, u8 seq, u8 type, u16 length,
+		       u8 *data, u8 *resp)
+{
+	u32 cmd1, cmd2;
+	u16 checksum = 0;
+	u16 length_le = cpu_to_le16(length);
+	bool retry = 0;
+	int i, rc, tries = 0;
+
+	cmd1 = (seq << 24) | (type << 16) | length_le;
+	memcpy(&cmd2, data, length);
+	cmd2 <<= ((4 - length) * 8);
+
+	/* checksum: sum of every bytes of cmd1, cmd2 */
+	for (i = 0; i < 4; i++) {
+		checksum += (cmd1 >> (i * 8)) & 0xFF;
+		checksum += (cmd2 >> (i * 8)) & 0xFF;
+	}
+
+	cmd2 |= checksum << ((2 - length) * 8);
+
+	/* Init OCB */
+	rc = driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_OR,
+				     OCB_OR_INIT0, OCB_OR_INIT1);
+	if (rc)
+		goto err;
+
+	rc = driver->bus_ops.putscom(driver->bus, OCB_STATUS_CONTROL_AND,
+				     OCB_AND_INIT0, OCB_AND_INIT1);
+	if (rc)
+		goto err;
+
+	/* Send command, 2nd half of the 64-bit addr is unused (write 0) */
+	rc = driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+				     driver->config.command_addr, 0);
+	if (rc)
+		goto err;
+
+	rc = driver->bus_ops.putscom(driver->bus, OCB_DATA, cmd1, cmd2);
+	if (rc)
+		goto err;
+
+	/* Trigger attention */
+	rc = driver->bus_ops.putscom(driver->bus, ATTN_DATA, ATTN0, ATTN1);
+	if (rc)
+		goto err;
+
+	/* Get response data */
+	rc = driver->bus_ops.putscom(driver->bus, OCB_ADDRESS,
+				     driver->config.response_addr, 0);
+	if (rc)
+		goto err;
+
+	do {
+		if (retry) {
+			set_current_state(TASK_INTERRUPTIBLE);
+			schedule_timeout(msecs_to_jiffies(CMD_IN_PRG_INT_MS));
+		}
+
+		rc = driver->bus_ops.getscom(driver->bus, OCB_DATA,
+					     (u64 *)resp);
+		if (rc)
+			goto err;
+
+		/* retry if we get "command in progress" return status */
+		retry = (resp[RESP_RETURN_STATUS] == RESP_RETURN_CMD_IN_PRG) &&
+			(tries++ < CMD_IN_PRG_RETRIES);
+	} while (retry);
+
+	switch (resp[RESP_RETURN_STATUS]) {
+	case RESP_RETURN_CMD_IN_PRG:
+		rc = -EALREADY;
+		break;
+	case RESP_RETURN_SUCCESS:
+		rc = 0;
+		break;
+	case RESP_RETURN_CMD_INVAL:
+	case RESP_RETURN_CMD_LEN:
+	case RESP_RETURN_DATA_INVAL:
+	case RESP_RETURN_CHKSUM:
+		rc = -EINVAL;
+		break;
+	case RESP_RETURN_OCC_ERR:
+		rc = -EREMOTE;
+		break;
+	default:
+		rc = -EFAULT;
+	}
+
+	return rc;
+
+err:
+	dev_err(driver->dev, "scom op failed rc:%d\n", rc);
+	return rc;
+}
+
+static int occ_get_all(struct occ *driver)
+{
+	int i = 0, rc;
+	u8 *occ_data;
+	u16 num_bytes;
+	const u8 poll_cmd_data = OCC_POLL_STAT_SENSOR;
+	struct device *dev = driver->dev;
+	struct occ_response *resp = &driver->response;
+
+	occ_data = devm_kzalloc(dev, OCC_DATA_MAX, GFP_KERNEL);
+	if (!occ_data)
+		return -ENOMEM;
+
+	rc = occ_send_cmd(driver, 0, OCC_POLL, 1, &poll_cmd_data, occ_data);
+	if (rc) {
+		dev_err(dev, "OCC poll failed: %d\n", rc);
+		goto out;
+	}
+
+	num_bytes = get_unaligned((u16 *)&occ_data[RESP_DATA_LENGTH]);
+	num_bytes = be16_to_cpu(num_bytes);
+	dev_dbg(dev, "OCC data length: %d\n", num_bytes);
+
+	if (num_bytes > OCC_DATA_MAX) {
+		dev_err(dev, "OCC data length must be < 4KB\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	if (num_bytes <= 0) {
+		dev_err(dev, "OCC data length is zero\n");
+		rc = -EINVAL;
+		goto out;
+	}
+
+	/* read remaining data */
+	for (i = 8; i < num_bytes + 8; i += 8) {
+		rc = driver->bus_ops.getscom(driver->bus, OCB_DATA,
+					     (u64 *)&occ_data[i]);
+		if (rc) {
+			dev_err(dev, "scom op failed rc:%d\n", rc);
+			goto out;
+		}
+	}
+
+	/* don't need more sanity checks; buffer is alloc'd for max response
+	 * size so we just check for valid data in parse_occ_response
+	 */
+	rc = parse_occ_response(driver, occ_data, resp);
+
+out:
+	devm_kfree(dev, occ_data);
+	return rc;
+}
+
+int occ_update_device(struct occ *driver)
+{
+	int rc = 0;
+
+	mutex_lock(&driver->update_lock);
+
+	if (time_after(jiffies, driver->last_updated + driver->update_interval)
+	    || !driver->valid) {
+		driver->valid = 1;
+
+		rc = occ_get_all(driver);
+		if (rc)
+			driver->valid = 0;
+
+		driver->last_updated = jiffies;
+	}
+
+	mutex_unlock(&driver->update_lock);
+
+	return rc;
+}
+EXPORT_SYMBOL(occ_update_device);
+
+void *occ_get_sensor(struct occ *driver, int sensor_type)
+{
+	int rc;
+
+	/* occ_update_device locks the update lock */
+	rc = occ_update_device(driver);
+	if (rc != 0) {
+		dev_err(driver->dev, "cannot get occ sensor data: %d\n",
+			rc);
+		return NULL;
+	}
+
+	return occ_get_sensor_by_type(&driver->response, sensor_type);
+}
+EXPORT_SYMBOL(occ_get_sensor);
+
+int occ_get_sensor_value(struct occ *occ, int sensor_type, int snum)
+{
+	return occ->ops.get_sensor_value(occ, sensor_type, snum);
+}
+EXPORT_SYMBOL(occ_get_sensor_value);
+
+int occ_get_sensor_id(struct occ *occ, int sensor_type, int snum)
+{
+	return occ->ops.get_sensor_id(occ, sensor_type, snum);
+}
+EXPORT_SYMBOL(occ_get_sensor_id);
+
+int occ_get_caps_value(struct occ *occ, void *sensor, int snum, int caps_field)
+{
+	return occ->ops.get_caps_value(sensor, snum, caps_field);
+}
+EXPORT_SYMBOL(occ_get_caps_value);
+
+void occ_get_response_blocks(struct occ *occ, struct occ_blocks **blocks)
+{
+	*blocks = &occ->response.data;
+}
+EXPORT_SYMBOL(occ_get_response_blocks);
+
+void occ_set_update_interval(struct occ *occ, unsigned long interval)
+{
+	occ->update_interval = msecs_to_jiffies(interval);
+}
+EXPORT_SYMBOL(occ_set_update_interval);
+
+int occ_set_user_powercap(struct occ *occ, u16 cap)
+{
+	u8 resp[8];
+
+	cap = cpu_to_be16(cap);
+
+	return occ_send_cmd(occ, 0, OCC_SET_USER_POWR_CAP, 2, (u8 *)&cap,
+			    resp);
+}
+EXPORT_SYMBOL(occ_set_user_powercap);
+
+struct occ *occ_start(struct device *dev, void *bus,
+		      struct occ_bus_ops *bus_ops, const struct occ_ops *ops,
+		      const struct occ_config *config)
+{
+	struct occ *driver = devm_kzalloc(dev, sizeof(struct occ), GFP_KERNEL);
+
+	if (!driver)
+		return ERR_PTR(-ENOMEM);
+
+	driver->dev = dev;
+	driver->bus = bus;
+	driver->bus_ops = *bus_ops;
+	driver->ops = *ops;
+	driver->config = *config;
+
+	driver->update_interval = HZ;
+	mutex_init(&driver->update_lock);
+
+	return driver;
+}
+EXPORT_SYMBOL(occ_start);
+
+int occ_stop(struct occ *occ)
+{
+	devm_kfree(occ->dev, occ);
+
+	return 0;
+}
+EXPORT_SYMBOL(occ_stop);
+
+MODULE_AUTHOR("Eddie James <eajames@us.ibm.com>");
+MODULE_DESCRIPTION("OCC hwmon core driver");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hwmon/occ/occ.h b/drivers/hwmon/occ/occ.h
new file mode 100644
index 0000000..8c08607
--- /dev/null
+++ b/drivers/hwmon/occ/occ.h
@@ -0,0 +1,86 @@
+/*
+ * occ.h - hwmon OCC driver
+ *
+ * This file contains data structures and function prototypes for common access
+ * between different bus protocols and host systems.
+ *
+ * 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_H__
+#define __OCC_H__
+
+#include "scom.h"
+
+struct device;
+struct occ;
+
+/* sensor_data_block_header
+ * structure to match the raw occ sensor block header
+ */
+struct sensor_data_block_header {
+	u8 sensor_type[4];
+	u8 reserved0;
+	u8 sensor_format;
+	u8 sensor_length;
+	u8 sensor_num;
+} __attribute__((packed, aligned(4)));
+
+struct sensor_data_block {
+	struct sensor_data_block_header header;
+	void *sensors;
+};
+
+enum sensor_type {
+	FREQ = 0,
+	TEMP,
+	POWER,
+	CAPS,
+	MAX_OCC_SENSOR_TYPE
+};
+
+struct occ_ops {
+	void (*parse_sensor)(u8 *data, void *sensor, int sensor_type, int off,
+			     int snum);
+	void *(*alloc_sensor)(int sensor_type, int num_sensors);
+	int (*get_sensor_value)(struct occ *driver, int sensor_type, int snum);
+	int (*get_sensor_id)(struct occ *driver, int sensor_type, int snum);
+	int (*get_caps_value)(void *sensor, int snum, int caps_field);
+};
+
+struct occ_config {
+	u32 command_addr;
+	u32 response_addr;
+};
+
+struct occ_blocks {
+	int sensor_block_id[MAX_OCC_SENSOR_TYPE];
+	struct sensor_data_block *blocks;
+};
+
+struct occ *occ_start(struct device *dev, void *bus,
+		      struct occ_bus_ops *bus_ops, const struct occ_ops *ops,
+		      const struct occ_config *config);
+int occ_stop(struct occ *occ);
+
+void *occ_get_sensor(struct occ *occ, int sensor_type);
+int occ_get_sensor_value(struct occ *occ, int sensor_type, int snum);
+int occ_get_sensor_id(struct occ *occ, int sensor_type, int snum);
+int occ_get_caps_value(struct occ *occ, void *sensor, int snum,
+		       int caps_field);
+void occ_get_response_blocks(struct occ *occ, struct occ_blocks **blocks);
+int occ_update_device(struct occ *driver);
+void occ_set_update_interval(struct occ *occ, unsigned long interval);
+int occ_set_user_powercap(struct occ *occ, u16 cap);
+
+#endif /* __OCC_H__ */
diff --git a/drivers/hwmon/occ/scom.h b/drivers/hwmon/occ/scom.h
new file mode 100644
index 0000000..c1da645
--- /dev/null
+++ b/drivers/hwmon/occ/scom.h
@@ -0,0 +1,47 @@
+/*
+ * scom.h - hwmon OCC driver
+ *
+ * This file contains data structures for scom operations to the OCC
+ *
+ * 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 __SCOM_H__
+#define __SCOM_H__
+
+/*
+ * occ_bus_ops - represent the low-level transfer methods to communicate with
+ * the OCC.
+ *
+ * getscom - OCC scom read
+ * @bus: handle to slave device
+ * @address: address
+ * @data: where to store data read from slave; buffer size must be at least
+ * eight bytes.
+ *
+ * Returns 0 on success or a negative errno on error
+ *
+ * putscom - OCC scom write
+ * @bus: handle to slave device
+ * @address: address
+ * @data0: first data byte to write
+ * @data1: second data byte to write
+ *
+ * Returns 0 on success or a negative errno on error
+ */
+struct occ_bus_ops {
+	int (*getscom)(void *bus, u32 address, u64 *data);
+	int (*putscom)(void *bus, u32 address, u32 data0, u32 data1);
+};
+
+#endif /* __SCOM_H__ */
-- 
1.9.1

^ permalink raw reply related

* [PATCH linux 0/6] drivers: hwmon: Add On-Chip Controller driver
From: eajames.ibm @ 2016-12-30 17:56 UTC (permalink / raw)
  To: linux
  Cc: jdelvare, corbet, mark.rutland, robh+dt, wsa, andrew, joel,
	devicetree, linux-doc, linux-hwmon, linux-i2c, linux-kernel,
	Edward A. James

From: "Edward A. James" <eajames@us.ibm.com>

This patchset adds a hwmon driver to support the OCC (On-Chip Controller)
on the IBM POWER8 and POWER9 processors, from a BMC (Baseboard Management
Controller). The OCC is an embedded processor that provides real time
power and thermal monitoring.

The driver provides an interface on a BMC to poll OCC sensor data, set
user power caps, and perform some basic OCC error handling. It interfaces
with userspace through hwmon.

The driver is currently functional only for the OCC on POWER8 chips.
Communicating with the POWER9 OCC requries FSI support.

Edward A. James (6):
  hwmon: Add core On-Chip Controller support for POWER CPUs
  hwmon: occ: Add sysfs interface
  hwmon: occ: Add I2C transport implementation for SCOM operations
  hwmon: occ: Add callbacks for parsing P8 OCC datastructures
  hwmon: occ: Add hwmon implementation for the P8 OCC
  hwmon: occ: Add callbacks for parsing P9 OCC datastructures

 .../devicetree/bindings/i2c/i2c-ibm-occ.txt        |  13 +
 Documentation/hwmon/occ                            | 100 ++++
 drivers/hwmon/Kconfig                              |   2 +
 drivers/hwmon/Makefile                             |   1 +
 drivers/hwmon/occ/Kconfig                          |  29 ++
 drivers/hwmon/occ/Makefile                         |   2 +
 drivers/hwmon/occ/occ.c                            | 544 +++++++++++++++++++++
 drivers/hwmon/occ/occ.h                            |  86 ++++
 drivers/hwmon/occ/occ_p8.c                         | 217 ++++++++
 drivers/hwmon/occ/occ_p8.h                         |  30 ++
 drivers/hwmon/occ/occ_scom_i2c.c                   |  73 +++
 drivers/hwmon/occ/occ_scom_i2c.h                   |  26 +
 drivers/hwmon/occ/occ_sysfs.c                      | 492 +++++++++++++++++++
 drivers/hwmon/occ/occ_sysfs.h                      |  52 ++
 drivers/hwmon/occ/p8_occ_i2c.c                     | 141 ++++++
 drivers/hwmon/occ/p9_occ.c                         | 243 +++++++++
 drivers/hwmon/occ/p9_occ.h                         |  30 ++
 drivers/hwmon/occ/scom.h                           |  47 ++
 18 files changed, 2128 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-ibm-occ.txt
 create mode 100644 Documentation/hwmon/occ
 create mode 100644 drivers/hwmon/occ/Kconfig
 create mode 100644 drivers/hwmon/occ/Makefile
 create mode 100644 drivers/hwmon/occ/occ.c
 create mode 100644 drivers/hwmon/occ/occ.h
 create mode 100644 drivers/hwmon/occ/occ_p8.c
 create mode 100644 drivers/hwmon/occ/occ_p8.h
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c
 create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h
 create mode 100644 drivers/hwmon/occ/occ_sysfs.c
 create mode 100644 drivers/hwmon/occ/occ_sysfs.h
 create mode 100644 drivers/hwmon/occ/p8_occ_i2c.c
 create mode 100644 drivers/hwmon/occ/p9_occ.c
 create mode 100644 drivers/hwmon/occ/p9_occ.h
 create mode 100644 drivers/hwmon/occ/scom.h

-- 
1.9.1


^ permalink raw reply

* Re: [PATCH 1/2 v2] iio: light: add DT bindings for Capella CM3605
From: Jonathan Cameron @ 2016-12-30 17:18 UTC (permalink / raw)
  To: Linus Walleij, linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Capella Microsystems,
	Kevin Tsai
In-Reply-To: <1482098155-16148-1-git-send-email-linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

On 18/12/16 21:55, Linus Walleij wrote:
> This adds device tree bindings for the Capella Microsystems CM3605
> ambient light sensor and short range proximity sensor.
> 
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: Capella Microsystems <capellamicro-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Kevin Tsai <ktsai-GubuWUlQtMwciDkP5Hr2oA@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Applied
> ---
> ChangeLog v1->v2:
> - Rename aset-resistance to aset-resistance-ohms
> ---
>  .../devicetree/bindings/iio/light/cm3605.txt       | 41 ++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/cm3605.txt
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/cm3605.txt b/Documentation/devicetree/bindings/iio/light/cm3605.txt
> new file mode 100644
> index 000000000000..56331a79f9ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/cm3605.txt
> @@ -0,0 +1,41 @@
> +Capella Microsystems CM3605
> +Ambient Light and Short Distance Proximity Sensor
> +
> +The CM3605 is an entirely analog part which however require quite a bit of
> +software logic to interface a host operating system.
> +
> +This ALS and proximity sensor was one of the very first deployed in mobile
> +handsets, notably it is used in the very first Nexus One Android phone from
> +2010.
> +
> +Required properties:
> +- compatible: must be: "capella,cm3605"
> +- aset-gpios: GPIO line controlling the ASET line (drive low
> +  to activate the ALS, should be flagged GPIO_ACTIVE_LOW)
> +- interrupts: the IRQ line (such as a GPIO) that is connected to
> +  the POUT (proximity sensor out) line. The edge detection must
> +  be set to IRQ_TYPE_EDGE_BOTH so as to detect movements toward
> +  and away from the proximity sensor.
> +- io-channels: the ADC channel used for converting the voltage from
> +  AOUT to a digital representation.
> +- io-channel-names: must be "aout"
> +
> +Optional properties:
> +- vdd-supply: regulator supplying VDD power to the component.
> +- capella,aset-resistance-ohms: the sensitivity calibration resistance,
> +  in Ohms. Valid values are: 50000, 100000, 300000 and 600000,
> +  as these are the resistance values that we are supplied with
> +  calibration curves for. If not supplied, 100 kOhm will be assumed
> +  but it is strongly recommended to supply this.
> +
> +Example:
> +
> +cm3605 {
> +	compatible = "capella,cm3605";
> +	vdd-supply = <&foo_reg>;
> +	aset-gpios = <&foo_gpio 1 GPIO_ACTIVE_LOW>;
> +	capella,aset-resistance-ohms = <100000>;
> +	interrupts = <1 IRQ_TYPE_EDGE_BOTH>;
> +	io-channels = <&adc 0x01>;
> +	io-channel-names = "aout";
> +};
> 

^ permalink raw reply

* Re: [PATCH v4 2/2] iio: adc: hx711: Add IIO driver for AVIA HX711
From: Jonathan Cameron @ 2016-12-30 17:04 UTC (permalink / raw)
  To: Andreas Klinger, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-iio-u79uwXL29TY76Z2rM5mHXA
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, pawel.moll-5wv7dgnIgG8,
	mark.rutland-5wv7dgnIgG8, ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg,
	galak-sgV2jX0FEOL9JmXXK+q4OQ, knaack.h-Mmb7MZpHnFY,
	lars-Qo5EllUWu/uELgA04lAiVw, pmeerw-jW+XmwGofnusTnJN9+BGXg
In-Reply-To: <20161221152514.GA14554@andreas>

On 21/12/16 15:25, Andreas Klinger wrote:
> This is the IIO driver for AVIA HX711 ADC which ist mostly used in weighting
> cells.
> 
> The protocol is quite simple and using GPIOs:
> One GPIO is used as clock (SCK) while another GPIO is read (DOUT)
> 
> The raw value read from the chip is delivered. 
> To get a weight one needs to subtract the zero offset and scale it.
> 
> Signed-off-by: Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>
I argue myself in an out of using fractional types for the scale (these aren't
well handled for writeable values as it's very hard to know what fraction we
should output!).

Otherwise a few points that may well repeat what Peter said.

Hard bit is done though (all that bit bashing) so only minor stuff to clean
up.

Thanks,

Jonathan
> ---
>  drivers/iio/adc/Kconfig  |  19 ++
>  drivers/iio/adc/Makefile |   1 +
>  drivers/iio/adc/hx711.c  | 466 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 486 insertions(+)
>  create mode 100644 drivers/iio/adc/hx711.c
> 
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 932de1f9d1e7..1dcf2ace1697 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -205,6 +205,25 @@ config HI8435
>  	  This driver can also be built as a module. If so, the module will be
>  	  called hi8435.
>  
> +config HX711
> +	tristate "AVIA HX711 ADC for weight cells"
> +	depends on GPIOLIB
> +	help
> +	  If you say yes here you get support for AVIA HX711 ADC which is used
> +	  for weigh cells
> +
> +	  This driver uses two GPIOs, one acts as the clock and controls the
> +	  channel selection and gain, the other one is used for the measurement
> +          data
> +
> +	  Currently the raw value is read from the chip and delivered.
> +	  To get an actual weight one needs to subtract the
> +	  zero offset and multiply by a scale factor.
> +	  This should be done in userspace.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called hx711.
> +
>  config INA2XX_ADC
>  	tristate "Texas Instruments INA2xx Power Monitors IIO driver"
>  	depends on I2C && !SENSORS_INA2XX
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index b1aa456e6af3..d46e289900ef 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -21,6 +21,7 @@ obj-$(CONFIG_CC10001_ADC) += cc10001_adc.o
>  obj-$(CONFIG_DA9150_GPADC) += da9150-gpadc.o
>  obj-$(CONFIG_EXYNOS_ADC) += exynos_adc.o
>  obj-$(CONFIG_HI8435) += hi8435.o
> +obj-$(CONFIG_HX711) += hx711.o
>  obj-$(CONFIG_IMX7D_ADC) += imx7d_adc.o
>  obj-$(CONFIG_INA2XX_ADC) += ina2xx-adc.o
>  obj-$(CONFIG_LP8788_ADC) += lp8788_adc.o
> diff --git a/drivers/iio/adc/hx711.c b/drivers/iio/adc/hx711.c
> new file mode 100644
> index 000000000000..f4b5f587e9c0
> --- /dev/null
> +++ b/drivers/iio/adc/hx711.c
> @@ -0,0 +1,466 @@
> +/*
> + * HX711: analog to digital converter for weight sensor module
> + *
> + * Copyright (c) 2016 Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@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/err.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +#include <linux/sched.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +/*
> + * gain to pulse and scale conversion
> + */
/* gain to pulse and scale conversion */
(we are very fussy about comment syntax, particularly as otherwise
I'll get inundated with patches fixing it ;)
> +#define HX711_CHAN_A		0	/* channel id for A  */
> +#define HX711_CHAN_B		1	/* channel id for B  */
> +
> +#define HX711_GAIN_MAX		3
> +
> +struct hx711_gain_to_scale {
> +	int			gain;
> +	int			gain_pulse;
> +	int			scale;
> +	int			channel;
> +};
> +
> +static struct hx711_gain_to_scale hx711_gain_to_scale[HX711_GAIN_MAX] = {
> +	{ 128, 1, 0, HX711_CHAN_A },
> +	{  32, 2, 0, HX711_CHAN_B },
> +	{  64, 3, 0, HX711_CHAN_A }
> +};
> +
> +static int hx711_get_gain_to_pulse(int gain)
> +{
> +	int i;
> +
> +	for (i = 0; i < HX711_GAIN_MAX; i++)
> +		if (hx711_gain_to_scale[i].gain == gain)
> +			return hx711_gain_to_scale[i].gain_pulse;
> +	return 1;
> +}
> +
> +static int hx711_get_gain_to_scale(int gain)
> +{
> +	int i;
> +
> +	for (i = 0; i < HX711_GAIN_MAX; i++)
> +		if (hx711_gain_to_scale[i].gain == gain)
> +			return hx711_gain_to_scale[i].scale;
> +	return 0;
> +}
> +
> +static int hx711_get_scale_to_gain(int scale)
> +{
> +	int i;
> +
> +	for (i = 0; i < HX711_GAIN_MAX; i++)
> +		if (hx711_gain_to_scale[i].scale == scale)
> +			return hx711_gain_to_scale[i].gain;
> +	return -EINVAL;
> +}
> +
> +struct hx711_data {
> +	struct device		*dev;
> +	struct gpio_desc	*gpiod_pd_sck;
> +	struct gpio_desc	*gpiod_dout;
> +	struct regulator	*reg_avdd;
> +	int			gain_set;	/* gain set on device */
> +	int			gain_chan_a;	/* gain for channel A */
> +	struct mutex		lock;
> +};
> +
> +static int hx711_read(struct hx711_data *hx711_data);
> +
> +static int hx711_reset(struct hx711_data *hx711_data)
> +{
> +	int ret;
> +	int val = gpiod_get_value(hx711_data->gpiod_dout);
> +
> +	if (val) {
> +		int i;
> +
> +		/*
> +		 * an examination with the oszilloscope indicated
> +		 * that the first value read after the reset is not stable
> +		 * if we reset too short;
> +		 * the shorter the reset cycle
> +		 * the less reliable the first value after reset is;
> +		 * there were no problems encountered with a value
> +		 * of 10 ms or higher
> +		 */
> +		gpiod_set_value(hx711_data->gpiod_pd_sck, 1);
> +		msleep(10);
> +		gpiod_set_value(hx711_data->gpiod_pd_sck, 0);
> +
> +		/*
> +		 * a maximum reset cycle time of 56 ms was measured.
> +		 * we round it up to 100 ms
> +		 */
> +		for (i = 0; i < 100; i++) {
> +			val = gpiod_get_value(hx711_data->gpiod_dout);
> +			if (!val)
> +				break;
> +			/* sleep at least 1 ms */
> +			msleep(1);
> +		}
> +
> +		/*
> +		 * after a reset the gain is 128 so we do a dummy read
> +		 *   to set the gain for the next read
> +		 */
> +		ret = hx711_read(hx711_data);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return val;
> +}
> +
> +static int hx711_cycle(struct hx711_data *hx711_data)
> +{
> +	int val;
> +
> +	/*
> +	 * if preempted for more then 60us while PD_SCK is high:
> +	 * hx711 is going in reset
> +	 * ==> measuring is false
> +	 */
> +	preempt_disable();
> +	gpiod_set_value(hx711_data->gpiod_pd_sck, 1);
> +	val = gpiod_get_value(hx711_data->gpiod_dout);
> +	/*
> +	 * here we are not waiting for 0.2 us as suggested by the datasheet,
> +	 * because the oszilloscope showed in a test scenario
> +	 * at least 1.15 us for PD_SCK high (T3 in datasheet)
> +	 * and 0.56 us for PD_SCK low on TI Sitara with 800 MHz
> +	 */
> +	gpiod_set_value(hx711_data->gpiod_pd_sck, 0);
> +	preempt_enable();
> +
> +	return val;
> +}
> +
> +static int hx711_read(struct hx711_data *hx711_data)
> +{
> +	int i, ret;
> +	int value = 0;
> +
> +	if (hx711_reset(hx711_data)) {
> +		dev_err(hx711_data->dev, "reset failed!");
> +		return -EIO;
> +	}
> +
> +	for (i = 0; i < 24; i++) {
> +		value <<= 1;
> +		ret = hx711_cycle(hx711_data);
> +		if (ret)
> +			value++;
> +	}
> +
> +	value ^= 0x800000;
> +
> +	for (i = 0; i < hx711_get_gain_to_pulse(hx711_data->gain_set); i++)
> +		hx711_cycle(hx711_data);
> +
> +	return value;
> +}
> +
> +static int hx711_set_gain_for_channel(struct hx711_data *hx711_data, int chan)
> +{
> +	int ret;
> +
> +	if (chan == HX711_CHAN_A) {
> +		if (hx711_data->gain_set == 32) {
> +			hx711_data->gain_set = hx711_data->gain_chan_a;
> +
> +			ret = hx711_read(hx711_data);
> +			if (ret < 0)
> +				return ret;
> +		}
> +	} else {
> +		if (hx711_data->gain_set != 32) {
> +			hx711_data->gain_set = 32;
> +
> +			ret = hx711_read(hx711_data);
> +			if (ret < 0)
> +				return ret;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +static int hx711_read_raw(struct iio_dev *iio_dev,
> +				const struct iio_chan_spec *chan,
> +				int *val, int *val2, long mask)
> +{
> +	struct hx711_data *hx711_data = iio_priv(iio_dev);
> +	int ret;
> +
> +	mutex_lock(&hx711_data->lock);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = hx711_set_gain_for_channel(hx711_data, chan->channel);
> +		if (ret < 0) {
> +			mutex_unlock(&hx711_data->lock);
> +			return ret;
> +		}
> +
> +		*val = hx711_read(hx711_data);
> +		mutex_unlock(&hx711_data->lock);
> +		if (*val < 0)
> +			return *val;
> +		return IIO_VAL_INT;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = hx711_get_gain_to_scale(hx711_data->gain_set);
> +		mutex_unlock(&hx711_data->lock);
> +		return IIO_VAL_INT_PLUS_NANO;
> +	default:
> +		mutex_unlock(&hx711_data->lock);
> +		return -EINVAL;
> +	}
> +}
> +
> +static int hx711_write_raw(struct iio_dev *iio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val,
> +				int val2,
> +				long mask)
> +{
> +	struct hx711_data *hx711_data = iio_priv(iio_dev);
> +	int ret;
> +	int gain;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		/*
> +		 * a scale greater than 1 mV per LSB is not possible
> +		 * with the HX711, therefore val must be 0
> +		 */
> +		if (val != 0)
> +			return -EINVAL;
> +
> +		mutex_lock(&hx711_data->lock);
> +
> +		gain = hx711_get_scale_to_gain(val2);
> +		if (gain < 0) {
> +			mutex_unlock(&hx711_data->lock);
> +			return gain;
> +		}
> +
> +		if (gain != hx711_data->gain_set) {
> +			hx711_data->gain_set = gain;
> +			if (gain != 32)
> +				hx711_data->gain_chan_a = gain;
> +
> +			ret = hx711_read(hx711_data);
> +			if (ret < 0) {
> +				mutex_unlock(&hx711_data->lock);
> +				return ret;
> +			}
> +		}
> +
> +		mutex_unlock(&hx711_data->lock);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hx711_write_raw_get_fmt(struct iio_dev *indio_dev,
> +		struct iio_chan_spec const *chan,
> +		long mask)
> +{
> +	return IIO_VAL_INT_PLUS_NANO;
> +}
> +
> +static ssize_t hx711_scale_available_show(struct device *dev,
> +				struct device_attribute *attr,
> +				char *buf)
> +{
> +	return sprintf(buf, "0.%09d 0.%09d 0.%09d\n",
> +			hx711_gain_to_scale[0].scale,
> +			hx711_gain_to_scale[2].scale,
> +			hx711_gain_to_scale[1].scale);
If you did use the fractional types as defined below, you could then
use the core functions to give you the pretty printed output.

Hmm.. Not all these scales are available for each of the channels.
You should have separate scale_availables
in_voltage0_scale_available, in_voltage1_scale_available
(though no actual need for the one that only has one scale)

It shouldn't be up to userspace to guess which is relevant to a given
channel.
> +}
> +
> +static IIO_DEVICE_ATTR(scale_available, S_IRUGO,
> +	hx711_scale_available_show, NULL, 0);
> +
> +static struct attribute *hx711_attributes[] = {
> +	&iio_dev_attr_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static struct attribute_group hx711_attribute_group = {
> +	.attrs = hx711_attributes,
> +};
> +
> +static const struct iio_info hx711_iio_info = {
> +	.driver_module		= THIS_MODULE,
> +	.read_raw		= hx711_read_raw,
> +	.write_raw		= hx711_write_raw,
> +	.write_raw_get_fmt	= hx711_write_raw_get_fmt,
> +	.attrs			= &hx711_attribute_group,
> +};
> +
> +static const struct iio_chan_spec hx711_chan_spec[] = {
> +	{
> +		.type = IIO_VOLTAGE,
> +		.channel = HX711_CHAN_A,
I'd be tempted to just make the two channels directly 0 and 1 and loose
this define throughout. Don't think it makes much difference to readability
elsewhare, but here it had me momentarily confused...
> +		.indexed = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +	{
> +		.type = IIO_VOLTAGE,
> +		.channel = HX711_CHAN_B,
> +		.indexed = 1,
> +		.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
> +		.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
> +	},
> +};
> +
> +static int hx711_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct hx711_data *hx711_data;
> +	struct iio_dev *iio;
It's mostly become convention to call this indio_dev but it doesn't
realy matter if you prefer iio.
> +	int ret;
> +	int i;
> +
> +	iio = devm_iio_device_alloc(dev, sizeof(struct hx711_data));
> +	if (!iio) {
> +		dev_err(dev, "failed to allocate IIO device\n");
> +		return -ENOMEM;
> +	}
> +
> +	hx711_data = iio_priv(iio);
> +	hx711_data->dev = dev;
> +
> +	mutex_init(&hx711_data->lock);
> +
> +	/* PD_SCK stands for power down and serial clock input of HX711
> +	 * in the driver it is an output
> +	 */
> +	hx711_data->gpiod_pd_sck = devm_gpiod_get(dev, "sck", GPIOD_OUT_LOW);
> +	if (IS_ERR(hx711_data->gpiod_pd_sck)) {
> +		dev_err(dev, "failed to get sck-gpiod: err=%ld\n",
> +					PTR_ERR(hx711_data->gpiod_pd_sck));
> +		return PTR_ERR(hx711_data->gpiod_pd_sck);
> +	}
> +
> +	/* DOUT stands for serial data output of HX711
Please use
/*
 * DOUT...

Same elsewhere for multiline comments. 
> +	 * for the driver it is an input
> +	 */
> +	hx711_data->gpiod_dout = devm_gpiod_get(dev, "dout", GPIOD_IN);
> +	if (IS_ERR(hx711_data->gpiod_dout)) {
> +		dev_err(dev, "failed to get dout-gpiod: err=%ld\n",
> +					PTR_ERR(hx711_data->gpiod_dout));
> +		return PTR_ERR(hx711_data->gpiod_dout);
> +	}
> +
> +	hx711_data->reg_avdd = devm_regulator_get(dev, "avdd");
> +	if (IS_ERR(hx711_data->reg_avdd))
> +		return PTR_ERR(hx711_data->reg_avdd);
> +
> +	ret = regulator_enable(hx711_data->reg_avdd);
> +	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * with
> +	 * full scale differential input range: AVDD / GAIN
> +	 * full scale output data: 2^24
> +	 * we can say:
> +	 *     AVDD / GAIN = 2^24
> +	 * therefore:
> +	 *     1 LSB = AVDD / GAIN / 2^24
> +	 * AVDD is in uV, but we need 10^-9 mV
> +	 * approximately to fit into a 32 bit number:
> +	 * 1 LSB = (AVDD * 100) / GAIN / 1678 [10^-9 mV]
> +	 */
> +	ret = regulator_get_voltage(hx711_data->reg_avdd);
> +	if (ret < 0)
> +		return ret;
> +	/* we need 10^-9 mV */
> +	ret *= 100;
> +
> +	for (i = 0; i < HX711_GAIN_MAX; i++)
> +		hx711_gain_to_scale[i].scale =
> +			ret / hx711_gain_to_scale[i].gain / 1678;
I wonder if we can manipulate things to be able to use one of the
fractional IIO types instead of doing this manually here. Will still
have to be careful with limits of the storage though.
Also, don't think we do anything clever with input of these right
now so you may be stuck doing it this way.
> +
> +	hx711_data->gain_set = 128;
> +	hx711_data->gain_chan_a = 128;
> +
> +	platform_set_drvdata(pdev, iio);
> +
> +	iio->name = "hx711";
> +	iio->dev.parent = &pdev->dev;
> +	iio->info = &hx711_iio_info;
> +	iio->modes = INDIO_DIRECT_MODE;
> +	iio->channels = hx711_chan_spec;
> +	iio->num_channels = ARRAY_SIZE(hx711_chan_spec);
> +
> +	return devm_iio_device_register(dev, iio);
> +}
> +
> +static int hx711_remove(struct platform_device *pdev)
> +{
> +	struct hx711_data *hx711_data;
> +	struct iio_dev *iio;
> +
> +	iio = platform_get_drvdata(pdev);
> +	hx711_data = iio_priv(iio);
> +
> +	regulator_disable(hx711_data->reg_avdd);
Ah. The usual fun of having using devm_ functions.
They are safe 'until' you do something in the probe before one of them that
needs to be unwound.

The result here is that we turn the power off before the userspace interface
for the device is removed (as the unregister occurs only after this remove
function is done).  Thus any reads in that period result in attempting to
talk to hardware that is turned off.

Hence, you need to not use the non managed version of iio_device_register
and manually call iio_device_unregister here. (nearly everyone makes this
mistake at least once!)
> +
> +	return 0;
> +}
> +
> +static const struct of_device_id of_hx711_match[] = {
> +	{ .compatible = "avia,hx711", },
> +	{},
> +};
> +
> +MODULE_DEVICE_TABLE(of, of_hx711_match);
> +
> +static struct platform_driver hx711_driver = {
> +	.probe		= hx711_probe,
> +	.remove		= hx711_remove,
> +	.driver		= {
> +		.name		= "hx711-gpio",
> +		.of_match_table	= of_hx711_match,
> +	},
> +};
> +
> +module_platform_driver(hx711_driver);
> +
> +MODULE_AUTHOR("Andreas Klinger <ak-n176/SwNRljddJNmlsFzeA@public.gmane.org>");
> +MODULE_DESCRIPTION("HX711 bitbanging driver - ADC for weight cells");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:hx711-gpio");
> +
> 

^ permalink raw reply

* Re: [PATCH v3 1/2] iio: imu: add support to lsm6dsx driver
From: Jonathan Cameron @ 2016-12-30 16:01 UTC (permalink / raw)
  To: Lorenzo Bianconi
  Cc: linux-iio-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA, lorenzo.bianconi-qxv4g6HH51o
In-Reply-To: <20161210090218.4609-2-lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>

On 10/12/16 09:02, Lorenzo Bianconi wrote:
> Add support to STM LSM6DS3-LSM6DSM 6-axis (acc + gyro) Mems sensor
> 
> http://www.st.com/resource/en/datasheet/lsm6ds3.pdf
> http://www.st.com/resource/en/datasheet/lsm6dsm.pdf
> 
> - continuous mode support
> - i2c support
> - spi support
> - sw fifo mode support
> - supported devices: lsm6ds3, lsm6dsm
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
Hi Lorenzo,

I've obviously not looked at this for a while and with fresh eyes I've
picked up on a few more little bits and bobs that would be nice to cleanup.

* I'm not keen on using the name of the device (string) as the driver_data
  element. Would be cleaner and more 'standard' to introduce an enum for that.
* It might be possible to bring all the asignments of hw elements into the
  core probe which would remove a small amount of duplication and make it
  a tiny bit more readable. (really minor point!)
* The use of claim_direct_mode and friends in read_raw is probably covering
  too many things.  There is no harm in allowing reading of the scale /
  sampling frequency whilst buffered operation is on going.

A few other trivial bits and bobs inline. Anyhow, nearly ready and a nice
driver for this complex bit of hardware. Looks like there are plenty of
other features that could be added later ;)

Jonathan
> ---
>  drivers/iio/imu/Kconfig                        |   1 +
>  drivers/iio/imu/Makefile                       |   2 +
>  drivers/iio/imu/st_lsm6dsx/Kconfig             |  23 +
>  drivers/iio/imu/st_lsm6dsx/Makefile            |   5 +
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h        | 133 +++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 437 +++++++++++++++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c   | 655 +++++++++++++++++++++++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c    |  88 ++++
>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c    | 126 +++++
>  9 files changed, 1470 insertions(+)
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/Kconfig
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/Makefile
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
>  create mode 100644 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> 
> diff --git a/drivers/iio/imu/Kconfig b/drivers/iio/imu/Kconfig
> index 1f1ad41..156630a 100644
> --- a/drivers/iio/imu/Kconfig
> +++ b/drivers/iio/imu/Kconfig
> @@ -39,6 +39,7 @@ config KMX61
>  	  be called kmx61.
>  
>  source "drivers/iio/imu/inv_mpu6050/Kconfig"
> +source "drivers/iio/imu/st_lsm6dsx/Kconfig"
>  
>  endmenu
>  
> diff --git a/drivers/iio/imu/Makefile b/drivers/iio/imu/Makefile
> index c71bcd3..8b563c3 100644
> --- a/drivers/iio/imu/Makefile
> +++ b/drivers/iio/imu/Makefile
> @@ -17,3 +17,5 @@ obj-y += bmi160/
>  obj-y += inv_mpu6050/
>  
>  obj-$(CONFIG_KMX61) += kmx61.o
> +
> +obj-y += st_lsm6dsx/
> diff --git a/drivers/iio/imu/st_lsm6dsx/Kconfig b/drivers/iio/imu/st_lsm6dsx/Kconfig
> new file mode 100644
> index 0000000..2ebcb74
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/Kconfig
> @@ -0,0 +1,23 @@
> +
> +config IIO_ST_LSM6DSX
> +	tristate "ST_LSM6DSx driver for STM 6-axis IMU MEMS sensors"
> +	depends on (I2C || SPI)
> +	select IIO_BUFFER
> +	select IIO_KFIFO_BUF
> +	select IIO_ST_LSM6DSX_I2C if (I2C)
> +	select IIO_ST_LSM6DSX_SPI if (SPI_MASTER)
> +	help
> +	  Say yes here to build support for STMicroelectronics LSM6DSx imu
> +	  sensor. Supported devices: lsm6ds3, lsm6dsm
> +
> +	  To compile this driver as a module, choose M here: the module
> +	  will be called st_lsm6dsx.
> +
> +config IIO_ST_LSM6DSX_I2C
> +	tristate
> +	depends on IIO_ST_LSM6DSX
> +
> +config IIO_ST_LSM6DSX_SPI
> +	tristate
> +	depends on IIO_ST_LSM6DSX
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/Makefile b/drivers/iio/imu/st_lsm6dsx/Makefile
> new file mode 100644
> index 0000000..35919fe
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/Makefile
> @@ -0,0 +1,5 @@
> +st_lsm6dsx-y := st_lsm6dsx_core.o st_lsm6dsx_buffer.o
> +
> +obj-$(CONFIG_IIO_ST_LSM6DSX) += st_lsm6dsx.o
> +obj-$(CONFIG_IIO_ST_LSM6DSX_I2C) += st_lsm6dsx_i2c.o
> +obj-$(CONFIG_IIO_ST_LSM6DSX_SPI) += st_lsm6dsx_spi.o
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> new file mode 100644
> index 0000000..5831f63
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx.h
> @@ -0,0 +1,133 @@
> +/*
> + * STMicroelectronics st_lsm6dsx sensor driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#ifndef ST_LSM6DSX_H
> +#define ST_LSM6DSX_H
> +
> +#include <linux/device.h>
> +
> +#define ST_LSM6DS3_DEV_NAME	"lsm6ds3"
> +#define ST_LSM6DSM_DEV_NAME	"lsm6dsm"
> +
> +#define ST_LSM6DSX_CHAN_SIZE		2
> +#define ST_LSM6DSX_SAMPLE_SIZE		6
> +#define ST_LSM6DSX_SAMPLE_DEPTH		(ST_LSM6DSX_SAMPLE_SIZE / \
> +					 ST_LSM6DSX_CHAN_SIZE)
> +
> +#define ST_LSM6DSX_RX_MAX_LENGTH	8
> +#define ST_LSM6DSX_TX_MAX_LENGTH	8
> +
> +struct st_lsm6dsx_transfer_buffer {
> +	u8 rx_buf[ST_LSM6DSX_RX_MAX_LENGTH];
> +	u8 tx_buf[ST_LSM6DSX_TX_MAX_LENGTH] ____cacheline_aligned;
> +};
> +
> +struct st_lsm6dsx_transfer_function {
> +	int (*read)(struct device *dev, u8 addr, int len, u8 *data);
> +	int (*write)(struct device *dev, u8 addr, int len, u8 *data);
> +};
> +
> +struct st_lsm6dsx_reg {
> +	u8 addr;
> +	u8 mask;
> +};
> +
> +struct st_lsm6dsx_settings {
> +	u8 wai;
> +	u16 max_fifo_size;
> +};
> +
> +enum st_lsm6dsx_sensor_id {
> +	ST_LSM6DSX_ID_ACC,
> +	ST_LSM6DSX_ID_GYRO,
> +	ST_LSM6DSX_ID_MAX,
> +};
> +
> +enum st_lsm6dsx_fifo_mode {
> +	ST_LSM6DSX_FIFO_BYPASS = 0x0,
> +	ST_LSM6DSX_FIFO_CONT = 0x6,
> +};
> +
> +/**
> + * struct st_lsm6dsx_sensor - ST IMU sensor instance
> + * @id: Sensor identifier.
> + * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> + * @gain: Configured sensor sensitivity.
> + * @odr: Output data rate of the sensor [Hz].
> + * @watermark: Sensor watermark level.
> + * @sip: Number of samples in a given pattern.
> + * @decimator: FIFO decimation factor.
> + * @decimator_mask: Sensor mask for decimation register.
> + * @delta_ts: Delta time between two consecutive interrupts.
> + * @ts: Latest timestamp from the interrupt handler.
> + */
> +struct st_lsm6dsx_sensor {
> +	enum st_lsm6dsx_sensor_id id;
> +	struct st_lsm6dsx_hw *hw;
> +
> +	u32 gain;
> +	u16 odr;
> +
> +	u16 watermark;
> +	u8 sip;
> +	u8 decimator;
> +	u8 decimator_mask;
> +
> +	s64 delta_ts;
> +	s64 ts;
> +};
> +
> +/**
> + * struct st_lsm6dsx_hw - ST IMU MEMS hw instance
> + * @name: Pointer to the device name (I2C name or SPI modalias).
> + * @dev: Pointer to instance of struct device (I2C or SPI).
> + * @irq: Device interrupt line (I2C or SPI).
> + * @lock: Mutex to protect read and write operations.
> + * @fifo_lock: Mutex to prevent concurrent access to the hw FIFO.
> + * @fifo_mode: FIFO operating mode supported by the device.
> + * @enable_mask: Enabled sensor bitmask.
> + * @sip: Total number of samples (acc/gyro) in a given pattern.
> + * @iio_devs: Pointers to acc/gyro iio_dev instances.
> + * @settings: Pointer to the specific sensor settings in use.
> + * @tf: Transfer function structure used by I/O operations.
> + * @tb: Transfer buffers used by SPI I/O operations.
> + */
> +struct st_lsm6dsx_hw {
> +	const char *name;
> +	struct device *dev;
> +	int irq;
> +
> +	struct mutex lock;
> +	struct mutex fifo_lock;
> +
> +	enum st_lsm6dsx_fifo_mode fifo_mode;
> +	u8 enable_mask;
> +	u8 sip;
> +
> +	struct iio_dev *iio_devs[ST_LSM6DSX_ID_MAX];
> +
> +	const struct st_lsm6dsx_settings *settings;
> +
> +	const struct st_lsm6dsx_transfer_function *tf;
> +	struct st_lsm6dsx_transfer_buffer tb;
> +};
> +
> +int st_lsm6dsx_probe(struct st_lsm6dsx_hw *hw);
> +int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor);
> +int st_lsm6dsx_sensor_disable(struct st_lsm6dsx_sensor *sensor);
> +int st_lsm6dsx_allocate_buffers(struct st_lsm6dsx_hw *hw);
> +int st_lsm6dsx_write_with_mask(struct st_lsm6dsx_hw *hw, u8 addr, u8 mask,
> +			       u8 val);
> +int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor,
> +				u16 watermark);
> +
> +#endif /* ST_LSM6DSX_H */
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> new file mode 100644
> index 0000000..7f4032d
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
> @@ -0,0 +1,437 @@
> +/*
> + * STMicroelectronics st_lsm6dsx FIFO buffer library driver
> + *
> + * LSM6DS3/LSM6DSM: The FIFO buffer can be configured to store data
> + * from gyroscope and accelerometer. Samples are queued without any tag
> + * according to a specific pattern based on 'FIFO data sets' (6 bytes each):
> + *  - 1st data set is reserved for gyroscope data
> + *  - 2nd data set is reserved for accelerometer data
> + * The FIFO pattern changes depending on the ODRs and decimation factors
> + * assigned to the FIFO data sets. The first sequence of data stored in FIFO
> + * buffer contains the data of all the enabled FIFO data sets
> + * (e.g. Gx, Gy, Gz, Ax, Ay, Az), then data are repeated depending on the
> + * value of the decimation factor and ODR set for each FIFO data set.
> + * FIFO supported modes:
> + *  - BYPASS: FIFO disabled
> + *  - CONTINUOUS: FIFO enabled. When the buffer is full, the FIFO index
> + *    restarts from the beginning and the oldest sample is overwritten
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +#include <linux/module.h>
> +#include <linux/interrupt.h>
> +#include <linux/irq.h>
> +#include <linux/iio/kfifo_buf.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define ST_LSM6DSX_REG_FIFO_THL_ADDR		0x06
> +#define ST_LSM6DSX_REG_FIFO_THH_ADDR		0x07
> +#define ST_LSM6DSX_FIFO_TH_MASK			GENMASK(11, 0)
> +#define ST_LSM6DSX_REG_FIFO_DEC_GXL_ADDR	0x08
> +#define ST_LSM6DSX_REG_FIFO_MODE_ADDR		0x0a
> +#define ST_LSM6DSX_FIFO_MODE_MASK		GENMASK(2, 0)
> +#define ST_LSM6DSX_FIFO_ODR_MASK		GENMASK(6, 3)
> +#define ST_LSM6DSX_REG_FIFO_DIFFL_ADDR		0x3a
> +#define ST_LSM6DSX_FIFO_DIFF_MASK		GENMASK(11, 0)
> +#define ST_LSM6DSX_FIFO_EMPTY_MASK		BIT(12)
> +#define ST_LSM6DSX_REG_FIFO_OUTL_ADDR		0x3e
> +
> +#define ST_LSM6DSX_MAX_FIFO_ODR_VAL		0x08
> +
> +struct st_lsm6dsx_decimator_entry {
> +	u8 decimator;
> +	u8 val;
> +};
> +
> +static const
> +struct st_lsm6dsx_decimator_entry st_lsm6dsx_decimator_table[] = {
> +	{  0, 0x0 },
> +	{  1, 0x1 },
> +	{  2, 0x2 },
> +	{  3, 0x3 },
> +	{  4, 0x4 },
> +	{  8, 0x5 },
> +	{ 16, 0x6 },
> +	{ 32, 0x7 },
> +};
> +
> +static int st_lsm6dsx_get_decimator_val(u8 val)
> +{
> +	const int max_size = ARRAY_SIZE(st_lsm6dsx_decimator_table);
> +	int i;
> +
> +	for (i = 0; i < max_size; i++)
> +		if (st_lsm6dsx_decimator_table[i].decimator == val)
> +			break;
> +
> +	return i == max_size ? 0 : st_lsm6dsx_decimator_table[i].val;
> +}
> +
> +static void st_lsm6dsx_get_max_min_odr(struct st_lsm6dsx_hw *hw,
> +				       u16 *max_odr, u16 *min_odr)
> +{
> +	struct st_lsm6dsx_sensor *sensor;
> +	int i;
> +
> +	*max_odr = 0, *min_odr = ~0;
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		sensor = iio_priv(hw->iio_devs[i]);
> +
> +		if (!(hw->enable_mask & BIT(sensor->id)))
> +			continue;
> +
> +		*max_odr = max_t(u16, *max_odr, sensor->odr);
> +		*min_odr = min_t(u16, *min_odr, sensor->odr);
> +	}
> +}
> +
> +static int st_lsm6dsx_update_decimators(struct st_lsm6dsx_hw *hw)
> +{
> +	struct st_lsm6dsx_sensor *sensor;
> +	u16 max_odr, min_odr, sip = 0;
> +	int err, i;
> +	u8 data;
> +
> +	st_lsm6dsx_get_max_min_odr(hw, &max_odr, &min_odr);
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		sensor = iio_priv(hw->iio_devs[i]);
> +
> +		/* update fifo decimators and sample in pattern */
> +		if (hw->enable_mask & BIT(sensor->id)) {
> +			sensor->sip = sensor->odr / min_odr;
> +			sensor->decimator = max_odr / sensor->odr;
> +			data = st_lsm6dsx_get_decimator_val(sensor->decimator);
> +		} else {
> +			sensor->sip = 0;
> +			sensor->decimator = 0;
> +			data = 0;
> +		}
> +
> +		err = st_lsm6dsx_write_with_mask(hw,
> +					ST_LSM6DSX_REG_FIFO_DEC_GXL_ADDR,
> +					sensor->decimator_mask, data);
> +		if (err < 0)
> +			return err;
> +
> +		sip += sensor->sip;
> +	}
> +	hw->sip = sip;
> +
> +	return 0;
> +}
> +
> +static int st_lsm6dsx_set_fifo_mode(struct st_lsm6dsx_hw *hw,
> +				    enum st_lsm6dsx_fifo_mode fifo_mode)
> +{
> +	u8 data;
> +	int err;
> +
> +	switch (fifo_mode) {
> +	case ST_LSM6DSX_FIFO_BYPASS:
> +		data = fifo_mode;
> +		break;
> +	case ST_LSM6DSX_FIFO_CONT:
> +		data = (ST_LSM6DSX_MAX_FIFO_ODR_VAL <<
> +			__ffs(ST_LSM6DSX_FIFO_ODR_MASK)) | fifo_mode;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_FIFO_MODE_ADDR,
> +			    sizeof(data), &data);
> +	if (err < 0)
> +		return err;
> +
> +	hw->fifo_mode = fifo_mode;
> +
> +	return 0;
> +}
> +
> +int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
> +{
> +	u16 fifo_watermark = ~0, cur_watermark, sip = 0;
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	struct st_lsm6dsx_sensor *cur_sensor;
> +	__le16 wdata;
> +	int i, err;
> +	u8 data;
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		cur_sensor = iio_priv(hw->iio_devs[i]);
> +
> +		if (!(hw->enable_mask & BIT(cur_sensor->id)))
> +			continue;
> +
> +		cur_watermark = (cur_sensor == sensor) ? watermark
> +						       : cur_sensor->watermark;
> +
> +		fifo_watermark = min_t(u16, fifo_watermark, cur_watermark);
> +		sip += cur_sensor->sip;
> +	}
> +
> +	if (!sip)
> +		return 0;
> +
> +	fifo_watermark = max_t(u16, fifo_watermark, sip);
> +	fifo_watermark = (fifo_watermark / sip) * sip;
> +	fifo_watermark = fifo_watermark * ST_LSM6DSX_SAMPLE_DEPTH;
> +
> +	mutex_lock(&hw->lock);
> +
> +	err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_THH_ADDR,
> +			   sizeof(data), &data);
> +	if (err < 0)
> +		goto out;
> +
> +	fifo_watermark = ((data & ~ST_LSM6DSX_FIFO_TH_MASK) << 8) |
> +			  (fifo_watermark & ST_LSM6DSX_FIFO_TH_MASK);
> +
> +	wdata = cpu_to_le16(fifo_watermark);
> +	err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_FIFO_THL_ADDR,
> +			    sizeof(wdata), (u8 *)&wdata);
> +out:
> +	mutex_unlock(&hw->lock);
> +
> +	return err < 0 ? err : 0;
> +}
> +
> +/**
> + * st_lsm6dsx_read_fifo() - LSM6DS3-LSM6DSM read FIFO routine
> + * @hw: Pointer to instance of struct st_lsm6dsx_hw.
> + *
> + * Read samples from the hw FIFO and push them to IIO buffers.
> + *
> + * Return: Number of bytes read from the FIFO
> + */
> +static int st_lsm6dsx_read_fifo(struct st_lsm6dsx_hw *hw)
> +{
> +	u8 buff[ALIGN(ST_LSM6DSX_SAMPLE_SIZE, sizeof(s64)) + sizeof(s64)];
> +	u16 fifo_len, pattern_len = hw->sip * ST_LSM6DSX_SAMPLE_SIZE;
> +	struct st_lsm6dsx_sensor *acc_sensor, *gyro_sensor;
> +	s64 acc_ts, acc_delta_ts, gyro_ts, gyro_delta_ts;
> +	int err, acc_sip, gyro_sip, read_len, samples;
> +	__le16 fifo_status;
> +
> +	err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_FIFO_DIFFL_ADDR,
> +			   sizeof(fifo_status), (u8 *)&fifo_status);
> +	if (err < 0)
> +		return err;
> +
> +	if (fifo_status & cpu_to_le16(ST_LSM6DSX_FIFO_EMPTY_MASK))
> +		return 0;
> +
> +	fifo_len = (le16_to_cpu(fifo_status) & ST_LSM6DSX_FIFO_DIFF_MASK) *
> +		   ST_LSM6DSX_CHAN_SIZE;
> +	samples = fifo_len / ST_LSM6DSX_SAMPLE_SIZE;
> +	fifo_len = (fifo_len / pattern_len) * pattern_len;
> +
> +	/*
> +	 * compute delta timestamp between two consecutive samples
> +	 * in order to estimate queueing time of data generated
> +	 * by the sensor
> +	 */
> +	acc_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_ACC]);
> +	acc_ts = acc_sensor->ts - acc_sensor->delta_ts;
> +	acc_delta_ts = div_s64(acc_sensor->delta_ts * acc_sensor->decimator,
> +			       samples);
> +
> +	gyro_sensor = iio_priv(hw->iio_devs[ST_LSM6DSX_ID_GYRO]);
> +	gyro_ts = gyro_sensor->ts - gyro_sensor->delta_ts;
> +	gyro_delta_ts = div_s64(gyro_sensor->delta_ts * gyro_sensor->decimator,
> +				samples);
> +
> +	for (read_len = 0; read_len < fifo_len; read_len += pattern_len) {
> +		gyro_sip = gyro_sensor->sip;
> +		acc_sip = acc_sensor->sip;
> +
It might be worth adding a description fo the data pattern here as well.
Good to have it next to the code. I think I follow how this works, but
am none too sure!
> +		while (acc_sip > 0 || gyro_sip > 0) {
> +			if (gyro_sip-- > 0) {
> +				err = hw->tf->read(hw->dev,
> +						ST_LSM6DSX_REG_FIFO_OUTL_ADDR,
> +						ST_LSM6DSX_SAMPLE_SIZE, buff);
> +				if (err < 0)
> +					return err;
> +
> +				iio_push_to_buffers_with_timestamp(
> +					hw->iio_devs[ST_LSM6DSX_ID_GYRO],
> +					buff, gyro_ts);
> +				gyro_ts += gyro_delta_ts;
> +			}
> +
> +			if (acc_sip-- > 0) {
> +				err = hw->tf->read(hw->dev,
> +						ST_LSM6DSX_REG_FIFO_OUTL_ADDR,
> +						ST_LSM6DSX_SAMPLE_SIZE, buff);
> +				if (err < 0)
> +					return err;
> +
> +				iio_push_to_buffers_with_timestamp(
> +					hw->iio_devs[ST_LSM6DSX_ID_ACC],
> +					buff, acc_ts);
> +				acc_ts += acc_delta_ts;
> +			}
> +		}
> +	}
> +
> +	return read_len;
> +}
> +
> +static int st_lsm6dsx_flush_fifo(struct st_lsm6dsx_hw *hw)
> +{
> +	int err;
> +
> +	mutex_lock(&hw->fifo_lock);
> +
> +	st_lsm6dsx_read_fifo(hw);
> +	err = st_lsm6dsx_set_fifo_mode(hw, ST_LSM6DSX_FIFO_BYPASS);
> +
> +	mutex_unlock(&hw->fifo_lock);
> +
> +	return err;
> +}
> +
> +static int st_lsm6dsx_update_fifo(struct iio_dev *iio_dev, bool enable)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int err;
> +
> +	if (hw->fifo_mode != ST_LSM6DSX_FIFO_BYPASS) {
> +		err = st_lsm6dsx_flush_fifo(hw);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	if (enable) {
> +		err = st_lsm6dsx_sensor_enable(sensor);
> +		if (err < 0)
> +			return err;
> +	} else {
> +		err = st_lsm6dsx_sensor_disable(sensor);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	err = st_lsm6dsx_update_decimators(hw);
> +	if (err < 0)
> +		return err;
> +
> +	err = st_lsm6dsx_update_watermark(sensor, sensor->watermark);
> +	if (err < 0)
> +		return err;
> +
> +	if (hw->enable_mask) {
> +		err = st_lsm6dsx_set_fifo_mode(hw, ST_LSM6DSX_FIFO_CONT);
> +		if (err < 0)
> +			return err;
> +
> +		/*
> +		 * store enable buffer timestamp as reference to compute
> +		 * first delta timestamp
> +		 */
> +		sensor->ts = iio_get_time_ns(iio_dev);
> +	}
> +
> +	return 0;
> +}
> +
> +static irqreturn_t st_lsm6dsx_handler_irq(int irq, void *private)
> +{
> +	struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
> +	struct st_lsm6dsx_sensor *sensor;
> +	int i;
> +
> +	if (!hw->sip)
> +		return IRQ_NONE;
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		sensor = iio_priv(hw->iio_devs[i]);
> +
> +		if (sensor->sip > 0) {
> +			s64 timestamp;
> +
> +			timestamp = iio_get_time_ns(hw->iio_devs[i]);
> +			sensor->delta_ts = timestamp - sensor->ts;
> +			sensor->ts = timestamp;
> +		}
> +	}
> +
> +	return IRQ_WAKE_THREAD;
> +}
> +
> +static irqreturn_t st_lsm6dsx_handler_thread(int irq, void *private)
> +{
> +	struct st_lsm6dsx_hw *hw = (struct st_lsm6dsx_hw *)private;
> +	int count;
> +
> +	mutex_lock(&hw->fifo_lock);
> +	count = st_lsm6dsx_read_fifo(hw);
> +	mutex_unlock(&hw->fifo_lock);
> +
> +	return !count ? IRQ_NONE : IRQ_HANDLED;
> +}
> +
> +static int st_lsm6dsx_buffer_preenable(struct iio_dev *iio_dev)
> +{
> +	return st_lsm6dsx_update_fifo(iio_dev, true);
> +}
> +
> +static int st_lsm6dsx_buffer_postdisable(struct iio_dev *iio_dev)
> +{
> +	return st_lsm6dsx_update_fifo(iio_dev, false);
> +}
> +
> +static const struct iio_buffer_setup_ops st_lsm6dsx_buffer_ops = {
> +	.preenable = st_lsm6dsx_buffer_preenable,
> +	.postdisable = st_lsm6dsx_buffer_postdisable,
> +};
> +
> +int st_lsm6dsx_allocate_buffers(struct st_lsm6dsx_hw *hw)
Consider renaming this function as it does a lot more than allocating
the buffers (such as requesting irqs)
> +{
> +	struct iio_buffer *buffer;
> +	unsigned long irq_type;
> +	int i, err;
> +
> +	irq_type = irqd_get_trigger_type(irq_get_irq_data(hw->irq));
> +
> +	switch (irq_type) {
> +	case IRQF_TRIGGER_HIGH:
> +	case IRQF_TRIGGER_RISING:
> +		break;
> +	default:
> +		dev_info(hw->dev, "mode %lx unsupported\n", irq_type);
> +		return -EINVAL;
> +	}
> +
> +	err = devm_request_threaded_irq(hw->dev, hw->irq,
> +					st_lsm6dsx_handler_irq,
> +					st_lsm6dsx_handler_thread,
> +					irq_type | IRQF_ONESHOT,
> +					hw->name, hw);
> +	if (err) {
> +		dev_err(hw->dev, "failed to request trigger irq %d\n",
> +			hw->irq);
> +		return err;
> +	}
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		buffer = devm_iio_kfifo_allocate(hw->dev);
> +		if (!buffer)
> +			return -ENOMEM;
> +
> +		iio_device_attach_buffer(hw->iio_devs[i], buffer);
> +		hw->iio_devs[i]->modes |= INDIO_BUFFER_SOFTWARE;
> +		hw->iio_devs[i]->setup_ops = &st_lsm6dsx_buffer_ops;
> +	}
> +
> +	return 0;
> +}
> +
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> new file mode 100644
> index 0000000..473315a
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
> @@ -0,0 +1,655 @@
> +/*
> + * STMicroelectronics st_lsm6dsx sensor driver
> + *
> + * The ST LSM6DSx IMU MEMS series consists of 3D digital accelerometer
> + * and 3D digital gyroscope system-in-package with a digital I2C/SPI serial
> + * interface standard output.
> + * LSM6DSx IMU MEMS series has a dynamic user-selectable full-scale
> + * acceleration range of +-2/+-4/+-8/+-16 g and an angular rate range of
> + * +-125/+-245/+-500/+-1000/+-2000 dps
> + * LSM6DSx series has an integrated First-In-First-Out (FIFO) buffer
> + * allowing dynamic batching of sensor data.
> + *
> + * Supported sensors:
> + * - LSM6DS3:
> + *   - Accelerometer/Gyroscope supported ODR [Hz]: 13, 26, 52, 104, 208, 416
> + *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
> + *   - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
> + *   - FIFO size: 8KB
> + *
> + * - LSM6DSM:
> + *   - Accelerometer/Gyroscope supported ODR [Hz]: 13, 26, 52, 104, 208, 416
> + *   - Accelerometer supported full-scale [g]: +-2/+-4/+-8/+-16
> + *   - Gyroscope supported full-scale [dps]: +-125/+-245/+-500/+-1000/+-2000
> + *   - FIFO size: 4KB
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/delay.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/sysfs.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define ST_LSM6DSX_REG_ACC_DEC_MASK		GENMASK(2, 0)
> +#define ST_LSM6DSX_REG_GYRO_DEC_MASK		GENMASK(5, 3)
> +#define ST_LSM6DSX_REG_INT1_ADDR		0x0d
> +#define ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK	BIT(3)
> +#define ST_LSM6DSX_REG_WHOAMI_ADDR		0x0f
> +#define ST_LSM6DSX_REG_RESET_ADDR		0x12
> +#define ST_LSM6DSX_REG_RESET_MASK		BIT(0)
> +#define ST_LSM6DSX_REG_BDU_ADDR			0x12
> +#define ST_LSM6DSX_REG_BDU_MASK			BIT(6)
> +#define ST_LSM6DSX_REG_INT2_ON_INT1_ADDR	0x13
> +#define ST_LSM6DSX_REG_INT2_ON_INT1_MASK	BIT(5)
> +#define ST_LSM6DSX_REG_ROUNDING_ADDR		0x16
> +#define ST_LSM6DSX_REG_ROUNDING_MASK		BIT(2)
> +#define ST_LSM6DSX_REG_LIR_ADDR			0x58
> +#define ST_LSM6DSX_REG_LIR_MASK			BIT(0)
> +
> +#define ST_LSM6DSX_REG_ACC_ODR_ADDR		0x10
> +#define ST_LSM6DSX_REG_ACC_ODR_MASK		GENMASK(7, 4)
> +#define ST_LSM6DSX_REG_ACC_FS_ADDR		0x10
> +#define ST_LSM6DSX_REG_ACC_FS_MASK		GENMASK(3, 2)
> +#define ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR		0x28
> +#define ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR		0x2a
> +#define ST_LSM6DSX_REG_ACC_OUT_Z_L_ADDR		0x2c
> +
> +#define ST_LSM6DSX_REG_GYRO_ODR_ADDR		0x11
> +#define ST_LSM6DSX_REG_GYRO_ODR_MASK		GENMASK(7, 4)
> +#define ST_LSM6DSX_REG_GYRO_FS_ADDR		0x11
> +#define ST_LSM6DSX_REG_GYRO_FS_MASK		GENMASK(3, 2)
> +#define ST_LSM6DSX_REG_GYRO_OUT_X_L_ADDR	0x22
> +#define ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR	0x24
> +#define ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR	0x26
> +
> +#define ST_LSM6DS3_WHOAMI			0x69
> +#define ST_LSM6DSM_WHOAMI			0x6a
> +
> +#define ST_LSM6DS3_MAX_FIFO_SIZE		8192
> +#define ST_LSM6DSM_MAX_FIFO_SIZE		4096
> +
> +#define ST_LSM6DSX_ACC_FS_2G_GAIN		IIO_G_TO_M_S_2(61)
> +#define ST_LSM6DSX_ACC_FS_4G_GAIN		IIO_G_TO_M_S_2(122)
> +#define ST_LSM6DSX_ACC_FS_8G_GAIN		IIO_G_TO_M_S_2(244)
> +#define ST_LSM6DSX_ACC_FS_16G_GAIN		IIO_G_TO_M_S_2(488)
> +
> +#define ST_LSM6DSX_GYRO_FS_245_GAIN		IIO_DEGREE_TO_RAD(4375)
> +#define ST_LSM6DSX_GYRO_FS_500_GAIN		IIO_DEGREE_TO_RAD(8750)
> +#define ST_LSM6DSX_GYRO_FS_1000_GAIN		IIO_DEGREE_TO_RAD(17500)
> +#define ST_LSM6DSX_GYRO_FS_2000_GAIN		IIO_DEGREE_TO_RAD(70000)
> +
> +struct st_lsm6dsx_odr {
> +	u16 hz;
> +	u8 val;
> +};
> +
> +#define ST_LSM6DSX_ODR_LIST_SIZE	6
> +struct st_lsm6dsx_odr_table_entry {
> +	struct st_lsm6dsx_reg reg;
> +	struct st_lsm6dsx_odr odr_avl[ST_LSM6DSX_ODR_LIST_SIZE];
> +};
> +
> +static const struct st_lsm6dsx_odr_table_entry st_lsm6dsx_odr_table[] = {
> +	[ST_LSM6DSX_ID_ACC] = {
> +		.reg = {
> +			.addr = ST_LSM6DSX_REG_ACC_ODR_ADDR,
> +			.mask = ST_LSM6DSX_REG_ACC_ODR_MASK,
> +		},
> +		.odr_avl[0] = {  13, 0x01 },
> +		.odr_avl[1] = {  26, 0x02 },
> +		.odr_avl[2] = {  52, 0x03 },
> +		.odr_avl[3] = { 104, 0x04 },
> +		.odr_avl[4] = { 208, 0x05 },
> +		.odr_avl[5] = { 416, 0x06 },
> +	},
> +	[ST_LSM6DSX_ID_GYRO] = {
> +		.reg = {
> +			.addr = ST_LSM6DSX_REG_GYRO_ODR_ADDR,
> +			.mask = ST_LSM6DSX_REG_GYRO_ODR_MASK,
> +		},
> +		.odr_avl[0] = {  13, 0x01 },
> +		.odr_avl[1] = {  26, 0x02 },
> +		.odr_avl[2] = {  52, 0x03 },
> +		.odr_avl[3] = { 104, 0x04 },
> +		.odr_avl[4] = { 208, 0x05 },
> +		.odr_avl[5] = { 416, 0x06 },
> +	}
> +};
> +
> +struct st_lsm6dsx_fs {
> +	u32 gain;
> +	u8 val;
> +};
> +
> +#define ST_LSM6DSX_FS_LIST_SIZE		4
> +struct st_lsm6dsx_fs_table_entry {
> +	struct st_lsm6dsx_reg reg;
> +	struct st_lsm6dsx_fs fs_avl[ST_LSM6DSX_FS_LIST_SIZE];
> +};
> +
> +static const struct st_lsm6dsx_fs_table_entry st_lsm6dsx_fs_table[] = {
> +	[ST_LSM6DSX_ID_ACC] = {
> +		.reg = {
> +			.addr = ST_LSM6DSX_REG_ACC_FS_ADDR,
> +			.mask = ST_LSM6DSX_REG_ACC_FS_MASK,
> +		},
> +		.fs_avl[0] = {  ST_LSM6DSX_ACC_FS_2G_GAIN, 0x0 },
> +		.fs_avl[1] = {  ST_LSM6DSX_ACC_FS_4G_GAIN, 0x2 },
> +		.fs_avl[2] = {  ST_LSM6DSX_ACC_FS_8G_GAIN, 0x3 },
> +		.fs_avl[3] = { ST_LSM6DSX_ACC_FS_16G_GAIN, 0x1 },
> +	},
> +	[ST_LSM6DSX_ID_GYRO] = {
> +		.reg = {
> +			.addr = ST_LSM6DSX_REG_GYRO_FS_ADDR,
> +			.mask = ST_LSM6DSX_REG_GYRO_FS_MASK,
> +		},
> +		.fs_avl[0] = {  ST_LSM6DSX_GYRO_FS_245_GAIN, 0x0 },
> +		.fs_avl[1] = {  ST_LSM6DSX_GYRO_FS_500_GAIN, 0x1 },
> +		.fs_avl[2] = { ST_LSM6DSX_GYRO_FS_1000_GAIN, 0x2 },
> +		.fs_avl[3] = { ST_LSM6DSX_GYRO_FS_2000_GAIN, 0x3 },
> +	}
> +};
> +
> +static const struct st_lsm6dsx_settings st_lsm6dsx_sensor_settings[] = {
> +	{
> +		.wai = ST_LSM6DS3_WHOAMI,
> +		.max_fifo_size = ST_LSM6DS3_MAX_FIFO_SIZE,
> +	},
> +	{
> +		.wai = ST_LSM6DSM_WHOAMI,
> +		.max_fifo_size = ST_LSM6DSM_MAX_FIFO_SIZE,
> +	},
> +};
> +
> +#define ST_LSM6DSX_CHANNEL(chan_type, addr, mod, scan_idx)		\
> +{									\
> +	.type = chan_type,						\
> +	.address = addr,						\
> +	.modified = 1,							\
> +	.channel2 = mod,						\
> +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |			\
> +			      BIT(IIO_CHAN_INFO_SCALE),			\
> +	.info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),	\
> +	.scan_index = scan_idx,						\
> +	.scan_type = {							\
> +		.sign = 's',						\
> +		.realbits = 16,						\
> +		.storagebits = 16,					\
> +		.endianness = IIO_LE,					\
> +	},								\
> +}
> +
> +static const struct iio_chan_spec st_lsm6dsx_acc_channels[] = {
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, ST_LSM6DSX_REG_ACC_OUT_X_L_ADDR,
> +			   IIO_MOD_X, 0),
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, ST_LSM6DSX_REG_ACC_OUT_Y_L_ADDR,
> +			   IIO_MOD_Y, 1),
> +	ST_LSM6DSX_CHANNEL(IIO_ACCEL, ST_LSM6DSX_REG_ACC_OUT_Z_L_ADDR,
> +			   IIO_MOD_Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +static const struct iio_chan_spec st_lsm6dsx_gyro_channels[] = {
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM6DSX_REG_GYRO_OUT_X_L_ADDR,
> +			   IIO_MOD_X, 0),
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM6DSX_REG_GYRO_OUT_Y_L_ADDR,
> +			   IIO_MOD_Y, 1),
> +	ST_LSM6DSX_CHANNEL(IIO_ANGL_VEL, ST_LSM6DSX_REG_GYRO_OUT_Z_L_ADDR,
> +			   IIO_MOD_Z, 2),
> +	IIO_CHAN_SOFT_TIMESTAMP(3),
> +};
> +
> +int st_lsm6dsx_write_with_mask(struct st_lsm6dsx_hw *hw, u8 addr, u8 mask,
> +			       u8 val)
> +{
> +	u8 data;
> +	int err;
> +
> +	mutex_lock(&hw->lock);
> +
> +	err = hw->tf->read(hw->dev, addr, sizeof(data), &data);
> +	if (err < 0) {
> +		dev_err(hw->dev, "failed to read %02x register\n", addr);
> +		goto out;
> +	}
> +
> +	data = (data & ~mask) | ((val << __ffs(mask)) & mask);
> +
> +	err = hw->tf->write(hw->dev, addr, sizeof(data), &data);
> +	if (err < 0)
> +		dev_err(hw->dev, "failed to write %02x register\n", addr);
> +
> +out:
> +	mutex_unlock(&hw->lock);
> +
> +	return err;
> +}
> +
> +static int st_lsm6dsx_check_whoami(struct st_lsm6dsx_hw *hw)
> +{
> +	int err, i;
> +	u8 data;
> +
> +	err = hw->tf->read(hw->dev, ST_LSM6DSX_REG_WHOAMI_ADDR, sizeof(data),
> +			   &data);
> +	if (err < 0) {
> +		dev_err(hw->dev, "failed to read whoami register\n");
> +		return err;
> +	}
> +
> +	for (i = 0; i < ARRAY_SIZE(st_lsm6dsx_sensor_settings); i++) {
> +		if (data == st_lsm6dsx_sensor_settings[i].wai) {
> +			hw->settings = &st_lsm6dsx_sensor_settings[i];
> +			break;
Normally we would sanity check that the expected whoami was the one read.
Here you are matching against all supported parts.  As we know what we
are expecting it would be nice to at least kick out a warning if we
infact find another supported part.  This would require you to pass
through the enum element from the enum suggested in the spi driver
review below.
> +		}
> +	}
> +
> +	if (i == ARRAY_SIZE(st_lsm6dsx_sensor_settings)) {
> +		dev_err(hw->dev, "unsupported whoami [%02x]\n", data);
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int st_lsm6dsx_set_full_scale(struct st_lsm6dsx_sensor *sensor,
> +				     u32 gain)
> +{
> +	enum st_lsm6dsx_sensor_id id = sensor->id;
> +	int i, err;
> +	u8 val;
> +
> +	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
> +		if (st_lsm6dsx_fs_table[id].fs_avl[i].gain == gain)
> +			break;
> +
> +	if (i == ST_LSM6DSX_FS_LIST_SIZE)
> +		return -EINVAL;
> +
> +	val = st_lsm6dsx_fs_table[id].fs_avl[i].val;
> +	err = st_lsm6dsx_write_with_mask(sensor->hw,
> +					 st_lsm6dsx_fs_table[id].reg.addr,
> +					 st_lsm6dsx_fs_table[id].reg.mask,
> +					 val);
> +	if (err < 0)
> +		return err;
> +
> +	sensor->gain = gain;
> +
> +	return 0;
> +}
> +
> +static int st_lsm6dsx_set_odr(struct st_lsm6dsx_sensor *sensor, u16 odr)
> +{
> +	enum st_lsm6dsx_sensor_id id = sensor->id;
> +	int i, err;
> +	u8 val;
> +
> +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
> +		if (st_lsm6dsx_odr_table[id].odr_avl[i].hz == odr)
> +			break;
> +
> +	if (i == ST_LSM6DSX_ODR_LIST_SIZE)
> +		return -EINVAL;
> +
> +	val = st_lsm6dsx_odr_table[id].odr_avl[i].val;
> +	err = st_lsm6dsx_write_with_mask(sensor->hw,
> +					 st_lsm6dsx_odr_table[id].reg.addr,
> +					 st_lsm6dsx_odr_table[id].reg.mask,
> +					 val);
> +	if (err < 0)
> +		return err;
> +
> +	sensor->odr = odr;
> +
> +	return 0;
> +}
> +
> +int st_lsm6dsx_sensor_enable(struct st_lsm6dsx_sensor *sensor)
> +{
> +	int err;
> +
> +	err = st_lsm6dsx_set_odr(sensor, sensor->odr);
> +	if (err < 0)
> +		return err;
> +
> +	sensor->hw->enable_mask |= BIT(sensor->id);
> +
> +	return 0;
> +}
> +
> +int st_lsm6dsx_sensor_disable(struct st_lsm6dsx_sensor *sensor)
> +{
> +	enum st_lsm6dsx_sensor_id id = sensor->id;
> +	int err;
> +
> +	err = st_lsm6dsx_write_with_mask(sensor->hw,
> +					 st_lsm6dsx_odr_table[id].reg.addr,
> +					 st_lsm6dsx_odr_table[id].reg.mask, 0);
> +	if (err < 0)
> +		return err;
> +
> +	sensor->hw->enable_mask &= ~BIT(id);
> +
> +	return 0;
> +}
> +
> +static int st_lsm6dsx_read_oneshot(struct st_lsm6dsx_sensor *sensor,
> +				   u8 addr, int *val)
> +{
> +	int err, delay;
> +	__le16 data;
> +
> +	err = st_lsm6dsx_sensor_enable(sensor);
> +	if (err < 0)
> +		return err;
> +
> +	delay = 1000000 / sensor->odr;
> +	usleep_range(delay, 2 * delay);
> +
> +	err = sensor->hw->tf->read(sensor->hw->dev, addr, sizeof(data),
> +				   (u8 *)&data);
> +	if (err < 0)
> +		return err;
> +
> +	st_lsm6dsx_sensor_disable(sensor);
> +
> +	*val = (s16)data;
> +
> +	return IIO_VAL_INT;
> +}
> +
> +static int st_lsm6dsx_read_raw(struct iio_dev *iio_dev,
> +			       struct iio_chan_spec const *ch,
> +			       int *val, int *val2, long mask)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	int ret;
> +
> +	ret = iio_device_claim_direct_mode(iio_dev);
> +	if (ret)
> +		return ret;
This feels like overkill.  You are protecting elements that shouldn't
need such heavy handed protection.  This prevent syou reading the
sampling frequency or scale whilst the buffer is enabled.

I'd move the claim and release to being just within the _RAW element
of the switch.
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		ret = st_lsm6dsx_read_oneshot(sensor, ch->address, val);
> +		break;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		*val = sensor->odr;
> +		ret = IIO_VAL_INT;
> +		break;
> +	case IIO_CHAN_INFO_SCALE:
> +		*val = 0;
> +		*val2 = sensor->gain;
> +		ret = IIO_VAL_INT_PLUS_MICRO;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	iio_device_release_direct_mode(iio_dev);
> +
> +	return ret;
> +}
> +
> +static int st_lsm6dsx_write_raw(struct iio_dev *iio_dev,
> +				struct iio_chan_spec const *chan,
> +				int val, int val2, long mask)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	int err;
> +
> +	err = iio_device_claim_direct_mode(iio_dev);
> +	if (err)
> +		return err;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_SCALE:
> +		err = st_lsm6dsx_set_full_scale(sensor, val2);
> +		break;
> +	case IIO_CHAN_INFO_SAMP_FREQ:
> +		err = st_lsm6dsx_set_odr(sensor, val);
> +		break;
> +	default:
> +		err = -EINVAL;
> +		break;
> +	}
> +
> +	iio_device_release_direct_mode(iio_dev);
> +
> +	return err;
> +}
> +
> +static int st_lsm6dsx_set_watermark(struct iio_dev *iio_dev, unsigned int val)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(iio_dev);
> +	struct st_lsm6dsx_hw *hw = sensor->hw;
> +	int err, max_fifo_len;
> +
> +	max_fifo_len = hw->settings->max_fifo_size / ST_LSM6DSX_SAMPLE_SIZE;
> +	if (val < 1 || val > max_fifo_len)
> +		return -EINVAL;
> +
> +	err = st_lsm6dsx_update_watermark(sensor, val);
> +	if (err < 0)
> +		return err;
> +
> +	sensor->watermark = val;
> +
> +	return 0;
> +}
> +
> +static ssize_t
> +st_lsm6dsx_sysfs_sampling_frequency_avail(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> +	enum st_lsm6dsx_sensor_id id = sensor->id;
> +	int i, len = 0;
> +
> +	for (i = 0; i < ST_LSM6DSX_ODR_LIST_SIZE; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "%d ",
> +				 st_lsm6dsx_odr_table[id].odr_avl[i].hz);
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static ssize_t st_lsm6dsx_sysfs_scale_avail(struct device *dev,
> +					    struct device_attribute *attr,
> +					    char *buf)
> +{
> +	struct st_lsm6dsx_sensor *sensor = iio_priv(dev_get_drvdata(dev));
> +	enum st_lsm6dsx_sensor_id id = sensor->id;
> +	int i, len = 0;
> +
> +	for (i = 0; i < ST_LSM6DSX_FS_LIST_SIZE; i++)
> +		len += scnprintf(buf + len, PAGE_SIZE - len, "0.%06u ",
> +				 st_lsm6dsx_fs_table[id].fs_avl[i].gain);
> +	buf[len - 1] = '\n';
> +
> +	return len;
> +}
> +
> +static IIO_DEV_ATTR_SAMP_FREQ_AVAIL(st_lsm6dsx_sysfs_sampling_frequency_avail);
> +static IIO_DEVICE_ATTR(in_accel_scale_available, 0444,
> +		       st_lsm6dsx_sysfs_scale_avail, NULL, 0);
> +static IIO_DEVICE_ATTR(in_anglvel_scale_available, 0444,
> +		       st_lsm6dsx_sysfs_scale_avail, NULL, 0);
> +
> +static struct attribute *st_lsm6dsx_acc_attributes[] = {
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_in_accel_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group st_lsm6dsx_acc_attribute_group = {
> +	.attrs = st_lsm6dsx_acc_attributes,
> +};
> +
> +static const struct iio_info st_lsm6dsx_acc_info = {
> +	.driver_module = THIS_MODULE,
> +	.attrs = &st_lsm6dsx_acc_attribute_group,
> +	.read_raw = st_lsm6dsx_read_raw,
> +	.write_raw = st_lsm6dsx_write_raw,
> +	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> +};
> +
> +static struct attribute *st_lsm6dsx_gyro_attributes[] = {
> +	&iio_dev_attr_sampling_frequency_available.dev_attr.attr,
> +	&iio_dev_attr_in_anglvel_scale_available.dev_attr.attr,
> +	NULL,
> +};
> +
> +static const struct attribute_group st_lsm6dsx_gyro_attribute_group = {
> +	.attrs = st_lsm6dsx_gyro_attributes,
> +};
> +
> +static const struct iio_info st_lsm6dsx_gyro_info = {
> +	.driver_module = THIS_MODULE,
> +	.attrs = &st_lsm6dsx_gyro_attribute_group,
> +	.read_raw = st_lsm6dsx_read_raw,
> +	.write_raw = st_lsm6dsx_write_raw,
> +	.hwfifo_set_watermark = st_lsm6dsx_set_watermark,
> +};
> +
> +static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
> +
> +static int st_lsm6dsx_init_device(struct st_lsm6dsx_hw *hw)
> +{
> +	int err;
> +	u8 data;
> +
> +	data = ST_LSM6DSX_REG_RESET_MASK;
> +	err = hw->tf->write(hw->dev, ST_LSM6DSX_REG_RESET_ADDR, sizeof(data),
> +			    &data);
> +	if (err < 0)
> +		return err;
> +
> +	msleep(200);
> +
> +	/* latch interrupts */
> +	err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_LIR_ADDR,
> +					 ST_LSM6DSX_REG_LIR_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	/* enable Block Data Update */
> +	err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_BDU_ADDR,
> +					 ST_LSM6DSX_REG_BDU_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_ROUNDING_ADDR,
> +					 ST_LSM6DSX_REG_ROUNDING_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	/* enable FIFO watermak interrupt */
> +	err = st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_INT1_ADDR,
> +					 ST_LSM6DSX_REG_FIFO_FTH_IRQ_MASK, 1);
> +	if (err < 0)
> +		return err;
> +
> +	/* redirect INT2 on INT1 */
> +	return st_lsm6dsx_write_with_mask(hw, ST_LSM6DSX_REG_INT2_ON_INT1_ADDR,
> +					  ST_LSM6DSX_REG_INT2_ON_INT1_MASK, 1);
> +}
> +
> +static struct iio_dev *st_lsm6dsx_alloc_iiodev(struct st_lsm6dsx_hw *hw,
> +					       enum st_lsm6dsx_sensor_id id)
> +{
> +	struct st_lsm6dsx_sensor *sensor;
> +	struct iio_dev *iio_dev;
> +
> +	iio_dev = devm_iio_device_alloc(hw->dev, sizeof(*sensor));
> +	if (!iio_dev)
> +		return NULL;
> +
> +	iio_dev->modes = INDIO_DIRECT_MODE;
> +	iio_dev->dev.parent = hw->dev;
> +	iio_dev->available_scan_masks = st_lsm6dsx_available_scan_masks;
> +
> +	sensor = iio_priv(iio_dev);
> +	sensor->id = id;
> +	sensor->hw = hw;
> +	sensor->odr = st_lsm6dsx_odr_table[id].odr_avl[0].hz;
> +	sensor->gain = st_lsm6dsx_fs_table[id].fs_avl[0].gain;
> +	sensor->watermark = 1;
> +
> +	switch (id) {
> +	case ST_LSM6DSX_ID_ACC:
> +		iio_dev->channels = st_lsm6dsx_acc_channels;
> +		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_acc_channels);
> +		iio_dev->name = "lsm6dsx_accel";
> +		iio_dev->info = &st_lsm6dsx_acc_info;
> +
> +		sensor->decimator_mask = ST_LSM6DSX_REG_ACC_DEC_MASK;
> +		break;
> +	case ST_LSM6DSX_ID_GYRO:
> +		iio_dev->channels = st_lsm6dsx_gyro_channels;
> +		iio_dev->num_channels = ARRAY_SIZE(st_lsm6dsx_gyro_channels);
> +		iio_dev->name = "lsm6dsx_gyro";
> +		iio_dev->info = &st_lsm6dsx_gyro_info;
> +
> +		sensor->decimator_mask = ST_LSM6DSX_REG_GYRO_DEC_MASK;
> +		break;
> +	default:
> +		return NULL;
> +	}
> +
> +	return iio_dev;
> +}
> +
> +int st_lsm6dsx_probe(struct st_lsm6dsx_hw *hw)
> +{
> +	int i, err;
> +
> +	mutex_init(&hw->lock);
> +	mutex_init(&hw->fifo_lock);
> +
> +	err = st_lsm6dsx_check_whoami(hw);
> +	if (err < 0)
> +		return err;
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		hw->iio_devs[i] = st_lsm6dsx_alloc_iiodev(hw, i);
> +		if (!hw->iio_devs[i])
> +			return -ENOMEM;
> +	}
> +
> +	err = st_lsm6dsx_init_device(hw);
> +	if (err < 0)
> +		return err;
> +
> +	if (hw->irq > 0) {
> +		err = st_lsm6dsx_allocate_buffers(hw);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	for (i = 0; i < ST_LSM6DSX_ID_MAX; i++) {
> +		err = devm_iio_device_register(hw->dev, hw->iio_devs[i]);
> +		if (err)
> +			return err;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(st_lsm6dsx_probe);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> new file mode 100644
> index 0000000..cb9a158
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_i2c.c
> @@ -0,0 +1,88 @@
> +/*
> + * STMicroelectronics st_lsm6dsx i2c driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/i2c.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +static int st_lsm6dsx_i2c_read(struct device *dev, u8 addr, int len, u8 *data)
> +{
> +	return i2c_smbus_read_i2c_block_data_or_emulated(to_i2c_client(dev),
> +							 addr, len, data);
> +}
> +
> +static int st_lsm6dsx_i2c_write(struct device *dev, u8 addr, int len, u8 *data)
> +{
> +	return i2c_smbus_write_i2c_block_data(to_i2c_client(dev), addr,
> +					      len, data);
> +}
> +
> +static const struct st_lsm6dsx_transfer_function st_lsm6dsx_transfer_fn = {
> +	.read = st_lsm6dsx_i2c_read,
> +	.write = st_lsm6dsx_i2c_write,
> +};
> +
> +static int st_lsm6dsx_i2c_probe(struct i2c_client *client,
> +				const struct i2c_device_id *id)
> +{
> +	struct st_lsm6dsx_hw *hw;
> +
> +	hw = devm_kzalloc(&client->dev, sizeof(*hw), GFP_KERNEL);
> +	if (!hw)
> +		return -ENOMEM;
> +
> +	i2c_set_clientdata(client, hw);
> +	hw->name = client->name;
> +	hw->dev = &client->dev;
> +	hw->irq = client->irq;
> +	hw->tf = &st_lsm6dsx_transfer_fn;
> +
> +	return st_lsm6dsx_probe(hw);
> +}
> +
> +static const struct of_device_id st_lsm6dsx_i2c_of_match[] = {
> +	{
> +		.compatible = "st,lsm6ds3",
> +		.data = ST_LSM6DS3_DEV_NAME,
> +	},
> +	{
> +		.compatible = "st,lsm6dsm",
> +		.data = ST_LSM6DSM_DEV_NAME,
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_lsm6dsx_i2c_of_match);
> +
> +static const struct i2c_device_id st_lsm6dsx_i2c_id_table[] = {
> +	{ ST_LSM6DS3_DEV_NAME },
> +	{ ST_LSM6DSM_DEV_NAME },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(i2c, st_lsm6dsx_i2c_id_table);
> +
> +static struct i2c_driver st_lsm6dsx_driver = {
> +	.driver = {
> +		.name = "st_lsm6dsx_i2c",
> +		.of_match_table = of_match_ptr(st_lsm6dsx_i2c_of_match),
> +	},
> +	.probe = st_lsm6dsx_i2c_probe,
> +	.id_table = st_lsm6dsx_i2c_id_table,
> +};
> +module_i2c_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx i2c driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> new file mode 100644
> index 0000000..f16eab5
> --- /dev/null
> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_spi.c
> @@ -0,0 +1,126 @@
> +/*
> + * STMicroelectronics st_lsm6dsx spi driver
> + *
> + * Copyright 2016 STMicroelectronics Inc.
> + *
> + * Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>
> + * Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>
> + *
> + * Licensed under the GPL-2.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/spi/spi.h>
> +#include <linux/slab.h>
> +#include <linux/of.h>
> +
> +#include "st_lsm6dsx.h"
> +
> +#define SENSORS_SPI_READ	BIT(7)
> +
> +static int st_lsm6dsx_spi_read(struct device *dev, u8 addr, int len,
> +			       u8 *data)
> +{
> +	struct spi_device *spi = to_spi_device(dev);
> +	struct st_lsm6dsx_hw *hw = spi_get_drvdata(spi);
> +	int err;
> +
> +	struct spi_transfer xfers[] = {
> +		{
> +			.tx_buf = hw->tb.tx_buf,
> +			.bits_per_word = 8,
> +			.len = 1,
> +		},
> +		{
> +			.rx_buf = hw->tb.rx_buf,
> +			.bits_per_word = 8,
> +			.len = len,
> +		}
> +	};
> +
> +	hw->tb.tx_buf[0] = addr | SENSORS_SPI_READ;
> +
> +	err = spi_sync_transfer(spi, xfers,  ARRAY_SIZE(xfers));
> +	if (err < 0)
> +		return err;
> +
> +	memcpy(data, hw->tb.rx_buf, len * sizeof(u8));
> +
> +	return len;
> +}
> +
> +static int st_lsm6dsx_spi_write(struct device *dev, u8 addr, int len,
> +				u8 *data)
> +{
> +	struct st_lsm6dsx_hw *hw;
> +	struct spi_device *spi;
> +
> +	if (len >= ST_LSM6DSX_TX_MAX_LENGTH)
> +		return -ENOMEM;
> +
> +	spi = to_spi_device(dev);
> +	hw = spi_get_drvdata(spi);
> +
> +	hw->tb.tx_buf[0] = addr;
> +	memcpy(&hw->tb.tx_buf[1], data, len);
> +
> +	return spi_write(spi, hw->tb.tx_buf, len + 1);
> +}
> +
> +static const struct st_lsm6dsx_transfer_function st_lsm6dsx_transfer_fn = {
> +	.read = st_lsm6dsx_spi_read,
> +	.write = st_lsm6dsx_spi_write,
> +};
> +
> +static int st_lsm6dsx_spi_probe(struct spi_device *spi)
> +{
> +	struct st_lsm6dsx_hw *hw;
> +
> +	hw = devm_kzalloc(&spi->dev, sizeof(*hw), GFP_KERNEL);
> +	if (!hw)
> +		return -ENOMEM;
> +
> +	spi_set_drvdata(spi, hw);
You could have moved this into the st_lsm6dsx_probe function by
using dev_set_drvdata(spi->dev).  Having done that...
> +	hw->name = spi->modalias;
> +	hw->dev = &spi->dev;
> +	hw->irq = spi->irq;
> +	hw->tf = &st_lsm6dsx_transfer_fn;
I'm not convinced having the hw structure visible here adds anything useful.
I'd have made the st_lsm6dsx_probe function just take the various elements
as separate parameters.  There would only be 4 of them and that would
bring all the setting of elements of hw into one place rather than scattered
as they are now.

(minor point though!)
> +
> +	return st_lsm6dsx_probe(hw);
> +}
> +
> +static const struct of_device_id st_lsm6dsx_spi_of_match[] = {
> +	{
> +		.compatible = "st,lsm6ds3",
> +		.data = ST_LSM6DS3_DEV_NAME,
> +	},
> +	{
> +		.compatible = "st,lsm6dsm",
> +		.data = ST_LSM6DSM_DEV_NAME,
I would have preferred the use of an enum to using strings in here.
You would have needed to provide the enum entries as data for the
struct spi_device_id entries as well.

This is just a bit 'unusual' and I like everything done in the predictable
way ;)
> +	},
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, st_lsm6dsx_spi_of_match);
> +
> +static const struct spi_device_id st_lsm6dsx_spi_id_table[] = {
> +	{ ST_LSM6DS3_DEV_NAME },
> +	{ ST_LSM6DSM_DEV_NAME },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(spi, st_lsm6dsx_spi_id_table);
> +
> +static struct spi_driver st_lsm6dsx_driver = {
> +	.driver = {
> +		.name = "st_lsm6dsx_spi",
> +		.of_match_table = of_match_ptr(st_lsm6dsx_spi_of_match),
> +	},
> +	.probe = st_lsm6dsx_spi_probe,
> +	.id_table = st_lsm6dsx_spi_id_table,
> +};
> +module_spi_driver(st_lsm6dsx_driver);
> +
> +MODULE_AUTHOR("Lorenzo Bianconi <lorenzo.bianconi-qxv4g6HH51o@public.gmane.org>");
> +MODULE_AUTHOR("Denis Ciocca <denis.ciocca-qxv4g6HH51o@public.gmane.org>");
> +MODULE_DESCRIPTION("STMicroelectronics st_lsm6dsx spi driver");
> +MODULE_LICENSE("GPL v2");
> 

--
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: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function
From: Tony Lindgren @ 2016-12-30 15:59 UTC (permalink / raw)
  To: Gary Bisson
  Cc: Linus Walleij, Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linux-OMAP
In-Reply-To: <CAAMH-ysDwyLst+EYtNNmO0qgAq4e=7ESxGYynDM1GOyDqXx3OQ@mail.gmail.com>

* Gary Bisson <gary.bisson@boundarydevices.com> [161230 07:43]:
> Hi Linus,
> 
> On Fri, Dec 30, 2016 at 3:39 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> > On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <tony@atomide.com> wrote:
> >
> >> Here are some changes to add generic helpers for managing pinctrl groups and
> >> functions.
> >
> > I applied it, screwed around with it and pushed to the build servers to see
> > if it survived.
> >
> > I really like the look of this and I hope lots of driver start to use it.
> >
> > Gary, I just applied your radix patches for i.MX, can you look if you can
> > use the GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUCNTIONS
> > that Tony invented and that I just merged to my devel branch in the
> > pinctrl tree?
> 
> Yes I will have a look. It does sound like a good idea. I'll share my
> findings beginning of next week.

OK great. Note that we may be able to come up also with a generic
iterator function for the node_to_map functions so maybe see if
you come up with some ideas for that while experimenting :)

Tony

^ permalink raw reply

* Re: [PATCH 2/5] pinctrl: core: Add generic pinctrl functions for managing groups
From: Tony Lindgren @ 2016-12-30 15:57 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linux-OMAP
In-Reply-To: <CACRpkdbNNfbq++NGhiSgKmaAN=VB_ZmV6YD2jSo2HgtpqaESBQ@mail.gmail.com>

* Linus Walleij <linus.walleij@linaro.org> [161230 06:12]:
> On Tue, Dec 27, 2016 at 6:20 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > We can add generic helpers for pin group handling for cases where the pin
> > controller driver does not need to use static arrays.
> >
> > Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> Patch applied.
> 
> > +config GENERIC_PINCTRL
> > +       bool
> 
> Then I renamed *this* to GENERIC_PINCTRL_GROUPS.
> (Not the other patch, I got confused because gmail threads the
> second and third patch together, sorry.)

Yeah OK nice, the renames make sense to me.

> Let's see if I manage to rebase the next patch on this.

I'll do some testing with your devel branch today to make
sure things still work for me.

Regards,

Tony

^ permalink raw reply

* Re: [PATCH 0/4] PCI: exynos: use the PHY generic framework
From: Krzysztof Kozlowski @ 2016-12-30 15:56 UTC (permalink / raw)
  To: Jaehoon Chung
  Cc: linux-pci, devicetree, linux-kernel, linux-samsung-soc, bhelgaas,
	robh+dt, mark.rutland, kgene, krzk, kishon, jingoohan1,
	vivek.gautam, pankaj.dubey, alim.akhtar, cpgs
In-Reply-To: <20161228103454.26467-1-jh80.chung@samsung.com>

On Wed, Dec 28, 2016 at 07:34:50PM +0900, Jaehoon Chung wrote:
> This patch is for using PHY generic framework.
> When Exynos5440 had upstreamed, there was no PHY subsytem.
> Now the using PHY framework is mandantory.
> In future, Exynos variant should be supported in pci-exynos.c
> 
> Theses pathces based on the below patches
>

Aren't you breaking DTB ABI here? If yes, then I don't mind but please
put it in the commit message.  Also order the patches in a way
preserving bisectability.

Best regards,
Krzysztof

> - Jaehoon's patches
> http://patchwork.ozlabs.org/patch/706998/
> http://patchwork.ozlabs.org/patch/706997/
> http://patchwork.ozlabs.org/patch/706995/
> http://patchwork.ozlabs.org/patch/706994/
> 
> - Srinivas's patch
> http://patchwork.ozlabs.org/patch/703530/
> 
> - Pankaj's patch
> http://patchwork.ozlabs.org/patch/708414/
> 
> Jaehoon Chung (4):
>   phy: exynos-pcie: Add support for Exynos PCIe phy
>   Documetation: samsung-phy: add the exynos-pcie-phy binding
>   Documetation: binding: modify the exynos5440 pcie binding
>   ARM: dts: exynos5440: support the phy-pcie node for pcie
> 
>  .../bindings/pci/samsung,exynos5440-pcie.txt       |  29 ++-
>  .../devicetree/bindings/phy/samsung-phy.txt        |  23 ++
>  arch/arm/boot/dts/exynos5440.dtsi                  |  44 +++-
>  drivers/pci/host/pci-exynos.c                      | 198 ++------------
>  drivers/phy/Kconfig                                |   9 +
>  drivers/phy/Makefile                               |   1 +
>  drivers/phy/phy-exynos-pcie.c                      | 289 +++++++++++++++++++++
>  7 files changed, 395 insertions(+), 198 deletions(-)
>  create mode 100644 drivers/phy/phy-exynos-pcie.c
> 
> -- 
> 2.10.2
> 

^ permalink raw reply

* Re: [PATCHv2 0/5] Add generic pinctrl helpers for managing groups and function
From: Gary Bisson @ 2016-12-30 15:43 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Tony Lindgren, Haojian Zhuang, Masahiro Yamada, Grygorii Strashko,
	Nishanth Menon, linux-gpio@vger.kernel.org,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
	Linux-OMAP
In-Reply-To: <CACRpkda9Lv-WF+fLysoEbqBPuMtPBwo_xQT7y0_CHVjC+cAL2Q@mail.gmail.com>

Hi Linus,

On Fri, Dec 30, 2016 at 3:39 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Dec 27, 2016 at 6:19 PM, Tony Lindgren <tony@atomide.com> wrote:
>
>> Here are some changes to add generic helpers for managing pinctrl groups and
>> functions.
>
> I applied it, screwed around with it and pushed to the build servers to see
> if it survived.
>
> I really like the look of this and I hope lots of driver start to use it.
>
> Gary, I just applied your radix patches for i.MX, can you look if you can
> use the GENERIC_PINCTRL_GROUPS and GENERIC_PINMUX_FUCNTIONS
> that Tony invented and that I just merged to my devel branch in the
> pinctrl tree?

Yes I will have a look. It does sound like a good idea. I'll share my
findings beginning of next week.

Regards,
Gary

^ permalink raw reply

* Re: [PATCH v2 3/4] ARM64: dts: exynos5433: use macros for pinctrl configuration on Exynos5433
From: Krzysztof Kozlowski @ 2016-12-30 15:17 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>

On Fri, Dec 30, 2016 at 02:32:39PM +0100, Linus Walleij wrote:
> On Fri, Dec 30, 2016 at 5:14 AM, Andi Shyti <andi.shyti@samsung.com> wrote:
> 
> > 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, I need the header. It would be much appreciated if you could
provide a tag or stable branch with it.

BTW, Andi, please follow the subject prefix convention (git log
--oneline arch/arm64/boot/dts/exynos).

Best regards,
Krzysztof

^ permalink raw reply

* Re: [PATCH 2/2] iio: misc: add support for GPIO power switches
From: Jonathan Cameron @ 2016-12-30 15:15 UTC (permalink / raw)
  To: Linus Walleij, Sebastian Reichel
  Cc: Bartosz Golaszewski, Hartmut Knaack, Lars-Peter Clausen,
	Peter Meerwald-Stadler, Rob Herring, Mark Rutland,
	linux-iio@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, Kevin Hilman, Patrick Titiano,
	Neil Armstrong, Alexandre Courbot, linux-gpio@vger.kernel.org,
	linux-pm@vger.kernel.org, Mark Brown, Liam Girdwood
In-Reply-To: <CACRpkdbjbtFwwQCdR3krWG0P4UMhWySTTkzqZFBZ6EGXPS=k8g@mail.gmail.com>

On 30/12/16 13:05, Linus Walleij wrote:
> n Thu, Dec 29, 2016 at 5:29 PM, Sebastian Reichel <sre@kernel.org> wrote:
>> On Wed, Dec 28, 2016 at 01:50:17PM +0100, Linus Walleij wrote:
>>> On Sun, Dec 11, 2016 at 11:21 PM, Bartosz Golaszewski
>>> <bgolaszewski@baylibre.com> wrote:
>>>
>>>> Some power-measuring ADCs work together with power load switches which
>>>> allow to power-cycle measured devices.
>>>>
>>>> An example use case would be measuring the power consumption of a
>>>> development board during boot using a power monitor such as TI INA226
>>>> and power-cycling the board remotely using a TPS229* power switch.
>>>>
>>>> Add an iio driver for simple GPIO power switches and expose a sysfs
>>>> attribute allowing to toggle their state.
>>>>
>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>
>>> I don't get this, isn't this doing the same as
>>> drivers/power/reset/gpio-poweroff.c
>>> ?
>>>
>>> With the only difference that the latter uses the standard syscall
>>> from pm_power_off to reboot the system instead of some random
>>> sysfs file.
>>
>> As far as I understand it, the TPS229 is used by Barzosz to poweroff
>> a remote system. The gpio-poweroff driver is used to poweroff the
>> local system.
> 
> Thanks yeah I understood this from the context of later patches.
> 
> Well if such a property is used it should be the property of the remote
> system per se, and the remote system should then also be desribed in
> DT, not half-described by dangling references at random nodes.
> 
> So this needs to be re-architected to either describe the remote system
> in DT and handle it in the kernel, or handle it all from userspace if it
> is a one-off non-product thing.
I'm not sure this is always true (though it might be here).  There is always
a need to describe the edge of the known world.  Be it that we have
an accelerometer - ultimately we could describe the device generating the
acceleration but we have no way of knowing what it is or what it will do..

What we have here is a rather simple case, but what about an I/O bank on
a PLC.  Obviously we can handle that as a bunch of GPIOs but if we know
more about it we should have means to describe that.  Say we know they are
always driving relays, we should be able to describe that additional known
stuff. Relays have characteristics such as bounce times etc that if described
would allow us to do the relevant filtering on inputs to handle this.

Here the fact it is a power supply switch should be describable.  Hard to
get right in a generic way though so in cases like this perhaps you
are right and it should just be left undescribed and handled in userspace.

Anyhow, here I think leaving it as a gpio interface is probably the way to
go, but I'm not sure that will always be the case.

Jonathan
> 
> Yours,
> Linus Walleij
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 


^ permalink raw reply

* Re: [PATCH v2 1/4] pinctrl: samsung: Fix the width of PINCFG_TYPE_DRV bitfields for Exynos5433
From: Krzysztof Kozlowski @ 2016-12-30 15:09 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
	linux-samsung-soc,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, stable,
	Andi Shyti
In-Reply-To: <CACRpkdbR3x14h38Gg7qpeWnswwD9qsS0zDUrrXCEJH-AdEB+cQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Dec 30, 2016 at 02:28:52PM +0100, Linus Walleij wrote:
> On Fri, Dec 30, 2016 at 5:14 AM, Andi Shyti <andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
> 
> > From: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> >
> > This patch fixes the wrong width of PINCFG_TYPE_DRV bitfields for Exynos5433
> > because PINCFG_TYPE_DRV of Exynos5433 has 4bit fields in the *_DRV
> > registers. Usually, other Exynos have 2bit field for PINCFG_TYPE_DRV.
> >
> > Fixes: 3c5ecc9ed353 ("pinctrl: exynos: Add support for Exynos5433")
> > Cc: stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: Tomasz Figa <tomasz.figa-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > Cc: Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > Cc: Kukjin Kim <kgene-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> > Cc: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> > Signed-off-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> 
> Nominally I think you should sign this off too Andi, as you are in the delivery
> path.
> 
> Patch applied for fixes.

That has to be signed by Andi... otherwise the chain is broken (and
there could be changes added inside).

Best regards,
Krzysztof
--
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 4/5] arm64: dts: exynos5433: Add bus dt node using VDD_INT for Exynos5433
From: Chanwoo Choi @ 2016-12-30 15:08 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Chanwoo Choi, javier, Kukjin Kim, Rob Herring, Sylwester Nawrocki,
	Tomasz Figa, myungjoo.ham@samsung.com, Kyungmin Park, devicetree,
	linux-samsung-soc, linux-arm-kernel, linux-kernel
In-Reply-To: <20161230145155.ecobfvtspviehlos@kozik-lap>

2016-12-30 23:51 GMT+09:00 Krzysztof Kozlowski <krzk@kernel.org>:
> On Fri, Dec 30, 2016 at 09:59:17AM +0900, Chanwoo Choi wrote:
>> Hi Krzysztof,
>>
>> On 2016년 12월 09일 02:52, Krzysztof Kozlowski wrote:
>> > On Thu, Dec 08, 2016 at 01:58:10PM +0900, Chanwoo Choi wrote:
>> >> This patch adds the bus nodes using VDD_INT for Exynos5433 SoC.
>> >> Exynos5433 has the following AMBA AXI buses to translate data
>> >> between DRAM and sub-blocks.
>> >>
>> >> Following list specify the detailed correlation between sub-block and clock:
>> >> - CLK_ACLK_G2D_{400|266}  : Bus clock for G2D (2D graphic engine)
>> >> - CLK_ACLK_MSCL_400       : Bus clock for MSCL (Memory to memory Scaler)
>> >> - CLK_ACLK_GSCL_333       : Bus clock for GSCL (General Scaler)
>> >> - CLK_SCLK_JPEG_MSCL      : Bus clock for JPEG
>> >> - CLK_ACLK_MFC_400        : Bus clock for MFC (Multi Format Codec)
>> >> - CLK_ACLK_HEVC_400       : Bus clock for HEVC (High Efficient Video Codec)
>> >> - CLK_ACLK_BUS0_400       : NoC(Network On Chip)'s bus clock for PERIC/PERIS/FSYS/MSCL
>> >> - CLK_ACLK_BUS1_400       : NoC's bus clock for MFC/HEVC/G3D
>> >> - CLK_ACLK_BUS2_400       : NoC's bus clock for GSCL/DISP/G2D/CAM0/CAM1/ISP
>> >>
>> >> Signed-off-by: Chanwoo Choi <cw00.choi@samsung.com>
>> >> ---
>> >>  arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi | 197 +++++++++++++++++++++++++
>> >>  arch/arm64/boot/dts/exynos/exynos5433.dtsi     |   1 +
>> >>  2 files changed, 198 insertions(+)
>> >>  create mode 100644 arch/arm64/boot/dts/exynos/exynos5433-bus.dtsi
>> >
>> > For the reference:
>> > Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>> >
>> > I'll queue it for v4.11, after this merge window.
>>
>> Could you please pick this patch3/4/5?
>> These patches were already reviewed by you.
>
> Not yet. I wanted to apply them few days ago but arm64 build is broken
> in 4.10-rc1 so I cannot auto-build them in my system. The arm64 is fixed
> already so I will apply them on top of 4.10-rc2 (when released).

OK. Thanks for reply.

-- 
Best Regards,
Chanwoo Choi

^ permalink raw reply

* Re: [PATCH 7/9] pinctrl: samsung: Add property to mark pad state as suitable for power down
From: Krzysztof Kozlowski @ 2016-12-30 15:05 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Krzysztof Kozlowski, linux-gpio-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Sylwester Nawrocki,
	Linus Walleij, Tomasz Figa, Ulf Hansson,
	Bartlomiej Zolnierkiewicz, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <bad5ef6a-6132-2029-8581-2e8b27f7a2bd-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

On Fri, Dec 30, 2016 at 12:55:27PM +0100, Marek Szyprowski wrote:
> Hi Krzysztof,
> 
> On 2016-12-27 16:33, Krzysztof Kozlowski wrote:
> > On Tue, Dec 27, 2016 at 11:30:57AM +0100, Marek Szyprowski wrote:
> > > On 2016-12-25 20:19, Krzysztof Kozlowski wrote:
> > > > On Fri, Dec 23, 2016 at 01:24:47PM +0100, Marek Szyprowski wrote:
> > > > > Add support for special property "samsung,off-state", which indicates a special
> > > > > state suitable for device's "sleep" state. Its pin values/properties should
> > > > > match the configuration in power down mode. It indicates that pin controller
> > > > > can notify runtime power management subsystem, that it is ready for runtime
> > > > > suspend if its all pins are configured for such state. This in turn might
> > > > > allow to turn respective power domain off to reduce power consumption.
> > > > > 
> > > > > Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> > > > > ---
> > > > >    Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt | 8 ++++++++
> > > > >    drivers/pinctrl/samsung/pinctrl-samsung.c                     | 4 ++++
> > > > >    drivers/pinctrl/samsung/pinctrl-samsung.h                     | 1 +
> > > > >    3 files changed, 13 insertions(+)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > > > index b7bd2e12a269..354eea0e7798 100644
> > > > > --- a/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > > > +++ b/Documentation/devicetree/bindings/pinctrl/samsung-pinctrl.txt
> > > > > @@ -105,6 +105,7 @@ Required Properties:
> > > > >      - samsung,pin-drv: Drive strength configuration.
> > > > >      - samsung,pin-pud-pdn: Pull up/down configuration in power down mode.
> > > > >      - samsung,pin-drv-pdn: Drive strength configuration in power down mode.
> > > > > +  - samsung,off-state: Mark this configuration as suitable for bank power off.
> > > > >      The values specified by these config properties should be derived from the
> > > > >      hardware manual and these values are programmed as-is into the pin
> > > > > @@ -113,6 +114,13 @@ Required Properties:
> > > > >      Note: A child should include atleast a pin function selection property or
> > > > >      pin configuration property (one or more) or both.
> > > > > +  Note: Special property "samsung,off-state" indicates that this state can
> > > > > +  be used for device's "sleep" pins state. Its pin values/properties should
> > > > > +  match the configuration in power down mode.
> > > > Why power down values cannot be used for sleep state? Why you need
> > > > separate pin control state? If pins values should match power down
> > > > configuration, then they could just be added to default state, couldn't
> > > > they?
> > > Separate sleep state is needed because of the pin control bindings and
> > > design.
> > > 
> > > A separate sleep state is required to let pin control client driver (in this
> > > case
> > > Exynos I2S driver) let to choose when it is okay to switch pads into sleep
> > > state and I see no other way to achieve this.
> > Maybe the pinctrl API should be extended for this? Doing this in DTS
> > just for purpose of passing information between drivers (consumer and
> > provider) looks odd.
> 
> Well, I don't know if it is odd or not, but that's how it is used now and I
> see
> no reason to reinvent wheel. Please check it yourself:
> $ git grep \"sleep\" arch/arm/boot/dts | wc -l
> 101

These drivers, at least not all of them, are not using the existence of
sleep mode configuration as a indication of possible runtime sleep. You
are mixing here different ideas.

> 
> > Anyway, you are proposing a new binding so please Cc devicetree mailing
> > list and device tree maintainers.
> 
> I'm just using the generic pinctrl bindings, but it might make some sense to
> add a note to Exynos i2s driver that a sleep pin control state is needed if
> one wants to get power domain to be turned off.

The samsung,off-state is a extension of the existing binding, so please
Cc the devicetree and maintainers. Why you see a problem in it?

Best regards,
Krzysztof

--
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


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