* [PATCH 0/2] i2c: sunxi: add P2WI controller support @ 2014-04-24 11:55 Boris BREZILLON 2014-04-24 11:55 ` [PATCH 1/2] i2c: sunxi: add P2WI DT bindings documentation Boris BREZILLON 2014-04-24 11:55 ` [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support Boris BREZILLON 0 siblings, 2 replies; 6+ messages in thread From: Boris BREZILLON @ 2014-04-24 11:55 UTC (permalink / raw) To: linux-arm-kernel Hello, This series adds support for the P2WI block used by some Allwinner boards to interface with the AXP221 PMIC. I'm still not sure on how this bus should be integrated into the I2C subsystem: There was a discussion about adding new I2C flags: [1], but, I want to reuse the existing axp209 driver [2] which in turn uses a regmap, and the regmap over I2C driver does not support setting specific flags when launching I2C transfer (thought this could be added to the regmap code ;-)). I decided to get rid of these new flags (I2C_PUSHPULL, I2C_M_DROP_ADDRESS, ...) in my first proposal, but let me know if you think this should be re-introduced. Moreover, some parts are missing (either because I need some informations on the HW blocks or because I don't know how it should be done): - the bus frequency config (the current version hardcode the bus frequency config): I need informations about the clk tree used by APB0 clks in order to properly implement missing clk drivers and define missing DT nodes. - the initialization (or I2C to P2WI mode switch) process: A specific value has to be written in one of the slave device register to switch the device from I2C to P2WI mode. The switch process is handled by the P2WI controller block (using the P2WI_PMCR register). The problem is that I don't know which register should be written and what's the value to switch to the P2WI mode (only the device driver knows it). How about encoding this in the DT, and then parse the child node during probe ? - address consistency check: I don't know when the slave device address should be saved in the p2wi struct (during probe using DT child node informations, at the first transfer, ...) Best Regards, Boris [1] http://www.spinics.net/lists/linux-i2c/msg15066.html [2] http://comments.gmane.org/gmane.comp.hardware.netbook.arm.sunxi/8947 Boris BREZILLON (2): i2c: sunxi: add P2WI DT bindings documentation i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support .../devicetree/bindings/i2c/i2c-sunxi-p2wi.txt | 27 ++ drivers/i2c/busses/Kconfig | 12 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-sunxi-p2wi.c | 317 +++++++++++++++++++++ 4 files changed, 357 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt create mode 100644 drivers/i2c/busses/i2c-sunxi-p2wi.c -- 1.8.3.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] i2c: sunxi: add P2WI DT bindings documentation 2014-04-24 11:55 [PATCH 0/2] i2c: sunxi: add P2WI controller support Boris BREZILLON @ 2014-04-24 11:55 ` Boris BREZILLON 2014-04-24 11:55 ` [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support Boris BREZILLON 1 sibling, 0 replies; 6+ messages in thread From: Boris BREZILLON @ 2014-04-24 11:55 UTC (permalink / raw) To: linux-arm-kernel P2WI (Push/Pull 2 Wire Interface) is an SMBus like bus used to communicate with some PMICs (like the AXP221). Document P2WI DT bindings which are pretty much the same as the one defined for the marvell's mv64xxx controller. Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> --- .../devicetree/bindings/i2c/i2c-sunxi-p2wi.txt | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt diff --git a/Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt b/Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt new file mode 100644 index 0000000..d59347d --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt @@ -0,0 +1,27 @@ + +* Allwinner P2WI (Push/Pull 2 Wire Interface) controller + +Required properties : + + - reg : Offset and length of the register set for the device. + - compatible : Should one of the following: + - "allwinner,sun6i-a31-p2wi" + - interrupts : The interrupt line connected to the P2WI peripheral. + - clocks : The gate clk connected to the P2WI peripheral. + - resets : The reset line connected to the P2WI peripheral. + +Optional properties : + + - clock-frequency : Desired P2WI bus clock frequency in Hz. If not set the +default frequency is 100kHz + +Example: + + p2wi at 01f03400 { + compatible = "allwinner,sun6i-a31-p2wi"; + reg = <0x01f03400 0x400>; + interrupts = <0 39 4>; + clocks = <&apb0_gates 3>; + clock-frequency = <6000000>; + resets = <&apb0_rst 3>; + }; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-04-24 11:55 [PATCH 0/2] i2c: sunxi: add P2WI controller support Boris BREZILLON 2014-04-24 11:55 ` [PATCH 1/2] i2c: sunxi: add P2WI DT bindings documentation Boris BREZILLON @ 2014-04-24 11:55 ` Boris BREZILLON 2014-04-24 13:29 ` Maxime Ripard 1 sibling, 1 reply; 6+ messages in thread From: Boris BREZILLON @ 2014-04-24 11:55 UTC (permalink / raw) To: linux-arm-kernel The P2WI looks like an SMBus controller which only supports byte data transfers. But, it differs from standard SMBus protocol on several aspects: - it supports only one slave device, and thus drop the address field - it adds a parity bit every 8bits of data - only one read access is required to read a byte (instead of a read followed by a write access in standard SMBus protocol) - there's no Ack bit after each byte transfer This means this bus cannot be used to interface with standard SMBus devices (the only known device to support this interface is the AXP221 PMIC). Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> --- drivers/i2c/busses/Kconfig | 12 ++ drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-sunxi-p2wi.c | 317 ++++++++++++++++++++++++++++++++++++ 3 files changed, 330 insertions(+) create mode 100644 drivers/i2c/busses/i2c-sunxi-p2wi.c diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index c94db1c..37e53d6 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -771,6 +771,18 @@ config I2C_STU300 This driver can also be built as a module. If so, the module will be called i2c-stu300. +config I2C_SUNXI_P2WI + tristate "Allwinner sunxi internal P2WI controller" + depends on ARCH_SUNXI + help + If you say yes to this option, support will be included for the + P2WI (Push/Pull 2 Wire Interface) controller embedded in some sunxi + SOCs. + The P2WI looks like an SMBus controller (which supports only byte + accesses), except that it only supports one slave device. + This interface is used to connect to specific PMIC devices (like the + AXP221). + config I2C_TEGRA tristate "NVIDIA Tegra internal I2C controller" depends on ARCH_TEGRA diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile index 18d18ff..c63d2ec 100644 --- a/drivers/i2c/busses/Makefile +++ b/drivers/i2c/busses/Makefile @@ -75,6 +75,7 @@ obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o obj-$(CONFIG_I2C_ST) += i2c-st.o obj-$(CONFIG_I2C_STU300) += i2c-stu300.o +obj-$(CONFIG_I2C_SUNXI_P2WI) += i2c-sunxi-p2wi.o obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o obj-$(CONFIG_I2C_WMT) += i2c-wmt.o diff --git a/drivers/i2c/busses/i2c-sunxi-p2wi.c b/drivers/i2c/busses/i2c-sunxi-p2wi.c new file mode 100644 index 0000000..e3fdd76 --- /dev/null +++ b/drivers/i2c/busses/i2c-sunxi-p2wi.c @@ -0,0 +1,317 @@ +/* + * P2WI (Push-Pull Two Wire Interface) bus driver. + * + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> + * + * This file is licensed under the terms of the GNU General Public License + * version 2. This program is licensed "as is" without any warranty of any + * kind, whether express or implied. + */ +#include <linux/kernel.h> +#include <linux/slab.h> +#include <linux/module.h> +#include <linux/spinlock.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/platform_device.h> +#include <linux/io.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/of_irq.h> +#include <linux/clk.h> +#include <linux/err.h> +#include <linux/delay.h> +#include <linux/clk.h> +#include <linux/reset.h> + + +/* P2WI registers */ +#define P2WI_CTRL 0x0 +#define P2WI_CCR 0x4 +#define P2WI_INTE 0x8 +#define P2WI_INTS 0xc +#define P2WI_DADDR0 0x10 +#define P2WI_DADDR1 0x14 +#define P2WI_DLEN 0x18 +#define P2WI_DATA0 0x1c +#define P2WI_DATA1 0x20 +#define P2WI_LCR 0x24 +#define P2WI_PMCR 0x28 + +/* CTRL fields */ +#define P2WI_START_TRANS (1 << 7) +#define P2WI_ABORT_TRANS (1 << 6) +#define P2WI_GLOBAL_INT_ENB (1 << 1) +#define P2WI_SOFT_RST (1 << 0) + +/* CLK CTRL fields */ +#define P2WI_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) +#define P2WI_CLK_DIV(v) ((v) & 0xff) + +/* STATUS fields */ +#define P2WI_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) +#define P2WI_LOAD_BSY (1 << 2) +#define P2WI_TRANS_ERR (1 << 1) +#define P2WI_TRANS_OVER (1 << 0) + +/* DATA LENGTH fields*/ +#define P2WI_READ (1 << 4) +#define P2WI_DATA_LENGTH(v) ((v - 1) & 0x7) + +/* LINE CTRL fields*/ +#define P2WI_SCL_STATE (1 << 5) +#define P2WI_SDA_STATE (1 << 4) +#define P2WI_SCL_CTL (1 << 3) +#define P2WI_SCL_CTL_EN (1 << 2) +#define P2WI_SDA_CTL (1 << 1) +#define P2WI_SDA_CTL_EN (1 << 0) + +/* PMU MODE CTRL fields */ +#define P2WI_PMU_INIT_SEND (1 << 31) +#define P2WI_PMU_INIT_DATA(v) (((v) & 0xff) << 16) +#define P2WI_PMU_MODE_REG(v) (((v) & 0xff) << 8) +#define P2WI_PMU_DEV_ADDR(v) ((v) & 0xff) + +struct p2wi { + struct i2c_adapter adapter; + struct completion complete; + unsigned int irq; + unsigned int status; + void __iomem *regs; + struct clk *clk; + struct reset_control *rstc; +}; + +static irqreturn_t p2wi_interrupt(int irq, void *dev_id) +{ + struct p2wi *p2wi = dev_id; + unsigned long status; + + status = readl(p2wi->regs + P2WI_INTS); + p2wi->status = status; + + /* Clear interrupts */ + status &= (P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER); + writel(status, p2wi->regs + P2WI_INTS); + + complete(&p2wi->complete); + + return IRQ_HANDLED; +} + +static u32 p2wi_functionality(struct i2c_adapter *adap) +{ + return I2C_FUNC_SMBUS_BYTE_DATA; +} + +static int p2wi_smbus_xfer(struct i2c_adapter *adap, u16 addr, + unsigned short flags, char read_write, + u8 command, int size, union i2c_smbus_data *data) +{ + struct p2wi *p2wi = i2c_get_adapdata(adap); + unsigned long dlen = P2WI_DATA_LENGTH(1); + + /* + * TODO: check address consistency. + * The P2WI bus only support one slave. As a result it does not use + * the I2C address except when you're switching the slave device from + * I2C to P2WI mode. + * We should at least verify that the addr argument is consistent with + * the slave device address. + */ + + if (addr > 0xff) { + dev_err(&adap->dev, "invalid P2WI address\n"); + return -EINVAL; + } + + /* + * TODO: handle switch to P2WI mode. + * At the moment, we're considering the slave device as already + * switchedto P2WI (which means the bootloader has to switch the slave + * device from I2C to P2WI mode). + * We need at least 3 informations to launch the switch process: + * - the slave device address (addr argument) + * - the mode register + * - the P2WI mode value (to write in the mode register) + */ + + if (!data) + return -EINVAL; + + writel(command, p2wi->regs + P2WI_DADDR0); + + if (read_write == I2C_SMBUS_READ) + dlen |= P2WI_READ; + else + writel(data->byte, p2wi->regs + P2WI_DATA0); + + writel(dlen, p2wi->regs + P2WI_DLEN); + + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_START_TRANS) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + reinit_completion(&p2wi->complete); + + writel(P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER, + p2wi->regs + P2WI_INTE); + + writel(P2WI_START_TRANS | P2WI_GLOBAL_INT_ENB, p2wi->regs + P2WI_CTRL); + + wait_for_completion(&p2wi->complete); + + if (p2wi->status & P2WI_LOAD_BSY) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + + if (p2wi->status & P2WI_TRANS_ERR) { + dev_err(&adap->dev, "P2WI bus xfer error\n"); + return -ENXIO; + } + + if (read_write == I2C_SMBUS_READ) + data->byte = readl(p2wi->regs + P2WI_DATA0); + + return 0; +} + +static const struct i2c_algorithm p2wi_algo = { + .smbus_xfer = p2wi_smbus_xfer, + .functionality = p2wi_functionality, +}; + +static const struct of_device_id p2wi_of_match_table[] = { + { .compatible = "allwinner,sun6i-a31-p2wi" }, + {} +}; +MODULE_DEVICE_TABLE(of, p2wi_of_match_table); + +static int p2wi_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *r; + struct p2wi *p2wi; + int ret; + + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); + if (!p2wi) { + dev_err(dev, "failed to allocate p2wi struct\n"); + return -ENOMEM; + } + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + p2wi->regs = devm_request_and_ioremap(dev, r); + if (IS_ERR(p2wi->regs)) { + ret = PTR_ERR(p2wi->regs); + dev_err(dev, "failed to retrieve iomem resource: %d\n", + ret); + return ret; + } + + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); + ret = platform_get_irq(pdev, 0); + if (ret < 0) { + dev_err(dev, "failed to retrieve irq: %d\n", ret); + return ret; + } + p2wi->irq = ret; + + p2wi->clk = devm_clk_get(dev, NULL); + if (IS_ERR(p2wi->clk)) { + ret = PTR_ERR(p2wi->clk); + dev_err(dev, "failed to retrieve clk: %d\n", + ret); + return ret; + } + + ret = clk_prepare_enable(p2wi->clk); + if (ret) { + dev_err(dev, "failed to enable clk: %d\n", ret); + return ret; + } + + p2wi->rstc = devm_reset_control_get(dev, NULL); + if (IS_ERR(p2wi->rstc)) { + ret = PTR_ERR(p2wi->rstc); + dev_err(dev, "failed to retrieve reset controller: %d\n", + ret); + goto err_clk_disable; + } + + ret = reset_control_deassert(p2wi->rstc); + if (ret) { + dev_err(dev, "failed to deassert reset line: %d\n", + ret); + goto err_clk_disable; + } + + init_completion(&p2wi->complete); + p2wi->adapter.dev.parent = dev; + p2wi->adapter.algo = &p2wi_algo; + p2wi->adapter.owner = THIS_MODULE; + p2wi->adapter.dev.of_node = pdev->dev.of_node; + platform_set_drvdata(pdev, p2wi); + i2c_set_adapdata(&p2wi->adapter, p2wi); + + ret = devm_request_irq(dev, p2wi->irq, p2wi_interrupt, 0, pdev->name, + p2wi); + if (ret) { + dev_err(dev, "can't register interrupt handler irq%d: %d\n", + p2wi->irq, ret); + goto err_reset_assert; + } + + writel(P2WI_SOFT_RST, p2wi->regs + P2WI_CTRL); + + /* + * TODO: retrieve (from DT) and setup bus frequency. + * At the moment, we do not configure the bus frequency, and rely on + * bootloader config. + * In order to properly configure the bus frequency we need the p2wi + * clk rate which in turn is based on the ahb0 clock, and, we do not + * know how to calculate the ahb0 clk rate. + */ + writel(P2WI_SDA_OUT_DELAY(1) | P2WI_CLK_DIV(8), p2wi->regs + P2WI_CCR); + + ret = i2c_add_adapter(&p2wi->adapter); + if (!ret) + return 0; + +err_reset_assert: + reset_control_assert(p2wi->rstc); + +err_clk_disable: + clk_disable_unprepare(p2wi->clk); + + return ret; +} + +static int p2wi_remove(struct platform_device *dev) +{ + struct p2wi *p2wi = platform_get_drvdata(dev); + + reset_control_assert(p2wi->rstc); + clk_disable_unprepare(p2wi->clk); + i2c_del_adapter(&p2wi->adapter); + + return 0; +} + +static struct platform_driver p2wi_driver = { + .probe = p2wi_probe, + .remove = p2wi_remove, + .driver = { + .owner = THIS_MODULE, + .name = "i2c-sunxi-p2wi", + .of_match_table = p2wi_of_match_table, + }, +}; +module_platform_driver(p2wi_driver); + +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon@free-electrons.com>"); +MODULE_DESCRIPTION("Allwinner P2WI driver"); +MODULE_LICENSE("GPL"); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-04-24 11:55 ` [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support Boris BREZILLON @ 2014-04-24 13:29 ` Maxime Ripard 2014-04-25 7:50 ` Boris BREZILLON 0 siblings, 1 reply; 6+ messages in thread From: Maxime Ripard @ 2014-04-24 13:29 UTC (permalink / raw) To: linux-arm-kernel On Thu, Apr 24, 2014 at 01:55:16PM +0200, Boris BREZILLON wrote: > The P2WI looks like an SMBus controller which only supports byte data > transfers. But, it differs from standard SMBus protocol on several > aspects: > - it supports only one slave device, and thus drop the address field > - it adds a parity bit every 8bits of data > - only one read access is required to read a byte (instead of a read > followed by a write access in standard SMBus protocol) > - there's no Ack bit after each byte transfer > > This means this bus cannot be used to interface with standard SMBus > devices (the only known device to support this interface is the AXP221 > PMIC). > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > --- > drivers/i2c/busses/Kconfig | 12 ++ > drivers/i2c/busses/Makefile | 1 + > drivers/i2c/busses/i2c-sunxi-p2wi.c | 317 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 330 insertions(+) > create mode 100644 drivers/i2c/busses/i2c-sunxi-p2wi.c > > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig > index c94db1c..37e53d6 100644 > --- a/drivers/i2c/busses/Kconfig > +++ b/drivers/i2c/busses/Kconfig > @@ -771,6 +771,18 @@ config I2C_STU300 > This driver can also be built as a module. If so, the module > will be called i2c-stu300. > > +config I2C_SUNXI_P2WI > + tristate "Allwinner sunxi internal P2WI controller" Since the A31 is the only SoC that uses it, maybe you can drop the sunxi and use either sun6i or A31 here? > + depends on ARCH_SUNXI > + help > + If you say yes to this option, support will be included for the > + P2WI (Push/Pull 2 Wire Interface) controller embedded in some sunxi > + SOCs. > + The P2WI looks like an SMBus controller (which supports only byte > + accesses), except that it only supports one slave device. > + This interface is used to connect to specific PMIC devices (like the > + AXP221). > + > config I2C_TEGRA > tristate "NVIDIA Tegra internal I2C controller" > depends on ARCH_TEGRA > diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile > index 18d18ff..c63d2ec 100644 > --- a/drivers/i2c/busses/Makefile > +++ b/drivers/i2c/busses/Makefile > @@ -75,6 +75,7 @@ obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o > obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o > obj-$(CONFIG_I2C_ST) += i2c-st.o > obj-$(CONFIG_I2C_STU300) += i2c-stu300.o > +obj-$(CONFIG_I2C_SUNXI_P2WI) += i2c-sunxi-p2wi.o > obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o > obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o > obj-$(CONFIG_I2C_WMT) += i2c-wmt.o > diff --git a/drivers/i2c/busses/i2c-sunxi-p2wi.c b/drivers/i2c/busses/i2c-sunxi-p2wi.c > new file mode 100644 > index 0000000..e3fdd76 > --- /dev/null > +++ b/drivers/i2c/busses/i2c-sunxi-p2wi.c > @@ -0,0 +1,317 @@ > +/* > + * P2WI (Push-Pull Two Wire Interface) bus driver. > + * > + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> > + * > + * This file is licensed under the terms of the GNU General Public License > + * version 2. This program is licensed "as is" without any warranty of any > + * kind, whether express or implied. > + */ > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/spinlock.h> > +#include <linux/i2c.h> > +#include <linux/interrupt.h> > +#include <linux/platform_device.h> > +#include <linux/io.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/of_irq.h> > +#include <linux/clk.h> > +#include <linux/err.h> > +#include <linux/delay.h> > +#include <linux/clk.h> > +#include <linux/reset.h> > + > + > +/* P2WI registers */ > +#define P2WI_CTRL 0x0 > +#define P2WI_CCR 0x4 > +#define P2WI_INTE 0x8 > +#define P2WI_INTS 0xc > +#define P2WI_DADDR0 0x10 > +#define P2WI_DADDR1 0x14 > +#define P2WI_DLEN 0x18 > +#define P2WI_DATA0 0x1c > +#define P2WI_DATA1 0x20 > +#define P2WI_LCR 0x24 > +#define P2WI_PMCR 0x28 > + > +/* CTRL fields */ > +#define P2WI_START_TRANS (1 << 7) > +#define P2WI_ABORT_TRANS (1 << 6) > +#define P2WI_GLOBAL_INT_ENB (1 << 1) > +#define P2WI_SOFT_RST (1 << 0) BIT() ? > +/* CLK CTRL fields */ > +#define P2WI_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) > +#define P2WI_CLK_DIV(v) ((v) & 0xff) > + > +/* STATUS fields */ > +#define P2WI_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) > +#define P2WI_LOAD_BSY (1 << 2) > +#define P2WI_TRANS_ERR (1 << 1) > +#define P2WI_TRANS_OVER (1 << 0) > + > +/* DATA LENGTH fields*/ > +#define P2WI_READ (1 << 4) > +#define P2WI_DATA_LENGTH(v) ((v - 1) & 0x7) > + > +/* LINE CTRL fields*/ > +#define P2WI_SCL_STATE (1 << 5) > +#define P2WI_SDA_STATE (1 << 4) > +#define P2WI_SCL_CTL (1 << 3) > +#define P2WI_SCL_CTL_EN (1 << 2) > +#define P2WI_SDA_CTL (1 << 1) > +#define P2WI_SDA_CTL_EN (1 << 0) > + > +/* PMU MODE CTRL fields */ > +#define P2WI_PMU_INIT_SEND (1 << 31) > +#define P2WI_PMU_INIT_DATA(v) (((v) & 0xff) << 16) > +#define P2WI_PMU_MODE_REG(v) (((v) & 0xff) << 8) > +#define P2WI_PMU_DEV_ADDR(v) ((v) & 0xff) I'd very much prefer if your bits were prefixed by the register name. That way, you directly know in which register that bit belong, without having to scroll across the whole driver. > + > +struct p2wi { > + struct i2c_adapter adapter; > + struct completion complete; > + unsigned int irq; > + unsigned int status; > + void __iomem *regs; > + struct clk *clk; > + struct reset_control *rstc; > +}; > + > +static irqreturn_t p2wi_interrupt(int irq, void *dev_id) > +{ > + struct p2wi *p2wi = dev_id; > + unsigned long status; > + > + status = readl(p2wi->regs + P2WI_INTS); > + p2wi->status = status; > + > + /* Clear interrupts */ > + status &= (P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER); > + writel(status, p2wi->regs + P2WI_INTS); > + > + complete(&p2wi->complete); > + > + return IRQ_HANDLED; > +} > + > +static u32 p2wi_functionality(struct i2c_adapter *adap) > +{ > + return I2C_FUNC_SMBUS_BYTE_DATA; > +} > + > +static int p2wi_smbus_xfer(struct i2c_adapter *adap, u16 addr, > + unsigned short flags, char read_write, > + u8 command, int size, union i2c_smbus_data *data) > +{ > + struct p2wi *p2wi = i2c_get_adapdata(adap); > + unsigned long dlen = P2WI_DATA_LENGTH(1); > + > + /* > + * TODO: check address consistency. > + * The P2WI bus only support one slave. As a result it does not use > + * the I2C address except when you're switching the slave device from > + * I2C to P2WI mode. > + * We should at least verify that the addr argument is consistent with > + * the slave device address. > + */ > + > + if (addr > 0xff) { > + dev_err(&adap->dev, "invalid P2WI address\n"); > + return -EINVAL; > + } > + > + /* > + * TODO: handle switch to P2WI mode. > + * At the moment, we're considering the slave device as already > + * switchedto P2WI (which means the bootloader has to switch the slave ^ you need a space here > + * device from I2C to P2WI mode). > + * We need at least 3 informations to launch the switch process: > + * - the slave device address (addr argument) > + * - the mode register > + * - the P2WI mode value (to write in the mode register) > + */ > + > + if (!data) > + return -EINVAL; > + > + writel(command, p2wi->regs + P2WI_DADDR0); > + > + if (read_write == I2C_SMBUS_READ) > + dlen |= P2WI_READ; > + else > + writel(data->byte, p2wi->regs + P2WI_DATA0); > + > + writel(dlen, p2wi->regs + P2WI_DLEN); > + > + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_START_TRANS) { > + dev_err(&adap->dev, "P2WI bus busy\n"); > + return -EBUSY; > + } > + > + reinit_completion(&p2wi->complete); > + > + writel(P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER, > + p2wi->regs + P2WI_INTE); > + > + writel(P2WI_START_TRANS | P2WI_GLOBAL_INT_ENB, p2wi->regs + P2WI_CTRL); > + > + wait_for_completion(&p2wi->complete); > + > + if (p2wi->status & P2WI_LOAD_BSY) { > + dev_err(&adap->dev, "P2WI bus busy\n"); > + return -EBUSY; > + } > + > + You can drop the extra line here. > + if (p2wi->status & P2WI_TRANS_ERR) { > + dev_err(&adap->dev, "P2WI bus xfer error\n"); > + return -ENXIO; > + } > + > + if (read_write == I2C_SMBUS_READ) > + data->byte = readl(p2wi->regs + P2WI_DATA0); > + > + return 0; > +} > + > +static const struct i2c_algorithm p2wi_algo = { > + .smbus_xfer = p2wi_smbus_xfer, > + .functionality = p2wi_functionality, > +}; > + > +static const struct of_device_id p2wi_of_match_table[] = { > + { .compatible = "allwinner,sun6i-a31-p2wi" }, > + {} > +}; > +MODULE_DEVICE_TABLE(of, p2wi_of_match_table); > + > +static int p2wi_probe(struct platform_device *pdev) > +{ > + struct device *dev = &pdev->dev; > + struct resource *r; > + struct p2wi *p2wi; > + int ret; > + > + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); > + if (!p2wi) { > + dev_err(dev, "failed to allocate p2wi struct\n"); > + return -ENOMEM; > + } > + > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + p2wi->regs = devm_request_and_ioremap(dev, r); > + if (IS_ERR(p2wi->regs)) { > + ret = PTR_ERR(p2wi->regs); > + dev_err(dev, "failed to retrieve iomem resource: %d\n", > + ret); > + return ret; > + } I don't see how it can work, since, when it fails, devm_request_and_ioremap returns NULL. You probably meant devm_ioremap_resource. Thanks! Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140424/4e584531/attachment.sig> ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-04-24 13:29 ` Maxime Ripard @ 2014-04-25 7:50 ` Boris BREZILLON 2014-04-25 7:58 ` Boris BREZILLON 0 siblings, 1 reply; 6+ messages in thread From: Boris BREZILLON @ 2014-04-25 7:50 UTC (permalink / raw) To: linux-arm-kernel Hi Maxime, On 24/04/2014 15:29, Maxime Ripard wrote: > On Thu, Apr 24, 2014 at 01:55:16PM +0200, Boris BREZILLON wrote: >> The P2WI looks like an SMBus controller which only supports byte data >> transfers. But, it differs from standard SMBus protocol on several >> aspects: >> - it supports only one slave device, and thus drop the address field >> - it adds a parity bit every 8bits of data >> - only one read access is required to read a byte (instead of a read >> followed by a write access in standard SMBus protocol) >> - there's no Ack bit after each byte transfer >> >> This means this bus cannot be used to interface with standard SMBus >> devices (the only known device to support this interface is the AXP221 >> PMIC). >> >> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> >> --- >> drivers/i2c/busses/Kconfig | 12 ++ >> drivers/i2c/busses/Makefile | 1 + >> drivers/i2c/busses/i2c-sunxi-p2wi.c | 317 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 330 insertions(+) >> create mode 100644 drivers/i2c/busses/i2c-sunxi-p2wi.c >> >> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >> index c94db1c..37e53d6 100644 >> --- a/drivers/i2c/busses/Kconfig >> +++ b/drivers/i2c/busses/Kconfig >> @@ -771,6 +771,18 @@ config I2C_STU300 >> This driver can also be built as a module. If so, the module >> will be called i2c-stu300. >> >> +config I2C_SUNXI_P2WI >> + tristate "Allwinner sunxi internal P2WI controller" > Since the A31 is the only SoC that uses it, maybe you can drop the > sunxi and use either sun6i or A31 here? It makes sense, I'll change the config option into I2C_SUNXI_P2WI and the source file name into i2c-sun6i-p2wi.c. >> + depends on ARCH_SUNXI >> + help >> + If you say yes to this option, support will be included for the >> + P2WI (Push/Pull 2 Wire Interface) controller embedded in some sunxi >> + SOCs. >> + The P2WI looks like an SMBus controller (which supports only byte >> + accesses), except that it only supports one slave device. >> + This interface is used to connect to specific PMIC devices (like the >> + AXP221). >> + >> config I2C_TEGRA >> tristate "NVIDIA Tegra internal I2C controller" >> depends on ARCH_TEGRA >> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile >> index 18d18ff..c63d2ec 100644 >> --- a/drivers/i2c/busses/Makefile >> +++ b/drivers/i2c/busses/Makefile >> @@ -75,6 +75,7 @@ obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o >> obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o >> obj-$(CONFIG_I2C_ST) += i2c-st.o >> obj-$(CONFIG_I2C_STU300) += i2c-stu300.o >> +obj-$(CONFIG_I2C_SUNXI_P2WI) += i2c-sunxi-p2wi.o >> obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o >> obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o >> obj-$(CONFIG_I2C_WMT) += i2c-wmt.o >> diff --git a/drivers/i2c/busses/i2c-sunxi-p2wi.c b/drivers/i2c/busses/i2c-sunxi-p2wi.c >> new file mode 100644 >> index 0000000..e3fdd76 >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-sunxi-p2wi.c >> @@ -0,0 +1,317 @@ >> +/* >> + * P2WI (Push-Pull Two Wire Interface) bus driver. >> + * >> + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> >> + * >> + * This file is licensed under the terms of the GNU General Public License >> + * version 2. This program is licensed "as is" without any warranty of any >> + * kind, whether express or implied. >> + */ >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/spinlock.h> >> +#include <linux/i2c.h> >> +#include <linux/interrupt.h> >> +#include <linux/platform_device.h> >> +#include <linux/io.h> >> +#include <linux/of.h> >> +#include <linux/of_device.h> >> +#include <linux/of_irq.h> >> +#include <linux/clk.h> >> +#include <linux/err.h> >> +#include <linux/delay.h> >> +#include <linux/clk.h> >> +#include <linux/reset.h> >> + >> + >> +/* P2WI registers */ >> +#define P2WI_CTRL 0x0 >> +#define P2WI_CCR 0x4 >> +#define P2WI_INTE 0x8 >> +#define P2WI_INTS 0xc >> +#define P2WI_DADDR0 0x10 >> +#define P2WI_DADDR1 0x14 >> +#define P2WI_DLEN 0x18 >> +#define P2WI_DATA0 0x1c >> +#define P2WI_DATA1 0x20 >> +#define P2WI_LCR 0x24 >> +#define P2WI_PMCR 0x28 >> + >> +/* CTRL fields */ >> +#define P2WI_START_TRANS (1 << 7) >> +#define P2WI_ABORT_TRANS (1 << 6) >> +#define P2WI_GLOBAL_INT_ENB (1 << 1) >> +#define P2WI_SOFT_RST (1 << 0) > BIT() ? Sure, I'll make use of the BIT macro wherever possible. > >> +/* CLK CTRL fields */ >> +#define P2WI_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) >> +#define P2WI_CLK_DIV(v) ((v) & 0xff) >> + >> +/* STATUS fields */ >> +#define P2WI_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) >> +#define P2WI_LOAD_BSY (1 << 2) >> +#define P2WI_TRANS_ERR (1 << 1) >> +#define P2WI_TRANS_OVER (1 << 0) >> + >> +/* DATA LENGTH fields*/ >> +#define P2WI_READ (1 << 4) >> +#define P2WI_DATA_LENGTH(v) ((v - 1) & 0x7) >> + >> +/* LINE CTRL fields*/ >> +#define P2WI_SCL_STATE (1 << 5) >> +#define P2WI_SDA_STATE (1 << 4) >> +#define P2WI_SCL_CTL (1 << 3) >> +#define P2WI_SCL_CTL_EN (1 << 2) >> +#define P2WI_SDA_CTL (1 << 1) >> +#define P2WI_SDA_CTL_EN (1 << 0) >> + >> +/* PMU MODE CTRL fields */ >> +#define P2WI_PMU_INIT_SEND (1 << 31) >> +#define P2WI_PMU_INIT_DATA(v) (((v) & 0xff) << 16) >> +#define P2WI_PMU_MODE_REG(v) (((v) & 0xff) << 8) >> +#define P2WI_PMU_DEV_ADDR(v) ((v) & 0xff) > I'd very much prefer if your bits were prefixed by the register > name. That way, you directly know in which register that bit belong, > without having to scroll across the whole driver. Okay. >> + >> +struct p2wi { >> + struct i2c_adapter adapter; >> + struct completion complete; >> + unsigned int irq; >> + unsigned int status; >> + void __iomem *regs; >> + struct clk *clk; >> + struct reset_control *rstc; >> +}; >> + >> +static irqreturn_t p2wi_interrupt(int irq, void *dev_id) >> +{ >> + struct p2wi *p2wi = dev_id; >> + unsigned long status; >> + >> + status = readl(p2wi->regs + P2WI_INTS); >> + p2wi->status = status; >> + >> + /* Clear interrupts */ >> + status &= (P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER); >> + writel(status, p2wi->regs + P2WI_INTS); >> + >> + complete(&p2wi->complete); >> + >> + return IRQ_HANDLED; >> +} >> + >> +static u32 p2wi_functionality(struct i2c_adapter *adap) >> +{ >> + return I2C_FUNC_SMBUS_BYTE_DATA; >> +} >> + >> +static int p2wi_smbus_xfer(struct i2c_adapter *adap, u16 addr, >> + unsigned short flags, char read_write, >> + u8 command, int size, union i2c_smbus_data *data) >> +{ >> + struct p2wi *p2wi = i2c_get_adapdata(adap); >> + unsigned long dlen = P2WI_DATA_LENGTH(1); >> + >> + /* >> + * TODO: check address consistency. >> + * The P2WI bus only support one slave. As a result it does not use >> + * the I2C address except when you're switching the slave device from >> + * I2C to P2WI mode. >> + * We should at least verify that the addr argument is consistent with >> + * the slave device address. >> + */ >> + >> + if (addr > 0xff) { >> + dev_err(&adap->dev, "invalid P2WI address\n"); >> + return -EINVAL; >> + } >> + >> + /* >> + * TODO: handle switch to P2WI mode. >> + * At the moment, we're considering the slave device as already >> + * switchedto P2WI (which means the bootloader has to switch the slave > ^ you need a space here > >> + * device from I2C to P2WI mode). >> + * We need at least 3 informations to launch the switch process: >> + * - the slave device address (addr argument) >> + * - the mode register >> + * - the P2WI mode value (to write in the mode register) >> + */ >> + >> + if (!data) >> + return -EINVAL; >> + >> + writel(command, p2wi->regs + P2WI_DADDR0); >> + >> + if (read_write == I2C_SMBUS_READ) >> + dlen |= P2WI_READ; >> + else >> + writel(data->byte, p2wi->regs + P2WI_DATA0); >> + >> + writel(dlen, p2wi->regs + P2WI_DLEN); >> + >> + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_START_TRANS) { >> + dev_err(&adap->dev, "P2WI bus busy\n"); >> + return -EBUSY; >> + } >> + >> + reinit_completion(&p2wi->complete); >> + >> + writel(P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER, >> + p2wi->regs + P2WI_INTE); >> + >> + writel(P2WI_START_TRANS | P2WI_GLOBAL_INT_ENB, p2wi->regs + P2WI_CTRL); >> + >> + wait_for_completion(&p2wi->complete); >> + >> + if (p2wi->status & P2WI_LOAD_BSY) { >> + dev_err(&adap->dev, "P2WI bus busy\n"); >> + return -EBUSY; >> + } >> + >> + > You can drop the extra line here. > >> + if (p2wi->status & P2WI_TRANS_ERR) { >> + dev_err(&adap->dev, "P2WI bus xfer error\n"); >> + return -ENXIO; >> + } >> + >> + if (read_write == I2C_SMBUS_READ) >> + data->byte = readl(p2wi->regs + P2WI_DATA0); >> + >> + return 0; >> +} >> + >> +static const struct i2c_algorithm p2wi_algo = { >> + .smbus_xfer = p2wi_smbus_xfer, >> + .functionality = p2wi_functionality, >> +}; >> + >> +static const struct of_device_id p2wi_of_match_table[] = { >> + { .compatible = "allwinner,sun6i-a31-p2wi" }, >> + {} >> +}; >> +MODULE_DEVICE_TABLE(of, p2wi_of_match_table); >> + >> +static int p2wi_probe(struct platform_device *pdev) >> +{ >> + struct device *dev = &pdev->dev; >> + struct resource *r; >> + struct p2wi *p2wi; >> + int ret; >> + >> + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); >> + if (!p2wi) { >> + dev_err(dev, "failed to allocate p2wi struct\n"); >> + return -ENOMEM; >> + } >> + >> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + p2wi->regs = devm_request_and_ioremap(dev, r); >> + if (IS_ERR(p2wi->regs)) { >> + ret = PTR_ERR(p2wi->regs); >> + dev_err(dev, "failed to retrieve iomem resource: %d\n", >> + ret); >> + return ret; >> + } > I don't see how it can work, since, when it fails, > devm_request_and_ioremap returns NULL. You probably meant > devm_ioremap_resource. Oops, just forgot devm_request_and_ioremap is returning NULL when it fails. I'll fix it. Thanks. Best Regards, Boris > Thanks! > Maxime > -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-04-25 7:50 ` Boris BREZILLON @ 2014-04-25 7:58 ` Boris BREZILLON 0 siblings, 0 replies; 6+ messages in thread From: Boris BREZILLON @ 2014-04-25 7:58 UTC (permalink / raw) To: linux-arm-kernel On 25/04/2014 09:50, Boris BREZILLON wrote: > Hi Maxime, > > On 24/04/2014 15:29, Maxime Ripard wrote: >> On Thu, Apr 24, 2014 at 01:55:16PM +0200, Boris BREZILLON wrote: >>> The P2WI looks like an SMBus controller which only supports byte data >>> transfers. But, it differs from standard SMBus protocol on several >>> aspects: >>> - it supports only one slave device, and thus drop the address field >>> - it adds a parity bit every 8bits of data >>> - only one read access is required to read a byte (instead of a read >>> followed by a write access in standard SMBus protocol) >>> - there's no Ack bit after each byte transfer >>> >>> This means this bus cannot be used to interface with standard SMBus >>> devices (the only known device to support this interface is the AXP221 >>> PMIC). >>> >>> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> >>> --- >>> drivers/i2c/busses/Kconfig | 12 ++ >>> drivers/i2c/busses/Makefile | 1 + >>> drivers/i2c/busses/i2c-sunxi-p2wi.c | 317 ++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 330 insertions(+) >>> create mode 100644 drivers/i2c/busses/i2c-sunxi-p2wi.c >>> >>> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig >>> index c94db1c..37e53d6 100644 >>> --- a/drivers/i2c/busses/Kconfig >>> +++ b/drivers/i2c/busses/Kconfig >>> @@ -771,6 +771,18 @@ config I2C_STU300 >>> This driver can also be built as a module. If so, the module >>> will be called i2c-stu300. >>> >>> +config I2C_SUNXI_P2WI >>> + tristate "Allwinner sunxi internal P2WI controller" >> Since the A31 is the only SoC that uses it, maybe you can drop the >> sunxi and use either sun6i or A31 here? > It makes sense, I'll change the config option into I2C_SUNXI_P2WI and > the source file name into i2c-sun6i-p2wi.c. I meant I2C_SUN6I_P2WI ;-). > >>> + depends on ARCH_SUNXI >>> + help >>> + If you say yes to this option, support will be included for the >>> + P2WI (Push/Pull 2 Wire Interface) controller embedded in some sunxi >>> + SOCs. >>> + The P2WI looks like an SMBus controller (which supports only byte >>> + accesses), except that it only supports one slave device. >>> + This interface is used to connect to specific PMIC devices (like the >>> + AXP221). >>> + >>> config I2C_TEGRA >>> tristate "NVIDIA Tegra internal I2C controller" >>> depends on ARCH_TEGRA >>> diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile >>> index 18d18ff..c63d2ec 100644 >>> --- a/drivers/i2c/busses/Makefile >>> +++ b/drivers/i2c/busses/Makefile >>> @@ -75,6 +75,7 @@ obj-$(CONFIG_I2C_SIMTEC) += i2c-simtec.o >>> obj-$(CONFIG_I2C_SIRF) += i2c-sirf.o >>> obj-$(CONFIG_I2C_ST) += i2c-st.o >>> obj-$(CONFIG_I2C_STU300) += i2c-stu300.o >>> +obj-$(CONFIG_I2C_SUNXI_P2WI) += i2c-sunxi-p2wi.o >>> obj-$(CONFIG_I2C_TEGRA) += i2c-tegra.o >>> obj-$(CONFIG_I2C_VERSATILE) += i2c-versatile.o >>> obj-$(CONFIG_I2C_WMT) += i2c-wmt.o >>> diff --git a/drivers/i2c/busses/i2c-sunxi-p2wi.c b/drivers/i2c/busses/i2c-sunxi-p2wi.c >>> new file mode 100644 >>> index 0000000..e3fdd76 >>> --- /dev/null >>> +++ b/drivers/i2c/busses/i2c-sunxi-p2wi.c >>> @@ -0,0 +1,317 @@ >>> +/* >>> + * P2WI (Push-Pull Two Wire Interface) bus driver. >>> + * >>> + * Author: Boris BREZILLON <boris.brezillon@free-electrons.com> >>> + * >>> + * This file is licensed under the terms of the GNU General Public License >>> + * version 2. This program is licensed "as is" without any warranty of any >>> + * kind, whether express or implied. >>> + */ >>> +#include <linux/kernel.h> >>> +#include <linux/slab.h> >>> +#include <linux/module.h> >>> +#include <linux/spinlock.h> >>> +#include <linux/i2c.h> >>> +#include <linux/interrupt.h> >>> +#include <linux/platform_device.h> >>> +#include <linux/io.h> >>> +#include <linux/of.h> >>> +#include <linux/of_device.h> >>> +#include <linux/of_irq.h> >>> +#include <linux/clk.h> >>> +#include <linux/err.h> >>> +#include <linux/delay.h> >>> +#include <linux/clk.h> >>> +#include <linux/reset.h> >>> + >>> + >>> +/* P2WI registers */ >>> +#define P2WI_CTRL 0x0 >>> +#define P2WI_CCR 0x4 >>> +#define P2WI_INTE 0x8 >>> +#define P2WI_INTS 0xc >>> +#define P2WI_DADDR0 0x10 >>> +#define P2WI_DADDR1 0x14 >>> +#define P2WI_DLEN 0x18 >>> +#define P2WI_DATA0 0x1c >>> +#define P2WI_DATA1 0x20 >>> +#define P2WI_LCR 0x24 >>> +#define P2WI_PMCR 0x28 >>> + >>> +/* CTRL fields */ >>> +#define P2WI_START_TRANS (1 << 7) >>> +#define P2WI_ABORT_TRANS (1 << 6) >>> +#define P2WI_GLOBAL_INT_ENB (1 << 1) >>> +#define P2WI_SOFT_RST (1 << 0) >> BIT() ? > Sure, I'll make use of the BIT macro wherever possible. > >>> +/* CLK CTRL fields */ >>> +#define P2WI_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) >>> +#define P2WI_CLK_DIV(v) ((v) & 0xff) >>> + >>> +/* STATUS fields */ >>> +#define P2WI_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) >>> +#define P2WI_LOAD_BSY (1 << 2) >>> +#define P2WI_TRANS_ERR (1 << 1) >>> +#define P2WI_TRANS_OVER (1 << 0) >>> + >>> +/* DATA LENGTH fields*/ >>> +#define P2WI_READ (1 << 4) >>> +#define P2WI_DATA_LENGTH(v) ((v - 1) & 0x7) >>> + >>> +/* LINE CTRL fields*/ >>> +#define P2WI_SCL_STATE (1 << 5) >>> +#define P2WI_SDA_STATE (1 << 4) >>> +#define P2WI_SCL_CTL (1 << 3) >>> +#define P2WI_SCL_CTL_EN (1 << 2) >>> +#define P2WI_SDA_CTL (1 << 1) >>> +#define P2WI_SDA_CTL_EN (1 << 0) >>> + >>> +/* PMU MODE CTRL fields */ >>> +#define P2WI_PMU_INIT_SEND (1 << 31) >>> +#define P2WI_PMU_INIT_DATA(v) (((v) & 0xff) << 16) >>> +#define P2WI_PMU_MODE_REG(v) (((v) & 0xff) << 8) >>> +#define P2WI_PMU_DEV_ADDR(v) ((v) & 0xff) >> I'd very much prefer if your bits were prefixed by the register >> name. That way, you directly know in which register that bit belong, >> without having to scroll across the whole driver. > Okay. > >>> + >>> +struct p2wi { >>> + struct i2c_adapter adapter; >>> + struct completion complete; >>> + unsigned int irq; >>> + unsigned int status; >>> + void __iomem *regs; >>> + struct clk *clk; >>> + struct reset_control *rstc; >>> +}; >>> + >>> +static irqreturn_t p2wi_interrupt(int irq, void *dev_id) >>> +{ >>> + struct p2wi *p2wi = dev_id; >>> + unsigned long status; >>> + >>> + status = readl(p2wi->regs + P2WI_INTS); >>> + p2wi->status = status; >>> + >>> + /* Clear interrupts */ >>> + status &= (P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER); >>> + writel(status, p2wi->regs + P2WI_INTS); >>> + >>> + complete(&p2wi->complete); >>> + >>> + return IRQ_HANDLED; >>> +} >>> + >>> +static u32 p2wi_functionality(struct i2c_adapter *adap) >>> +{ >>> + return I2C_FUNC_SMBUS_BYTE_DATA; >>> +} >>> + >>> +static int p2wi_smbus_xfer(struct i2c_adapter *adap, u16 addr, >>> + unsigned short flags, char read_write, >>> + u8 command, int size, union i2c_smbus_data *data) >>> +{ >>> + struct p2wi *p2wi = i2c_get_adapdata(adap); >>> + unsigned long dlen = P2WI_DATA_LENGTH(1); >>> + >>> + /* >>> + * TODO: check address consistency. >>> + * The P2WI bus only support one slave. As a result it does not use >>> + * the I2C address except when you're switching the slave device from >>> + * I2C to P2WI mode. >>> + * We should at least verify that the addr argument is consistent with >>> + * the slave device address. >>> + */ >>> + >>> + if (addr > 0xff) { >>> + dev_err(&adap->dev, "invalid P2WI address\n"); >>> + return -EINVAL; >>> + } >>> + >>> + /* >>> + * TODO: handle switch to P2WI mode. >>> + * At the moment, we're considering the slave device as already >>> + * switchedto P2WI (which means the bootloader has to switch the slave >> ^ you need a space here >> >>> + * device from I2C to P2WI mode). >>> + * We need at least 3 informations to launch the switch process: >>> + * - the slave device address (addr argument) >>> + * - the mode register >>> + * - the P2WI mode value (to write in the mode register) >>> + */ >>> + >>> + if (!data) >>> + return -EINVAL; >>> + >>> + writel(command, p2wi->regs + P2WI_DADDR0); >>> + >>> + if (read_write == I2C_SMBUS_READ) >>> + dlen |= P2WI_READ; >>> + else >>> + writel(data->byte, p2wi->regs + P2WI_DATA0); >>> + >>> + writel(dlen, p2wi->regs + P2WI_DLEN); >>> + >>> + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_START_TRANS) { >>> + dev_err(&adap->dev, "P2WI bus busy\n"); >>> + return -EBUSY; >>> + } >>> + >>> + reinit_completion(&p2wi->complete); >>> + >>> + writel(P2WI_LOAD_BSY | P2WI_TRANS_ERR | P2WI_TRANS_OVER, >>> + p2wi->regs + P2WI_INTE); >>> + >>> + writel(P2WI_START_TRANS | P2WI_GLOBAL_INT_ENB, p2wi->regs + P2WI_CTRL); >>> + >>> + wait_for_completion(&p2wi->complete); >>> + >>> + if (p2wi->status & P2WI_LOAD_BSY) { >>> + dev_err(&adap->dev, "P2WI bus busy\n"); >>> + return -EBUSY; >>> + } >>> + >>> + >> You can drop the extra line here. >> >>> + if (p2wi->status & P2WI_TRANS_ERR) { >>> + dev_err(&adap->dev, "P2WI bus xfer error\n"); >>> + return -ENXIO; >>> + } >>> + >>> + if (read_write == I2C_SMBUS_READ) >>> + data->byte = readl(p2wi->regs + P2WI_DATA0); >>> + >>> + return 0; >>> +} >>> + >>> +static const struct i2c_algorithm p2wi_algo = { >>> + .smbus_xfer = p2wi_smbus_xfer, >>> + .functionality = p2wi_functionality, >>> +}; >>> + >>> +static const struct of_device_id p2wi_of_match_table[] = { >>> + { .compatible = "allwinner,sun6i-a31-p2wi" }, >>> + {} >>> +}; >>> +MODULE_DEVICE_TABLE(of, p2wi_of_match_table); >>> + >>> +static int p2wi_probe(struct platform_device *pdev) >>> +{ >>> + struct device *dev = &pdev->dev; >>> + struct resource *r; >>> + struct p2wi *p2wi; >>> + int ret; >>> + >>> + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); >>> + if (!p2wi) { >>> + dev_err(dev, "failed to allocate p2wi struct\n"); >>> + return -ENOMEM; >>> + } >>> + >>> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >>> + p2wi->regs = devm_request_and_ioremap(dev, r); >>> + if (IS_ERR(p2wi->regs)) { >>> + ret = PTR_ERR(p2wi->regs); >>> + dev_err(dev, "failed to retrieve iomem resource: %d\n", >>> + ret); >>> + return ret; >>> + } >> I don't see how it can work, since, when it fails, >> devm_request_and_ioremap returns NULL. You probably meant >> devm_ioremap_resource. > Oops, just forgot devm_request_and_ioremap is returning NULL when it fails. > I'll fix it. > > Thanks. > > Best Regards, > > Boris >> Thanks! >> Maxime >> -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-04-25 7:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-04-24 11:55 [PATCH 0/2] i2c: sunxi: add P2WI controller support Boris BREZILLON 2014-04-24 11:55 ` [PATCH 1/2] i2c: sunxi: add P2WI DT bindings documentation Boris BREZILLON 2014-04-24 11:55 ` [PATCH 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support Boris BREZILLON 2014-04-24 13:29 ` Maxime Ripard 2014-04-25 7:50 ` Boris BREZILLON 2014-04-25 7:58 ` Boris BREZILLON
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).