* [PATCH 1/1] i2c: designware: Consolidate default functionality bits
From: Alexander Stein @ 2016-11-21 10:43 UTC (permalink / raw)
To: Jarkko Nikula, Andy Shevchenko, Mika Westerberg, Wolfram Sang
Cc: Alexander Stein, linux-i2c
Use a common place for default functionality bits for both platform
and pci driver.
Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
---
This patch requires commit c3ae106050b9 ("i2c: designware: Implement support
for SMBus block read and write") which is included in current i2c/for-next
branch.
BTW: Do merrifield and medfield actually not support 10bit addressing?
drivers/i2c/busses/i2c-designware-core.h | 8 ++++++++
drivers/i2c/busses/i2c-designware-pcidrv.c | 9 +--------
drivers/i2c/busses/i2c-designware-platdrv.c | 9 +--------
3 files changed, 10 insertions(+), 16 deletions(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 0d44d2a..26250b4 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -22,6 +22,14 @@
*
*/
+#include <linux/i2c.h>
+
+#define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
+ I2C_FUNC_SMBUS_BYTE | \
+ I2C_FUNC_SMBUS_BYTE_DATA | \
+ I2C_FUNC_SMBUS_WORD_DATA | \
+ I2C_FUNC_SMBUS_BLOCK_DATA | \
+ I2C_FUNC_SMBUS_I2C_BLOCK)
#define DW_IC_CON_MASTER 0x1
#define DW_IC_CON_SPEED_STD 0x2
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 8ffe2da..300802e7 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -71,13 +71,6 @@ struct dw_pci_controller {
DW_IC_CON_SLAVE_DISABLE | \
DW_IC_CON_RESTART_EN)
-#define DW_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C | \
- I2C_FUNC_SMBUS_BYTE | \
- I2C_FUNC_SMBUS_BYTE_DATA | \
- I2C_FUNC_SMBUS_WORD_DATA | \
- I2C_FUNC_SMBUS_BLOCK_DATA | \
- I2C_FUNC_SMBUS_I2C_BLOCK)
-
/* Merrifield HCNT/LCNT/SDA hold time */
static struct dw_scl_sda_cfg mrfld_config = {
.ss_hcnt = 0x2f8,
@@ -250,7 +243,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
}
dev->functionality = controller->functionality |
- DW_DEFAULT_FUNCTIONALITY;
+ DW_IC_DEFAULT_FUNCTIONALITY;
dev->master_cfg = controller->bus_cfg;
if (controller->scl_sda_cfg) {
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 7429dfa..08153ea 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -219,14 +219,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
if (r)
return r;
- dev->functionality =
- I2C_FUNC_I2C |
- I2C_FUNC_10BIT_ADDR |
- I2C_FUNC_SMBUS_BYTE |
- I2C_FUNC_SMBUS_BYTE_DATA |
- I2C_FUNC_SMBUS_WORD_DATA |
- I2C_FUNC_SMBUS_BLOCK_DATA |
- I2C_FUNC_SMBUS_I2C_BLOCK;
+ dev->functionality = I2C_FUNC_10BIT_ADDR | DW_IC_DEFAULT_FUNCTIONALITY;
dev->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
DW_IC_CON_RESTART_EN;
--
2.7.3
^ permalink raw reply related
* Re: [PATCH 1/2] i2c: designware: report short transfers
From: Mika Westerberg @ 2016-11-21 10:32 UTC (permalink / raw)
To: Russell King
Cc: Andrew Jackson, Liviu Dudau, Wolfram Sang, Jarkko Nikula,
Andy Shevchenko, linux-i2c, linux-arm-kernel
In-Reply-To: <E1c7p12-0000R0-S8@rmk-PC.armlinux.org.uk>
On Fri, Nov 18, 2016 at 07:40:04PM +0000, Russell King wrote:
> Rather than reporting success for a short transfer due to interrupt
> latency, report an error both to the caller, as well as to the kernel
> log.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
^ permalink raw reply
* Re: [BUG] i2c-designware silently fails on long transfers
From: Mika Westerberg @ 2016-11-21 10:29 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Andrew Jackson, Liviu Dudau, Wolfram Sang, Jarkko Nikula,
Andy Shevchenko, linux-i2c, linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>
On Fri, Nov 18, 2016 at 07:35:42PM +0000, Russell King - ARM Linux wrote:
> With reference to this commit:
>
> commit d39f77b06a712fcba6185a20bb209e357923d980
> Author: Andrew Jackson <Andrew.Jackson@arm.com>
> Date: Fri Nov 7 12:10:44 2014 +0000
>
> i2c: designware: prevent early stop on TX FIFO empty
>
> If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN
> set to zero, allowing the TX FIFO to become empty causes a STOP
> condition to be generated on the I2C bus. If the transmit FIFO
> threshold is set too high, an erroneous STOP condition can be
> generated on long transfers - particularly where the interrupt
> latency is extended.
>
> Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>
> Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
> Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
>
> The TDA998x driver issues long I2C transfers to read the EDID from the
> device - and userspace can also issue large transfers too. However,
> if a DW core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set as
> zero, the above commit doesn't seem to solve the problem. During
> boot, with the patch below, I see:
>
> [ 1.736549] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x10
> [ 1.736564] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
> [ 1.736608] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [ 1.736799] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
> [ 1.736819] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
> ...
> [ 1.737986] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [ 1.738010] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
> [ 1.738034] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
> [ 1.738039] random: fast init done
> [ 1.740120] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x714
> [ 1.740231] i2c_dw_xfer: ffffffc97657b770:1 -> ffffffc97657b770:1 (0:0) [0 0 3 0] 8 [tx:ffffffc976682380:47] [rx:ffffffc9766823c9:55]
> [ 1.740249] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 93
> [ 1.746979] Raw EDID:
> [ 1.747934] 00 ff ff ff ff ff ff 00 34 a9 96 a2 01 01 01 01
> [ 1.752342] 00 17 01 03 80 80 48 78 0a da ff a3 58 4a a2 29
> [ 1.756748] 17 49 4b 21 08 00 31 40 45 40 61 40 81 80 01 01
> [ 1.761153] 01 01 01 01 01 01 02 3a 80 d0 72 38 2d 40 10 2c
> [ 1.765555] 45 80 ba 88 21 00 00 1e 02 00 d0 4e 30 09 12 54
> [ 1.769958] 01 08 02 00 23 36 01 40 01 05 00 80 a1 4c 4b 49
> [ 1.774361] 22 00 00 40 03 00 28 00 23 01 20 00 01 88 00 01
> [ 1.778762] 08 00 00 40 00 02 03 04 0a 00 80 00 02 00 00 40
>
> The significant thing is the "i2c_dw_xfer" line, where I add a print of
> the current state. Here, we can see that the transfer is mid-way, but
> a stop condition has been generated by the hardware, leaving 55 bytes
> to be received.
>
> Unfortunately, the i2c-designware driver ignores this, and believes that
> the transfer completed both fully and successfully, but returns bogus
> data to userspace or the kernel driver. That's really _bad_ behaviour
> by the driver - it should at least return an error.
Totally agree.
> This problem is _soo_ bad that on my Juno, I can't run Xorg (it hits
> this every time we try to read the EDID) nor can I boot with the TV
> connected (it hits this every boot as well.)
>
> I'd go as far as to say that the i2c-designware hardware, when
> configured with this option set to zero, is fundamentally broken for OS
> which do not provide any guarantee for interrupt latency, such as Linux.
>
> The commit above tries to mitigate this by reducing the Tx FIFO
> threshold, so the interrupt is raised sooner, but that's clearly not
> enough for reliable operation.
>
> Another mitigation would be to lower the I2C bus frequency on Juno from
> 400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
> However, even that isn't going to be reliable - even going to 100kHz
> isn't going to allow the above case to be solved - the interrupt is
> delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
> at 100kHz. (9 * 16 / (100*10^3)).
>
> So, I think all hope is lost for i2c-designware on Juno to cope with
> reading the EDID from TDA998x reliably.
:-(
I wonder if we can get it work more reliably by using DMA (provided that
there are DMA channels available for I2C in Juno)? That would allow the
hardware to perform longer reads without relying on how fast the
interrupt handler is able to empty the Rx FIFO.
^ permalink raw reply
* [Patch V5 2/2] i2c: imx: add low power i2c bus driver
From: Gao Pan @ 2016-11-21 7:51 UTC (permalink / raw)
To: wsa, u.kleine-koenig, cmo, robh, vz, wsa-dev
Cc: linux-i2c, pandy.gao, frank.li, fugang.duan
In-Reply-To: <1479714704-3014-1-git-send-email-pandy.gao@nxp.com>
This patch adds lpi2c bus driver to support new i.MX products
which use lpi2c instead of the old imx i2c.
The lpi2c can continue operating in stop mode when an appropriate
clock is available. It is also designed for low CPU overhead with
DMA offloading of FIFO register accesses.
Signed-off-by: Gao Pan <pandy.gao@nxp.com>
Reviewed-by: Fugang Duan <B38611@freescale.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
---
V2:
-stop i2c transfer under the wrong condition
-add timeout check in while() domain
V3:
-fix typo inside commit message and the driver.
V4:
As Vladimir Zapolskiy's review, the version do below changes:
-split devicetree binding with driver
-prepare clk in probe()
-replace "1<<n" BIT(n)
-remove unnecessary variables
-fix typo
-use module_platform_driver()
-use i2c_add_adapter()
V5:
-unprepare clk in lpi2c_imx_remove()
-disable clk when lpi2c_imx_config() fails
-unprepare clk when i2c_add_adapter() fails
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-imx-lpi2c.c | 649 +++++++++++++++++++++++++++++++++++++
3 files changed, 660 insertions(+)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..7cff9ec 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -597,6 +597,16 @@ config I2C_IMX
This driver can also be built as a module. If so, the module
will be called i2c-imx.
+config I2C_IMX_LPI2C
+ tristate "IMX Low Power I2C interface"
+ depends on ARCH_MXC || COMPILE_TEST
+ help
+ Say Y here if you want to use the Low Power IIC bus controller
+ on the Freescale i.MX processors.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-imx-lpi2c.
+
config I2C_IOP3XX
tristate "Intel IOPx3xx and IXP4xx on-chip I2C interface"
depends on ARCH_IOP32X || ARCH_IOP33X || ARCH_IXP4XX || ARCH_IOP13XX
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 29764cc..05195cd 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -56,6 +56,7 @@ obj-$(CONFIG_I2C_HIX5HD2) += i2c-hix5hd2.o
obj-$(CONFIG_I2C_IBM_IIC) += i2c-ibm_iic.o
obj-$(CONFIG_I2C_IMG) += i2c-img-scb.o
obj-$(CONFIG_I2C_IMX) += i2c-imx.o
+obj-$(CONFIG_I2C_IMX_LPI2C) += i2c-imx-lpi2c.o
obj-$(CONFIG_I2C_IOP3XX) += i2c-iop3xx.o
obj-$(CONFIG_I2C_JZ4780) += i2c-jz4780.o
obj-$(CONFIG_I2C_KEMPLD) += i2c-kempld.o
diff --git a/drivers/i2c/busses/i2c-imx-lpi2c.c b/drivers/i2c/busses/i2c-imx-lpi2c.c
new file mode 100644
index 0000000..af5d1b6
--- /dev/null
+++ b/drivers/i2c/busses/i2c-imx-lpi2c.c
@@ -0,0 +1,649 @@
+/*
+ * This is i.MX low power i2c controller driver.
+ *
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ *
+ * 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/clk.h>
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+
+#define DRIVER_NAME "imx-lpi2c"
+
+#define LPI2C_PARAM 0x04 /* i2c RX/TX FIFO size */
+#define LPI2C_MCR 0x10 /* i2c contrl register */
+#define LPI2C_MSR 0x14 /* i2c status register */
+#define LPI2C_MIER 0x18 /* i2c interrupt enable */
+#define LPI2C_MCFGR0 0x20 /* i2c master configuration */
+#define LPI2C_MCFGR1 0x24 /* i2c master configuration */
+#define LPI2C_MCFGR2 0x28 /* i2c master configuration */
+#define LPI2C_MCFGR3 0x2C /* i2c master configuration */
+#define LPI2C_MCCR0 0x48 /* i2c master clk configuration */
+#define LPI2C_MCCR1 0x50 /* i2c master clk configuration */
+#define LPI2C_MFCR 0x58 /* i2c master FIFO control */
+#define LPI2C_MFSR 0x5C /* i2c master FIFO status */
+#define LPI2C_MTDR 0x60 /* i2c master TX data register */
+#define LPI2C_MRDR 0x70 /* i2c master RX data register */
+
+/* i2c command */
+#define TRAN_DATA 0X00
+#define RECV_DATA 0X01
+#define GEN_STOP 0X02
+#define RECV_DISCARD 0X03
+#define GEN_START 0X04
+#define START_NACK 0X05
+#define START_HIGH 0X06
+#define START_HIGH_NACK 0X07
+
+#define MCR_MEN BIT(0)
+#define MCR_RST BIT(1)
+#define MCR_DOZEN BIT(2)
+#define MCR_DBGEN BIT(3)
+#define MCR_RTF BIT(8)
+#define MCR_RRF BIT(9)
+#define MSR_TDF BIT(0)
+#define MSR_RDF BIT(1)
+#define MSR_SDF BIT(9)
+#define MSR_NDF BIT(10)
+#define MSR_ALF BIT(11)
+#define MSR_MBF BIT(24)
+#define MSR_BBF BIT(25)
+#define MIER_TDIE BIT(0)
+#define MIER_RDIE BIT(1)
+#define MIER_SDIE BIT(9)
+#define MIER_NDIE BIT(10)
+#define MCFGR1_AUTOSTOP BIT(8)
+#define MCFGR1_IGNACK BIT(9)
+#define MRDR_RXEMPTY BIT(14)
+
+#define I2C_CLK_RATIO 2
+#define CHUNK_DATA 256
+
+#define LPI2C_RX_FIFOSIZE 4
+#define LPI2C_TX_FIFOSIZE 4
+
+#define LPI2C_DEFAULT_RATE 100000
+#define STARDARD_MAX_BITRATE 400000
+#define FAST_MAX_BITRATE 1000000
+#define FAST_PLUS_MAX_BITRATE 3400000
+#define HIGHSPEED_MAX_BITRATE 5000000
+
+enum lpi2c_imx_mode {
+ STANDARD, /* 100+Kbps */
+ FAST, /* 400+Kbps */
+ FAST_PLUS, /* 1.0+Mbps */
+ HS, /* 3.4+Mbps */
+ ULTRA_FAST, /* 5.0+Mbps */
+};
+
+enum lpi2c_imx_pincfg {
+ TWO_PIN_OD,
+ TWO_PIN_OO,
+ TWO_PIN_PP,
+ FOUR_PIN_PP,
+};
+
+struct lpi2c_imx_struct {
+ struct i2c_adapter adapter;
+ struct clk *clk;
+ void __iomem *base;
+ __u8 *rx_buf;
+ __u8 *tx_buf;
+ struct completion complete;
+ unsigned int msglen;
+ unsigned int delivered;
+ unsigned int block_data;
+ unsigned int bitrate;
+ enum lpi2c_imx_mode mode;
+};
+
+static void lpi2c_imx_intctrl(struct lpi2c_imx_struct *lpi2c_imx,
+ unsigned int enable)
+{
+ writel(enable, lpi2c_imx->base + LPI2C_MIER);
+}
+
+static int lpi2c_imx_bus_busy(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned long orig_jiffies = jiffies;
+ unsigned int temp;
+
+ while (1) {
+ temp = readl(lpi2c_imx->base + LPI2C_MSR);
+
+ /* check for arbitration lost, clear if set */
+ if (temp & MSR_ALF) {
+ writel(temp, lpi2c_imx->base + LPI2C_MSR);
+ return -EAGAIN;
+ }
+
+ if (temp & (MSR_BBF | MSR_MBF))
+ break;
+
+ if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "bus not work\n");
+ return -ETIMEDOUT;
+ }
+ schedule();
+ }
+
+ return 0;
+}
+
+static void lpi2c_imx_set_mode(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int bitrate = lpi2c_imx->bitrate;
+ enum lpi2c_imx_mode mode;
+
+ if (bitrate < STARDARD_MAX_BITRATE)
+ mode = STANDARD;
+ else if (bitrate < FAST_MAX_BITRATE)
+ mode = FAST;
+ else if (bitrate < FAST_PLUS_MAX_BITRATE)
+ mode = FAST_PLUS;
+ else if (bitrate < HIGHSPEED_MAX_BITRATE)
+ mode = HS;
+ else
+ mode = ULTRA_FAST;
+
+ lpi2c_imx->mode = mode;
+}
+
+static int lpi2c_imx_start(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msgs)
+{
+ unsigned int temp;
+ u8 read;
+
+ temp = readl(lpi2c_imx->base + LPI2C_MCR);
+ temp |= MCR_RRF | MCR_RTF;
+ writel(temp, lpi2c_imx->base + LPI2C_MCR);
+ writel(0x7f00, lpi2c_imx->base + LPI2C_MSR);
+
+ read = msgs->flags & I2C_M_RD;
+ temp = (msgs->addr << 1 | read) | (GEN_START << 8);
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+
+ return lpi2c_imx_bus_busy(lpi2c_imx);
+}
+
+static void lpi2c_imx_stop(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned long orig_jiffies = jiffies;
+ unsigned int temp;
+
+ writel(GEN_STOP << 8, lpi2c_imx->base + LPI2C_MTDR);
+
+ do {
+ temp = readl(lpi2c_imx->base + LPI2C_MSR);
+ if (temp & MSR_SDF)
+ break;
+
+ if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "stop timeout\n");
+ break;
+ }
+ schedule();
+
+ } while (1);
+}
+
+/* CLKLO = I2C_CLK_RATIO * CLKHI, SETHOLD = CLKHI, DATAVD = CLKHI/2 */
+static int lpi2c_imx_config(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ u8 prescale, filt, sethold, clkhi, clklo, datavd;
+ unsigned int clk_rate, clk_cycle;
+ enum lpi2c_imx_pincfg pincfg;
+ unsigned int temp;
+
+ lpi2c_imx_set_mode(lpi2c_imx);
+
+ clk_rate = clk_get_rate(lpi2c_imx->clk);
+ if (lpi2c_imx->mode == HS || lpi2c_imx->mode == ULTRA_FAST)
+ filt = 0;
+ else
+ filt = 2;
+
+ for (prescale = 0; prescale <= 7; prescale++) {
+ clk_cycle = clk_rate / ((1 << prescale) * lpi2c_imx->bitrate)
+ - 3 - (filt >> 1);
+ clkhi = (clk_cycle + I2C_CLK_RATIO) / (I2C_CLK_RATIO + 1);
+ clklo = clk_cycle - clkhi;
+ if (clklo < 64)
+ break;
+ }
+
+ if (prescale > 7)
+ return -EINVAL;
+
+ /* set MCFGR1: PINCFG, PRESCALE, IGNACK */
+ if (lpi2c_imx->mode == ULTRA_FAST)
+ pincfg = TWO_PIN_OO;
+ else
+ pincfg = TWO_PIN_OD;
+ temp = prescale | pincfg << 24;
+
+ if (lpi2c_imx->mode == ULTRA_FAST)
+ temp |= MCFGR1_IGNACK;
+
+ writel(temp, lpi2c_imx->base + LPI2C_MCFGR1);
+
+ /* set MCFGR2: FILTSDA, FILTSCL */
+ temp = (filt << 16) | (filt << 24);
+ writel(temp, lpi2c_imx->base + LPI2C_MCFGR2);
+
+ /* set MCCR: DATAVD, SETHOLD, CLKHI, CLKLO */
+ sethold = clkhi;
+ datavd = clkhi >> 1;
+ temp = datavd << 24 | sethold << 16 | clkhi << 8 | clklo;
+
+ if (lpi2c_imx->mode == HS)
+ writel(temp, lpi2c_imx->base + LPI2C_MCCR1);
+ else
+ writel(temp, lpi2c_imx->base + LPI2C_MCCR0);
+
+ return 0;
+}
+
+static int lpi2c_imx_master_enable(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int temp;
+ int ret;
+
+ ret = clk_enable(lpi2c_imx->clk);
+ if (ret)
+ return ret;
+
+ temp = MCR_RST;
+ writel(temp, lpi2c_imx->base + LPI2C_MCR);
+ writel(0, lpi2c_imx->base + LPI2C_MCR);
+
+ ret = lpi2c_imx_config(lpi2c_imx);
+ if (ret)
+ goto clk_disable;
+
+ temp = readl(lpi2c_imx->base + LPI2C_MCR);
+ temp |= MCR_MEN;
+ writel(temp, lpi2c_imx->base + LPI2C_MCR);
+
+ return 0;
+
+clk_disable:
+ clk_disable(lpi2c_imx->clk);
+
+ return ret;
+}
+
+static int lpi2c_imx_master_disable(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ u32 temp;
+
+ temp = readl(lpi2c_imx->base + LPI2C_MCR);
+ temp &= ~MCR_MEN;
+ writel(temp, lpi2c_imx->base + LPI2C_MCR);
+
+ clk_disable(lpi2c_imx->clk);
+
+ return 0;
+}
+
+static int lpi2c_imx_msg_complete(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned long timeout;
+
+ timeout = wait_for_completion_timeout(&lpi2c_imx->complete, HZ);
+
+ return timeout ? 0 : -ETIMEDOUT;
+}
+
+static int lpi2c_imx_txfifo_empty(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned long orig_jiffies = jiffies;
+ u32 txcnt;
+
+ do {
+ txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
+
+ if (readl(lpi2c_imx->base + LPI2C_MSR) & MSR_NDF) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "NDF detected\n");
+ return -EIO;
+ }
+
+ if (time_after(jiffies, orig_jiffies + msecs_to_jiffies(500))) {
+ dev_dbg(&lpi2c_imx->adapter.dev, "txfifo empty timeout\n");
+ return -ETIMEDOUT;
+ }
+ schedule();
+
+ } while (txcnt);
+
+ return 0;
+}
+
+static void lpi2c_imx_set_tx_watermark(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ writel(LPI2C_TX_FIFOSIZE >> 1, lpi2c_imx->base + LPI2C_MFCR);
+}
+
+static void lpi2c_imx_set_rx_watermark(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int temp, remaining;
+
+ remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
+
+ if (remaining > (LPI2C_RX_FIFOSIZE >> 1))
+ temp = LPI2C_RX_FIFOSIZE >> 1;
+ else
+ temp = 0;
+
+ writel(temp << 16, lpi2c_imx->base + LPI2C_MFCR);
+}
+
+static void lpi2c_imx_write_txfifo(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int data, txcnt;
+
+ txcnt = readl(lpi2c_imx->base + LPI2C_MFSR) & 0xff;
+
+ while (txcnt < LPI2C_TX_FIFOSIZE) {
+ if (lpi2c_imx->delivered == lpi2c_imx->msglen)
+ break;
+
+ data = lpi2c_imx->tx_buf[lpi2c_imx->delivered++];
+ writel(data, lpi2c_imx->base + LPI2C_MTDR);
+ txcnt++;
+ }
+
+ if (lpi2c_imx->delivered < lpi2c_imx->msglen)
+ lpi2c_imx_intctrl(lpi2c_imx, MIER_TDIE | MIER_NDIE);
+ else
+ complete(&lpi2c_imx->complete);
+}
+
+static void lpi2c_imx_read_rxfifo(struct lpi2c_imx_struct *lpi2c_imx)
+{
+ unsigned int blocklen, remaining;
+ unsigned int temp, data;
+
+ do {
+ data = readl(lpi2c_imx->base + LPI2C_MRDR);
+ if (data & MRDR_RXEMPTY)
+ break;
+
+ lpi2c_imx->rx_buf[lpi2c_imx->delivered++] = data & 0xff;
+ } while (1);
+
+ /*
+ * First byte is the length of remaining packet in the SMBus block
+ * data read. Add it to msgs->len.
+ */
+ if (lpi2c_imx->block_data) {
+ blocklen = lpi2c_imx->rx_buf[0];
+ lpi2c_imx->msglen += blocklen;
+ }
+
+ remaining = lpi2c_imx->msglen - lpi2c_imx->delivered;
+
+ if (!remaining) {
+ complete(&lpi2c_imx->complete);
+ return;
+ }
+
+ /* not finished, still waiting for rx data */
+ lpi2c_imx_set_rx_watermark(lpi2c_imx);
+
+ /* multiple receive commands */
+ if (lpi2c_imx->block_data) {
+ lpi2c_imx->block_data = 0;
+ temp = remaining;
+ temp |= (RECV_DATA << 8);
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+ } else if (!(lpi2c_imx->delivered & 0xff)) {
+ temp = (remaining > CHUNK_DATA ? CHUNK_DATA : remaining) - 1;
+ temp |= (RECV_DATA << 8);
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+ }
+
+ lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE);
+}
+
+static void lpi2c_imx_write(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msgs)
+{
+ lpi2c_imx->tx_buf = msgs->buf;
+ lpi2c_imx_set_tx_watermark(lpi2c_imx);
+ lpi2c_imx_write_txfifo(lpi2c_imx);
+}
+
+static void lpi2c_imx_read(struct lpi2c_imx_struct *lpi2c_imx,
+ struct i2c_msg *msgs)
+{
+ unsigned int temp;
+
+ lpi2c_imx->rx_buf = msgs->buf;
+ lpi2c_imx->block_data = msgs->flags & I2C_M_RECV_LEN;
+
+ lpi2c_imx_set_rx_watermark(lpi2c_imx);
+ temp = msgs->len > CHUNK_DATA ? CHUNK_DATA - 1 : msgs->len - 1;
+ temp |= (RECV_DATA << 8);
+ writel(temp, lpi2c_imx->base + LPI2C_MTDR);
+
+ lpi2c_imx_intctrl(lpi2c_imx, MIER_RDIE | MIER_NDIE);
+}
+
+static int lpi2c_imx_xfer(struct i2c_adapter *adapter,
+ struct i2c_msg *msgs, int num)
+{
+ struct lpi2c_imx_struct *lpi2c_imx = i2c_get_adapdata(adapter);
+ unsigned int temp;
+ int i, result;
+
+ result = lpi2c_imx_master_enable(lpi2c_imx);
+ if (result)
+ return result;
+
+ for (i = 0; i < num; i++) {
+ result = lpi2c_imx_start(lpi2c_imx, &msgs[i]);
+ if (result)
+ goto disable;
+
+ /* quick smbus */
+ if (num == 1 && msgs[0].len == 0)
+ goto stop;
+
+ lpi2c_imx->delivered = 0;
+ lpi2c_imx->msglen = msgs[i].len;
+ init_completion(&lpi2c_imx->complete);
+
+ if (msgs[i].flags & I2C_M_RD)
+ lpi2c_imx_read(lpi2c_imx, &msgs[i]);
+ else
+ lpi2c_imx_write(lpi2c_imx, &msgs[i]);
+
+ result = lpi2c_imx_msg_complete(lpi2c_imx);
+ if (result)
+ goto stop;
+
+ if (!(msgs[i].flags & I2C_M_RD)) {
+ result = lpi2c_imx_txfifo_empty(lpi2c_imx);
+ if (result)
+ goto stop;
+ }
+ }
+
+stop:
+ lpi2c_imx_stop(lpi2c_imx);
+
+ temp = readl(lpi2c_imx->base + LPI2C_MSR);
+ if ((temp & MSR_NDF) && !result)
+ result = -EIO;
+
+disable:
+ lpi2c_imx_master_disable(lpi2c_imx);
+
+ dev_dbg(&lpi2c_imx->adapter.dev, "<%s> exit with: %s: %d\n", __func__,
+ (result < 0) ? "error" : "success msg",
+ (result < 0) ? result : num);
+
+ return (result < 0) ? result : num;
+}
+
+static irqreturn_t lpi2c_imx_isr(int irq, void *dev_id)
+{
+ struct lpi2c_imx_struct *lpi2c_imx = dev_id;
+ unsigned int temp;
+
+ lpi2c_imx_intctrl(lpi2c_imx, 0);
+ temp = readl(lpi2c_imx->base + LPI2C_MSR);
+
+ if (temp & MSR_RDF) {
+ lpi2c_imx_read_rxfifo(lpi2c_imx);
+ return IRQ_HANDLED;
+ }
+
+ if (temp & MSR_TDF) {
+ lpi2c_imx_write_txfifo(lpi2c_imx);
+ return IRQ_HANDLED;
+ }
+
+ complete(&lpi2c_imx->complete);
+
+ return IRQ_HANDLED;
+}
+
+static u32 lpi2c_imx_func(struct i2c_adapter *adapter)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL |
+ I2C_FUNC_SMBUS_READ_BLOCK_DATA;
+}
+
+static struct i2c_algorithm lpi2c_imx_algo = {
+ .master_xfer = lpi2c_imx_xfer,
+ .functionality = lpi2c_imx_func,
+};
+
+static const struct of_device_id lpi2c_imx_of_match[] = {
+ { .compatible = "fsl,imx7ulp-lpi2c" },
+ { .compatible = "fsl,imx8dv-lpi2c" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, lpi2c_imx_of_match);
+
+static int lpi2c_imx_probe(struct platform_device *pdev)
+{
+ struct lpi2c_imx_struct *lpi2c_imx;
+ struct resource *res;
+ int irq, ret;
+
+ lpi2c_imx = devm_kzalloc(&pdev->dev, sizeof(*lpi2c_imx), GFP_KERNEL);
+ if (!lpi2c_imx)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ lpi2c_imx->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(lpi2c_imx->base))
+ return PTR_ERR(lpi2c_imx->base);
+
+ irq = platform_get_irq(pdev, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "can't get irq number\n");
+ return irq;
+ }
+
+ lpi2c_imx->adapter.owner = THIS_MODULE;
+ lpi2c_imx->adapter.algo = &lpi2c_imx_algo;
+ lpi2c_imx->adapter.dev.parent = &pdev->dev;
+ lpi2c_imx->adapter.dev.of_node = pdev->dev.of_node;
+ strlcpy(lpi2c_imx->adapter.name, pdev->name,
+ sizeof(lpi2c_imx->adapter.name));
+
+ lpi2c_imx->clk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(lpi2c_imx->clk)) {
+ dev_err(&pdev->dev, "can't get I2C peripheral clock\n");
+ return PTR_ERR(lpi2c_imx->clk);
+ }
+
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "clock-frequency", &lpi2c_imx->bitrate);
+ if (ret)
+ lpi2c_imx->bitrate = LPI2C_DEFAULT_RATE;
+
+ ret = devm_request_irq(&pdev->dev, irq, lpi2c_imx_isr, 0,
+ pdev->name, lpi2c_imx);
+ if (ret) {
+ dev_err(&pdev->dev, "can't claim irq %d\n", irq);
+ return ret;
+ }
+
+ i2c_set_adapdata(&lpi2c_imx->adapter, lpi2c_imx);
+ platform_set_drvdata(pdev, lpi2c_imx);
+
+ ret = clk_prepare(lpi2c_imx->clk);
+ if (ret) {
+ dev_err(&pdev->dev, "clk prepare failed %d\n", ret);
+ return ret;
+ }
+
+ ret = i2c_add_adapter(&lpi2c_imx->adapter);
+ if (ret)
+ goto clk_unprepare;
+
+ dev_info(&lpi2c_imx->adapter.dev, "LPI2C adapter registered\n");
+
+ return 0;
+
+clk_unprepare:
+ clk_unprepare(lpi2c_imx->clk);
+
+ return ret;
+}
+
+static int lpi2c_imx_remove(struct platform_device *pdev)
+{
+ struct lpi2c_imx_struct *lpi2c_imx = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&lpi2c_imx->adapter);
+
+ clk_unprepare(lpi2c_imx->clk);
+
+ return 0;
+}
+
+static struct platform_driver lpi2c_imx_driver = {
+ .probe = lpi2c_imx_probe,
+ .remove = lpi2c_imx_remove,
+ .driver = {
+ .name = DRIVER_NAME,
+ .of_match_table = lpi2c_imx_of_match,
+ },
+};
+
+module_platform_driver(lpi2c_imx_driver);
+
+MODULE_AUTHOR("Gao Pan <pandy.gao@nxp.com>");
+MODULE_DESCRIPTION("I2C adapter driver for LPI2C bus");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [patch v9 1/1] i2c: add master driver for mellanox systems
From: vadimp @ 2016-11-20 16:56 UTC (permalink / raw)
To: wsa; +Cc: linux-i2c, linux-kernel, jiri, Vadim Pasternak, Michael Shych
From: Vadim Pasternak <vadimp@mellanox.com>
Device driver for Mellanox I2C controller logic, implemented in Lattice
CPLD device.
Device supports:
- Master mode
- One physical bus
- Polling mode
The Kconfig currently controlling compilation of this code is:
drivers/i2c/busses/Kconfig:config I2C_MLXCPLD
Signed-off-by: Michael Shych <michaelsh@mellanox.com>
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@mellanox.com>
Reviewed-by: Vladimir Zapolskiy <vz@mleia.com>
---
v8->v9
Comments pointed out by Wolfram:
- fix typo in comment;
- use quirk parameters for the message number validation;
v7->v8
Comments pointed out by Wolfram:
- Remove descriptions for two structures, since the members have
self-explaining names;
- Populate the structure i2c_adapter_quirks, so core will make length
validation, as a result remove mlxcpld_i2c_invalid_len,
and move common length calculation to mlxcpld_i2c_xfer. Consider
removing of mlxcpld_i2c_check_msg_params after core quirks is able
to handle messages full validation;
- Use ENXIO error code for NACK;
v6->v7
Comments pointed out by Peter:
- Fix grammar in doc file;
- Fix description for CMD in doc file;
- Fix grammar for NUM_DATA, DATAx in doc file;
- Rename mlxcpld_i2c_curr_transf to mlxcpld_i2c_curr_xfer;
- Make code in mlxcpld_i2c_lpc_write_buf more readable and
mlxcpld_i2c_lpc_read_buf, compact and elegant;
- Fix multiline comments;
v5->v6:
Comments pointed out by Vladimir:
- Drop the line with module path from the header;
- In description of mlxcpld_i2c_priv remove lpc_gen_dec_reg asnd dev_id;
- In mlxcpld_i2c_priv change type of the filed base_addr to u16 for
the alignment with in/out and remove unused dev_id;
- Fix misspelling in comment for mlxcpld_i2c_invalid_len;
- Remove comment regarding EBUSY return in mlxcpld_i2c_check_busy;
- Use sizeof of the target storage in allocation in probe routine;
v4->v5:
Comments pointed out by Vladimir:
- Remove "default n" from Kconfig;
- Fix the comments for timeout and pool time;
- Optimize error flow in mlxcpld_i2c_probe;
v3->v4:
Comments pointed out by Vladimir:
- Set default to no in Kconfig;
- Make mlxcpld_i2c_plat_dev static and add empty line before the
declaration;
- In function mlxcpld_i2c_invalid_len remove (msg->len < 0), since len is
unsigned;
- Remove unused symbol mlxcpld_i2c_plat_dev;
- Remove extra spaces in comments to mlxcpld_i2c_check_msg_params;
- Remove unnecessary round braces in mlxcpld_i2c_set_transf_data;
- Remove the assignment of 'i' variable in mlxcpld_i2c_wait_for_tc;
- Add extra line in mlxcpld_i2c_xfer;
- Move assignment of the adapter's fields retries and nr inside
mlxcpld_i2c_adapter declaration;
v2->v3:
Comments pointed out by Vladimir:
- Use tab symbol as indentation in Kconfig
- Add the Kconfig section preserving the alphabetical order - added
within "Other I2C/SMBus bus drivers" after I2C_ELEKTOR (but after this
sections others are not follow alphabetical);
- Change license to dual;
- Replace ADRR with ADDR in macros;
- Remove unused macros: MLXCPLD_LPCI2C_LPF_DFLT,
MLXCPLD_LPCI2C_HALF_CYC_100, MLXCPLD_LPCI2C_I2C_HOLD_100,
MLXCPLD_LPCI2C_HALF_CYC_REG, MLXCPLD_LPCI2C_I2C_HOLD_REG;
- Fix checkpatch warnings (**/ and the end of comment);
- Add empty line before structures mlxcpld_i2c_regs,
mlxcpld_i2c_curr_transf, mlxcpld_i2c_priv;
- Remove unused structure mlxcpld_i2c_regs;
- Remove from mlxcpld_i2c_priv the next fields:
retr_num, poll_time, block_sz, xfer_to; use instead macros
respectively: MLXCPLD_I2C_RETR_NUM, MLXCPLD_I2C_POLL_TIME,
MLXCPLD_I2C_DATA_REG_SZ, MLXCPLD_I2C_XFER_TO;
- In mlxcpld_i2c_invalid_len remove unnecessary else;
- Optimize mlxcpld_i2c_set_transf_data;
- mlxcpld_i2c_reset - add empty lines after/before mutex
lock/unlock;
- mlxcpld_i2c_wait_for_free - cover case timeout is equal
MLXCPLD_I2C_XFER_TO;
- mlxcpld_i2c_wait_for_tc:
- Do not assign err in declaration (also err is removed);
- Insert empty line before case MLXCPLD_LPCI2C_ACK_IND;
- inside case MLXCPLD_LPCI2C_ACK_IND - avoid unnecessary
indentation;
- Remove case MLXCPLD_LPCI2C_ERR_IND and remove this macro;
- Add empty lines in mlxcpld_i2c_xfer before/after mutex_lock/
mutex_unlock;
- In mlxcpld_i2c_probe add emtpy line after platform_set_drvdata;
- Replace platfrom handle pdev in mlxcpld_i2c_priv with the pointer
to the structure device;
- Place assignment of base_addr near the others;
- Enclose e-mail with <>;
Fixes added by Vadim:
- Change structure description format according to
Documentation/kernel-documentation.rst guideline;
- mlxcpld_i2c_wait_for_tc: return error if status reaches default case;
v1->v2
Fixes added by Vadim:
- Put new record in Makefile in alphabetic order;
- Remove http://www.mellanox.com from MAINTAINERS record;
---
Documentation/i2c/busses/i2c-mlxcpld | 47 ++++
MAINTAINERS | 8 +
drivers/i2c/busses/Kconfig | 11 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-mlxcpld.c | 504 +++++++++++++++++++++++++++++++++++
5 files changed, 571 insertions(+)
create mode 100644 Documentation/i2c/busses/i2c-mlxcpld
create mode 100644 drivers/i2c/busses/i2c-mlxcpld.c
diff --git a/Documentation/i2c/busses/i2c-mlxcpld b/Documentation/i2c/busses/i2c-mlxcpld
new file mode 100644
index 0000000..4e46c44
--- /dev/null
+++ b/Documentation/i2c/busses/i2c-mlxcpld
@@ -0,0 +1,47 @@
+Driver i2c-mlxcpld
+
+Author: Michael Shych <michaelsh@mellanox.com>
+
+This is the Mellanox I2C controller logic, implemented in Lattice CPLD
+device.
+Device supports:
+ - Master mode.
+ - One physical bus.
+ - Polling mode.
+
+This controller is equipped within the next Mellanox systems:
+"msx6710", "msx6720", "msb7700", "msn2700", "msx1410", "msn2410", "msb7800",
+"msn2740", "msn2100".
+
+The next transaction types are supported:
+ - Receive Byte/Block.
+ - Send Byte/Block.
+ - Read Byte/Block.
+ - Write Byte/Block.
+
+Registers:
+CTRL 0x1 - control reg.
+ Resets all the registers.
+HALF_CYC 0x4 - cycle reg.
+ Configure the width of I2C SCL half clock cycle (in 4 LPC_CLK
+ units).
+I2C_HOLD 0x5 - hold reg.
+ OE (output enable) is delayed by value set to this register
+ (in LPC_CLK units)
+CMD 0x6 - command reg.
+ Bit 0, 0 = write, 1 = read.
+ Bits [7:1] - the 7bit Address of the I2C device.
+ It should be written last as it triggers an I2C transaction.
+NUM_DATA 0x7 - data size reg.
+ Number of data bytes to write in read transaction
+NUM_ADDR 0x8 - address reg.
+ Number of address bytes to write in read transaction.
+STATUS 0x9 - status reg.
+ Bit 0 - transaction is completed.
+ Bit 4 - ACK/NACK.
+DATAx 0xa - 0x54 - 68 bytes data buffer regs.
+ For write transaction address is specified in four first bytes
+ (DATA1 - DATA4), data starting from DATA4.
+ For read transactions address is sent in a separate transaction and
+ specified in the four first bytes (DATA0 - DATA3). Data is read
+ starting from DATA0.
diff --git a/MAINTAINERS b/MAINTAINERS
index 411e3b8..26d05f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7881,6 +7881,14 @@ W: http://www.mellanox.com
Q: http://patchwork.ozlabs.org/project/netdev/list/
F: drivers/net/ethernet/mellanox/mlxsw/
+MELLANOX MLXCPLD I2C DRIVER
+M: Vadim Pasternak <vadimp@mellanox.com>
+M: Michael Shych <michaelsh@mellanox.com>
+L: linux-i2c@vger.kernel.org
+S: Supported
+F: drivers/i2c/busses/i2c-mlxcpld.c
+F: Documentation/i2c/busses/i2c-mlxcpld
+
MELLANOX MLXCPLD LED DRIVER
M: Vadim Pasternak <vadimp@mellanox.com>
L: linux-leds@vger.kernel.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d252276..6399cea 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1150,6 +1150,17 @@ config I2C_ELEKTOR
This support is also available as a module. If so, the module
will be called i2c-elektor.
+config I2C_MLXCPLD
+ tristate "Mellanox I2C driver"
+ depends on X86_64
+ help
+ This exposes the Mellanox platform I2C busses to the linux I2C layer
+ for X86 based systems.
+ Controller is implemented as CPLD logic.
+
+ This driver can also be built as a module. If so, the module will be
+ called as i2c-mlxcpld.
+
config I2C_PCA_ISA
tristate "PCA9564/PCA9665 on an ISA bus"
depends on ISA
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 29764cc..645bf08 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_I2C_BCM_KONA) += i2c-bcm-kona.o
obj-$(CONFIG_I2C_BRCMSTB) += i2c-brcmstb.o
obj-$(CONFIG_I2C_CROS_EC_TUNNEL) += i2c-cros-ec-tunnel.o
obj-$(CONFIG_I2C_ELEKTOR) += i2c-elektor.o
+obj-$(CONFIG_I2C_MLXCPLD) += i2c-mlxcpld.o
obj-$(CONFIG_I2C_OPAL) += i2c-opal.o
obj-$(CONFIG_I2C_PCA_ISA) += i2c-pca-isa.o
obj-$(CONFIG_I2C_SIBYTE) += i2c-sibyte.o
diff --git a/drivers/i2c/busses/i2c-mlxcpld.c b/drivers/i2c/busses/i2c-mlxcpld.c
new file mode 100644
index 0000000..d271e6a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-mlxcpld.c
@@ -0,0 +1,504 @@
+/*
+ * Copyright (c) 2016 Mellanox Technologies. All rights reserved.
+ * Copyright (c) 2016 Michael Shych <michaels@mellanox.com>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * 3. Neither the names of the copyright holders nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * Alternatively, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") version 2 as published by the Free
+ * Software Foundation.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ * POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+/* General defines */
+#define MLXPLAT_CPLD_LPC_I2C_BASE_ADDR 0x2000
+#define MLXCPLD_I2C_DEVICE_NAME "i2c_mlxcpld"
+#define MLXCPLD_I2C_VALID_FLAG (I2C_M_RECV_LEN | I2C_M_RD)
+#define MLXCPLD_I2C_BUS_NUM 1
+#define MLXCPLD_I2C_DATA_REG_SZ 36
+#define MLXCPLD_I2C_MAX_ADDR_LEN 4
+#define MLXCPLD_I2C_RETR_NUM 2
+#define MLXCPLD_I2C_XFER_TO 500000 /* usec */
+#define MLXCPLD_I2C_POLL_TIME 2000 /* usec */
+
+/* LPC I2C registers */
+#define MLXCPLD_LPCI2C_LPF_REG 0x0
+#define MLXCPLD_LPCI2C_CTRL_REG 0x1
+#define MLXCPLD_LPCI2C_HALF_CYC_REG 0x4
+#define MLXCPLD_LPCI2C_I2C_HOLD_REG 0x5
+#define MLXCPLD_LPCI2C_CMD_REG 0x6
+#define MLXCPLD_LPCI2C_NUM_DAT_REG 0x7
+#define MLXCPLD_LPCI2C_NUM_ADDR_REG 0x8
+#define MLXCPLD_LPCI2C_STATUS_REG 0x9
+#define MLXCPLD_LPCI2C_DATA_REG 0xa
+
+/* LPC I2C masks and parametres */
+#define MLXCPLD_LPCI2C_RST_SEL_MASK 0x1
+#define MLXCPLD_LPCI2C_TRANS_END 0x1
+#define MLXCPLD_LPCI2C_STATUS_NACK 0x10
+#define MLXCPLD_LPCI2C_NO_IND 0
+#define MLXCPLD_LPCI2C_ACK_IND 1
+#define MLXCPLD_LPCI2C_NACK_IND 2
+
+struct mlxcpld_i2c_curr_xfer {
+ u8 cmd;
+ u8 addr_width;
+ u8 data_len;
+ u8 msg_num;
+ struct i2c_msg *msg;
+};
+
+struct mlxcpld_i2c_priv {
+ struct i2c_adapter adap;
+ u32 base_addr;
+ struct mutex lock;
+ struct mlxcpld_i2c_curr_xfer xfer;
+ struct device *dev;
+};
+
+static void mlxcpld_i2c_lpc_write_buf(u8 *data, u8 len, u32 addr)
+{
+ int i;
+
+ for (i = 0; i < len - len % 4; i += 4)
+ outl(*(u32 *)(data + i), addr + i);
+ for (; i < len; ++i)
+ outb(*(data + i), addr + i);
+}
+
+static void mlxcpld_i2c_lpc_read_buf(u8 *data, u8 len, u32 addr)
+{
+ int i;
+
+ for (i = 0; i < len - len % 4; i += 4)
+ *(u32 *)(data + i) = inl(addr + i);
+ for (; i < len; ++i)
+ *(data + i) = inb(addr + i);
+}
+
+static void mlxcpld_i2c_read_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ *(data) = inb(addr);
+ break;
+ case 2:
+ *((u16 *)data) = inw(addr);
+ break;
+ case 3:
+ *((u16 *)data) = inw(addr);
+ *(data + 2) = inb(addr + 2);
+ break;
+ case 4:
+ *((u32 *)data) = inl(addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_read_buf(data, datalen, addr);
+ break;
+ }
+}
+
+static void mlxcpld_i2c_write_comm(struct mlxcpld_i2c_priv *priv, u8 offs,
+ u8 *data, u8 datalen)
+{
+ u32 addr = priv->base_addr + offs;
+
+ switch (datalen) {
+ case 1:
+ outb(*(data), addr);
+ break;
+ case 2:
+ outw(*((u16 *)data), addr);
+ break;
+ case 3:
+ outw(*((u16 *)data), addr);
+ outb(*(data + 2), addr + 2);
+ break;
+ case 4:
+ outl(*((u32 *)data), addr);
+ break;
+ default:
+ mlxcpld_i2c_lpc_write_buf(data, datalen, addr);
+ break;
+ }
+}
+
+/*
+ * Check validity of received i2c messages parameters.
+ * Returns 0 if OK, other - in case of invalid parameters.
+ */
+static int mlxcpld_i2c_check_msg_params(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num)
+{
+ int i;
+
+ if (!num) {
+ dev_err(priv->dev, "Incorrect 0 num of messages\n");
+ return -EINVAL;
+ }
+
+ if (unlikely(msgs[0].addr > 0x7f)) {
+ dev_err(priv->dev, "Invalid address 0x%03x\n",
+ msgs[0].addr);
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num; ++i) {
+ if (unlikely(!msgs[i].buf)) {
+ dev_err(priv->dev, "Invalid buf in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ if (unlikely(msgs[0].addr != msgs[i].addr)) {
+ dev_err(priv->dev, "Invalid addr in msg[%d]\n",
+ i);
+ return -EINVAL;
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Check if transfer is completed and status of operation.
+ * Returns 0 - transfer completed (both ACK or NACK),
+ * negative - transfer isn't finished.
+ */
+static int mlxcpld_i2c_check_status(struct mlxcpld_i2c_priv *priv, int *status)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END) {
+ if (val & MLXCPLD_LPCI2C_STATUS_NACK)
+ /*
+ * The slave is unable to accept the data. No such
+ * slave, command not understood, or unable to accept
+ * any more data.
+ */
+ *status = MLXCPLD_LPCI2C_NACK_IND;
+ else
+ *status = MLXCPLD_LPCI2C_ACK_IND;
+ return 0;
+ }
+ *status = MLXCPLD_LPCI2C_NO_IND;
+
+ return -EIO;
+}
+
+static void mlxcpld_i2c_set_transf_data(struct mlxcpld_i2c_priv *priv,
+ struct i2c_msg *msgs, int num,
+ u8 comm_len)
+{
+ priv->xfer.msg = msgs;
+ priv->xfer.msg_num = num;
+
+ /*
+ * All upper layers currently are never use transfer with more than
+ * 2 messages. Actually, it's also not so relevant in Mellanox systems
+ * because of HW limitation. Max size of transfer is not more than 32
+ * bytes in the current x86 LPCI2C bridge.
+ */
+ priv->xfer.cmd = msgs[num - 1].flags & I2C_M_RD;
+
+ if (priv->xfer.cmd == I2C_M_RD && comm_len != msgs[0].len) {
+ priv->xfer.addr_width = msgs[0].len;
+ priv->xfer.data_len = comm_len - priv->xfer.addr_width;
+ } else {
+ priv->xfer.addr_width = 0;
+ priv->xfer.data_len = comm_len;
+ }
+}
+
+/* Reset CPLD LPCI2C block */
+static void mlxcpld_i2c_reset(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mutex_lock(&priv->lock);
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+ val &= ~MLXCPLD_LPCI2C_RST_SEL_MASK;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CTRL_REG, &val, 1);
+
+ mutex_unlock(&priv->lock);
+}
+
+/* Make sure the CPLD is ready to start transmitting. */
+static int mlxcpld_i2c_check_busy(struct mlxcpld_i2c_priv *priv)
+{
+ u8 val;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_STATUS_REG, &val, 1);
+
+ if (val & MLXCPLD_LPCI2C_TRANS_END)
+ return 0;
+
+ return -EIO;
+}
+
+static int mlxcpld_i2c_wait_for_free(struct mlxcpld_i2c_priv *priv)
+{
+ int timeout = 0;
+
+ do {
+ if (!mlxcpld_i2c_check_busy(priv))
+ break;
+ usleep_range(MLXCPLD_I2C_POLL_TIME / 2, MLXCPLD_I2C_POLL_TIME);
+ timeout += MLXCPLD_I2C_POLL_TIME;
+ } while (timeout <= MLXCPLD_I2C_XFER_TO);
+
+ if (timeout > MLXCPLD_I2C_XFER_TO)
+ return -ETIMEDOUT;
+
+ return 0;
+}
+
+/*
+ * Wait for master transfer to complete.
+ * It puts current process to sleep until we get interrupt or timeout expires.
+ * Returns the number of transferred or read bytes or error (<0).
+ */
+static int mlxcpld_i2c_wait_for_tc(struct mlxcpld_i2c_priv *priv)
+{
+ int status, i, timeout = 0;
+ u8 datalen;
+
+ do {
+ usleep_range(MLXCPLD_I2C_POLL_TIME / 2, MLXCPLD_I2C_POLL_TIME);
+ if (!mlxcpld_i2c_check_status(priv, &status))
+ break;
+ timeout += MLXCPLD_I2C_POLL_TIME;
+ } while (status == 0 && timeout < MLXCPLD_I2C_XFER_TO);
+
+ switch (status) {
+ case MLXCPLD_LPCI2C_NO_IND:
+ return -ETIMEDOUT;
+
+ case MLXCPLD_LPCI2C_ACK_IND:
+ if (priv->xfer.cmd != I2C_M_RD)
+ return (priv->xfer.addr_width + priv->xfer.data_len);
+
+ if (priv->xfer.msg_num == 1)
+ i = 0;
+ else
+ i = 1;
+
+ if (!priv->xfer.msg[i].buf)
+ return -EINVAL;
+
+ /*
+ * Actual read data len will be always the same as
+ * requested len. 0xff (line pull-up) will be returned
+ * if slave has no data to return. Thus don't read
+ * MLXCPLD_LPCI2C_NUM_DAT_REG reg from CPLD.
+ */
+ datalen = priv->xfer.data_len;
+
+ mlxcpld_i2c_read_comm(priv, MLXCPLD_LPCI2C_DATA_REG,
+ priv->xfer.msg[i].buf, datalen);
+
+ return datalen;
+
+ case MLXCPLD_LPCI2C_NACK_IND:
+ return -ENXIO;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static void mlxcpld_i2c_xfer_msg(struct mlxcpld_i2c_priv *priv)
+{
+ int i, len = 0;
+ u8 cmd;
+
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_DAT_REG,
+ &priv->xfer.data_len, 1);
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_NUM_ADDR_REG,
+ &priv->xfer.addr_width, 1);
+
+ for (i = 0; i < priv->xfer.msg_num; i++) {
+ if ((priv->xfer.msg[i].flags & I2C_M_RD) != I2C_M_RD) {
+ /* Don't write to CPLD buffer in read transaction */
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_DATA_REG +
+ len, priv->xfer.msg[i].buf,
+ priv->xfer.msg[i].len);
+ len += priv->xfer.msg[i].len;
+ }
+ }
+
+ /*
+ * Set target slave address with command for master transfer.
+ * It should be latest executed function before CPLD transaction.
+ */
+ cmd = (priv->xfer.msg[0].addr << 1) | priv->xfer.cmd;
+ mlxcpld_i2c_write_comm(priv, MLXCPLD_LPCI2C_CMD_REG, &cmd, 1);
+}
+
+/*
+ * Generic lpc-i2c transfer.
+ * Returns the number of processed messages or error (<0).
+ */
+static int mlxcpld_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ struct mlxcpld_i2c_priv *priv = i2c_get_adapdata(adap);
+ u8 comm_len = 0;
+ int i, err;
+
+ err = mlxcpld_i2c_check_msg_params(priv, msgs, num);
+ if (err) {
+ dev_err(priv->dev, "Incorrect message\n");
+ return err;
+ }
+
+ for (i = 0; i < num; ++i)
+ comm_len += msgs[i].len;
+
+ /* Check bus state */
+ if (mlxcpld_i2c_wait_for_free(priv)) {
+ dev_err(priv->dev, "LPCI2C bridge is busy\n");
+
+ /*
+ * Usually it means something serious has happened.
+ * We can not have unfinished previous transfer
+ * so it doesn't make any sense to try to stop it.
+ * Probably we were not able to recover from the
+ * previous error.
+ * The only reasonable thing - is soft reset.
+ */
+ mlxcpld_i2c_reset(priv);
+ if (mlxcpld_i2c_check_busy(priv)) {
+ dev_err(priv->dev, "LPCI2C bridge is busy after reset\n");
+ return -EIO;
+ }
+ }
+
+ mlxcpld_i2c_set_transf_data(priv, msgs, num, comm_len);
+
+ mutex_lock(&priv->lock);
+
+ /* Do real transfer. Can't fail */
+ mlxcpld_i2c_xfer_msg(priv);
+
+ /* Wait for transaction complete */
+ err = mlxcpld_i2c_wait_for_tc(priv);
+
+ mutex_unlock(&priv->lock);
+
+ return err < 0 ? err : num;
+}
+
+static u32 mlxcpld_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm mlxcpld_i2c_algo = {
+ .master_xfer = mlxcpld_i2c_xfer,
+ .functionality = mlxcpld_i2c_func
+};
+
+static struct i2c_adapter_quirks mlxcpld_i2c_quirks = {
+ .flags = I2C_AQ_COMB_WRITE_THEN_READ,
+ .max_read_len = MLXCPLD_I2C_DATA_REG_SZ - MLXCPLD_I2C_MAX_ADDR_LEN,
+ .max_write_len = MLXCPLD_I2C_DATA_REG_SZ,
+ .max_comb_1st_msg_len = 4,
+};
+
+static struct i2c_adapter mlxcpld_i2c_adapter = {
+ .owner = THIS_MODULE,
+ .name = "i2c-mlxcpld",
+ .class = I2C_CLASS_HWMON | I2C_CLASS_SPD,
+ .algo = &mlxcpld_i2c_algo,
+ .quirks = &mlxcpld_i2c_quirks,
+ .retries = MLXCPLD_I2C_RETR_NUM,
+ .nr = MLXCPLD_I2C_BUS_NUM,
+};
+
+static int mlxcpld_i2c_probe(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv;
+ int err;
+
+ priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ mutex_init(&priv->lock);
+ platform_set_drvdata(pdev, priv);
+
+ priv->dev = &pdev->dev;
+
+ /* Register with i2c layer */
+ mlxcpld_i2c_adapter.timeout = usecs_to_jiffies(MLXCPLD_I2C_XFER_TO);
+ priv->adap = mlxcpld_i2c_adapter;
+ priv->adap.dev.parent = &pdev->dev;
+ priv->base_addr = MLXPLAT_CPLD_LPC_I2C_BASE_ADDR;
+ i2c_set_adapdata(&priv->adap, priv);
+
+ err = i2c_add_numbered_adapter(&priv->adap);
+ if (err)
+ mutex_destroy(&priv->lock);
+
+ return err;
+}
+
+static int mlxcpld_i2c_remove(struct platform_device *pdev)
+{
+ struct mlxcpld_i2c_priv *priv = platform_get_drvdata(pdev);
+
+ i2c_del_adapter(&priv->adap);
+ mutex_destroy(&priv->lock);
+
+ return 0;
+}
+
+static struct platform_driver mlxcpld_i2c_driver = {
+ .probe = mlxcpld_i2c_probe,
+ .remove = mlxcpld_i2c_remove,
+ .driver = {
+ .name = MLXCPLD_I2C_DEVICE_NAME,
+ },
+};
+
+module_platform_driver(mlxcpld_i2c_driver);
+
+MODULE_AUTHOR("Michael Shych <michaels@mellanox.com>");
+MODULE_DESCRIPTION("Mellanox I2C-CPLD controller driver");
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_ALIAS("platform:i2c-mlxcpld");
--
2.1.4
^ permalink raw reply related
* Re: Looking for guidance to support 74CBTLV3253 mux
From: Peter Rosin @ 2016-11-20 8:53 UTC (permalink / raw)
To: linux-i2c
In-Reply-To: <990cdcd2-e118-d056-0e80-defeb018bd63@axentia.se>
On 2016-11-11 13:18, Peter Rosin wrote:
> On 2016-11-11 13:04, Peter Rosin wrote:
>> On 2016-11-11 12:47, MikeB wrote:
>>> On Thu, Nov 10, 2016 at 8:01 AM, Peter Rosin <peda@axentia.se> wrote:
>>>>> There is however one requirement, the CPLD-mux must be "mux-locked"
>>>>> as explained in Documentation/i2c/i2c-topology. Otherwise accesses
>>>>> to devices behind that mux will lock the PCA9547 mux for the full
>>>>> duration of the transaction and thus lock out any attempts to update
>>>>> the CPLD mux as part of the transaction.
>>>
>>> Thank you for the informative reply. I was unfamiliar with the new
>>> mux and parent locking schemes. They do make the solution for this
>>> problem much easier to deal with. I'm working with release 4.4
>>> (Ubuntu Xenial), so the mux/parent locking hadn't shown up on my radar
>>> yet. Unfortunately, the target release for my work is and will remain
>>> 4.4 for a while.
>>
>> Lucky you :-)
>>
>> Anyway, you might be able to backport the changes?
>>
>> Start with:
>>
>> a7ab72390b77062420fb50e4451f71c9321aae05
>> "i2c: mux: add common data for every i2c-mux instance"
>>
>> through to
>>
>> 23fe440c59b9f08afe108e7ec7b6714cb2a3b955
>> "i2c: mux: drop old unused i2c-mux api"
>>
>> And then:
>>
>> 785d32c7a9638a96730686663564d1842f7c2f03
>> "i2c: allow adapter drivers to override the adapter locking"
>>
>> through to
>>
>> f76b724ee915415c52068c622ca48c652c1bd10c
>> "i2c-mux: document i2c muxes and elaborate on parent-/mux-locked muxes"
>
> Correction, those two commits became
> 8320f495cf441d593f7cd4f30e6b63455be71a2c and
> 2254d24aff3ab472dca287aef0123e8f0e06a14a when they got upstream.
>
>> There are a handful of commits right after that that are also
>> related, but they should be irrelevant to you...
>>
>> Haven't tried it though, YMMV, and I might have forgotten a patch
>> or two that came in later, but I don't remember any serious fallout...
For the archives, this is a bugfix for one of the commits mentioned:
ad092de60f865c1ad94221bd06d381ecea446cc8
"i2c: i2c-mux-pca954x: fix deselect enabling for device-tree"
Cheers,
Peter
^ permalink raw reply
* RE: [patch v8 1/1] i2c: add master driver for mellanox systems
From: Vadim Pasternak @ 2016-11-20 9:15 UTC (permalink / raw)
To: Wolfram Sang
Cc: wsa@the-dreams.de, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, jiri@resnulli.us, Michael Shych
In-Reply-To: <20161120090212.GA1554@katana>
> -----Original Message-----
> From: Wolfram Sang [mailto:wsa-dev@sang-engineering.com]
> Sent: Sunday, November 20, 2016 11:02 AM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: wsa@the-dreams.de; linux-i2c@vger.kernel.org; linux-
> kernel@vger.kernel.org; jiri@resnulli.us; Michael Shych
> <michaelsh@mellanox.com>
> Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems
>
>
> > Possibly I should add validation for "num" parameter at the beginning of
> mlxcpld_i2c_xfer?
>
> There is 'max_num_msgs' in the quirk structure.
Yes, I see it.
So I should just set max_num_msgs to 1, right?
Thanks you,
Vadim.
/
>
> Thanks,
>
> Wolfram
^ permalink raw reply
* Re: [patch v8 1/1] i2c: add master driver for mellanox systems
From: Wolfram Sang @ 2016-11-20 9:02 UTC (permalink / raw)
To: Vadim Pasternak
Cc: wsa@the-dreams.de, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, jiri@resnulli.us, Michael Shych
In-Reply-To: <AM5PR0501MB2097315FB75BFEF681512187A2B20@AM5PR0501MB2097.eurprd05.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 165 bytes --]
> Possibly I should add validation for "num" parameter at the beginning of mlxcpld_i2c_xfer?
There is 'max_num_msgs' in the quirk structure.
Thanks,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* RE: [patch v8 1/1] i2c: add master driver for mellanox systems
From: Vadim Pasternak @ 2016-11-20 5:52 UTC (permalink / raw)
To: Wolfram Sang
Cc: wsa@the-dreams.de, linux-i2c@vger.kernel.org,
linux-kernel@vger.kernel.org, jiri@resnulli.us, Michael Shych
In-Reply-To: <20161119210444.GB2856@katana>
Hi Wolfram,
> -----Original Message-----
> From: Wolfram Sang [mailto:wsa-dev@sang-engineering.com]
> Sent: Saturday, November 19, 2016 11:05 PM
> To: Vadim Pasternak <vadimp@mellanox.com>
> Cc: wsa@the-dreams.de; linux-i2c@vger.kernel.org; linux-
> kernel@vger.kernel.org; jiri@resnulli.us; Michael Shych
> <michaelsh@mellanox.com>
> Subject: Re: [patch v8 1/1] i2c: add master driver for mellanox systems
>
> Hi,
>
> looks mostly good. I just found this comment which needs clarification:
>
> > + /*
> > + * All upper layers currently are never use transfer with more than
> > + * 2 messages.
>
> Have you really checked ALL of the upper layers? And even if so, there is the dev-
> interface to userspace which allows for arbitrary I2C transfers using I2C_RDWR.
>
We are using the next drivers on our switches: max11603, lm75, pmbus, mlxsw_minimal (last one is new, I wrote it to support Mellanox ASIC and it's has 32 byte limitation for I2C transaction chunk) and at24.
I am planning to add some code in the next patch, for handling bufferization for message length greater then data buffer. Allowed by HW.
Possibly I should add validation for "num" parameter at the beginning of mlxcpld_i2c_xfer?
> > Actually, it's also not so relevant in Mellanox systems
> > + * because of HW limitation.
>
> What kind of HW limitation do you mean here? Can Mellanox send more than
> two messages?
>
> > Max size of transfer is o more than 20B
>
> "is o more"? Typo?
Typo.
Max size of transfer is not more than 32 bytes.
>
> > + * in current x86 LPCI2C bridge.
>
> What does that mean in result?
In case more than 32 bytes of data length will be set in transaction, such message will fail length validation.
The 32 is HW limitation - 32 registers of DATAx.
In new version of LPCI2C logic, there is 64 registers, which can be used for data, and I am going to support it in next patch for this driver.
>
> Regards,
>
> Wolfram
Thanks,
Vadim.
^ permalink raw reply
* Re: [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
From: Peter Rosin @ 2016-11-19 22:08 UTC (permalink / raw)
To: Jonathan Cameron, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <e462780e-305b-2cb9-1acd-c66dee312fac-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
On 2016-11-19 16:49, Jonathan Cameron wrote:
> On 17/11/16 21:48, Peter Rosin wrote:
>> When a multiplexer changes how an iio device behaves (for example
>> by feeding different signals to an ADC), this driver can be used
>> create one virtual iio channel for each multiplexer state.
>>
>> Depends on the generic multiplexer driver.
> I'm not really following what all the ext info stuff in here is about.
> Could you add a little more description of that?
Certainly. I have two needs for this series. The first one is simple
when it comes to the iio part and complex because the mux is shared
between three 8-way muxes on three of the inputs to an ADS-1015 ADC.
The forth ADC line to the ADS-1015 is not muxed. Those three muxes
are of course GPIO-controlled and share GPIO pins. And the GPIO
pins also control an i2c bus that is muxed 8-ways as well. There are
eight (possible) batteries, and we monitor voltage/current/temp with
the 3 muxed ADC lines, and 8 chargers sit on i2c behind the i2c mux.
I guess it felt natural for the HW designer to select battery with
the GPIO lines, but that do not fit too well with the code as it
is without this series...
For this first need, the iio mux does not need ext_info.
The second need is simple in the mux part but worse in the iio
department. It's another 8-way mux that simply muxes an ADC line,
so that is simple. However, the ADC line is the envelope detector
that just got added to linux-next, and it has two ext_info
attributes that needs to be set differently for different mux
states. Two of the states need "invert" to be false, the rest need
"invert" to be true. And it is also preferable to have different
values for "compare_interval" for different mux states since the
signals on the diffrent mux states have the different frequency
characteristics.
True, I could have the ext-info attributes go straight through
the mux, and just start by writing values to "invert"
and "compare_interval", and only then read a sample. But then I
would need to lock out other users during the duration of this
transaction. I believe that the best place to put that lock is
in the iio mux (when it locks its control-mux) and not leave it
to userspace to solve this in some brittle cooperative manner.
> Perhaps an example of how it is used and what the resulting interface
> looks like?
The resulting interface is just a copy of the (ext_info) interface
exposed by the parent channel (with a cache that is rewritten to
the parent on every iio mux state change). I have plans to add code
to not rewrite ext_info attributes that have never been changed in
any mux state.
Below I have an example file listing.
device0 is a dpot (mcp4561).
device1 is a dac (dpot-dac, wrapping the above dpot).
device2 is an adc (envelope-detector, wrapping the above dac)
device3 is a mux (iio-mux, wrapping the above adc)
The 8-way iio-mux have no signals attached on mux states 0 and 1, which
is why the first channel for device 3 is in_altvoltage2.
Ultimately, I would like some knob to hide devices 0, 1 and 2 from
userspace. They need/should only be visible to in-kernel users. Or
is there such a knob already?
Cheers,
Peter
$ ls /sys/bus/iio/devices/iio\:device*
/sys/bus/iio/devices/iio:device0:
dev out_resistance_raw_available
name out_resistance_scale
of_node power
out_resistance0_raw subsystem
out_resistance1_raw uevent
/sys/bus/iio/devices/iio:device1:
dev out_voltage0_scale
name power
of_node subsystem
out_voltage0_raw uevent
out_voltage0_raw_available
/sys/bus/iio/devices/iio:device2:
dev name
in_altvoltage0_compare_interval of_node
in_altvoltage0_invert power
in_altvoltage0_raw subsystem
in_altvoltage0_scale uevent
/sys/bus/iio/devices/iio:device3:
dev in_altvoltage5_raw
in_altvoltage2_compare_interval in_altvoltage5_scale
in_altvoltage2_invert in_altvoltage6_compare_interval
in_altvoltage2_raw in_altvoltage6_invert
in_altvoltage2_scale in_altvoltage6_raw
in_altvoltage3_compare_interval in_altvoltage6_scale
in_altvoltage3_invert in_altvoltage7_compare_interval
in_altvoltage3_raw in_altvoltage7_invert
in_altvoltage3_scale in_altvoltage7_raw
in_altvoltage4_compare_interval in_altvoltage7_scale
in_altvoltage4_invert name
in_altvoltage4_raw of_node
in_altvoltage4_scale power
in_altvoltage5_compare_interval subsystem
in_altvoltage5_invert uevent
^ permalink raw reply
* [PULL REQUEST] i2c for 4.9
From: Wolfram Sang @ 2016-11-19 21:31 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-i2c, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]
Linus,
here are some I2C driver bugfixes (and one documentation fix) for you.
Please pull. Note that I currently need to use a different email address
until the problems with my main mail provider are settled.
Thanks,
Wolfram
The following changes since commit bc33b0ca11e3df467777a4fa7639ba488c9d4911:
Linux 4.9-rc4 (2016-11-05 16:23:36 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current
for you to fetch changes up to ad092de60f865c1ad94221bd06d381ecea446cc8:
i2c: i2c-mux-pca954x: fix deselect enabling for device-tree (2016-11-19 21:49:29 +0100)
----------------------------------------------------------------
Alex Hemme (1):
i2c: i2c-mux-pca954x: fix deselect enabling for device-tree
Linus Walleij (1):
i2c: mux: fix up dependencies
Peter Rosin (1):
i2c: Documentation: i2c-topology: fix minor whitespace nit
Wei Yongjun (1):
i2c: digicolor: use clk_disable_unprepare instead of clk_unprepare
Wolfram Sang (1):
i2c: mux: demux-pinctrl: make drivers with no pinctrl work again
Documentation/i2c/i2c-topology | 4 ++--
drivers/i2c/Kconfig | 1 -
drivers/i2c/busses/i2c-digicolor.c | 2 +-
drivers/i2c/muxes/Kconfig | 1 +
drivers/i2c/muxes/i2c-demux-pinctrl.c | 22 ++++++++++++++++++++--
drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++--
6 files changed, 26 insertions(+), 8 deletions(-)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [v2] i2c: xlp9xx: ACPI support for I2C clients
From: Wolfram Sang @ 2016-11-19 21:10 UTC (permalink / raw)
To: Tanmay Jagdale; +Cc: wsa, linux-i2c, jayachandran.chandrashekaran
In-Reply-To: <1474886583-10569-1-git-send-email-tanmay.jagdale@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 565 bytes --]
On Mon, Sep 26, 2016 at 04:13:03PM +0530, Tanmay Jagdale wrote:
> The ACPI companion of the adapter has to be set for I2C controller
> code to read and attach the slave devices described in the ACPI table
> with the I2CSerialBus resource descriptor. Used ACPI_COMPANION_SET
> macro to set this.
>
> Signed-off-by: Tanmay Jagdale <tanmay.jagdale@broadcom.com>
So, you did resend the patch and I missed it. Please ignore what I wrote
in the last mail. Thanks for resending properly and again sorry for
overlooking this patch.
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [patch v8 1/1] i2c: add master driver for mellanox systems
From: Wolfram Sang @ 2016-11-19 21:04 UTC (permalink / raw)
To: vadimp; +Cc: wsa, linux-i2c, linux-kernel, jiri, Michael Shych
In-Reply-To: <1479455458-157627-1-git-send-email-vadimp@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 676 bytes --]
Hi,
looks mostly good. I just found this comment which needs clarification:
> + /*
> + * All upper layers currently are never use transfer with more than
> + * 2 messages.
Have you really checked ALL of the upper layers? And even if so, there
is the dev-interface to userspace which allows for arbitrary I2C
transfers using I2C_RDWR.
> Actually, it's also not so relevant in Mellanox systems
> + * because of HW limitation.
What kind of HW limitation do you mean here? Can Mellanox send more than
two messages?
> Max size of transfer is o more than 20B
"is o more"? Typo?
> + * in current x86 LPCI2C bridge.
What does that mean in result?
Regards,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] i2c: i2c-mux-pca954x: fix deselect enabling for device-tree
From: Wolfram Sang @ 2016-11-19 20:49 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Alex Hemme, Wolfram Sang, MakeB, linux-i2c, stable,
Ziyang Wu
In-Reply-To: <1479548918-21006-1-git-send-email-peda@axentia.se>
[-- Attachment #1: Type: text/plain, Size: 716 bytes --]
On Sat, Nov 19, 2016 at 10:48:38AM +0100, Peter Rosin wrote:
> From: Alex Hemme <ahemme@cisco.com>
>
> Deselect functionality can be ignored for device-trees with
> "i2c-mux-idle-disconnect" entries if no platform_data is available.
> By enabling the deselect functionality outside the platform_data
> block the logic works as it did in previous kernels.
>
> Fixes: 7fcac9807175 ("i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core")
> Cc: <stable@vger.kernel.org> # v4.7+
> Signed-off-by: Alex Hemme <ahemme@cisco.com>
> Signed-off-by: Ziyang Wu <ziywu@cisco.com>
> [touched up a few minor issues /peda]
> Signed-off-by: Peter Rosin <peda@axentia.se>
Applied to for-current, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v2 5/7] iio: multiplexer: new iio category and iio-mux driver
From: Jonathan Cameron @ 2016-11-19 15:49 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio
In-Reply-To: <1479419289-17553-6-git-send-email-peda@axentia.se>
On 17/11/16 21:48, Peter Rosin wrote:
> When a multiplexer changes how an iio device behaves (for example
> by feeding different signals to an ADC), this driver can be used
> create one virtual iio channel for each multiplexer state.
>
> Depends on the generic multiplexer driver.
I'm not really following what all the ext info stuff in here is about.
Could you add a little more description of that?
Perhaps an example of how it is used and what the resulting interface
looks like?
Thanks,
Jonathan
> ---
> drivers/iio/Kconfig | 1 +
> drivers/iio/Makefile | 1 +
> drivers/iio/multiplexer/Kconfig | 17 ++
> drivers/iio/multiplexer/Makefile | 6 +
> drivers/iio/multiplexer/iio-mux.c | 444 ++++++++++++++++++++++++++++++++++++++
> 5 files changed, 469 insertions(+)
> create mode 100644 drivers/iio/multiplexer/Kconfig
> create mode 100644 drivers/iio/multiplexer/Makefile
> create mode 100644 drivers/iio/multiplexer/iio-mux.c
>
> diff --git a/drivers/iio/Kconfig b/drivers/iio/Kconfig
> index 6743b18194fb..dcb541d0d70e 100644
> --- a/drivers/iio/Kconfig
> +++ b/drivers/iio/Kconfig
> @@ -82,6 +82,7 @@ source "drivers/iio/humidity/Kconfig"
> source "drivers/iio/imu/Kconfig"
> source "drivers/iio/light/Kconfig"
> source "drivers/iio/magnetometer/Kconfig"
> +source "drivers/iio/multiplexer/Kconfig"
> source "drivers/iio/orientation/Kconfig"
> if IIO_TRIGGER
> source "drivers/iio/trigger/Kconfig"
> diff --git a/drivers/iio/Makefile b/drivers/iio/Makefile
> index 87e4c4369e2f..f9879c29cf6f 100644
> --- a/drivers/iio/Makefile
> +++ b/drivers/iio/Makefile
> @@ -27,6 +27,7 @@ obj-y += humidity/
> obj-y += imu/
> obj-y += light/
> obj-y += magnetometer/
> +obj-y += multiplexer/
> obj-y += orientation/
> obj-y += potentiometer/
> obj-y += pressure/
> diff --git a/drivers/iio/multiplexer/Kconfig b/drivers/iio/multiplexer/Kconfig
> new file mode 100644
> index 000000000000..31cbe4509366
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Kconfig
> @@ -0,0 +1,17 @@
> +#
> +# Multiplexer drivers
> +#
> +# When adding new entries keep the list in alphabetical order
> +
> +menu "Multiplexers"
> +
> +config IIO_MUX
> + tristate "IIO multiplexer driver"
> + depends on OF && MUX_GPIO
> + help
> + Say yes here to build support for the IIO multiplexer.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called iio-mux.
> +
> +endmenu
> diff --git a/drivers/iio/multiplexer/Makefile b/drivers/iio/multiplexer/Makefile
> new file mode 100644
> index 000000000000..68be3c4abd07
> --- /dev/null
> +++ b/drivers/iio/multiplexer/Makefile
> @@ -0,0 +1,6 @@
> +#
> +# Makefile for industrial I/O multiplexer drivers
> +#
> +
> +# When adding new entries keep the list in alphabetical order
> +obj-$(CONFIG_IIO_MUX) += iio-mux.o
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> new file mode 100644
> index 000000000000..2a8d00da990b
> --- /dev/null
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -0,0 +1,444 @@
> +/*
> + * IIO multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * Author: Peter Rosin <peda@axentia.se>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/iio/consumer.h>
> +#include <linux/iio/iio.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_ext_info_cache {
> + char *data;
> + size_t size;
> +};
> +
> +struct mux_child {
> + u32 state;
> +
> + struct mux_ext_info_cache *ext_info_cache;
> +};
> +
> +struct mux {
> + u32 cached_state;
> + struct mux_control *control;
> + struct iio_channel *parent;
> + struct iio_dev *indio_dev;
> + struct iio_chan_spec *c;
> + struct iio_chan_spec_ext_info *ext_info;
> + struct mux_child *child;
> +};
> +
> +static int iio_mux_select(struct mux *mux, int idx)
> +{
> + struct mux_child *child = &mux->child[idx];
> + int ret;
> + int i;
> +
> + ret = mux_control_select(mux->control, child->state);
> + if (ret < 0) {
> + mux->cached_state = -1;
> + return ret;
> + }
> +
> + if (mux->cached_state == child->state)
> + return 0;
> +
I don't follow what is going on here.. Perhaps you could explain
futher?
> + if (mux->c[idx].ext_info) {
> + for (i = 0; mux->c[idx].ext_info[i].name; ++i) {
> + const char *attr = mux->c[idx].ext_info[i].name;
> + struct mux_ext_info_cache *cache;
> +
> + cache = &child->ext_info_cache[i];
> +
> + if (cache->size < 0)
> + continue;
> +
> + ret = iio_write_channel_ext_info(mux->parent, attr,
> + cache->data,
> + cache->size);
> +
> + if (ret < 0) {
> + mux_control_deselect(mux->control);
> + mux->cached_state = -1;
> + return ret;
> + }
> + }
> + }
> + mux->cached_state = child->state;
> +
> + return 0;
> +}
> +
> +static void iio_mux_deselect(struct mux *mux)
> +{
> + mux_control_deselect(mux->control);
> +}
> +
> +static int mux_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct mux *mux = iio_priv(indio_dev);
> + int idx = chan - mux->c;
> + int ret;
> +
> + ret = iio_mux_select(mux, idx);
> + if (ret < 0)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_read_channel_raw(mux->parent, val);
> + break;
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = iio_read_channel_scale(mux->parent, val, val2);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + }
> +
> + iio_mux_deselect(mux);
> +
> + return ret;
> +}
> +
> +static int mux_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + struct mux *mux = iio_priv(indio_dev);
> + int idx = chan - mux->c;
> + int ret;
> +
> + ret = iio_mux_select(mux, idx);
> + if (ret < 0)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + *type = IIO_VAL_INT;
> + ret = iio_read_avail_channel_raw(mux->parent, vals, length);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + }
> +
> + iio_mux_deselect(mux);
> +
> + return ret;
> +}
> +
> +static int mux_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct mux *mux = iio_priv(indio_dev);
> + int idx = chan - mux->c;
> + int ret;
> +
> + ret = iio_mux_select(mux, idx);
> + if (ret < 0)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = iio_write_channel_raw(mux->parent, val);
> + break;
> +
> + default:
> + ret = -EINVAL;
> + }
> +
> + iio_mux_deselect(mux);
> +
> + return ret;
> +}
> +
> +static const struct iio_info mux_info = {
> + .read_raw = mux_read_raw,
> + .read_avail = mux_read_avail,
> + .write_raw = mux_write_raw,
> + .driver_module = THIS_MODULE,
> +};
> +
> +static ssize_t mux_read_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> + struct iio_chan_spec const *chan, char *buf)
> +{
> + struct mux *mux = iio_priv(indio_dev);
> + int idx = chan - mux->c;
> + ssize_t ret;
> +
> + ret = iio_mux_select(mux, idx);
> + if (ret < 0)
> + return ret;
> +
> + ret = iio_read_channel_ext_info(mux->parent,
> + mux->ext_info[private].name,
> + buf);
> +
> + iio_mux_deselect(mux);
> +
> + return ret;
> +}
> +
> +static ssize_t mux_write_ext_info(struct iio_dev *indio_dev, uintptr_t private,
> + struct iio_chan_spec const *chan,
> + const char *buf, size_t len)
> +{
> + struct device *dev = indio_dev->dev.parent;
> + struct mux *mux = iio_priv(indio_dev);
> + int idx = chan - mux->c;
> + char *new;
> + ssize_t ret;
> +
> + ret = iio_mux_select(mux, idx);
> + if (ret < 0)
> + return ret;
> +
> + new = devm_kmemdup(dev, buf, len + 1, GFP_KERNEL);
> + if (!new) {
> + iio_mux_deselect(mux);
> + return -ENOMEM;
> + }
> +
> + new[len] = 0;
> +
> + ret = iio_write_channel_ext_info(mux->parent,
> + mux->ext_info[private].name,
> + buf, len);
> + if (ret < 0) {
> + iio_mux_deselect(mux);
> + devm_kfree(dev, new);
> + return ret;
> + }
> +
> + devm_kfree(dev, mux->child[idx].ext_info_cache[private].data);
> + mux->child[idx].ext_info_cache[private].data = new;
> + mux->child[idx].ext_info_cache[private].size = len;
> +
> + iio_mux_deselect(mux);
> +
> + return ret;
> +}
> +
> +static int mux_configure_channel(struct device *dev, struct mux *mux,
> + struct device_node *child_np, int idx)
> +{
> + struct mux_child *child = &mux->child[idx];
> + struct iio_chan_spec *c = &mux->c[idx];
> + const struct iio_chan_spec *pc = mux->parent->channel;
> + char *page;
> + int num_ext_info;
> + int i;
> + int ret;
> +
> + c->indexed = 1;
> + c->channel = idx;
> + c->output = pc->output;
> + c->datasheet_name = child_np->name;
> + c->ext_info = mux->ext_info;
> +
> + ret = iio_get_channel_type(mux->parent, &c->type);
> + if (ret < 0) {
> + dev_err(dev, "failed to get parent channel type\n");
> + return ret;
> + }
> +
> + if (iio_channel_has_info(pc, IIO_CHAN_INFO_RAW))
> + c->info_mask_separate |= BIT(IIO_CHAN_INFO_RAW);
> + if (iio_channel_has_info(pc, IIO_CHAN_INFO_SCALE))
> + c->info_mask_separate |= BIT(IIO_CHAN_INFO_SCALE);
> +
> + if (iio_channel_has_available(pc, IIO_CHAN_INFO_RAW))
> + c->info_mask_separate_available |= BIT(IIO_CHAN_INFO_RAW);
> +
> + ret = of_property_read_u32(child_np, "reg", &child->state);
> + if (ret < 0) {
> + dev_err(dev, "no reg property for node '%s'\n", child_np->name);
> + return ret;
> + }
> +
> + for (i = 0; i < idx; ++i) {
> + if (mux->child[i].state == child->state) {
> + dev_err(dev, "double use of reg %d\n", child->state);
> + return -EINVAL;
> + }
> + }
> +
> + num_ext_info = iio_get_channel_ext_info_count(mux->parent);
> + if (num_ext_info) {
> + page = devm_kzalloc(dev, PAGE_SIZE, GFP_KERNEL);
> + if (!page)
> + return -ENOMEM;
> + }
> + child->ext_info_cache = devm_kzalloc(dev,
> + sizeof(*child->ext_info_cache) *
> + num_ext_info, GFP_KERNEL);
> + for (i = 0; i < num_ext_info; ++i) {
> + child->ext_info_cache[i].size = -1;
> +
> + if (!pc->ext_info[i].write)
> + continue;
> + if (!pc->ext_info[i].read)
> + continue;
> +
> + ret = iio_read_channel_ext_info(mux->parent,
> + mux->ext_info[i].name,
> + page);
> + if (ret < 0) {
> + dev_err(dev, "failed to get ext_info '%s'\n",
> + pc->ext_info[i].name);
> + return ret;
> + }
> + if (ret >= PAGE_SIZE) {
> + dev_err(dev, "too large ext_info '%s'\n",
> + pc->ext_info[i].name);
> + return -EINVAL;
> + }
> +
> + child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
> + GFP_KERNEL);
> + child->ext_info_cache[i].data[ret] = 0;
> + child->ext_info_cache[i].size = ret;
> + }
> +
> + if (num_ext_info)
> + devm_kfree(dev, page);
> +
> + return 0;
> +}
> +
> +static int mux_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *child_np;
> + struct iio_dev *indio_dev;
> + struct iio_channel *parent;
> + struct mux *mux;
> + int sizeof_ext_info;
> + int children;
> + int sizeof_priv;
> + int i;
> + int ret;
> +
> + if (!np)
> + return -ENODEV;
> +
> + parent = devm_iio_channel_get(dev, "parent");
> + if (IS_ERR(parent)) {
> + if (PTR_ERR(parent) != -EPROBE_DEFER)
> + dev_err(dev, "failed to get parent channel\n");
> + return PTR_ERR(parent);
> + }
> +
> + sizeof_ext_info = iio_get_channel_ext_info_count(parent);
> + if (sizeof_ext_info) {
> + sizeof_ext_info += 1; /* one extra entry for the sentinel */
> + sizeof_ext_info *= sizeof(*mux->ext_info);
> + }
> +
> + children = of_get_child_count(np);
> + if (children <= 0) {
> + dev_err(dev, "not even a single child\n");
> + return -EINVAL;
> + }
> +
> + sizeof_priv = sizeof(*mux);
> + sizeof_priv += sizeof(*mux->child) * children;
> + sizeof_priv += sizeof(*mux->c) * children;
> + sizeof_priv += sizeof_ext_info;
> +
> + indio_dev = devm_iio_device_alloc(dev, sizeof_priv);
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + mux = iio_priv(indio_dev);
> + mux->child = (struct mux_child *)(mux + 1);
> + mux->c = (struct iio_chan_spec *)(mux->child + children);
> +
> + platform_set_drvdata(pdev, indio_dev);
> +
> + mux->parent = parent;
> + mux->cached_state = -1;
> +
> + indio_dev->name = dev_name(dev);
> + indio_dev->dev.parent = dev;
> + indio_dev->info = &mux_info;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = mux->c;
> + indio_dev->num_channels = children;
> + if (sizeof_ext_info) {
> + mux->ext_info = devm_kmemdup(dev,
> + parent->channel->ext_info,
> + sizeof_ext_info, GFP_KERNEL);
> + if (!mux->ext_info)
> + return -ENOMEM;
> +
> + for (i = 0; mux->ext_info[i].name; ++i) {
> + if (parent->channel->ext_info[i].read)
> + mux->ext_info[i].read = mux_read_ext_info;
> + if (parent->channel->ext_info[i].write)
> + mux->ext_info[i].write = mux_write_ext_info;
> + mux->ext_info[i].private = i;
> + }
> + }
> +
> + i = 0;
> + for_each_child_of_node(np, child_np) {
> + ret = mux_configure_channel(dev, mux, child_np, i);
> + if (ret < 0)
> + return ret;
> + i++;
> + }
> +
> + mux->control = devm_mux_control_get(dev, "mux");
> + if (IS_ERR(mux->control)) {
> + if (PTR_ERR(mux->control) != -EPROBE_DEFER)
> + dev_err(dev, "failed to get control-mux\n");
> + return PTR_ERR(mux->control);
> + }
> +
> + ret = devm_iio_device_register(dev, indio_dev);
> + if (ret) {
> + dev_err(dev, "failed to register iio device\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mux_match[] = {
> + { .compatible = "iio-mux" },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_match);
> +
> +static struct platform_driver mux_driver = {
> + .probe = mux_probe,
> + .driver = {
> + .name = "iio-mux",
> + .of_match_table = mux_match,
> + },
> +};
> +module_platform_driver(mux_driver);
> +
> +MODULE_DESCRIPTION("IIO multiplexer driver");
> +MODULE_AUTHOR("Peter Rosin <peda@axentia.se>");
> +MODULE_LICENSE("GPL v2");
>
^ permalink raw reply
* Re: [RFC PATCH v2 3/7] iio: inkern: api for manipulating ext_info of iio channels
From: Jonathan Cameron @ 2016-11-19 15:38 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479419289-17553-4-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 17/11/16 21:48, Peter Rosin wrote:
> Extend the inkern api with functions for reading and writing ext_info
> of iio channels.
I'd like Lars' feedback on this one.
Superficially looks fine to me but I am not as familiar with this interface
as Lars is ;) (he wrote it IIRC:)
> ---
> drivers/iio/inkern.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/iio/consumer.h | 6 +++++
> 2 files changed, 61 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index cfca17ba2535..a8099b164222 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -850,3 +850,58 @@ int iio_write_channel_raw(struct iio_channel *chan, int val)
> return ret;
> }
> EXPORT_SYMBOL_GPL(iio_write_channel_raw);
> +
> +int iio_get_channel_ext_info_count(struct iio_channel *chan)
> +{
> + const struct iio_chan_spec_ext_info *ext_info;
> + unsigned int i = 0;
> +
> + if (!chan->channel->ext_info)
> + return i;
> +
> + for (ext_info = chan->channel->ext_info; ext_info->name; ext_info++)
> + ++i;
> +
> + return i;
> +}
> +EXPORT_SYMBOL_GPL(iio_get_channel_ext_info_count);
> +
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> + const char *attr, char *buf)
> +{
> + const struct iio_chan_spec_ext_info *ext_info;
> +
> + if (!chan->channel->ext_info)
> + return -EINVAL;
> +
> + for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
> + if (strcmp(attr, ext_info->name))
> + continue;
> +
> + return ext_info->read(chan->indio_dev, ext_info->private,
> + chan->channel, buf);
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(iio_read_channel_ext_info);
> +
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> + const char *buf, size_t len)
> +{
> + const struct iio_chan_spec_ext_info *ext_info;
> +
> + if (!chan->channel->ext_info)
> + return -EINVAL;
> +
> + for (ext_info = chan->channel->ext_info; ext_info->name; ++ext_info) {
> + if (strcmp(attr, ext_info->name))
> + continue;
> +
> + return ext_info->write(chan->indio_dev, ext_info->private,
> + chan->channel, buf, len);
> + }
> +
> + return -EINVAL;
> +}
> +EXPORT_SYMBOL_GPL(iio_write_channel_ext_info);
> diff --git a/include/linux/iio/consumer.h b/include/linux/iio/consumer.h
> index 9a4f336d8b4a..471dece8729a 100644
> --- a/include/linux/iio/consumer.h
> +++ b/include/linux/iio/consumer.h
> @@ -299,4 +299,10 @@ int iio_read_channel_scale(struct iio_channel *chan, int *val,
> int iio_convert_raw_to_processed(struct iio_channel *chan, int raw,
> int *processed, unsigned int scale);
>
> +int iio_get_channel_ext_info_count(struct iio_channel *chan);
> +ssize_t iio_read_channel_ext_info(struct iio_channel *chan,
> + const char *attr, char *buf);
> +ssize_t iio_write_channel_ext_info(struct iio_channel *chan, const char *attr,
> + const char *buf, size_t len);
> +
> #endif
>
^ permalink raw reply
* Re: [RFC PATCH v2 2/7] misc: minimal mux subsystem and gpio-based mux controller
From: Jonathan Cameron @ 2016-11-19 15:34 UTC (permalink / raw)
To: Peter Rosin, linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1479419289-17553-3-git-send-email-peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org>
On 17/11/16 21:48, Peter Rosin wrote:
> When both the iio subsystem and the i2c subsystem wants to update
> the same mux, there needs to be some coordination. Invent a new
> minimal "mux" subsystem that handles this.
I'd probably put something more general in the description. Lots of things
may need the same infrastructure. This is just an example.
Few bits inline.
Also, I suspect you will fairly rapidly have a need for a strobe signal
as well. A lot of mux chips that are more than 2 way seem to have them to
allow multiple chips to be synchronized.
>
> Add a single backend driver for this new subsystem that can
> control gpio based multiplexers.
> ---
> drivers/misc/Kconfig | 6 +
> drivers/misc/Makefile | 2 +
> drivers/misc/mux-core.c | 299 ++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/misc/mux-gpio.c | 115 +++++++++++++++++++
> include/linux/mux.h | 53 +++++++++
> 5 files changed, 475 insertions(+)
> create mode 100644 drivers/misc/mux-core.c
> create mode 100644 drivers/misc/mux-gpio.c
> create mode 100644 include/linux/mux.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 64971baf11fa..9e119bb78d82 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -766,6 +766,12 @@ config PANEL_BOOT_MESSAGE
> An empty message will only clear the display at driver init time. Any other
> printf()-formatted message is valid with newline and escape codes.
>
> +config MUX_GPIO
> + tristate "GPIO-controlled MUX controller"
> + depends on OF
> + help
> + GPIO-controlled MUX controller
> +
> source "drivers/misc/c2port/Kconfig"
> source "drivers/misc/eeprom/Kconfig"
> source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 31983366090a..92b547bcbac1 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -53,6 +53,8 @@ obj-$(CONFIG_ECHO) += echo/
> obj-$(CONFIG_VEXPRESS_SYSCFG) += vexpress-syscfg.o
> obj-$(CONFIG_CXL_BASE) += cxl/
> obj-$(CONFIG_PANEL) += panel.o
> +obj-$(CONFIG_MUX_GPIO) += mux-core.o
> +obj-$(CONFIG_MUX_GPIO) += mux-gpio.o
>
> lkdtm-$(CONFIG_LKDTM) += lkdtm_core.o
> lkdtm-$(CONFIG_LKDTM) += lkdtm_bugs.o
> diff --git a/drivers/misc/mux-core.c b/drivers/misc/mux-core.c
> new file mode 100644
> index 000000000000..7a8bf003a92c
> --- /dev/null
> +++ b/drivers/misc/mux-core.c
> @@ -0,0 +1,299 @@
> +/*
> + * Multiplexer subsystem
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define pr_fmt(fmt) "mux-core: " fmt
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/slab.h>
> +
> +static struct bus_type mux_bus_type = {
> + .name = "mux",
> +};
> +
> +static int __init mux_init(void)
> +{
> + return bus_register(&mux_bus_type);
> +}
> +
> +static void __exit mux_exit(void)
> +{
> + bus_unregister(&mux_bus_type);
> +}
> +
> +static DEFINE_IDA(mux_ida);
> +
> +static void mux_control_release(struct device *dev)
> +{
> + struct mux_control *mux = to_mux_control(dev);
> +
> + ida_simple_remove(&mux_ida, mux->id);
> + kfree(mux);
> +}
> +
> +static struct device_type mux_control_type = {
> + .name = "mux-control",
> + .release = mux_control_release,
> +};
> +
> +/*
> + * Allocate a mux-control, plus an extra memory area for private use
> + * by the caller.
> + */
> +struct mux_control *mux_control_alloc(size_t sizeof_priv)
> +{
> + struct mux_control *mux;
> +
Worth planning ahead for spi controlled muxes and others that need their
structures to be carefully aligned to avoid dma cacheline fun?
Easy enough to add later I guess.
> + mux = kzalloc(sizeof(*mux) + sizeof_priv, GFP_KERNEL);
> + if (!mux)
> + return NULL;
> +
> + mux->dev.bus = &mux_bus_type;
> + mux->dev.type = &mux_control_type;
> + device_initialize(&mux->dev);
> + dev_set_drvdata(&mux->dev, mux);
> +
> + init_rwsem(&mux->lock);
> + mux->priv = mux + 1;
Needed? Or just do it with a bit of pointer math where the access is needed?
> +
> + mux->id = ida_simple_get(&mux_ida, 0, 0, GFP_KERNEL);
> + if (mux->id < 0) {
> + pr_err("mux-controlX failed to get device id\n");
> + kfree(mux);
> + return NULL;
> + }
> + dev_set_name(&mux->dev, "mux:control%d", mux->id);
> +
> + mux->cached_state = -1;
> + mux->idle_state = -1;
> +
> + return mux;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_alloc);
> +
> +/*
> + * Register the mux-control, thus readying it for use.
Either single line comment style - or perhaps kernel doc the lot...
> + */
> +int mux_control_register(struct mux_control *mux)
> +{
> + /* If the calling driver did not initialize of_node, do it here */
> + if (!mux->dev.of_node && mux->dev.parent)
> + mux->dev.of_node = mux->dev.parent->of_node;
> +
> + return device_add(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_register);
> +
> +/*
> + * Take the mux-control off-line.
> + */
> +void mux_control_unregister(struct mux_control *mux)
> +{
> + device_del(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_unregister);
> +
> +/*
> + * Put away the mux-control for good.
> + */
> +void mux_control_put(struct mux_control *mux)
> +{
> + if (!mux)
> + return;
> + put_device(&mux->dev);
> +}
> +EXPORT_SYMBOL_GPL(mux_control_put);
> +
> +static int mux_control_set(struct mux_control *mux, int state)
> +{
> + int ret = mux->ops->set(mux, state);
> +
> + mux->cached_state = ret < 0 ? -1 : state;
> +
> + return ret;
> +}
> +
> +/*
> + * Select the given multiplexer channel. Call mux_control_deselect()
> + * when the operation is complete on the multiplexer channel, and the
> + * multiplexer is free for others to use.
> + */
> +int mux_control_select(struct mux_control *mux, int state)
> +{
> + int ret;
> +
> + if (down_read_trylock(&mux->lock)) {
> + if (mux->cached_state == state)
> + return 0;
> +
> + /* Sigh, the mux needs updating... */
> + up_read(&mux->lock);
> + }
> +
> + /* ...or it's just contended. */
> + down_write(&mux->lock);
> +
> + if (mux->cached_state == state) {
> + /*
> + * Hmmm, someone else changed the mux to my liking.
> + * That makes me wonder how long I waited for nothing...
> + */
> + downgrade_write(&mux->lock);
> + return 0;
> + }
> +
> + ret = mux_control_set(mux, state);
> + if (ret < 0) {
> + if (mux->idle_state != -1)
> + mux_control_set(mux, mux->idle_state);
> +
> + up_write(&mux->lock);
> + return ret;
> + }
> +
> + downgrade_write(&mux->lock);
> +
> + return 1;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_select);
> +
> +/*
> + * Deselect the previously selected multiplexer channel.
> + */
> +int mux_control_deselect(struct mux_control *mux)
> +{
> + int ret = 0;
> +
> + if (mux->idle_state != -1 && mux->cached_state != mux->idle_state)
> + ret = mux_control_set(mux, mux->idle_state);
> +
> + up_read(&mux->lock);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_deselect);
> +
> +static int of_dev_node_match(struct device *dev, void *data)
> +{
> + return dev->of_node == data;
> +}
> +
> +static struct mux_control *of_find_mux_by_node(struct device_node *np)
> +{
> + struct device *dev;
> +
> + dev = bus_find_device(&mux_bus_type, NULL, np, of_dev_node_match);
> +
> + return dev ? to_mux_control(dev) : NULL;
> +}
> +
> +static struct mux_control *of_mux_control_get(struct device_node *np, int index)
> +{
> + struct device_node *mux_np;
> + struct mux_control *mux;
> +
> + mux_np = of_parse_phandle(np, "control-muxes", index);
> + if (!mux_np)
> + return NULL;
> +
> + mux = of_find_mux_by_node(mux_np);
> + of_node_put(mux_np);
> +
> + return mux;
> +}
> +
> +/*
> + * Get a named mux.
> + */
> +struct mux_control *mux_control_get(struct device *dev, const char *mux_name)
> +{
> + struct device_node *np = dev->of_node;
> + struct mux_control *mux;
> + int index;
> +
> + index = of_property_match_string(np, "control-mux-names", mux_name);
> + if (index < 0) {
> + dev_err(dev, "failed to get control-mux %s:%s(%i)\n",
> + np->full_name, mux_name ?: "", index);
> + return ERR_PTR(index);
> + }
> +
> + mux = of_mux_control_get(np, index);
> + if (!mux)
> + return ERR_PTR(-EPROBE_DEFER);
> +
> + return mux;
> +}
> +EXPORT_SYMBOL_GPL(mux_control_get);
> +
> +static void devm_mux_control_free(struct device *dev, void *res)
> +{
> + struct mux_control *mux = *(struct mux_control **)res;
> +
> + mux_control_put(mux);
> +}
> +
> +/*
> + * Get a named mux, with resource management.
> + */
Guess it might be elsewhere in patch set but remember to add this to
the global list of devm interfaces (in Documentation somewhere.. IIRC)
> +struct mux_control *devm_mux_control_get(struct device *dev,
> + const char *mux_name)
> +{
> + struct mux_control **ptr, *mux;
> +
> + ptr = devres_alloc(devm_mux_control_free, sizeof(*ptr), GFP_KERNEL);
> + if (!ptr)
> + return ERR_PTR(-ENOMEM);
> +
> + mux = mux_control_get(dev, mux_name);
> + if (IS_ERR(mux)) {
> + devres_free(ptr);
> + return mux;
> + }
> +
> + *ptr = mux;
> + devres_add(dev, ptr);
> +
> + return mux;
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_get);
> +
> +static int devm_mux_control_match(struct device *dev, void *res, void *data)
> +{
> + struct mux_control **r = res;
> +
> + if (!r || !*r) {
> + WARN_ON(!r || !*r);
> + return 0;
> + }
> +
> + return *r == data;
> +}
> +
> +/*
> + * Resource-managed version mux_control_put.
> + */
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux)
> +{
> + WARN_ON(devres_release(dev, devm_mux_control_free,
> + devm_mux_control_match, mux));
> +}
> +EXPORT_SYMBOL_GPL(devm_mux_control_put);
> +
> +subsys_initcall(mux_init);
> +module_exit(mux_exit);
> +
> +MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org");
> +MODULE_DESCRIPTION("MUX subsystem");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/misc/mux-gpio.c b/drivers/misc/mux-gpio.c
> new file mode 100644
> index 000000000000..2ddd7fb24078
> --- /dev/null
> +++ b/drivers/misc/mux-gpio.c
> @@ -0,0 +1,115 @@
> +/*
> + * GPIO-controlled multiplexer driver
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/err.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/module.h>
> +#include <linux/mux.h>
> +#include <linux/of.h>
> +#include <linux/of_platform.h>
> +#include <linux/platform_device.h>
> +
> +struct mux_gpio {
> + struct gpio_descs *gpios;
> +};
> +
> +static int mux_gpio_set(struct mux_control *mux, int val)
> +{
> + struct mux_gpio *mux_gpio = mux->priv;
> + int i;
> +
> + for (i = 0; i < mux_gpio->gpios->ndescs; i++)
> + gpiod_set_value_cansleep(mux_gpio->gpios->desc[i],
> + val & (1 << i));
> +
> + return 0;
> +}
> +
> +static const struct mux_control_ops mux_gpio_ops = {
> + .set = mux_gpio_set,
> +};
> +
> +static const struct of_device_id mux_gpio_dt_ids[] = {
> + { .compatible = "mux-gpio", },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, mux_gpio_dt_ids);
> +
> +static int mux_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = pdev->dev.of_node;
> + struct mux_control *mux;
> + struct mux_gpio *mux_gpio;
> + u32 idle_state;
> + int ret;
> +
> + if (!np)
> + return -ENODEV;
> +
> + mux = mux_control_alloc(sizeof(*mux_gpio));
> + if (!mux)
> + return -ENOMEM;
> + mux_gpio = mux->priv;
> + mux->dev.parent = dev;
> + mux->ops = &mux_gpio_ops;
> +
> + platform_set_drvdata(pdev, mux);
> +
> + mux_gpio->gpios = devm_gpiod_get_array(dev, "mux", GPIOD_OUT_LOW);
> + if (IS_ERR(mux_gpio->gpios)) {
> + if (PTR_ERR(mux_gpio->gpios) != -EPROBE_DEFER)
> + dev_err(dev, "failed to get gpios\n");
> + mux_control_put(mux);
> + return PTR_ERR(mux_gpio->gpios);
> + }
> +
> + ret = of_property_read_u32(np, "idle-state", &idle_state);
> + if (ret >= 0) {
> + if (idle_state >= (1 << mux_gpio->gpios->ndescs)) {
> + dev_err(dev, "invalid idle-state %u\n", idle_state);
> + return -EINVAL;
> + }
> + mux->idle_state = idle_state;
> + }
> +
> + ret = mux_control_register(mux);
> + if (ret < 0) {
> + dev_err(dev, "failed to register mux_control\n");
> + mux_control_put(mux);
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int mux_gpio_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct mux_control *mux = to_mux_control(dev);
> +
> + mux_control_unregister(mux);
> + mux_control_put(mux);
> + return 0;
> +}
> +
> +static struct platform_driver mux_gpio_driver = {
> + .driver = {
> + .name = "mux-gpio",
> + .of_match_table = of_match_ptr(mux_gpio_dt_ids),
> + },
> + .probe = mux_gpio_probe,
> + .remove = mux_gpio_remove,
> +};
> +module_platform_driver(mux_gpio_driver);
> +
> +MODULE_AUTHOR("Peter Rosin <peda-koto5C5qi+TLoDKTGw+V6w@public.gmane.org");
> +MODULE_DESCRIPTION("GPIO-controlled multiplexer driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mux.h b/include/linux/mux.h
> new file mode 100644
> index 000000000000..5b21b8184056
> --- /dev/null
> +++ b/include/linux/mux.h
> @@ -0,0 +1,53 @@
> +/*
> + * mux.h - definitions for the multiplexer interface
> + *
> + * Copyright (C) 2016 Axentia Technologies AB
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_MUX_H
> +#define _LINUX_MUX_H
> +
> +#include <linux/device.h>
> +#include <linux/rwsem.h>
> +
> +struct mux_control;
> +
> +struct mux_control_ops {
> + int (*set)(struct mux_control *mux, int reg);
> +};
> +
> +struct mux_control {
> + struct rw_semaphore lock; /* protects the state of the mux */
> +
> + struct device dev;
> + int id;
> +
> + int cached_state;
> + int idle_state;
> +
> + const struct mux_control_ops *ops;
> +
> + void *priv;
> +};
> +
> +#define to_mux_control(x) container_of((x), struct mux_control, dev)
> +
> +struct mux_control *mux_control_alloc(size_t sizeof_priv);
> +int mux_control_register(struct mux_control *mux);
> +void mux_control_unregister(struct mux_control *mux);
> +void mux_control_put(struct mux_control *mux);
> +
> +int mux_control_select(struct mux_control *mux, int state);
> +int mux_control_deselect(struct mux_control *mux);
> +
> +struct mux_control *mux_control_get(struct device *dev,
> + const char *mux_name);
> +struct mux_control *devm_mux_control_get(struct device *dev,
> + const char *mux_name);
> +void devm_mux_control_put(struct device *dev, struct mux_control *mux);
> +
> +#endif /* _LINUX_MUX_H */
>
^ permalink raw reply
* Re: [RFC PATCH v2 1/7] dt-bindings: document devicetree bindings for mux-gpio
From: Jonathan Cameron @ 2016-11-19 15:21 UTC (permalink / raw)
To: Peter Rosin, Rob Herring
Cc: linux-kernel, Wolfram Sang, Mark Rutland, Hartmut Knaack,
Lars-Peter Clausen, Peter Meerwald-Stadler, Arnd Bergmann,
Greg Kroah-Hartman, linux-i2c, devicetree, linux-iio
In-Reply-To: <a6a6610e-6827-a069-36e6-ca30a7110836@axentia.se>
On 18/11/16 16:59, Peter Rosin wrote:
> On 2016-11-18 16:35, Rob Herring wrote:
>> On Thu, Nov 17, 2016 at 10:48:03PM +0100, Peter Rosin wrote:
>>> ---
>>> .../devicetree/bindings/misc/mux-gpio.txt | 79 ++++++++++++++++++++++
>>> 1 file changed, 79 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/misc/mux-gpio.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/misc/mux-gpio.txt b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> new file mode 100644
>>> index 000000000000..73699a37824f
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/misc/mux-gpio.txt
>>> @@ -0,0 +1,79 @@
>>> +GPIO-based multiplexer controller bindings
>>> +
>>> +Define what GPIO pins are used to control a multiplexer. Or several
>>> +multiplexers, if the same pins control more than one multiplexer.
>>
>> I think this makes sense in your case, but I think it is too complicated
>> for a non-shared case. Perhaps mux-gpios should be used directly (i.e.
>> in the adc-mux node) and control-muxes only used for the shared case.
>>
>> Part of me feels like you are working around in DT the GPIO subsystem
>> limitation that it can't share GPIO lines. Either this could be fixed
>> in some way in the GPIO subsystem, or the mux subsys could deal with it.
>> You just have to look up if you already have a mux registered for the
>> same GPIOs. Of course, that may make the mux subsys pretty much GPIO
>> only, but I'm having a hard time thinking how you would have shared
>> muxes that are not GPIO controlled. Any other control would be
>> integrated into the mux itself.
>
> But if someone wants to mux an adc line with a mux that is some kind of
> integrated i2c device, you'd have to add extra code to the iio muxer
> driver to handle that case. Or fork it. Or build something like the
> i2c muxer infrastructure and separate out the mux control in small
> drivers and handle the generic iio muxing centrally. But then someone
> else uses that i2c device to instead mux an i2c bus, and you'd end up
> with code duplication when that same muxer control code is added under
> drivers/i2c/muxes.
>
> With the proposed solution, this is unified.
>
> I'd just hate to see drivers for muxers added under drivers/i2c/muxes
> that do little more that control a mux that happens to be used to mux
> an i2c bus, but are generic muxers that could equally well mux something
> else. Even if the control is integrated into the mux, what the mux is
> actually used for should perhaps not determine where its driver should
> live.
>
> Anyway, I don't know what to make with your suggestion, I just don't
> see the path forward (not enough experience with the kernel and/or gpio
> code). And it would be a limited solution (GPIO only,a s you say) so it
> doesn't feel right.
Also worth pointing out here the possibility of multi pole muxes...
Relays are ultimately muxes as well (be it slow ones ;)
A quick google fed me:
TI SN74LS153 for example. This one is digital only though...
Analog option (in both senses) is:
http://www.analog.com/media/en/technical-documentation/data-sheets/ADG888.pdf
So these 'look' the same as two single muxes wired to the same GPIOs.
>
> Is there perhaps some way to keep the complicated shared case work as
> is (or equivalently, the exact details are not important), and also
> provide a simpler in-node thingy to glue a mux control to a consumer
> w/o pointing to it with a phandle, but still have the same mux driver
> handle both cases? No, I'm not a devicetree guru, so I don't see a
> solution for that either, but maybe someone else does?
>
> Perhaps the consumer could look for the mux control in first the
> phandle, as in my proposal. If not found, it could also look for
> a mux provider bound to child node.
>
> adc-mux {
> compatible = "iio-mux";
> io-channels = <&adc 0>;
> io-channel-names = "parent";
>
> mux-control {
> compatible = "mux-gpio";
> mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> <&pioA 1 GPIO_ACTIVE_HIGH>;
> };
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> sync-1@0 {
> reg = <0>;
> };
> /* ... */
> };
>
> Or perhaps look in a parent node:
>
> mux-control {
> compatible = "mux-gpio";
> mux-gpios = <&pioA 0 GPIO_ACTIVE_HIGH>,
> <&pioA 1 GPIO_ACTIVE_HIGH>;
>
> adc-mux {
> compatible = "iio-mux";
> io-channels = <&adc 0>;
> io-channel-names = "parent";
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> sync-1@0 {
> reg = <0>;
> };
> /* ... */
> };
> };
>
> With the last suggestion, you could have multiple children of the
> mux-control node for the complicated case where it controls more
> than one mux. Not too bad? Hmm, what does the driver for the
> mux-control node have to do to have drivers tied to its children?
>
> Maybe this last layout should be the only thing supported? Good
> enough for me anyway...
>
> Cheers,
> Peter
>
> PS. I will take care of the other comments for the next round.
>
^ permalink raw reply
* [PATCH] i2c: i2c-mux-pca954x: fix deselect enabling for device-tree
From: Peter Rosin @ 2016-11-19 9:48 UTC (permalink / raw)
To: linux-kernel
Cc: Alex Hemme, Wolfram Sang, Wolfram Sang, MakeB, linux-i2c, stable,
Ziyang Wu, Peter Rosin
From: Alex Hemme <ahemme@cisco.com>
Deselect functionality can be ignored for device-trees with
"i2c-mux-idle-disconnect" entries if no platform_data is available.
By enabling the deselect functionality outside the platform_data
block the logic works as it did in previous kernels.
Fixes: 7fcac9807175 ("i2c: i2c-mux-pca954x: convert to use an explicit i2c mux core")
Cc: <stable@vger.kernel.org> # v4.7+
Signed-off-by: Alex Hemme <ahemme@cisco.com>
Signed-off-by: Ziyang Wu <ziywu@cisco.com>
[touched up a few minor issues /peda]
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/i2c/muxes/i2c-mux-pca954x.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Hi!
I got this regression report and patch off-list, and am now
feeding it on to the appropriate places.
I would like to thank Alex (and team?) for finding the bug and
providing a fix for the bug that I introduced.
Cheers,
Peter
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 1091346f2480..8bc3d36d2837 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -268,9 +268,9 @@ static int pca954x_probe(struct i2c_client *client,
/* discard unconfigured channels */
break;
idle_disconnect_pd = pdata->modes[num].deselect_on_exit;
- data->deselect |= (idle_disconnect_pd
- || idle_disconnect_dt) << num;
}
+ data->deselect |= (idle_disconnect_pd ||
+ idle_disconnect_dt) << num;
ret = i2c_mux_add_adapter(muxc, force, num, class);
--
2.1.4
^ permalink raw reply related
* Re: [PATCH v3 4/5] i2c: designware: Add slave mode as separated driver
From: kbuild test robot @ 2016-11-18 22:27 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Luis.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <36abadc931ab0814019c9b2214886bcb4e4ce5c1.1479410047.git.lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 6345 bytes --]
Hi Luis,
[auto build test ERROR on v4.9-rc5]
[also build test ERROR on next-20161117]
[cannot apply to wsa/i2c/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Luis-Oliveira/i2c-designware-Add-slave-support/20161118-193236
config: x86_64-randconfig-s4-11190538 (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All error/warnings (new ones prefixed by >>):
drivers/i2c/busses/i2c-designware-slave.c: In function 'i2c_dw_irq_handler_slave':
>> drivers/i2c/busses/i2c-designware-slave.c:295:3: error: implicit declaration of function 'i2c_slave_event' [-Werror=implicit-function-declaration]
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
^~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-designware-slave.c:295:31: error: 'I2C_SLAVE_WRITE_REQUESTED' undeclared (first use in this function)
i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
^~~~~~~~~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-slave.c:295:31: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/i2c/busses/i2c-designware-slave.c:302:6: error: 'I2C_SLAVE_WRITE_RECEIVED' undeclared (first use in this function)
I2C_SLAVE_WRITE_RECEIVED, &val)) {
^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-designware-slave.c:314:7: error: 'I2C_SLAVE_READ_REQUESTED' undeclared (first use in this function)
I2C_SLAVE_READ_REQUESTED, &val))
^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-designware-slave.c:320:36: error: 'I2C_SLAVE_READ_PROCESSED' undeclared (first use in this function)
if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
^~~~~~~~~~~~~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-designware-slave.c:324:31: error: 'I2C_SLAVE_STOP' undeclared (first use in this function)
i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
^~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-slave.c: At top level:
>> drivers/i2c/busses/i2c-designware-slave.c:370:2: error: unknown field 'reg_slave' specified in initializer
.reg_slave = i2c_dw_reg_slave,
^
>> drivers/i2c/busses/i2c-designware-slave.c:370:15: warning: excess elements in struct initializer
.reg_slave = i2c_dw_reg_slave,
^~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-slave.c:370:15: note: (near initialization for 'i2c_dw_algo')
>> drivers/i2c/busses/i2c-designware-slave.c:371:2: error: unknown field 'unreg_slave' specified in initializer
.unreg_slave = i2c_dw_unreg_slave,
^
drivers/i2c/busses/i2c-designware-slave.c:371:17: warning: excess elements in struct initializer
.unreg_slave = i2c_dw_unreg_slave,
^~~~~~~~~~~~~~~~~~
drivers/i2c/busses/i2c-designware-slave.c:371:17: note: (near initialization for 'i2c_dw_algo')
cc1: some warnings being treated as errors
vim +/i2c_slave_event +295 drivers/i2c/busses/i2c-designware-slave.c
289 dw_readl(dev, DW_IC_CLR_START_DET);
290 if (stat & DW_IC_INTR_ACTIVITY)
291 dw_readl(dev, DW_IC_CLR_ACTIVITY);
292 if (stat & DW_IC_INTR_RX_OVER)
293 dw_readl(dev, DW_IC_CLR_RX_OVER);
294 if ((stat & DW_IC_INTR_RX_FULL) && (stat & DW_IC_INTR_STOP_DET))
> 295 i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_REQUESTED, &val);
296
297 if (slave_activity) {
298 if (stat & DW_IC_INTR_RD_REQ) {
299 if (stat & DW_IC_INTR_RX_FULL) {
300 val = dw_readl(dev, DW_IC_DATA_CMD);
301 if (!i2c_slave_event(dev->slave,
> 302 I2C_SLAVE_WRITE_RECEIVED, &val)) {
303 dev_dbg(dev->dev, "Byte %X acked! ",
304 val);
305 }
306 dw_readl(dev, DW_IC_CLR_RD_REQ);
307 stat = i2c_dw_read_clear_intrbits_slave(dev);
308 } else {
309 dw_readl(dev, DW_IC_CLR_RD_REQ);
310 dw_readl(dev, DW_IC_CLR_RX_UNDER);
311 stat = i2c_dw_read_clear_intrbits_slave(dev);
312 }
313 if (!i2c_slave_event(dev->slave,
> 314 I2C_SLAVE_READ_REQUESTED, &val))
315 dw_writel(dev, val, DW_IC_DATA_CMD);
316 }
317 }
318
319 if (stat & DW_IC_INTR_RX_DONE) {
> 320 if (!i2c_slave_event(dev->slave, I2C_SLAVE_READ_PROCESSED,
321 &val))
322 dw_readl(dev, DW_IC_CLR_RX_DONE);
323
> 324 i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
325 stat = i2c_dw_read_clear_intrbits_slave(dev);
326 return true;
327 }
328
329 if (stat & DW_IC_INTR_RX_FULL) {
330 val = dw_readl(dev, DW_IC_DATA_CMD);
331 if (!i2c_slave_event(dev->slave, I2C_SLAVE_WRITE_RECEIVED,
332 &val))
333 dev_dbg(dev->dev, "Byte %X acked! ", val);
334 } else {
335 i2c_slave_event(dev->slave, I2C_SLAVE_STOP, &val);
336 stat = i2c_dw_read_clear_intrbits_slave(dev);
337 }
338
339 if (stat & DW_IC_INTR_TX_OVER) {
340 dw_readl(dev, DW_IC_CLR_TX_OVER);
341 return true;
342 }
343 return true;
344 }
345
346 static irqreturn_t i2c_dw_isr_slave(int this_irq, void *dev_id)
347 {
348 struct dw_i2c_dev *dev = dev_id;
349 u32 stat, enabled, mode;
350
351 enabled = dw_readl(dev, DW_IC_ENABLE);
352 mode = dw_readl(dev, DW_IC_CON);
353 stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
354
355 dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled,
356 stat);
357 if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
358 return IRQ_NONE;
359
360 stat = i2c_dw_read_clear_intrbits_slave(dev);
361 if (!i2c_dw_irq_handler_slave(dev))
362 return IRQ_NONE;
363
364 complete(&dev->cmd_complete);
365 return IRQ_HANDLED;
366 }
367
368 static struct i2c_algorithm i2c_dw_algo = {
369 .functionality = i2c_dw_func,
> 370 .reg_slave = i2c_dw_reg_slave,
> 371 .unreg_slave = i2c_dw_unreg_slave,
372 };
373
374 void i2c_dw_disable_slave(struct dw_i2c_dev *dev)
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25799 bytes --]
^ permalink raw reply
* Re: [v1] i2c: mux: pca954x : Supports ACPI
From: Peter Rosin @ 2016-11-18 20:48 UTC (permalink / raw)
To: tnhuynh, Wolfram Sang, linux-i2c, linux-kernel
Cc: Loc Ho, Thang Nguyen, Phong Vo, patches
In-Reply-To: <1479481269-16363-1-git-send-email-tnhuynh@apm.com>
Hi,
Thanks for your patch!
Some small suggestions inline.
On 2016-11-18 16:01, tnhuynh@apm.com wrote:
> From: Tin Huynh <tnhuynh@apm.com>
>
> This patch enable ACPI support for mux-pca954x driver.
>
> Signed-off-by: Tin Huynh <tnhuynh@apm.com>
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 25 ++++++++++++++++++++++++-
> 1 files changed, 24 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 1091346..e7ef93b 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -42,6 +42,7 @@
> #include <linux/i2c/pca954x.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/acpi.h>
Please keep the includes sorted.
> #include <linux/of_device.h>
> #include <linux/pm.h>
> #include <linux/slab.h>
> @@ -120,6 +121,19 @@ struct pca954x {
> };
> MODULE_DEVICE_TABLE(i2c, pca954x_id);
>
> +static const struct acpi_device_id pca954x_acpi_ids[] = {
> + { "PCA9540", pca_9540 },
I would write that as:
{ .id = "PCA9540", .driver_data = pca_9540, },
But that doesn't seem common for other acpi_device_id tables.
I wonder why?
> + { "PCA9542", pca_9540 },
> + { "PCA9543", pca_9543 },
> + { "PCA9544", pca_9544 },
> + { "PCA9545", pca_9545 },
> + { "PCA9546", pca_9545 },
> + { "PCA9547", pca_9547 },
> + { "PCA9548", pca_9548 },
> + { }
> +};
> +MODULE_DEVICE_TABLE(acpi, pca954x_acpi_ids);
This table should be #ifdef CONFIG_ACPI.
> +
> #ifdef CONFIG_OF
> static const struct of_device_id pca954x_of_match[] = {
> { .compatible = "nxp,pca9540", .data = &chips[pca_9540] },
> @@ -245,8 +259,16 @@ static int pca954x_probe(struct i2c_client *client,
> match = of_match_device(of_match_ptr(pca954x_of_match), &client->dev);
> if (match)
> data->chip = of_device_get_match_data(&client->dev);
> - else
> + else if (id) {
> data->chip = &chips[id->driver_data];
> + } else {
> + const struct acpi_device_id *id;
Please don't shadow the outer id variable.
Cheers,
Peter
> +
> + id = acpi_match_device(pca954x_acpi_ids, &client->dev);
> + if (!id)
> + return -ENODEV;
> + data->chip = &chips[id->driver_data];
> + }
>
> data->last_chan = 0; /* force the first selection */
>
> @@ -321,6 +343,7 @@ static int pca954x_resume(struct device *dev)
> .name = "pca954x",
> .pm = &pca954x_pm,
> .of_match_table = of_match_ptr(pca954x_of_match),
> + .acpi_match_table = ACPI_PTR(pca954x_acpi_ids),
> },
> .probe = pca954x_probe,
> .remove = pca954x_remove,
>
^ permalink raw reply
* [PATCH 2/2] i2c: designware: fix rx fifo depth tracking
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
To: Andrew Jackson, Liviu Dudau, Mika Westerberg, Wolfram Sang,
Jarkko Nikula, Andy Shevchenko
Cc: linux-i2c, linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>
When loading the TX fifo to receive bytes on the I2C bus, we incorrectly
count the number of bytes:
rx_limit = dev->rx_fifo_depth - dw_readl(dev, DW_IC_RXFLR);
while (buf_len > 0 && tx_limit > 0 && rx_limit > 0) {
if (rx_limit - dev->rx_outstanding <= 0)
break;
rx_limit--;
dev->rx_outstanding++;
}
DW_IC_RXFLR indicates how many bytes are available to be read in the
FIFO, dev->rx_fifo_depth is the FIFO size, and dev->rx_outstanding is
the number of bytes that we've requested to be read so far, but which
have not been read.
Firstly, increasing dev->rx_outstanding and decreasing rx_limit and then
comparing them results in each byte consuming "two" bytes in this
tracking, so this is obviously wrong.
Secondly, the number of bytes that _could_ be received into the FIFO at
any time is the number of bytes we have so far requested but not yet
read from the FIFO - in other words dev->rx_outstanding.
So, in order to request enough bytes to fill the RX FIFO, we need to
request dev->rx_fifo_depth - dev->rx_outstanding bytes.
Modifying the code thusly results in us reaching the maximum number of
bytes outstanding each time we queue more "receive" operations, provided
the transfer allows that to happen.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
With this applied, I now get:
[ 1.726388] i2c_designware 7ffa0000.i2c: transfer terminated early - interrupt latency too high?
[ 1.733813] tda998x 0-0070: Error -5 reading from 0x900
[ 1.737708] tda998x 0-0070: failed to read edid block 0: -5
[ 1.743683] tda998x 0-0070: failed to read EDID
which is a more graceful failure than letting DRM detect the bad
transfer by checking the EDID checksum and hoping that the untransferred
bytes don't result in the EDID checksum succeeding.
drivers/i2c/busses/i2c-designware-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..9703fe392543 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -611,7 +611,7 @@ i2c_dw_xfer_msg(struct dw_i2c_dev *dev)
if (msgs[dev->msg_write_idx].flags & I2C_M_RD) {
/* avoid rx buffer overrun */
- if (rx_limit - dev->rx_outstanding <= 0)
+ if (dev->rx_outstanding >= dev->rx_fifo_depth)
break;
dw_writel(dev, cmd | 0x100, DW_IC_DATA_CMD);
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] i2c: designware: report short transfers
From: Russell King @ 2016-11-18 19:40 UTC (permalink / raw)
To: Andrew Jackson, Liviu Dudau, Mika Westerberg, Wolfram Sang,
Jarkko Nikula, Andy Shevchenko
Cc: linux-i2c, linux-arm-kernel
In-Reply-To: <20161118193542.GO1041@n2100.armlinux.org.uk>
Rather than reporting success for a short transfer due to interrupt
latency, report an error both to the caller, as well as to the kernel
log.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/i2c/busses/i2c-designware-core.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 9703fe392543..c53058d6139c 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -758,7 +758,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
}
/* no error */
- if (likely(!dev->cmd_err)) {
+ if (likely(!dev->cmd_err && !dev->status)) {
ret = num;
goto done;
}
@@ -768,6 +768,11 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
ret = i2c_dw_handle_tx_abort(dev);
goto done;
}
+
+ if (dev->status)
+ dev_err(dev->dev,
+ "transfer terminated early - interrupt latency too high?\n");
+
ret = -EIO;
done:
--
2.7.4
^ permalink raw reply related
* [BUG] i2c-designware silently fails on long transfers
From: Russell King - ARM Linux @ 2016-11-18 19:35 UTC (permalink / raw)
To: Andrew Jackson, Liviu Dudau, Mika Westerberg, Wolfram Sang,
Jarkko Nikula, Andy Shevchenko
Cc: linux-i2c, linux-arm-kernel
With reference to this commit:
commit d39f77b06a712fcba6185a20bb209e357923d980
Author: Andrew Jackson <Andrew.Jackson@arm.com>
Date: Fri Nov 7 12:10:44 2014 +0000
i2c: designware: prevent early stop on TX FIFO empty
If the Designware core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN
set to zero, allowing the TX FIFO to become empty causes a STOP
condition to be generated on the I2C bus. If the transmit FIFO
threshold is set too high, an erroneous STOP condition can be
generated on long transfers - particularly where the interrupt
latency is extended.
Signed-off-by: Andrew Jackson <Andrew.Jackson@arm.com>
Signed-off-by: Liviu Dudau <Liviu.Dudau@arm.com>
Tested-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Wolfram Sang <wsa@the-dreams.de>
The TDA998x driver issues long I2C transfers to read the EDID from the
device - and userspace can also issue large transfers too. However,
if a DW core is configured with IC_EMPTYFIFO_HOLD_MASTER_EN set as
zero, the above commit doesn't seem to solve the problem. During
boot, with the patch below, I see:
[ 1.736549] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x10
[ 1.736564] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
[ 1.736608] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.736799] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[ 1.736819] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x510
...
[ 1.737986] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.738010] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x514
[ 1.738034] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x504
[ 1.738039] random: fast init done
[ 1.740120] i2c_designware 7ffa0000.i2c: i2c_dw_isr: enabled=0x1 stat=0x714
[ 1.740231] i2c_dw_xfer: ffffffc97657b770:1 -> ffffffc97657b770:1 (0:0) [0 0 3 0] 8 [tx:ffffffc976682380:47] [rx:ffffffc9766823c9:55]
[ 1.740249] [drm:drm_edid_block_valid] *ERROR* EDID checksum is invalid, remainder is 93
[ 1.746979] Raw EDID:
[ 1.747934] 00 ff ff ff ff ff ff 00 34 a9 96 a2 01 01 01 01
[ 1.752342] 00 17 01 03 80 80 48 78 0a da ff a3 58 4a a2 29
[ 1.756748] 17 49 4b 21 08 00 31 40 45 40 61 40 81 80 01 01
[ 1.761153] 01 01 01 01 01 01 02 3a 80 d0 72 38 2d 40 10 2c
[ 1.765555] 45 80 ba 88 21 00 00 1e 02 00 d0 4e 30 09 12 54
[ 1.769958] 01 08 02 00 23 36 01 40 01 05 00 80 a1 4c 4b 49
[ 1.774361] 22 00 00 40 03 00 28 00 23 01 20 00 01 88 00 01
[ 1.778762] 08 00 00 40 00 02 03 04 0a 00 80 00 02 00 00 40
The significant thing is the "i2c_dw_xfer" line, where I add a print of
the current state. Here, we can see that the transfer is mid-way, but
a stop condition has been generated by the hardware, leaving 55 bytes
to be received.
Unfortunately, the i2c-designware driver ignores this, and believes that
the transfer completed both fully and successfully, but returns bogus
data to userspace or the kernel driver. That's really _bad_ behaviour
by the driver - it should at least return an error.
This problem is _soo_ bad that on my Juno, I can't run Xorg (it hits
this every time we try to read the EDID) nor can I boot with the TV
connected (it hits this every boot as well.)
I'd go as far as to say that the i2c-designware hardware, when
configured with this option set to zero, is fundamentally broken for OS
which do not provide any guarantee for interrupt latency, such as Linux.
The commit above tries to mitigate this by reducing the Tx FIFO
threshold, so the interrupt is raised sooner, but that's clearly not
enough for reliable operation.
Another mitigation would be to lower the I2C bus frequency on Juno from
400kHz to 100kHz, so that there's 4x longer IRQ latency possible.
However, even that isn't going to be reliable - even going to 100kHz
isn't going to allow the above case to be solved - the interrupt is
delayed by around 2ms, and it takes about 1.4ms to send/receive 16 bytes
at 100kHz. (9 * 16 / (100*10^3)).
So, I think all hope is lost for i2c-designware on Juno to cope with
reading the EDID from TDA998x reliably.
I have one patch which solves a problem in the accounting of bytes, and
another to ensure that we return an error for an incomplete transfer,
both will be sent threaded to this mail.
diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 11e866d05368..060ae9e5a916 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -752,6 +752,15 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
goto done;
}
+ printk(KERN_DEBUG "%s: %p:%d -> %p:%d (%d:%d) [%x %x %x %x] %d [tx:%p:%d] [rx:%p:%d]\n",
+ __func__, msgs, num,
+ dev->msgs, dev->msgs_num,
+ dev->msg_write_idx, dev->msg_read_idx,
+ dev->cmd_err, dev->msg_err, dev->status, dev->abort_source,
+ dev->rx_outstanding,
+ dev->tx_buf, dev->tx_buf_len,
+ dev->rx_buf, dev->rx_buf_len);
+
if (dev->msg_err) {
ret = dev->msg_err;
goto done;
@@ -857,7 +866,7 @@ static irqreturn_t i2c_dw_isr(int this_irq, void *dev_id)
enabled = dw_readl(dev, DW_IC_ENABLE);
stat = dw_readl(dev, DW_IC_RAW_INTR_STAT);
- dev_dbg(dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
+ dev_printk(KERN_DEBUG, dev->dev, "%s: enabled=%#x stat=%#x\n", __func__, enabled, stat);
if (!enabled || !(stat & ~DW_IC_INTR_ACTIVITY))
return IRQ_NONE;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related
* Re: [PATCH v3 3/5] i2c: designware: Add slave definitions
From: Rob Herring @ 2016-11-18 17:01 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
mark.rutland-5wv7dgnIgG8, jarkko.nikula-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <1479472552.22212.23.camel-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
On Fri, Nov 18, 2016 at 02:35:52PM +0200, Andy Shevchenko wrote:
> On Fri, 2016-11-18 at 11:19 +0000, Luis Oliveira wrote:
> > - Add slave defintitions to i2c-designware-core
> > - Changes in Kconfig to auto-enable I2C_SLAVE when compiling the
> > modules
> > - Add compatible string to designware-core.txt explaining the
> > devicetree bindings
> >
>
>
> > --- a/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-designware.txt
> > @@ -2,7 +2,9 @@
> >
> > Required properties :
> >
> > - - compatible : should be "snps,designware-i2c"
> > + - compatible : should be:
> > + - "snps,designware-i2c" to setup the hardware block as I2C master.
> > + - "snps,designware-i2c-slave" to setup the hardware block as I2C
> > slave.
>
> Not sure about this one.
>
> Compatible string is more generic than list of modes. Basically you have
> to add a property which selects mode.
Yes, agreed. And come up with a common property.
> DT people's ACK is a must for this change.
>
>
> --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -470,6 +470,7 @@ config I2C_DESIGNWARE_CORE
> > config I2C_DESIGNWARE_PLATFORM
> > tristate "Synopsys DesignWare Platform"
> > select I2C_DESIGNWARE_CORE
> > + select I2C_SLAVE
> >
>
> Common rule, generic dependencies usually go first
>
> select I2C_SLAVE
> select I2C_DESIGNWARE_CORE
>
> --
> Andy Shevchenko <andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> Intel Finland Oy
--
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
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