* Re: [PATCH] i2c: busses: constify i2c_algorithm structures
From: Ludovic Desroches @ 2017-02-06 16:12 UTC (permalink / raw)
To: Bhumika Goyal
Cc: julia.lawall, ludovic.desroches, wsa, sonic.zhang, jarkko.nikula,
andriy.shevchenko, mika.westerberg, jdelvare, patrice.chotard,
linux-i2c, linux-kernel, adi-buildroot-devel, linux-arm-kernel,
kernel
In-Reply-To: <1485540377-13686-1-git-send-email-bhumirks@gmail.com>
On Fri, Jan 27, 2017 at 11:36:17PM +0530, Bhumika Goyal wrote:
> Declare i2c_algorithm structures as const as they are only stored in the
> algo field of an i2c_adapter structure. This field is of type const, so
> i2c_algorithm structures having this property can be made const too.
> Done using Coccinelle:
>
> @r disable optional_qualifier@
> identifier i;
> position p;
> @@
> static struct i2c_algorithm i@p={...};
>
> @ok@
> identifier r.i,x;
> position p;
> struct i2c_adapter adapter;
> struct xlr_i2c_private priv;
> @@
> (
> adapter.algo=&i@p;
> |
> priv.adap.algo=&i@p;
> )
>
> @bad@
> position p!={r.p,ok.p};
> identifier r.i;
> @@
> i@p
>
> @depends on !bad disable optional_qualifier@
> identifier r.i;
> @@
> +const
> struct i2c_algorithm i;
>
> File size details before and after patching.
> First line of every .o file shows the file size before patching
> and second line shows the size after patching.
>
> text data bss dec hex filename
>
> 9138 1328 8 10474 28ea drivers/i2c/busses/i2c-at91.o
> 9178 1280 8 10466 28e2 drivers/i2c/busses/i2c-at91.o
>
> 7555 657 8 8220 201c i2c/busses/i2c-designware-core.o
> 7619 617 8 8244 2034 i2c/busses/i2c-designware-core.o
>
> 7240 2240 120 9600 2580 drivers/i2c/busses/i2c-eg20t.o
> 7304 2176 120 9600 2580 drivers/i2c/busses/i2c-eg20t.o
>
> 2399 248 8 2655 a5f drivers/i2c/busses/i2c-emev2.o
> 2455 192 8 2655 a5f drivers/i2c/busses/i2c-emev2.o
>
> 3766 688 8 4462 116e i2c/busses/i2c-imx-lpi2c.o
> 3814 632 8 4454 1166 i2c/busses/i2c-imx-lpi2c.o
>
> 11793 4280 16 16089 3ed9 drivers/i2c/busses/i2c-imx.o
> 11857 4216 16 16089 3ed9 drivers/i2c/busses/i2c-imx.o
>
> 4046 656 8 4710 1266 drivers/i2c/busses/i2c-nforce2.o
> 4110 592 8 4710 1266 drivers/i2c/busses/i2c-nforce2.o
>
> 1397 464 0 1861 745 i2c/busses/i2c-robotfuzz-osif.o
> 1437 408 0 1845 735 i2c/busses/i2c-robotfuzz-osif.o
>
> 9622 1216 16 10854 2a66 i2c/busses/i2c-sh_mobile.o
> 9662 1160 16 10838 2a56 i2c/busses/i2c-sh_mobile.o
>
> 4936 576 8 5520 1590 drivers/i2c/busses/i2c-st.o
> 5000 512 8 5520 1590 drivers/i2c/busses/i2c-st.o
>
> 3404 248 8 3660 e4c i2c/busses/i2c-xgene-slimpro.o
> 3460 192 8 3660 e4c i2c/busses/i2c-xgene-slimpro.o
>
> 2741 600 8 3349 d15 drivers/i2c/busses/i2c-xlp9xx.o
> 2797 544 8 3349 d15 drivers/i2c/busses/i2c-xlp9xx.o
>
> 2643 248 8 2899 b53 drivers/i2c/busses/i2c-xlr.o
> 2707 192 8 2907 b5b drivers/i2c/busses/i2c-xlr.o
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
> ---
> drivers/i2c/busses/i2c-at91.c | 2 +-
Acked-by: Ludovic Desroches <ludovic.desroches@microchip.com>
Regards
Ludovic
^ permalink raw reply
* Re: [PATCH] i2c: at91: ensure state is restored after suspending
From: Ludovic Desroches @ 2017-02-06 14:35 UTC (permalink / raw)
To: Alexandre Belloni
Cc: Wolfram Sang, Ludovic Desroches, linux-i2c, linux-arm-kernel,
linux-kernel, Cyrille Pitchen
In-Reply-To: <20170130172533.31995-1-alexandre.belloni@free-electrons.com>
Hi,
I add Cyrille for FIFO part.
On Mon, Jan 30, 2017 at 06:25:33PM +0100, Alexandre Belloni wrote:
> When going to suspend, the I2C registers may be lost because the power to
> VDDcore is cut. Save them and restore them when resuming.
>
> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> ---
> drivers/i2c/busses/i2c-at91.c | 24 ++++++++++++++++++++++++
> 1 file changed, 24 insertions(+)
>
> diff --git a/drivers/i2c/busses/i2c-at91.c b/drivers/i2c/busses/i2c-at91.c
> index 0b86c6173e07..633bdd899952 100644
> --- a/drivers/i2c/busses/i2c-at91.c
> +++ b/drivers/i2c/busses/i2c-at91.c
> @@ -140,6 +140,12 @@ struct at91_twi_dev {
> unsigned transfer_status;
> struct i2c_adapter adapter;
> unsigned twi_cwgr_reg;
> + struct {
> + u32 mmr;
> + u32 imr;
> + u32 fmr;
> + u32 fimr;
> + } cache;
Do we really need it?
For instance MMR is only written in at91_twi_xfer(). I am not sure we can
continue the transfer when resuming. I assume we will need to perform a new
transfer from a proper state.
Moreover at91_init_twi_bus() is called on resume, it will perform a reset and
it seems it can lead to strange behaviors:
"Note that issuing a software while transmitting might leave a slave in
an unknown state holding the TWD line. In such case, a Bus Clear Command
will allow to make the slave release the TWD line (the first frame
sent afterwards might not be received properly by the slave)."
I think fimr is useless, AT91_TWI_FIER and AT91_TWI_FIDR are not used in
the current code.
Regards
Ludovic
> struct at91_twi_pdata *pdata;
> bool use_dma;
> bool use_alt_cmd;
> @@ -1172,6 +1178,15 @@ static int at91_twi_runtime_resume(struct device *dev)
>
> static int at91_twi_suspend_noirq(struct device *dev)
> {
> + struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
> +
> + twi_dev->cache.mmr = at91_twi_read(twi_dev, AT91_TWI_MMR);
> + twi_dev->cache.imr = at91_twi_read(twi_dev, AT91_TWI_IMR);
> + if (twi_dev->fifo_size) {
> + twi_dev->cache.fmr = at91_twi_read(twi_dev, AT91_TWI_FMR);
> + twi_dev->cache.fimr = at91_twi_read(twi_dev, AT91_TWI_FIMR);
> + }
> +
> if (!pm_runtime_status_suspended(dev))
> at91_twi_runtime_suspend(dev);
>
> @@ -1180,6 +1195,7 @@ static int at91_twi_suspend_noirq(struct device *dev)
>
> static int at91_twi_resume_noirq(struct device *dev)
> {
> + struct at91_twi_dev *twi_dev = dev_get_drvdata(dev);
> int ret;
>
> if (!pm_runtime_status_suspended(dev)) {
> @@ -1191,6 +1207,14 @@ static int at91_twi_resume_noirq(struct device *dev)
> pm_runtime_mark_last_busy(dev);
> pm_request_autosuspend(dev);
>
> + at91_init_twi_bus(twi_dev);
> + at91_twi_write(twi_dev, AT91_TWI_MMR, twi_dev->cache.mmr);
> + at91_twi_write(twi_dev, AT91_TWI_IER, twi_dev->cache.imr);
> + if (twi_dev->fifo_size) {
> + at91_twi_write(twi_dev, AT91_TWI_FMR, twi_dev->cache.fmr);
> + at91_twi_write(twi_dev, AT91_TWI_FIER, twi_dev->cache.fimr);
> + }
> +
> return 0;
> }
>
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH] i2c: designwear: Fix clk warning on suspend/resume
From: Jarkko Nikula @ 2017-02-06 9:32 UTC (permalink / raw)
To: John Stultz, lkml
Cc: Andy Shevchenko, Mika Westerberg, Wolfram Sang, linux-i2c
In-Reply-To: <1486162893-12313-1-git-send-email-john.stultz@linaro.org>
Hi
On 04.02.2017 01:01, John Stultz wrote:
> On my HiKey board, I'm seeing clk warnings on suspend/resume, which
> seem to be caused by runtime pm suspending the device, then the same
> suspend hook being called again on suspend time.
>
> Thus this patch adds suspend state tracking to avoid runtime pm and
> suspend causing double suspend calls on i2c-designware-platdrv.
>
> Feedback would be greatly appreciated!
>
> Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Wolfram Sang <wsa@the-dreams.de>
> Cc: linux-i2c@vger.kernel.org
> Signed-off-by: John Stultz <john.stultz@linaro.org>
> ---
> drivers/i2c/busses/i2c-designware-core.h | 1 +
> drivers/i2c/busses/i2c-designware-platdrv.c | 9 +++++++++
> 2 files changed, 10 insertions(+)
>
...
> @@ -340,8 +340,12 @@ static int dw_i2c_plat_suspend(struct device *dev)
> struct platform_device *pdev = to_platform_device(dev);
> struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
>
> + if (i_dev->suspended)
> + return 0;
> +
> i2c_dw_disable(i_dev);
> i2c_dw_plat_prepare_clk(i_dev, false);
> + i_dev->suspended = true;
>
This feels a band-aid fix. At quick look some drivers appear to use
SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume) in their dev_pm_ops.
Could you try does it help in your case and I'll check our platforms.
--
Jarkko
^ permalink raw reply
* [PATCH 1/3] of: fix of_node leak caused in of_find_node_opts_by_path
From: Qi Hou @ 2017-02-06 4:55 UTC (permalink / raw)
To: peda, stable, robh, linux-i2c
Cc: leif.lindholm, pantelis.antoniou, grant.likely, bruce.ashfield,
paul.gortmaker, xiao.zhang
During stepping down the tree, parent node is gotten first and its refcount is
increased with of_node_get() in __of_get_next_child(). Since it just being used
as tmp node, its refcount must be decreased with of_node_put() after traversing
its child nodes.
Or, its refcount will never be descreased to ZERO, then it will never be freed,
as well as other related memory blocks.
To fix this, decrease refcount of parent with of_node_put() after
__of_find_node_by_path().
Signed-off-by: Qi Hou <qi.hou@windriver.com>
Acked-by: Peter Rosin <peda@axentia.se>
---
drivers/of/base.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d4bea3c..ce8206f 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -842,8 +842,11 @@ struct device_node *of_find_node_opts_by_path(const char *path, const char **opt
if (!np)
np = of_node_get(of_root);
while (np && *path == '/') {
+ struct device_node *tmp = np;
+
path++; /* Increment past '/' delimiter */
np = __of_find_node_by_path(np, path);
+ of_node_put(tmp);
path = strchrnul(path, '/');
if (separator && separator < path)
break;
--
1.9.1
^ permalink raw reply related
* [PATCH v4 1/3] dt: bindings: add documentation for zx2967 family i2c controller
From: Baoyou Xie @ 2017-02-06 3:26 UTC (permalink / raw)
To: andy.shevchenko, jun.nie, baoyou.xie, wsa, robh+dt, mark.rutland
Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel
This patch adds dt-binding documentation for zx2967 family
i2c controller.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
Acked-by: Rob Herring <robh@kernel.org>
---
.../devicetree/bindings/i2c/i2c-zx2967.txt | 24 ++++++++++++++++++++++
1 file changed, 24 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-zx2967.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt b/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt
new file mode 100644
index 0000000..a528374
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-zx2967.txt
@@ -0,0 +1,24 @@
+ZTE zx2967 I2C controller
+
+Required properties:
+ - compatible: must be "zte,zx296718-i2c"
+ - reg: physical address and length of the device registers
+ - interrupts: a single interrupt specifier
+ - clocks: clock for the device
+ - #address-cells: should be <1>
+ - #size-cells: should be <0>
+ - clock-frequency: the desired I2C bus clock frequency.
+
+Examples:
+
+ i2c@112000 {
+ compatible = "zte,zx296718-i2c";
+ reg = <0x00112000 0x1000>;
+ interrupts = <GIC_SPI 112 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&osc24m>;
+ #address-cells = <1>
+ #size-cells = <0>;
+ clock-frequency = <1600000>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2c_global_pin>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v4 3/3] i2c: zx2967: add i2c controller driver for ZTE's zx2967 family
From: Baoyou Xie @ 2017-02-06 3:26 UTC (permalink / raw)
To: andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jun.nie-QSEj5FYQhm4dnm+yROfE0A, baoyou.xie-QSEj5FYQhm4dnm+yROfE0A,
wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1486351569-4814-1-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
This patch adds i2c controller driver for ZTE's zx2967 family.
Signed-off-by: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/i2c/busses/Kconfig | 9 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-zx2967.c | 688 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 698 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-zx2967.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index e4a603e..d983e36 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1246,4 +1246,13 @@ config I2C_OPAL
This driver can also be built as a module. If so, the module will be
called as i2c-opal.
+config I2C_ZX2967
+ tristate "ZTE zx2967 I2C support"
+ depends on ARCH_ZX
+ default y
+ help
+ Selecting this option will add ZX2967 I2C driver.
+ This driver can also be built as a module. If so, the module will be
+ called i2c-zx2967.
+
endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index beb4809..16b2901 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
obj-$(CONFIG_I2C_XLR) += i2c-xlr.o
obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
+obj-$(CONFIG_I2C_ZX2967) += i2c-zx2967.o
# External I2C/SMBus adapter drivers
obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-zx2967.c b/drivers/i2c/busses/i2c-zx2967.c
new file mode 100644
index 0000000..7b3471a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-zx2967.c
@@ -0,0 +1,688 @@
+/*
+ * ZTE's zx2967 family i2c bus controller driver
+ *
+ * Copyright (C) 2017 ZTE Ltd.
+ *
+ * Author: Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ *
+ * License terms: GNU General Public License (GPL) version 2
+ */
+
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define REG_CMD 0x04
+#define REG_DEVADDR_H 0x0C
+#define REG_DEVADDR_L 0x10
+#define REG_CLK_DIV_FS 0x14
+#define REG_CLK_DIV_HS 0x18
+#define REG_WRCONF 0x1C
+#define REG_RDCONF 0x20
+#define REG_DATA 0x24
+#define REG_STAT 0x28
+
+#define I2C_STOP 0
+#define I2C_MASTER BIT(0)
+#define I2C_ADDR_MODE_TEN BIT(1)
+#define I2C_IRQ_MSK_ENABLE BIT(3)
+#define I2C_RW_READ BIT(4)
+#define I2C_CMB_RW_EN BIT(5)
+#define I2C_START BIT(6)
+#define I2C_ADDR_MODE_TEN BIT(1)
+
+#define I2C_WFIFO_RESET BIT(7)
+#define I2C_RFIFO_RESET BIT(7)
+
+#define I2C_IRQ_ACK_CLEAR BIT(7)
+#define I2C_INT_MASK GENMASK(6, 0)
+
+#define I2C_TRANS_DONE BIT(0)
+#define I2C_ERROR_DEVICE BIT(1)
+#define I2C_ERROR_DATA BIT(2)
+#define I2C_ERROR_MASK GENMASK(2, 1)
+
+#define I2C_SR_BUSY BIT(6)
+
+#define I2C_SR_EDEVICE BIT(1)
+#define I2C_SR_EDATA BIT(2)
+
+#define I2C_FIFO_MAX 16
+
+#define I2C_TIMEOUT msecs_to_jiffies(1000)
+
+struct zx2967_i2c_info {
+ spinlock_t lock;
+ struct device *dev;
+ struct i2c_adapter adap;
+ struct clk *clk;
+ struct completion complete;
+ u32 clk_freq;
+ void __iomem *reg_base;
+ size_t residue;
+ int irq;
+ int msg_rd;
+ u8 *buf;
+ u8 access_cnt;
+ bool is_suspended;
+};
+
+static void zx2967_i2c_writel(struct zx2967_i2c_info *zx_i2c,
+ u32 val, unsigned long reg)
+{
+ writel_relaxed(val, zx_i2c->reg_base + reg);
+}
+
+static u32 zx2967_i2c_readl(struct zx2967_i2c_info *zx_i2c, unsigned long reg)
+{
+ return readl_relaxed(zx_i2c->reg_base + reg);
+}
+
+static void zx2967_i2c_writesb(struct zx2967_i2c_info *zx_i2c,
+ void *data, unsigned long reg, int len)
+{
+ writesb(zx_i2c->reg_base + reg, data, len);
+}
+
+static void zx2967_i2c_readsb(struct zx2967_i2c_info *zx_i2c,
+ void *data, unsigned long reg, int len)
+{
+ readsb(zx_i2c->reg_base + reg, data, len);
+}
+
+static void zx2967_i2c_start_ctrl(struct zx2967_i2c_info *zx_i2c)
+{
+ u32 status;
+ u32 ctl;
+
+ status = zx2967_i2c_readl(zx_i2c, REG_STAT);
+ status |= I2C_IRQ_ACK_CLEAR;
+ zx2967_i2c_writel(zx_i2c, status, REG_STAT);
+
+ ctl = zx2967_i2c_readl(zx_i2c, REG_CMD);
+ if (zx_i2c->msg_rd)
+ ctl |= I2C_RW_READ;
+ else
+ ctl &= ~I2C_RW_READ;
+ ctl &= ~I2C_CMB_RW_EN;
+ ctl |= I2C_START;
+ zx2967_i2c_writel(zx_i2c, ctl, REG_CMD);
+}
+
+static void zx2967_i2c_flush_fifos(struct zx2967_i2c_info *zx_i2c)
+{
+ u32 val;
+ u32 offset;
+
+ if (zx_i2c->msg_rd) {
+ offset = REG_RDCONF;
+ val = I2C_RFIFO_RESET;
+ } else {
+ offset = REG_WRCONF;
+ val = I2C_WFIFO_RESET;
+ }
+
+ val |= zx2967_i2c_readl(zx_i2c, offset);
+ zx2967_i2c_writel(zx_i2c, val, offset);
+}
+
+static int zx2967_i2c_empty_rx_fifo(struct zx2967_i2c_info *zx_i2c, u32 size)
+{
+ u8 val[I2C_FIFO_MAX] = {0};
+ int i;
+
+ if (size > I2C_FIFO_MAX) {
+ dev_err(zx_i2c->dev, "fifo size %d over the max value %d\n",
+ size, I2C_FIFO_MAX);
+ return -EINVAL;
+ }
+
+ zx2967_i2c_readsb(zx_i2c, val, REG_DATA, size);
+ for (i = 0; i < size; i++) {
+ *(zx_i2c->buf++) = val[i];
+ zx_i2c->residue--;
+ if (zx_i2c->residue <= 0)
+ break;
+ }
+
+ barrier();
+
+ return 0;
+}
+
+static int zx2967_i2c_fill_tx_fifo(struct zx2967_i2c_info *zx_i2c)
+{
+ u8 *buf = zx_i2c->buf;
+ size_t residue = zx_i2c->residue;
+
+ if (residue == 0) {
+ dev_err(zx_i2c->dev, "residue is %d\n", (int)residue);
+ return -EINVAL;
+ }
+
+ if (residue <= I2C_FIFO_MAX) {
+ zx2967_i2c_writesb(zx_i2c, buf, REG_DATA, residue);
+
+ /* Again update before writing to FIFO to make sure isr sees. */
+ zx_i2c->residue = 0;
+ zx_i2c->buf = NULL;
+ } else {
+ zx2967_i2c_writesb(zx_i2c, buf, REG_DATA, I2C_FIFO_MAX);
+ zx_i2c->residue -= I2C_FIFO_MAX;
+ zx_i2c->buf += I2C_FIFO_MAX;
+ }
+
+ barrier();
+
+ return 0;
+}
+
+static int zx2967_i2c_reset_hardware(struct zx2967_i2c_info *zx_i2c)
+{
+ u32 val;
+ u32 clk_div;
+ u32 status;
+
+ val = I2C_MASTER | I2C_IRQ_MSK_ENABLE;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+
+ clk_div = clk_get_rate(zx_i2c->clk) / zx_i2c->clk_freq - 1;
+ zx2967_i2c_writel(zx_i2c, clk_div, REG_CLK_DIV_FS);
+ zx2967_i2c_writel(zx_i2c, clk_div, REG_CLK_DIV_HS);
+
+ zx2967_i2c_writel(zx_i2c, I2C_FIFO_MAX - 1, REG_WRCONF);
+ zx2967_i2c_writel(zx_i2c, I2C_FIFO_MAX - 1, REG_RDCONF);
+ zx2967_i2c_writel(zx_i2c, 1, REG_RDCONF);
+
+ zx2967_i2c_flush_fifos(zx_i2c);
+
+ status = zx2967_i2c_readl(zx_i2c, REG_STAT);
+ if (status & I2C_SR_BUSY)
+ return -EBUSY;
+ if (status & (I2C_SR_EDEVICE | I2C_SR_EDATA))
+ return -EIO;
+
+ enable_irq(zx_i2c->irq);
+
+ return 0;
+}
+
+static void zx2967_i2c_isr_clr(struct zx2967_i2c_info *zx_i2c)
+{
+ u32 status;
+
+ status = zx2967_i2c_readl(zx_i2c, REG_STAT);
+ status |= I2C_IRQ_ACK_CLEAR;
+ zx2967_i2c_writel(zx_i2c, status, REG_STAT);
+}
+
+static irqreturn_t zx2967_i2c_isr(int irq, void *dev_id)
+{
+ u32 status;
+ struct zx2967_i2c_info *zx_i2c = (struct zx2967_i2c_info *)dev_id;
+ unsigned long flags;
+
+ spin_lock_irqsave(&zx_i2c->lock, flags);
+
+ status = zx2967_i2c_readl(zx_i2c, REG_STAT) & I2C_INT_MASK;
+ zx2967_i2c_isr_clr(zx_i2c);
+
+ if (status & I2C_ERROR_MASK) {
+ spin_unlock_irqrestore(&zx_i2c->lock, flags);
+ return IRQ_HANDLED;
+ }
+
+ if (status & I2C_TRANS_DONE)
+ complete(&zx_i2c->complete);
+
+ spin_unlock_irqrestore(&zx_i2c->lock, flags);
+
+ return IRQ_HANDLED;
+}
+
+static void zx2967_enable_tenbit(struct zx2967_i2c_info *zx_i2c, __u16 addr)
+{
+ u16 val = (addr >> 7) & 0x7;
+
+ if (val > 0) {
+ zx2967_i2c_writel(zx_i2c, val, REG_DEVADDR_H);
+ val = (zx2967_i2c_readl(zx_i2c, REG_CMD)) | I2C_ADDR_MODE_TEN;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+ }
+}
+
+static int
+zx2967_i2c_xfer_read_bytes(struct zx2967_i2c_info *zx_i2c, u32 bytes)
+{
+ unsigned long time_left;
+
+ reinit_completion(&zx_i2c->complete);
+ zx2967_i2c_writel(zx_i2c, bytes - 1, REG_RDCONF);
+ zx2967_i2c_start_ctrl(zx_i2c);
+
+ time_left = wait_for_completion_timeout(&zx_i2c->complete,
+ I2C_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(zx_i2c->dev, "read i2c transfer timed out\n");
+ disable_irq(zx_i2c->irq);
+ zx2967_i2c_reset_hardware(zx_i2c);
+ return -EIO;
+ }
+
+ return zx2967_i2c_empty_rx_fifo(zx_i2c, bytes);
+}
+
+static int zx2967_i2c_xfer_read(struct zx2967_i2c_info *zx_i2c)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < zx_i2c->access_cnt; i++) {
+ ret = zx2967_i2c_xfer_read_bytes(zx_i2c, I2C_FIFO_MAX);
+ if (ret)
+ return ret;
+ }
+
+ if (zx_i2c->residue > 0) {
+ ret = zx2967_i2c_xfer_read_bytes(zx_i2c, I2C_FIFO_MAX);
+ if (ret)
+ return ret;
+ }
+
+ zx_i2c->residue = 0;
+ zx_i2c->access_cnt = 0;
+ return 0;
+}
+
+static int
+zx2967_i2c_xfer_write_bytes(struct zx2967_i2c_info *zx_i2c, u32 bytes)
+{
+ unsigned long time_left;
+ int ret;
+
+ reinit_completion(&zx_i2c->complete);
+
+ ret = zx2967_i2c_fill_tx_fifo(zx_i2c);
+ if (ret)
+ return ret;
+
+ zx2967_i2c_start_ctrl(zx_i2c);
+
+ time_left = wait_for_completion_timeout(&zx_i2c->complete,
+ I2C_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(zx_i2c->dev, "write i2c transfer timed out\n");
+ disable_irq(zx_i2c->irq);
+ zx2967_i2c_reset_hardware(zx_i2c);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int zx2967_i2c_xfer_write(struct zx2967_i2c_info *zx_i2c)
+{
+ int ret;
+ int i;
+
+ for (i = 0; i < zx_i2c->access_cnt; i++) {
+ ret = zx2967_i2c_xfer_write_bytes(zx_i2c, I2C_FIFO_MAX);
+ if (ret)
+ return ret;
+ }
+
+ if (zx_i2c->residue > 0) {
+ ret = zx2967_i2c_xfer_write_bytes(zx_i2c, I2C_FIFO_MAX);
+ if (ret)
+ return ret;
+ }
+
+ zx_i2c->residue = 0;
+ zx_i2c->access_cnt = 0;
+ return 0;
+}
+
+static int zx2967_i2c_xfer_msg(struct zx2967_i2c_info *zx_i2c,
+ struct i2c_msg *msg)
+{
+ if (msg->len == 0)
+ return -EINVAL;
+
+ zx2967_i2c_flush_fifos(zx_i2c);
+
+ zx_i2c->buf = msg->buf;
+ zx_i2c->residue = msg->len;
+ zx_i2c->access_cnt = msg->len / I2C_FIFO_MAX;
+ zx_i2c->msg_rd = (msg->flags & I2C_M_RD);
+
+ if (zx_i2c->msg_rd)
+ return zx2967_i2c_xfer_read(zx_i2c);
+
+ return zx2967_i2c_xfer_write(zx_i2c);
+}
+
+static int zx2967_i2c_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
+ int ret;
+ int i;
+
+ if (zx_i2c->is_suspended)
+ return -EBUSY;
+
+ zx2967_i2c_writel(zx_i2c, (msgs->addr & 0x7f), REG_DEVADDR_L);
+ zx2967_i2c_writel(zx_i2c, (msgs->addr >> 7) & 0x7, REG_DEVADDR_H);
+ if (zx2967_i2c_readl(zx_i2c, REG_DEVADDR_H) > 0)
+ zx2967_enable_tenbit(zx_i2c, msgs->addr);
+
+ for (i = 0; i < num; i++) {
+ ret = zx2967_i2c_xfer_msg(zx_i2c, &msgs[i]);
+ if (ret)
+ return ret;
+ if (num > 1)
+ usleep_range(1000, 2000);
+ }
+
+ return num;
+}
+
+static void
+zx2967_smbus_xfer_prepare(struct zx2967_i2c_info *zx_i2c, u16 addr,
+ char read_write, u8 command, int size,
+ union i2c_smbus_data *data)
+{
+ u32 val;
+
+ val = zx2967_i2c_readl(zx_i2c, REG_RDCONF);
+ val |= I2C_RFIFO_RESET;
+ zx2967_i2c_writel(zx_i2c, val, REG_RDCONF);
+ zx2967_i2c_writel(zx_i2c, (addr & 0x7f), REG_DEVADDR_L);
+
+ zx2967_enable_tenbit(zx_i2c, addr);
+ val = zx2967_i2c_readl(zx_i2c, REG_CMD);
+ val &= ~I2C_RW_READ;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+
+ switch (size) {
+ case I2C_SMBUS_BYTE:
+ zx2967_i2c_writel(zx_i2c, command, REG_DATA);
+ break;
+ case I2C_SMBUS_BYTE_DATA:
+ zx2967_i2c_writel(zx_i2c, command, REG_DATA);
+ if (read_write == I2C_SMBUS_WRITE)
+ zx2967_i2c_writel(zx_i2c, data->byte, REG_DATA);
+ break;
+ case I2C_SMBUS_WORD_DATA:
+ zx2967_i2c_writel(zx_i2c, command, REG_DATA);
+ if (read_write == I2C_SMBUS_WRITE) {
+ zx2967_i2c_writel(zx_i2c, (data->word >> 8), REG_DATA);
+ zx2967_i2c_writel(zx_i2c, (data->word & 0xff),
+ REG_DATA);
+ }
+ break;
+ }
+}
+
+static int zx2967_smbus_xfer_read(struct zx2967_i2c_info *zx_i2c, int size,
+ union i2c_smbus_data *data)
+{
+ unsigned long time_left;
+ u8 buf[2];
+ u32 val;
+
+ reinit_completion(&zx_i2c->complete);
+
+ val = zx2967_i2c_readl(zx_i2c, REG_CMD);
+ val |= I2C_CMB_RW_EN;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+
+ val = zx2967_i2c_readl(zx_i2c, REG_CMD);
+ val |= I2C_START;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+
+ time_left = wait_for_completion_timeout(&zx_i2c->complete,
+ I2C_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(zx_i2c->dev, "i2c read transfer timed out\n");
+ disable_irq(zx_i2c->irq);
+ zx2967_i2c_reset_hardware(zx_i2c);
+ return -EIO;
+ }
+
+ usleep_range(1000, 2000);
+ switch (size) {
+ case I2C_SMBUS_BYTE:
+ case I2C_SMBUS_BYTE_DATA:
+ val = zx2967_i2c_readl(zx_i2c, REG_DATA);
+ data->byte = val;
+ break;
+ case I2C_SMBUS_WORD_DATA:
+ case I2C_SMBUS_PROC_CALL:
+ buf[0] = zx2967_i2c_readl(zx_i2c, REG_DATA);
+ buf[1] = zx2967_i2c_readl(zx_i2c, REG_DATA);
+ data->word = (buf[0] << 8) | buf[1];
+ break;
+ default:
+ dev_warn(zx_i2c->dev, "Unsupported transaction %d\n", size);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static int zx2967_smbus_xfer_write(struct zx2967_i2c_info *zx_i2c)
+{
+ unsigned long time_left;
+ u32 val;
+
+ reinit_completion(&zx_i2c->complete);
+ val = zx2967_i2c_readl(zx_i2c, REG_CMD);
+ val |= I2C_START;
+ zx2967_i2c_writel(zx_i2c, val, REG_CMD);
+
+ time_left = wait_for_completion_timeout(&zx_i2c->complete,
+ I2C_TIMEOUT);
+ if (time_left == 0) {
+ dev_err(zx_i2c->dev, "i2c write transfer timed out\n");
+ disable_irq(zx_i2c->irq);
+ zx2967_i2c_reset_hardware(zx_i2c);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int zx2967_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+ unsigned short flags, char read_write,
+ u8 command, int size, union i2c_smbus_data *data)
+{
+ struct zx2967_i2c_info *zx_i2c = i2c_get_adapdata(adap);
+
+ if (size == I2C_SMBUS_QUICK)
+ read_write = I2C_SMBUS_WRITE;
+
+ switch (size) {
+ case I2C_SMBUS_QUICK:
+ case I2C_SMBUS_BYTE:
+ case I2C_SMBUS_BYTE_DATA:
+ case I2C_SMBUS_WORD_DATA:
+ zx2967_smbus_xfer_prepare(zx_i2c, addr, read_write,
+ command, size, data);
+ break;
+ default:
+ dev_warn(&adap->dev, "Unsupported transaction %d\n", size);
+ return -EOPNOTSUPP;
+ }
+
+ if (read_write == I2C_SMBUS_READ)
+ return zx2967_smbus_xfer_read(zx_i2c, size, data);
+
+ return zx2967_smbus_xfer_write(zx_i2c);
+}
+
+#define ZX2967_I2C_FUNCS (I2C_FUNC_SMBUS_QUICK | I2C_FUNC_SMBUS_BYTE | \
+ I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA | \
+ I2C_FUNC_SMBUS_BLOCK_DATA | I2C_FUNC_SMBUS_PROC_CALL | \
+ I2C_FUNC_I2C | I2C_FUNC_SMBUS_I2C_BLOCK)
+
+static u32 zx2967_i2c_func(struct i2c_adapter *adap)
+{
+ return ZX2967_I2C_FUNCS;
+}
+
+static int __maybe_unused zx2967_i2c_suspend(struct device *dev)
+{
+ struct zx2967_i2c_info *zx_i2c = dev_get_drvdata(dev);
+
+ zx_i2c->is_suspended = true;
+ clk_disable_unprepare(zx_i2c->clk);
+
+ return 0;
+}
+
+static int __maybe_unused zx2967_i2c_resume(struct device *dev)
+{
+ struct zx2967_i2c_info *zx_i2c = dev_get_drvdata(dev);
+
+ zx_i2c->is_suspended = false;
+ clk_prepare_enable(zx_i2c->clk);
+
+ return 0;
+}
+
+#ifdef CONFIG_PM
+static const struct dev_pm_ops zx2967_i2c_dev_pm_ops = {
+ .suspend = zx2967_i2c_suspend,
+ .resume = zx2967_i2c_resume,
+};
+#define ZX2967_I2C_DEV_PM_OPS (&zx2967_i2c_dev_pm_ops)
+#else
+#define ZX2967_I2C_DEV_PM_OPS NULL
+#endif
+
+static const struct i2c_algorithm zx2967_i2c_algo = {
+ .master_xfer = zx2967_i2c_xfer,
+ .smbus_xfer = zx2967_smbus_xfer,
+ .functionality = zx2967_i2c_func,
+};
+
+static const struct of_device_id zx2967_i2c_of_match[] = {
+ { .compatible = "zte,zx296718-i2c", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, zx2967_i2c_of_match);
+
+static int zx2967_i2c_probe(struct platform_device *pdev)
+{
+ struct zx2967_i2c_info *zx_i2c;
+ void __iomem *reg_base;
+ struct resource *res;
+ struct clk *clk;
+ int ret;
+
+ zx_i2c = devm_kzalloc(&pdev->dev, sizeof(*zx_i2c), GFP_KERNEL);
+ if (!zx_i2c)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ reg_base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(reg_base))
+ return PTR_ERR(reg_base);
+
+ clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(clk)) {
+ dev_err(&pdev->dev, "missing controller clock");
+ return PTR_ERR(clk);
+ }
+
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to enable i2c_clk\n");
+ return ret;
+ }
+
+ ret = platform_get_irq(pdev, 0);
+ if (ret < 0)
+ return ret;
+ zx_i2c->irq = ret;
+
+ ret = device_property_read_u32(&pdev->dev, "clock-frequency",
+ &zx_i2c->clk_freq);
+ if (ret) {
+ dev_err(&pdev->dev, "missing clock-frequency");
+ return ret;
+ }
+
+ zx_i2c->reg_base = reg_base;
+ zx_i2c->clk = clk;
+ zx_i2c->dev = &pdev->dev;
+
+ spin_lock_init(&zx_i2c->lock);
+ init_completion(&zx_i2c->complete);
+ platform_set_drvdata(pdev, zx_i2c);
+
+ ret = zx2967_i2c_reset_hardware(zx_i2c);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to initialize i2c controller\n");
+ goto err_clk_unprepare;
+ }
+
+ ret = devm_request_irq(&pdev->dev, zx_i2c->irq,
+ zx2967_i2c_isr, 0, dev_name(&pdev->dev), zx_i2c);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to request irq %i\n", zx_i2c->irq);
+ goto err_clk_unprepare;
+ }
+
+ i2c_set_adapdata(&zx_i2c->adap, zx_i2c);
+ zx_i2c->adap.owner = THIS_MODULE;
+ zx_i2c->adap.class = I2C_CLASS_DEPRECATED;
+ strlcpy(zx_i2c->adap.name, "zx2967 i2c adapter",
+ sizeof(zx_i2c->adap.name));
+ zx_i2c->adap.algo = &zx2967_i2c_algo;
+ zx_i2c->adap.dev.parent = &pdev->dev;
+ zx_i2c->adap.nr = pdev->id;
+ zx_i2c->adap.dev.of_node = pdev->dev.of_node;
+
+ ret = i2c_add_numbered_adapter(&zx_i2c->adap);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add zx2967 i2c adapter\n");
+ goto err_clk_unprepare;
+ }
+
+ return 0;
+
+err_clk_unprepare:
+ clk_disable_unprepare(zx_i2c->clk);
+ return ret;
+}
+
+static int zx2967_i2c_remove(struct platform_device *pdev)
+{
+ struct zx2967_i2c_info *zx_i2c = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&zx_i2c->adap);
+ clk_disable_unprepare(zx_i2c->clk);
+
+ return 0;
+}
+
+static struct platform_driver zx2967_i2c_driver = {
+ .probe = zx2967_i2c_probe,
+ .remove = zx2967_i2c_remove,
+ .driver = {
+ .name = "zx2967_i2c",
+ .of_match_table = zx2967_i2c_of_match,
+ .pm = ZX2967_I2C_DEV_PM_OPS,
+ },
+};
+module_platform_driver(zx2967_i2c_driver);
+
+MODULE_AUTHOR("Baoyou Xie <baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>");
+MODULE_DESCRIPTION("ZTE zx2967 I2C Bus Controller driver");
+MODULE_LICENSE("GPL v2");
--
2.7.4
--
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 v4 2/3] MAINTAINERS: add zx2967 i2c controller driver to ARM ZTE architecture
From: Baoyou Xie @ 2017-02-06 3:26 UTC (permalink / raw)
To: andy.shevchenko, jun.nie, baoyou.xie, wsa, robh+dt, mark.rutland
Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel
In-Reply-To: <1486351569-4814-1-git-send-email-baoyou.xie@linaro.org>
Add the zx2967 i2c controller driver as maintained by ARM ZTE
architecture maintainers, as they're parts of the core IP.
Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
---
MAINTAINERS | 2 ++
1 file changed, 2 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 5fb9b62..4e81e2b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1987,10 +1987,12 @@ L: linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
S: Maintained
F: arch/arm/mach-zx/
F: drivers/clk/zte/
+F: drivers/i2c/busses/i2c-zx2967.c
F: drivers/reset/reset-zx2967.c
F: drivers/soc/zte/
F: Documentation/devicetree/bindings/arm/zte.txt
F: Documentation/devicetree/bindings/clock/zx296702-clk.txt
+F: Documentation/devicetree/bindings/i2c/i2c-zx2967.txt
F: Documentation/devicetree/bindings/reset/zte,zx2967-reset.txt
F: Documentation/devicetree/bindings/soc/zte/
F: include/dt-bindings/soc/zx*.h
--
2.7.4
^ permalink raw reply related
* [PATCH v6 3/3] spi: acpi: Initialize modalias from of_compatible
From: Dan O'Donovan @ 2017-02-05 16:30 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486312214-20770-1-git-send-email-dan@emutex.com>
When using devicetree spi_device.modalias is set to the compatible
string with the vendor prefix removed. For SPI devices described via
ACPI the spi_device.modalias string is initialized by acpi_device_hid.
When using ACPI and DT ids this string ends up something like "PRP0001".
Change acpi_register_spi_device to use the of_compatible property if
present. This makes it easier to instantiate spi drivers through ACPI
with DT ids.
Signed-off-by: Dan O'Donovan <dan@emutex.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e..ad7f638 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1722,13 +1722,15 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
return AE_OK;
}
+ acpi_set_modalias(adev, acpi_device_hid(adev), spi->modalias,
+ sizeof(spi->modalias));
+
if (spi->irq < 0)
spi->irq = acpi_dev_gpio_irq_get(adev, 0);
acpi_device_set_enumerated(adev);
adev->power.flags.ignore_parent = true;
- strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
if (spi_add_device(spi)) {
adev->power.flags.ignore_parent = false;
dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v6 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
From: Dan O'Donovan @ 2017-02-05 16:30 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486312214-20770-1-git-send-email-dan@emutex.com>
When using devicetree stuff like i2c_client.name or spi_device.modalias
is initialized to the first DT compatible id with the vendor prefix
stripped. Since some drivers rely on this try to replicate it when using
ACPI with DT ids.
Signed-off-by: Dan O'Donovan <dan@emutex.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
drivers/acpi/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/acpi/acpi_bus.h | 2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 95855cb..80cb5eb 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -677,6 +677,48 @@ static bool acpi_of_match_device(struct acpi_device *adev,
return false;
}
+static bool acpi_of_modalias(struct acpi_device *adev,
+ char *modalias, size_t len)
+{
+ const union acpi_object *of_compatible;
+ const union acpi_object *obj;
+ const char *str, *chr;
+
+ of_compatible = adev->data.of_compatible;
+ if (!of_compatible)
+ return false;
+
+ if (of_compatible->type == ACPI_TYPE_PACKAGE)
+ obj = of_compatible->package.elements;
+ else /* Must be ACPI_TYPE_STRING. */
+ obj = of_compatible;
+
+ str = obj->string.pointer;
+ chr = strchr(str, ',');
+ strlcpy(modalias, chr ? chr + 1 : str, len);
+
+ return true;
+}
+
+/**
+ * acpi_set_modalias - Set modalias using "compatible" property or supplied ID
+ * @adev: ACPI device object to match
+ * @default_id: ID string to use as default if no compatible string found
+ * @modalias: Pointer to buffer that modalias value will be copied into
+ * @len: Length of modalias buffer
+ *
+ * This is a counterpart of of_modalias_node() for struct acpi_device objects.
+ * If there is a compatible string for @adev, it will be copied to @modalias
+ * with the vendor prefix stripped; otherwise, @default_id will be used.
+ */
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+ char *modalias, size_t len)
+{
+ if (!acpi_of_modalias(adev, modalias, len))
+ strlcpy(modalias, default_id, len);
+}
+EXPORT_SYMBOL_GPL(acpi_set_modalias);
+
static bool __acpi_match_device_cls(const struct acpi_device_id *id,
struct acpi_hardware_id *hwid)
{
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..ef0ae8a 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -522,6 +522,8 @@ void acpi_bus_trim(struct acpi_device *start);
acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids);
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+ char *modalias, size_t len);
int acpi_create_dir(struct acpi_device *);
void acpi_remove_dir(struct acpi_device *);
--
2.7.4
^ permalink raw reply related
* [PATCH v6 2/3] i2c: acpi: Initialize info.type from of_compatible
From: Dan O'Donovan @ 2017-02-05 16:30 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486312214-20770-1-git-send-email-dan@emutex.com>
When using devicetree i2c_board_info.type is set to the compatible
string with the vendor prefix removed. For I2C devices described via
ACPI the i2c_board_info.type string is set to the ACPI device name. When
using ACPI and DT ids this string ends up something like "PRP0001:00".
If the of_compatible property is present try to use that instead. This
makes it easier to instantiate i2c drivers through ACPI with DT ids.
Signed-off-by: Dan O'Donovan <dan@emutex.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Acked-by: Wolfram Sang <wsa@the-dreams.de>
---
drivers/i2c/i2c-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 583e950..1e52395 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -221,7 +221,8 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
acpi_dev_free_resource_list(&resource_list);
- strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+ acpi_set_modalias(adev, dev_name(&adev->dev), info->type,
+ sizeof(info->type));
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH v6 0/3] Init device ids from ACPI of_compatible
From: Dan O'Donovan @ 2017-02-05 16:30 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486195228-10929-1-git-send-email-dan@emutex.com>
When using devicetree, stuff like i2c_client.name or spi_device.modalias
is initialized to the first DT compatible id with the vendor prefix
stripped. Since some drivers rely on this in order to differentiate between
hardware variants, try to replicate it when using ACPI with DT ids.
This also makes it so that the i2c_device_id parameter passed to probe is
non-NULL when matching with ACPI and DT ids.
Tested using ACPI overlays but there is no actual dependency. This series
just extends the PRP0001 feature to be more useful for I2C/SPI.
The patches only touches the ACPI-specific parts of the i2c and spi core.
Here is an example .dsl for an SPI accelerometer connected to minnowboard max:
Device (ACCL)
{
Name (_ADR, Zero)
Name (_HID, "PRP0001")
Name (_UID, One)
Method (_CRS, 0, Serialized)
{
Name (RBUF, ResourceTemplate ()
{
SPISerialBus(1, PolarityLow, FourWireMode, 16,
ControllerInitiated, 1000000, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.SPI1",)
GpioInt (Edge, ActiveHigh, Exclusive, PullDown, 0x0000,
"\\_SB.GPO2", 0x00, ResourceConsumer, , )
{ // Pin list
1
}
})
Return (RBUF)
}
Name (_DSD, Package ()
{
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package ()
{
Package () {"compatible", "st,lis3dh"},
}
})
}
Credit to Leonard Crestez for the original version of this patch set.
Link to v5: http://www.spinics.net/lists/linux-acpi/msg71902.html
Changes:
* Change formatting of arguments passed to acpi_set_modalias (Mark Brown)
Link to v4: http://www.spinics.net/lists/linux-acpi/msg71657.html
Changes:
* add acpi_set_modalias wrapper to handle fallback to ACPI ID (Mark Brown)
Link to v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)
Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)
Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
Changes:
* Rebase on after acpi overlays got it.
* Change acpi_of_modalias outlen param to size_t
* Use {} after else
Dan O'Donovan (3):
ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible
drivers/acpi/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
drivers/i2c/i2c-core.c | 3 ++-
drivers/spi/spi.c | 4 +++-
include/acpi/acpi_bus.h | 2 ++
4 files changed, 49 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v5 3/3] spi: acpi: Initialize modalias from of_compatible
From: Mark Brown @ 2017-02-04 10:16 UTC (permalink / raw)
To: Dan O'Donovan
Cc: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel
In-Reply-To: <1486195228-10929-4-git-send-email-dan@emutex.com>
[-- Attachment #1: Type: text/plain, Size: 310 bytes --]
On Sat, Feb 04, 2017 at 08:00:28AM +0000, Dan O'Donovan wrote:
> + acpi_set_modalias(adev, acpi_device_hid(adev),
> + spi->modalias, sizeof(spi->modalias));
> +
The formatting here is really weird (why isn't the third argument on the
line above?) but otherwise
Acked-by: Mark Brown <broonie@kernel.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH v5 3/3] spi: acpi: Initialize modalias from of_compatible
From: Dan O'Donovan @ 2017-02-04 8:00 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486195228-10929-1-git-send-email-dan@emutex.com>
When using devicetree spi_device.modalias is set to the compatible
string with the vendor prefix removed. For SPI devices described via
ACPI the spi_device.modalias string is initialized by acpi_device_hid.
When using ACPI and DT ids this string ends up something like "PRP0001".
Change acpi_register_spi_device to use the of_compatible property if
present. This makes it easier to instantiate spi drivers through ACPI
with DT ids.
Signed-off-by: Dan O'Donovan <dan@emutex.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
drivers/spi/spi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index 656dd3e..d9afc32 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -1722,13 +1722,15 @@ static acpi_status acpi_register_spi_device(struct spi_master *master,
return AE_OK;
}
+ acpi_set_modalias(adev, acpi_device_hid(adev),
+ spi->modalias, sizeof(spi->modalias));
+
if (spi->irq < 0)
spi->irq = acpi_dev_gpio_irq_get(adev, 0);
acpi_device_set_enumerated(adev);
adev->power.flags.ignore_parent = true;
- strlcpy(spi->modalias, acpi_device_hid(adev), sizeof(spi->modalias));
if (spi_add_device(spi)) {
adev->power.flags.ignore_parent = false;
dev_err(&master->dev, "failed to add SPI device %s from ACPI\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v5 2/3] i2c: acpi: Initialize info.type from of_compatible
From: Dan O'Donovan @ 2017-02-04 8:00 UTC (permalink / raw)
To: linux-acpi-u79uwXL29TY76Z2rM5mHXA, Rafael J . Wysocki,
Jarkko Nikula, Mika Westerberg, Mark Brown
Cc: Len Brown, linux-i2c-u79uwXL29TY76Z2rM5mHXA, Wolfram Sang,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dan O'Donovan
In-Reply-To: <1486195228-10929-1-git-send-email-dan-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org>
When using devicetree i2c_board_info.type is set to the compatible
string with the vendor prefix removed. For I2C devices described via
ACPI the i2c_board_info.type string is set to the ACPI device name. When
using ACPI and DT ids this string ends up something like "PRP0001:00".
If the of_compatible property is present try to use that instead. This
makes it easier to instantiate i2c drivers through ACPI with DT ids.
Signed-off-by: Dan O'Donovan <dan-M3NBUjLqch7QT0dZR+AlfA@public.gmane.org>
Reviewed-by: Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Tested-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Acked-by: Wolfram Sang <wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
---
drivers/i2c/i2c-core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 583e950..28d2181 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -221,7 +221,8 @@ static int i2c_acpi_get_info(struct acpi_device *adev,
acpi_dev_free_resource_list(&resource_list);
- strlcpy(info->type, dev_name(&adev->dev), sizeof(info->type));
+ acpi_set_modalias(adev, dev_name(&adev->dev),
+ info->type, sizeof(info->type));
return 0;
}
--
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" 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 v5 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
From: Dan O'Donovan @ 2017-02-04 8:00 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1486195228-10929-1-git-send-email-dan@emutex.com>
When using devicetree stuff like i2c_client.name or spi_device.modalias
is initialized to the first DT compatible id with the vendor prefix
stripped. Since some drivers rely on this try to replicate it when using
ACPI with DT ids.
Signed-off-by: Dan O'Donovan <dan@emutex.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
---
drivers/acpi/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
include/acpi/acpi_bus.h | 2 ++
2 files changed, 44 insertions(+)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 95855cb..80cb5eb 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -677,6 +677,48 @@ static bool acpi_of_match_device(struct acpi_device *adev,
return false;
}
+static bool acpi_of_modalias(struct acpi_device *adev,
+ char *modalias, size_t len)
+{
+ const union acpi_object *of_compatible;
+ const union acpi_object *obj;
+ const char *str, *chr;
+
+ of_compatible = adev->data.of_compatible;
+ if (!of_compatible)
+ return false;
+
+ if (of_compatible->type == ACPI_TYPE_PACKAGE)
+ obj = of_compatible->package.elements;
+ else /* Must be ACPI_TYPE_STRING. */
+ obj = of_compatible;
+
+ str = obj->string.pointer;
+ chr = strchr(str, ',');
+ strlcpy(modalias, chr ? chr + 1 : str, len);
+
+ return true;
+}
+
+/**
+ * acpi_set_modalias - Set modalias using "compatible" property or supplied ID
+ * @adev: ACPI device object to match
+ * @default_id: ID string to use as default if no compatible string found
+ * @modalias: Pointer to buffer that modalias value will be copied into
+ * @len: Length of modalias buffer
+ *
+ * This is a counterpart of of_modalias_node() for struct acpi_device objects.
+ * If there is a compatible string for @adev, it will be copied to @modalias
+ * with the vendor prefix stripped; otherwise, @default_id will be used.
+ */
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+ char *modalias, size_t len)
+{
+ if (!acpi_of_modalias(adev, modalias, len))
+ strlcpy(modalias, default_id, len);
+}
+EXPORT_SYMBOL_GPL(acpi_set_modalias);
+
static bool __acpi_match_device_cls(const struct acpi_device_id *id,
struct acpi_hardware_id *hwid)
{
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 4242c31..ef0ae8a 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -522,6 +522,8 @@ void acpi_bus_trim(struct acpi_device *start);
acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids);
+void acpi_set_modalias(struct acpi_device *adev, const char *default_id,
+ char *modalias, size_t len);
int acpi_create_dir(struct acpi_device *);
void acpi_remove_dir(struct acpi_device *);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 0/3] Init device ids from ACPI of_compatible
From: Dan O'Donovan @ 2017-02-04 8:00 UTC (permalink / raw)
To: linux-acpi, Rafael J . Wysocki, Jarkko Nikula, Mika Westerberg,
Mark Brown
Cc: Len Brown, linux-i2c, Wolfram Sang, linux-spi, linux-kernel,
Dan O'Donovan
In-Reply-To: <1485523815-9629-1-git-send-email-dan@emutex.com>
When using devicetree, stuff like i2c_client.name or spi_device.modalias
is initialized to the first DT compatible id with the vendor prefix
stripped. Since some drivers rely on this in order to differentiate between
hardware variants, try to replicate it when using ACPI with DT ids.
This also makes it so that the i2c_device_id parameter passed to probe is
non-NULL when matching with ACPI and DT ids.
Tested using ACPI overlays but there is no actual dependency. This series
just extends the PRP0001 feature to be more useful for I2C/SPI.
The patches only touches the ACPI-specific parts of the i2c and spi core.
Here is an example .dsl for an SPI accelerometer connected to minnowboard max:
Device (ACCL)
{
Name (_ADR, Zero)
Name (_HID, "PRP0001")
Name (_UID, One)
Method (_CRS, 0, Serialized)
{
Name (RBUF, ResourceTemplate ()
{
SPISerialBus(1, PolarityLow, FourWireMode, 16,
ControllerInitiated, 1000000, ClockPolarityLow,
ClockPhaseFirst, "\\_SB.SPI1",)
GpioInt (Edge, ActiveHigh, Exclusive, PullDown, 0x0000,
"\\_SB.GPO2", 0x00, ResourceConsumer, , )
{ // Pin list
1
}
})
Return (RBUF)
}
Name (_DSD, Package ()
{
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package ()
{
Package () {"compatible", "st,lis3dh"},
}
})
}
Credit to Leonard Crestez for the original version of this patch set.
Link to v4: http://www.spinics.net/lists/linux-acpi/msg71657.html
Changes:
* add acpi_set_modalias wrapper to handle fallback to ACPI ID (Mark Brown)
Link to v3: http://www.spinics.net/lists/linux-acpi/msg71531.html
Changes:
* Minor cosmetic/readability improvements (Andy Shevchenko)
Link to v2: https://lkml.org/lkml/2016/7/13/392
Changes:
* Use appropriate subject prefix for each subsystem (Mark Brown)
* Use ACPI info as before if getting OF info fails (Mark Brown)
* Minor cosmetic/readability improvements (Rafael J. Wysocki)
Link to v1: https://www.spinics.net/lists/linux-acpi/msg66469.html
Changes:
* Rebase on after acpi overlays got it.
* Change acpi_of_modalias outlen param to size_t
* Use {} after else
Dan O'Donovan (3):
ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
i2c: acpi: Initialize info.type from of_compatible
spi: acpi: Initialize modalias from of_compatible
drivers/acpi/bus.c | 42 ++++++++++++++++++++++++++++++++++++++++++
drivers/i2c/i2c-core.c | 3 ++-
drivers/spi/spi.c | 4 +++-
include/acpi/acpi_bus.h | 2 ++
4 files changed, 49 insertions(+), 2 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH v3 3/3] i2c: zx2967: add i2c controller driver for ZTE's zx2967 family
From: Shawn Guo @ 2017-02-04 7:15 UTC (permalink / raw)
To: Baoyou Xie
Cc: andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
jun.nie-QSEj5FYQhm4dnm+yROfE0A, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
xie.baoyou-Th6q7B73Y6EnDS1+zs4M5A,
chen.chaokai-Th6q7B73Y6EnDS1+zs4M5A,
wang.qiang01-Th6q7B73Y6EnDS1+zs4M5A
In-Reply-To: <1485435631-32642-3-git-send-email-baoyou.xie-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Only a couple of comments below. Otherwise, the patch looks fine to me.
On Thu, Jan 26, 2017 at 09:00:31PM +0800, Baoyou Xie wrote:
<snip>
> +static int zx2967_i2c_flush_fifos(struct zx2967_i2c_info *zx_i2c)
> +{
> + u32 val;
> + u32 offset;
> +
> + if (zx_i2c->msg_rd) {
> + offset = REG_RDCONF;
> + val = I2C_RFIFO_RESET;
> + } else {
> + offset = REG_WRCONF;
> + val = I2C_WFIFO_RESET;
> + }
> +
> + val |= zx2967_i2c_readl(zx_i2c, offset);
> + zx2967_i2c_writel(zx_i2c, val, offset);
> +
> + return 0;
Since the function will never return other values than 0, should we
simply define the return type as void?
> +}
<snip>
> +static int zx2967_i2c_probe(struct platform_device *pdev)
> +{
> + struct zx2967_i2c_info *zx_i2c;
> + void __iomem *reg_base;
> + struct resource *res;
> + struct clk *clk;
> + int ret;
> +
> + zx_i2c = devm_kzalloc(&pdev->dev, sizeof(*zx_i2c), GFP_KERNEL);
> + if (!zx_i2c)
> + return -ENOMEM;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + reg_base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(reg_base))
> + return PTR_ERR(reg_base);
> +
> + clk = devm_clk_get(&pdev->dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(&pdev->dev, "missing controller clock");
> + return PTR_ERR(clk);
> + }
> +
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to enable i2c_clk\n");
> + return ret;
> + }
> +
> + ret = platform_get_irq(pdev, 0);
> + if (ret < 0)
> + return ret;
> + zx_i2c->irq = ret;
> +
> + ret = device_property_read_u32(&pdev->dev, "clock-frequency",
> + &zx_i2c->clk_freq);
> + if (ret) {
> + dev_err(&pdev->dev, "missing clock-frequency");
> + return ret;
> + }
> +
> + zx_i2c->reg_base = reg_base;
> + zx_i2c->clk = clk;
> + zx_i2c->dev = &pdev->dev;
> +
> + zx_i2c->pin_ctrl = devm_pinctrl_get_select_default(&pdev->dev);
> + if (IS_ERR(zx_i2c->pin_ctrl)) {
> + if (PTR_ERR(zx_i2c->pin_ctrl) == -EPROBE_DEFER)
> + return -EPROBE_DEFER;
> + dev_info(&pdev->dev, "no pinctrl handle\n");
> + }
You do not need this boilerplate code for request "default" pin state,
since device core will automatically issue the call. See section "Pin
control requests from drivers" in Documentation/pinctrl.txt for more
info.
Shawn
--
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: SMBus not found
From: Hrvoje T @ 2017-02-04 1:55 UTC (permalink / raw)
To: Jean Delvare; +Cc: linux-i2c
In-Reply-To: <CANE=DpWovf4BO31Dqzj72hN=8etRLwFbyrDMZMzqzFNFtRe2cg@mail.gmail.com>
Hi Jean,
thank you for a quick reply.
My hardware is HP Probook 6470b laptop, kernel 4.8.0-37-generic,
lm-sensors version 1:3.4.0-3. I did
# modprobe i2c-dev
# i2cdetect -l
and got this:
i2c-3 i2c i915 gmbus dpc I2C adapter
i2c-1 i2c i915 gmbus vga I2C adapter
i2c-8 i2c DPDDC-D I2C adapter
i2c-6 i2c DPDDC-B I2C adapter
i2c-4 i2c i915 gmbus dpb I2C adapter
i2c-2 i2c i915 gmbus panel I2C adapter
i2c-0 i2c i915 gmbus ssc I2C adapter
i2c-7 i2c DPDDC-C I2C adapter
i2c-5 i2c i915 gmbus dpd I2C adapter
I would like to find out CAS latency times of my RAM memory modules
without opening the laptop. I guess those are on DIMM slots. dmidecode
-t memory gives:
Handle 0x0008, DMI type 17, 34 bytes
Memory Device
Array Handle: 0x0005
Error Information Handle: Not Provided
Total Width: 64 bits
Data Width: 64 bits
Size: 4096 MB
Form Factor: SODIMM
Set: None
Locator: Bottom-Slot 2(under)
Bank Locator: BANK 2
Type: DDR3
Type Detail: Synchronous
Speed: 1600 MHz
Manufacturer: Ramaxel
Serial Number: 44BBE80E
Asset Tag: 9876543210
Part Number: RMT3160ED58E9W1600
Rank: Unknown
Configured Clock Speed: Unknown
and lspci:
00:00.0 Host bridge: Intel Corporation 3rd Gen Core processor DRAM
Controller (rev 09)
00:02.0 VGA compatible controller: Intel Corporation 3rd Gen Core
processor Graphics Controller (rev 09)
00:14.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB xHCI Host Controller (rev 04)
00:16.0 Communication controller: Intel Corporation 7 Series/C210
Series Chipset Family MEI Controller #1 (rev 04)
00:19.0 Ethernet controller: Intel Corporation 82579V Gigabit Network
Connection (rev 04)
00:1a.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB Enhanced Host Controller #2 (rev 04)
00:1b.0 Audio device: Intel Corporation 7 Series/C210 Series Chipset
Family High Definition Audio Controller (rev 04)
00:1c.0 PCI bridge: Intel Corporation 7 Series/C210 Series Chipset
Family PCI Express Root Port 1 (rev c4)
00:1c.1 PCI bridge: Intel Corporation 7 Series/C210 Series Chipset
Family PCI Express Root Port 2 (rev c4)
00:1c.2 PCI bridge: Intel Corporation 7 Series/C210 Series Chipset
Family PCI Express Root Port 3 (rev c4)
00:1c.3 PCI bridge: Intel Corporation 7 Series/C210 Series Chipset
Family PCI Express Root Port 4 (rev c4)
00:1d.0 USB controller: Intel Corporation 7 Series/C210 Series Chipset
Family USB Enhanced Host Controller #1 (rev 04)
00:1f.0 ISA bridge: Intel Corporation HM76 Express Chipset LPC
Controller (rev 04)
00:1f.2 SATA controller: Intel Corporation 7 Series Chipset Family
6-port SATA Controller [AHCI mode] (rev 04)
23:00.0 FireWire (IEEE 1394): JMicron Technology Corp. IEEE 1394 Host
Controller (rev 30)
23:00.1 System peripheral: JMicron Technology Corp. SD/MMC Host
Controller (rev 30)
23:00.2 SD Host controller: JMicron Technology Corp. Standard SD Host
Controller (rev 30)
24:00.0 Network controller: Broadcom Limited BCM43228 802.11a/b/g/n
I know I could find out latency times on the manufacturer's web page,
but I would like to know is it possible via i2c? I'm noob in Linux and
bad at english, sorry for my mistakes and thanks for your help in
advance.
BR
Hrvoje
*trying without html from gmail one more time
^ permalink raw reply
* [PATCH] i2c: designwear: Fix clk warning on suspend/resume
From: John Stultz @ 2017-02-03 23:01 UTC (permalink / raw)
To: lkml
Cc: John Stultz, Jarkko Nikula, Andy Shevchenko, Mika Westerberg,
Wolfram Sang, linux-i2c
On my HiKey board, I'm seeing clk warnings on suspend/resume, which
seem to be caused by runtime pm suspending the device, then the same
suspend hook being called again on suspend time.
Thus this patch adds suspend state tracking to avoid runtime pm and
suspend causing double suspend calls on i2c-designware-platdrv.
Feedback would be greatly appreciated!
Cc: Jarkko Nikula <jarkko.nikula@linux.intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Wolfram Sang <wsa@the-dreams.de>
Cc: linux-i2c@vger.kernel.org
Signed-off-by: John Stultz <john.stultz@linaro.org>
---
drivers/i2c/busses/i2c-designware-core.h | 1 +
drivers/i2c/busses/i2c-designware-platdrv.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 26250b4..386cacb 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -126,6 +126,7 @@ struct dw_i2c_dev {
void (*release_lock)(struct dw_i2c_dev *dev);
bool pm_runtime_disabled;
bool dynamic_tar_update_enabled;
+ bool suspended;
};
#define ACCESS_SWAP 0x00000001
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6ce4313..54084fe 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -340,8 +340,12 @@ static int dw_i2c_plat_suspend(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+ if (i_dev->suspended)
+ return 0;
+
i2c_dw_disable(i_dev);
i2c_dw_plat_prepare_clk(i_dev, false);
+ i_dev->suspended = true;
return 0;
}
@@ -351,11 +355,16 @@ static int dw_i2c_plat_resume(struct device *dev)
struct platform_device *pdev = to_platform_device(dev);
struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev);
+ if (!i_dev->suspended)
+ return 0;
+
i2c_dw_plat_prepare_clk(i_dev, true);
if (!i_dev->pm_runtime_disabled)
i2c_dw_init(i_dev);
+ i_dev->suspended = false;
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] i2c: piix4: Request the SMBUS semaphore inside the mutex
From: Jean Delvare @ 2017-02-03 15:17 UTC (permalink / raw)
To: Ricardo Ribalda Delgado
Cc: Wolfram Sang, Andy Shevchenko, linux-i2c, linux-kernel
In-Reply-To: <20170202191516.4476-1-ricardo.ribalda@gmail.com>
Hi Ricardo,
On Thu, 2 Feb 2017 20:15:16 +0100, Ricardo Ribalda Delgado wrote:
> SMBSLVCNT must be protected with the piix4_mutex_sb800 in order to avoid
> multiple buses accessing to the semaphore at the same time.
>
> Reported-by: Jean Delvare <jdelvare@suse.de>
> Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
> ---
> drivers/i2c/busses/i2c-piix4.c | 12 +++++++-----
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
> index 73cc6799cc59..c21ca7bf2efe 100644
> --- a/drivers/i2c/busses/i2c-piix4.c
> +++ b/drivers/i2c/busses/i2c-piix4.c
> @@ -592,6 +592,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> u8 port;
> int retval;
>
> + mutex_lock(&piix4_mutex_sb800);
> +
> /* Request the SMBUS semaphore, avoid conflicts with the IMC */
> smbslvcnt = inb_p(SMBSLVCNT);
> do {
> @@ -605,10 +607,10 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
> usleep_range(1000, 2000);
> } while (--retries);
> /* SMBus is still owned by the IMC, we give up */
> - if (!retries)
> + if (!retries) {
> + mutex_unlock(&piix4_mutex_sb800);
> return -EBUSY;
> -
> - mutex_lock(&piix4_mutex_sb800);
> + }
>
> outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
> smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
> @@ -623,11 +625,11 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
>
> outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1);
>
> - mutex_unlock(&piix4_mutex_sb800);
> -
> /* Release the semaphore */
> outb_p(smbslvcnt | 0x20, SMBSLVCNT);
>
> + mutex_unlock(&piix4_mutex_sb800);
> +
> return retval;
> }
>
Thanks for the quick fix.
Signed-off-by: Jean Delvare <jdelvare@suse.de>
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: SMBus not found
From: Jean Delvare @ 2017-02-03 10:53 UTC (permalink / raw)
To: Hrvoje T, linux-i2c
In-Reply-To: <CANE=DpU===GBGNY0+QJCKuw295kkYSEPN-oeaT7cG6DRcNdjiA@mail.gmail.com>
Hi Hrvoje,
Please use plain text (no HTML) when writing to development lists.
On jeu., 2017-02-02 at 14:50 +0100, Hrvoje T wrote:
> When I run sensors-detect I get an answer:
> Do you want to probe the I2C/SMBus adapters now? (YES/no): yes
> Sorry, no supported PCI bus adapters found.
> Is tehere anything else I can try?
You did not tell which version of sensors-detect you tried, nor what
kernel you are running, nor what your hardware is. What kind of help
are you expecting?
You should probably not be using sensors-detect in the first place
anyway. i2cdetect (from the i2c-tools package) is a much better tool
for what you are trying to do.
> Is it possible to dump addresses 0x50-0x57 on this bus with i2cdump
> and run decode-dimms -x on them? How could I do this when I don't
> know which bus is SMBus?
This question makes no sense as "this bus" is clearly undefined at this
point. First check what I2C/SMBus busses are available on your system:
# modprobe i2c-dev
# i2cdetect -l
Then either run i2cdetect / i2cdump on the relevant bus if it was
listed, or try to figure out why it is not listed (looking for it with
lspci maybe, if your SMBus controller is a PCI device.)
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply
* Re: [PATCH v8 07/12] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
From: Peter Rosin @ 2017-02-03 8:25 UTC (permalink / raw)
To: Rob Herring
Cc: linux-kernel@vger.kernel.org, Wolfram Sang, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Andrew Morton,
linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
linux-iio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <CAL_JsqLCdM5s8FYqys-m5wpr4QeeE-rQaRXCt+rjQz3qm+E2qA@mail.gmail.com>
On 2017-02-02 17:08, Rob Herring wrote:
> On Tue, Jan 31, 2017 at 1:36 AM, Peter Rosin <peda@axentia.se> wrote:
>> If you see this new driver as something that is superseding the existing
>> i2c-mux-gpio driver, I'm sad to inform you that the code is not simply
>> not there. i2c-mux-gpio has acpi support and users may provide platform
>> data from code. The existing i2c-mux-gpio driver also has the below
>> mentioned locking heuristic. Adding all those things to the new driver
>> would make it big and unwieldy and having even more unwanted tentacles
>> into other subsystems. And why should it be only i2c-mux-gpio that is
>> merged into this new i2c-mux driver? Why not implement a mux-pinctrl
>> driver for the new mux subsustem and then merge i2c-mux-pinctrl as well?
>> I say no, that way lies madness.
>
> Sounds like a good idea to me. I'm not saying you need to merge any of
> them right now though (that's Wolfram's call).
If we're pedantic I probably have some stake in it too, being the i2c-mux
maintainer and all. But, agreed, I arrived quite late to the Linux kernel
party and my opinion might perhaps not carry all that much weight...
> None of this has anything to do with the binding though. Compatible
> strings should be specific. That's not up for debate. Whether the
Ok, I'm going to focus on the compatible string for a minute and leave
the implementation details for some other discussion.
> driver bound to a compatible string is common or specific to that
> compatible string is completely up to the OS. That decision can change
> over time, but the binding should not.
So, there's the existing compatible "i2c-mux-gpio" ("i2c-gpio-mux" is
wrong) that you seem to suggest is what I should stick to. I object to
that.
As you say, the bindings and compatible strings should describe hardware,
and you also state they should be specific. I agree. But, why are you
then apparently suggesting (by implication) that for this (hypothetical)
hardware...
.----.
|SDA0|-----------.
|SCL0|---------. |
| | | |
| | .-------.
| | |adg792a|
| | | |
|ADC0|------|D1 S1A|---- signal A
| | | S1B|---- signal B
| | | S1C|---- signal C
| | | S1D|---- signal D
| | | |
|SDA1|---+--|D2 S2A|---- i2s segment foo A
|SCL1|-. | | S2B|---- i2s segment foo B
'----' | | | S2C|---- i2s segment foo C
| | | S2D|---- i2s segment foo D
| | | |
| '--|D3 S3A|---- i2s segment bar A
| | S3B|---- i2s segment bar B
| | S3C|---- i2s segment bar C
| | S3D|---- i2s segment bar D
| '-------'
| A B C D A B C D (feed SCL1 to each of
| | | | | | | | | the 8 muxed segments)
'------------------+-+-+-+---+-+-+-'
...the devicetree should be like below?
&i2c0 {
mux: mux-controller@50 {
compatible = "adi,adg792a";
reg = <0x50>;
#mux-control-cells = <1>;
};
};
adc-mux {
compatible = "io-channel-mux";
io-channels = <&adc 0>;
io-channel-names = "parent";
mux-controls = <&mux 0>;
...
};
i2c-mux-foo {
compatible = "i2c-mux-gpio";
i2c-parent = <&i2c1>;
mux-controls = <&mux 1>;
...
};
i2c-mux-bar {
compatible = "i2c-mux-gpio";
i2c-parent = <&i2c1>;
mux-controls = <&mux 2>;
...
};
There must be some disconnect, because those "i2c-mux-gpio" compatible
strings are just dead wrong. There simply are no gpio pins involved at
all and that "gpio" suffix is just totally out of place.
So, since you are not happy with "i2c-mux-simple", "i2c-mux-generic" or
"i2c-mux" that I have suggested, can you please come up with something
that is good enough for the above?
Or, are you /really/ suggesting "i2c-mux-gpio"?
Cheers,
peda
^ permalink raw reply
* [PATCH] i2c: piix4: Request the SMBUS semaphore inside the mutex
From: Ricardo Ribalda Delgado @ 2017-02-02 19:15 UTC (permalink / raw)
To: Jean Delvare, Wolfram Sang, Andy Shevchenko, linux-i2c,
linux-kernel
Cc: Ricardo Ribalda Delgado
SMBSLVCNT must be protected with the piix4_mutex_sb800 in order to avoid
multiple buses accessing to the semaphore at the same time.
Reported-by: Jean Delvare <jdelvare@suse.de>
Signed-off-by: Ricardo Ribalda Delgado <ricardo.ribalda@gmail.com>
---
drivers/i2c/busses/i2c-piix4.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/drivers/i2c/busses/i2c-piix4.c b/drivers/i2c/busses/i2c-piix4.c
index 73cc6799cc59..c21ca7bf2efe 100644
--- a/drivers/i2c/busses/i2c-piix4.c
+++ b/drivers/i2c/busses/i2c-piix4.c
@@ -592,6 +592,8 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
u8 port;
int retval;
+ mutex_lock(&piix4_mutex_sb800);
+
/* Request the SMBUS semaphore, avoid conflicts with the IMC */
smbslvcnt = inb_p(SMBSLVCNT);
do {
@@ -605,10 +607,10 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
usleep_range(1000, 2000);
} while (--retries);
/* SMBus is still owned by the IMC, we give up */
- if (!retries)
+ if (!retries) {
+ mutex_unlock(&piix4_mutex_sb800);
return -EBUSY;
-
- mutex_lock(&piix4_mutex_sb800);
+ }
outb_p(piix4_port_sel_sb800, SB800_PIIX4_SMB_IDX);
smba_en_lo = inb_p(SB800_PIIX4_SMB_IDX + 1);
@@ -623,11 +625,11 @@ static s32 piix4_access_sb800(struct i2c_adapter *adap, u16 addr,
outb_p(smba_en_lo, SB800_PIIX4_SMB_IDX + 1);
- mutex_unlock(&piix4_mutex_sb800);
-
/* Release the semaphore */
outb_p(smbslvcnt | 0x20, SMBSLVCNT);
+ mutex_unlock(&piix4_mutex_sb800);
+
return retval;
}
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v8 07/12] dt-bindings: i2c: i2c-mux-simple: document i2c-mux-simple bindings
From: Rob Herring @ 2017-02-02 16:08 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel@vger.kernel.org, Wolfram Sang, Mark Rutland,
Jonathan Cameron, Hartmut Knaack, Lars-Peter Clausen,
Peter Meerwald-Stadler, Jonathan Corbet, Andrew Morton,
linux-i2c@vger.kernel.org, devicetree@vger.kernel.org,
linux-iio@vger.kernel.org, linux-doc@vger.kernel.org
In-Reply-To: <cdc958c3-2496-be52-6407-bf9aefdfa965@axentia.se>
On Tue, Jan 31, 2017 at 1:36 AM, Peter Rosin <peda@axentia.se> wrote:
> On 2017-01-30 18:20, Rob Herring wrote:
>> On Sat, Jan 28, 2017 at 4:42 PM, Peter Rosin <peda@axentia.se> wrote:
>>> On 2017-01-27 20:39, Rob Herring wrote:
>>>> On Wed, Jan 18, 2017 at 04:57:10PM +0100, Peter Rosin wrote:
>>>>> Describe how a generic multiplexer controller is used to mux an i2c bus.
>>>>>
>>>>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>>>>> Signed-off-by: Peter Rosin <peda@axentia.se>
>>>>> ---
>>>>> .../devicetree/bindings/i2c/i2c-mux-simple.txt | 81 ++++++++++++++++++++++
>>>>> 1 file changed, 81 insertions(+)
>>>>> create mode 100644 Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>>> new file mode 100644
>>>>> index 000000000000..253d5027843b
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/i2c/i2c-mux-simple.txt
>>>>> @@ -0,0 +1,81 @@
>>>>> +Simple I2C Bus Mux
>>>>> +
>>>>> +This binding describes an I2C bus multiplexer that uses a mux controller
>>>>> +from the mux subsystem to route the I2C signals.
>>>>> +
>>>>> + .-----. .-----.
>>>>> + | dev | | dev |
>>>>> + .------------. '-----' '-----'
>>>>> + | SoC | | |
>>>>> + | | .--------+--------'
>>>>> + | .------. | .------+ child bus A, on MUX value set to 0
>>>>> + | | I2C |-|--| Mux |
>>>>> + | '------' | '--+---+ child bus B, on MUX value set to 1
>>>>> + | .------. | | '----------+--------+--------.
>>>>> + | | MUX- | | | | | |
>>>>> + | | Ctrl |-|-----+ .-----. .-----. .-----.
>>>>> + | '------' | | dev | | dev | | dev |
>>>>> + '------------' '-----' '-----' '-----'
>>>>> +
>>>>> +Required properties:
>>>>> +- compatible: i2c-mux-simple,mux-locked or i2c-mux-simple,parent-locked
>>>>
>>>> Not a fan of using "simple" nor the ','. Perhaps lock type should be
>>>> separate property.
>>>
>>> How about just i2c-mux for the compatible? Because i2c-mux-mux (which
>>> follows the naming of previous i2c muxes) looks really stupid. Or
>>> perhaps i2c-mux-generic?
>>
>> I like "generic" only slightly more than "simple". :)
>>
>> If the mux is gpio controlled, then it should still be called
>> i2c-gpio-mux. Let's not invent brand new bindings when current ones
>> are easily extended. We already have pretty generic names here, let's
>> not make them more generic.
>
> Ahh, but the whole point of this new driver is that it does not know
> if the mux is gpio controlled, if it is controlled by an adg792 chip
> or whatever might appear down the line. So, I think i2c-mux-gpio (and
> i2c-gpio-mux) is actively wrong.
That's fine for the driver, but the DT bindings should reflect the
h/w. Be happy that i2c-gpio-mux is allowed. I could easily say it
needs to be actual part numbers for compatible strings.
How is it okay that we now have 2 drivers for the same thing: the
i2c-gpio-mux driver and this i2c mux-controller driver with a gpio
mux-controller driver. I'm surprised Wolfram is okay with that.
> This new driver is in fact an i2c-mux driver that (in this aspect) is more
> generic than any of the existing i2c-mux drivers, hence my suggestion.
If it is more generic, then it should be able to replace existing drivers.
> If you see this new driver as something that is superseding the existing
> i2c-mux-gpio driver, I'm sad to inform you that the code is not simply
> not there. i2c-mux-gpio has acpi support and users may provide platform
> data from code. The existing i2c-mux-gpio driver also has the below
> mentioned locking heuristic. Adding all those things to the new driver
> would make it big and unwieldy and having even more unwanted tentacles
> into other subsystems. And why should it be only i2c-mux-gpio that is
> merged into this new i2c-mux driver? Why not implement a mux-pinctrl
> driver for the new mux subsustem and then merge i2c-mux-pinctrl as well?
> I say no, that way lies madness.
Sounds like a good idea to me. I'm not saying you need to merge any of
them right now though (that's Wolfram's call).
None of this has anything to do with the binding though. Compatible
strings should be specific. That's not up for debate. Whether the
driver bound to a compatible string is common or specific to that
compatible string is completely up to the OS. That decision can change
over time, but the binding should not.
>>> I'm also happy to have the lock type as a separate property. One lock
>>> type, e.g. parent-locked, could be the default and adding a 'mux-locked'
>>> property could change that. Would that be ok?
>>
>> I prefer this. Then existing bindings can use it.
>>
>>> Or should it be a requirement that one of 'mux-locked'/'parent-locked'
>>> is always present?
>>
>> I would make it boolean and make not present be the more common case.
>> Not present could also mean determined via other means as you have
>> today with existing bindings. Maybe then you need both properties.
>
> Ok. Lets define that every type of i2c-mux have a default locking.
> If no property is found that overrides this default, a heuristic may
> be applied that is only allowed to very defensively use non-default
> locking (like the current heuristics in i2c-mux-gpio/i2c-mux-pinctrl
> attempt to do, they only adjust the locking for impossible cases that
> would invariably lead to a deadlock with the default locking). Then,
> there should never exist a need to enforce the default locking with
> a property.
>
> And given that no current i2c-mux that is mux-locked by default even
> have the ability to be parent-locked, there is (currently) no need
> for more than a bool 'mux-locked' property.
Okay, good.
>>>> I'm not sure I get the mux vs. parent locked fully. How do I determine
>>>> what type I have? We already have bindings for several types of i2c
>>>> muxes. How does the locking annotation fit into those?
>>>
>>> We have briefly discussed this before [1] in the context of i2c-mux-gpio
>>> and i2c-mux-pinctrl, when I added the mux-locked/parent-locked distinction
>>> to the i2c-mux infrastructure (it wasn't named mux-locked/parent-locked
>>> way back then though). There is more detail on what the difference is
>>> between the two in Documentation/i2c/i2c-topology.
>>>
>>> Side note regarding your remark "use an I2C controlled mux instead"; it
>>> appears that I'm not alone [2] with this kind of requirement...
>>>
>>> [1] https://lkml.org/lkml/2016/1/6/437
>>> [2] http://marc.info/?t=147877959100002&r=1&w=2
>>>
>>> But, now that I have pondered on this for a year or so, I firmly
>>> believe it was a mistake to have the code in i2c-mux-gpio and
>>> i2c-mux-pinctrl automatically try to deduce if the mux should be
>>> mux-locked or parent-locked. It might be easy to make that call
>>> in some trivial cases, but it is not difficult to dream up
>>> scenarios where it would be extremely hard for the code to get
>>> this decision right. It's just fragile. But now we have code in
>>> those two muxes that has unwanted tentacles into the guts of the
>>> gpio and pinctrl subsystems. Hopefully those unwanted tentacles
>>> can be replaced with something based on device links? However, it
>>> is still not hard to come up with scenarios that will require
>>> manual intervention in order to select the right kind of i2c mux
>>> locking. So, I fear that we have inadequate code trying to make a
>>> decision automatically, and that we at some point down the line
>>> will have some impossible case needing a binding that trumps the
>>> heuristic. Why have a heuristic at all in that case? In short, it
>>> should have been a binding from the start, methinks.
>>>
>>> That was a long rant regarding i2c-mux-gpio and i2c-mux-pinctrl.
>>> I obviously think it is bad to have this new i2c mux go in the
>>> same direction as those two drivers. I.e. I think a binding that
>>> specifies the desired locking is preferable to any attempted
>>> heuristic.
>>
>> Having a separate, common property(ies) for these would solve this then.
>
> Yep, the plan is now to make this new i2c-mux parent-locked by default
> and then look for an optional mux-locked property, thus eliminating the
> comma and the suffix in the compatible string, leaving only whatever is
> eventually picked from 'i2c-mux-simple', 'i2c-mux', 'i2c-mux-generic' and
> 'i2c-mux-whatever'.
>
> Do I need to re-document the mux-locked/parent-locked difference? I.e.,
> is it enough to refer to Documentation/i2c/i2c-topology, or does the
> Documentation/devicetree subdir need to self-contained?
We do try to keep things self contained, but I'm okay with not
duplicating everything in the binding. Just try to summarize what
mux-locked means.
Rob
^ permalink raw reply
* Re: [PATCH v4 1/3] ACPI / bus: Export acpi_of_modalias equiv of of_modalias_node
From: Dan O'Donovan @ 2017-02-02 13:30 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: linux-acpi, Andy Shevchenko, Jarkko Nikula, Mika Westerberg,
Mark Brown, Len Brown, linux-i2c, Wolfram Sang, linux-spi,
linux-kernel
In-Reply-To: <2538517.VnXrn8zi1x@aspire.rjw.lan>
On 02/02/2017 11:41 AM, Rafael J. Wysocki wrote:
> On Friday, January 27, 2017 01:30:13 PM Dan O'Donovan wrote:
>> From: Crestez Dan Leonard <leonard.crestez@intel.com>
>>
>> When using devicetree stuff like i2c_client.name or spi_device.modalias
>> is initialized to the first DT compatible id with the vendor prefix
>> stripped. Since some drivers rely on this try to replicate it when using
>> ACPI with DT ids.
>>
>> Signed-off-by: Crestez Dan Leonard <leonard.crestez@intel.com>
>> Signed-off-by: Dan O'Donovan <dan@emutex.com>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>> Reviewed-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
>> Tested-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> First off, the original author of the patch doesn't seem to be reachable under
> the Intel address any more, so his S-o-b is meaningless and it shouldn't be
> there. In such cases you can send the patch as From: you and give a credit
> to the original author in the changelog.
>
> Second, as Mark said ->
>
>> ---
>> drivers/acpi/bus.c | 35 +++++++++++++++++++++++++++++++++++
>> include/acpi/acpi_bus.h | 1 +
>> 2 files changed, 36 insertions(+)
>>
>> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
>> index 95855cb..8b9657f 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -677,6 +677,41 @@ static bool acpi_of_match_device(struct acpi_device *adev,
>> return false;
>> }
>>
>> +/**
>> + * acpi_of_modalias - Like of_modalias_node for ACPI with DT ids
>> + * @adev: ACPI device object to match.
>> + * @outstr: Pointer to buffer for result
>> + * @outlen: Length of outstr value
>> + *
>> + * This is a counterpart of of_modalias_node() for struct acpi_device
>> + * objects. If there is a compatible string for @adev, copy it to the
>> + * @outstr location with the vendor prefix stripped.
>> + *
>> + * Returns 0 on success or negative errno on failure.
>> + */
>> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen)
> -> this could be a bool function and you could provide something like
> acpi_set_modalias() doing the entire
>
> if (!acpi_of_modalias(adev, outstr, outlen))
> strlcpy(outstr, dev_name(&adev->dev), outlen);
>
> thing, so that its users don't have to duplicate this conditional.
>
> And then acpi_of_modalias() could be static and the only thing to export
> would be acpi_set_modalias().
Thanks Mark and Rafael for the latest feedback. Those suggestions make
sense. I'll send an updated patch-set soon.
>
>> +{
>> + const union acpi_object *of_compatible;
>> + const union acpi_object *obj;
>> + const char *str, *chr;
>> +
>> + of_compatible = adev->data.of_compatible;
>> + if (!of_compatible)
>> + return -ENODEV;
>> +
>> + if (of_compatible->type == ACPI_TYPE_PACKAGE)
>> + obj = of_compatible->package.elements;
>> + else /* Must be ACPI_TYPE_STRING. */
>> + obj = of_compatible;
>> +
>> + str = obj->string.pointer;
>> + chr = strchr(str, ',');
>> + strlcpy(outstr, chr ? chr + 1 : str, outlen);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(acpi_of_modalias);
>> +
>> static bool __acpi_match_device_cls(const struct acpi_device_id *id,
>> struct acpi_hardware_id *hwid)
>> {
>> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
>> index 4242c31..351b4a4 100644
>> --- a/include/acpi/acpi_bus.h
>> +++ b/include/acpi/acpi_bus.h
>> @@ -522,6 +522,7 @@ void acpi_bus_trim(struct acpi_device *start);
>> acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
>> int acpi_match_device_ids(struct acpi_device *device,
>> const struct acpi_device_id *ids);
>> +int acpi_of_modalias(struct acpi_device *adev, char *outstr, size_t outlen);
>> int acpi_create_dir(struct acpi_device *);
>> void acpi_remove_dir(struct acpi_device *);
> Thanks,
> Rafael
>
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox