* [PATCH v5 0/2] i2c: sunxi: add P2WI controller support @ 2014-06-10 13:47 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 UTC (permalink / raw) To: Wolfram Sang Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c, Boris BREZILLON Hello, This series adds support for the P2WI block used by some Allwinner boards to interface with the AXP221 PMIC. Best Regards, Boris Changes since v4: - add P2WI protocol description in driver header comment - remove unneeded i2c address check - remove unneeded irq field from p2wi struct Changes since v3: - update the DT bindings doc - fix a comment that was no longer true Changes since v2: - drop the initialization (switch from I2C to P2WI mode) part - print devm_ioremap_resource err code 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 | 41 +++ drivers/i2c/busses/Kconfig | 12 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-sun6i-p2wi.c | 356 +++++++++++++++++++++ 4 files changed, 410 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt create mode 100644 drivers/i2c/busses/i2c-sun6i-p2wi.c -- 1.8.3.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 0/2] i2c: sunxi: add P2WI controller support @ 2014-06-10 13:47 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 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. Best Regards, Boris Changes since v4: - add P2WI protocol description in driver header comment - remove unneeded i2c address check - remove unneeded irq field from p2wi struct Changes since v3: - update the DT bindings doc - fix a comment that was no longer true Changes since v2: - drop the initialization (switch from I2C to P2WI mode) part - print devm_ioremap_resource err code 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 | 41 +++ drivers/i2c/busses/Kconfig | 12 + drivers/i2c/busses/Makefile | 1 + drivers/i2c/busses/i2c-sun6i-p2wi.c | 356 +++++++++++++++++++++ 4 files changed, 410 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt create mode 100644 drivers/i2c/busses/i2c-sun6i-p2wi.c -- 1.8.3.2 ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 1/2] i2c: sunxi: add P2WI DT bindings documentation 2014-06-10 13:47 ` Boris BREZILLON @ 2014-06-10 13:47 ` Boris BREZILLON -1 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 UTC (permalink / raw) To: Wolfram Sang Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c, Boris BREZILLON 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 | 41 ++++++++++++++++++++++ 1 file changed, 41 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..6b76548 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt @@ -0,0 +1,41 @@ + +* 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 + +A P2WI may contain one child node encoding a P2WI slave device. + +Slave device properties: + Required properties: + - reg : the I2C slave address used during the initialization + process to switch from I2C to P2WI mode + +Example: + + p2wi@01f03400 { + compatible = "allwinner,sun6i-a31-p2wi"; + reg = <0x01f03400 0x400>; + interrupts = <0 39 4>; + clocks = <&apb0_gates 3>; + clock-frequency = <6000000>; + resets = <&apb0_rst 3>; + + axp221: pmic@68 { + compatible = "x-powers,axp221"; + reg = <0x68>; + + /* ... */ + }; + }; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 1/2] i2c: sunxi: add P2WI DT bindings documentation @ 2014-06-10 13:47 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 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 | 41 ++++++++++++++++++++++ 1 file changed, 41 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..6b76548 --- /dev/null +++ b/Documentation/devicetree/bindings/i2c/i2c-sunxi-p2wi.txt @@ -0,0 +1,41 @@ + +* 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 + +A P2WI may contain one child node encoding a P2WI slave device. + +Slave device properties: + Required properties: + - reg : the I2C slave address used during the initialization + process to switch from I2C to P2WI mode + +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>; + + axp221: pmic at 68 { + compatible = "x-powers,axp221"; + reg = <0x68>; + + /* ... */ + }; + }; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 13:47 ` Boris BREZILLON @ 2014-06-10 13:47 ` Boris BREZILLON -1 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 UTC (permalink / raw) To: Wolfram Sang Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c, Boris BREZILLON The P2WI controller 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 write followed by a read 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-sun6i-p2wi.c | 356 ++++++++++++++++++++++++++++++++++++ 3 files changed, 369 insertions(+) create mode 100644 drivers/i2c/busses/i2c-sun6i-p2wi.c diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index c94db1c..09bce1c 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_SUN6I_P2WI + tristate "Allwinner sun6i 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..58feeb9 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_SUN6I_P2WI) += i2c-sun6i-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-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c new file mode 100644 index 0000000..4f7b93f --- /dev/null +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c @@ -0,0 +1,356 @@ +/* + * 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. + * + * The P2WI controller 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 write + * followed by a read 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). + * + */ +#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_CTRL_START_TRANS BIT(7) +#define P2WI_CTRL_ABORT_TRANS BIT(6) +#define P2WI_CTRL_GLOBAL_INT_ENB BIT(1) +#define P2WI_CTRL_SOFT_RST BIT(0) + +/* CLK CTRL fields */ +#define P2WI_CCR_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) +#define P2WI_CCR_MAX_CLK_DIV 0xff +#define P2WI_CCR_CLK_DIV(v) ((v) & P2WI_CCR_MAX_CLK_DIV) + +/* STATUS fields */ +#define P2WI_INTS_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) +#define P2WI_INTS_LOAD_BSY BIT(2) +#define P2WI_INTS_TRANS_ERR BIT(1) +#define P2WI_INTS_TRANS_OVER BIT(0) + +/* DATA LENGTH fields*/ +#define P2WI_DLEN_READ BIT(4) +#define P2WI_DLEN_DATA_LENGTH(v) ((v - 1) & 0x7) + +/* LINE CTRL fields*/ +#define P2WI_LCR_SCL_STATE BIT(5) +#define P2WI_LCR_SDA_STATE BIT(4) +#define P2WI_LCR_SCL_CTL BIT(3) +#define P2WI_LCR_SCL_CTL_EN BIT(2) +#define P2WI_LCR_SDA_CTL BIT(1) +#define P2WI_LCR_SDA_CTL_EN BIT(0) + +/* PMU MODE CTRL fields */ +#define P2WI_PMCR_PMU_INIT_SEND BIT(31) +#define P2WI_PMCR_PMU_INIT_DATA(v) (((v) & 0xff) << 16) +#define P2WI_PMCR_PMU_MODE_REG(v) (((v) & 0xff) << 8) +#define P2WI_PMCR_PMU_DEV_ADDR(v) ((v) & 0xff) + +#define P2WI_MAX_FREQ 6000000 + +struct p2wi { + struct i2c_adapter adapter; + struct completion complete; + unsigned int status; + void __iomem *regs; + struct clk *clk; + struct reset_control *rstc; + int slave_addr; +}; + +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_INTS_LOAD_BSY | P2WI_INTS_TRANS_ERR | + P2WI_INTS_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_DLEN_DATA_LENGTH(1); + + if (p2wi->slave_addr >= 0 && addr != p2wi->slave_addr) { + dev_err(&adap->dev, "invalid P2WI address\n"); + return -EINVAL; + } + + if (!data) + return -EINVAL; + + writel(command, p2wi->regs + P2WI_DADDR0); + + if (read_write == I2C_SMBUS_READ) + dlen |= P2WI_DLEN_READ; + else + writel(data->byte, p2wi->regs + P2WI_DATA0); + + writel(dlen, p2wi->regs + P2WI_DLEN); + + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_CTRL_START_TRANS) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + reinit_completion(&p2wi->complete); + + writel(P2WI_INTS_LOAD_BSY | P2WI_INTS_TRANS_ERR | P2WI_INTS_TRANS_OVER, + p2wi->regs + P2WI_INTE); + + writel(P2WI_CTRL_START_TRANS | P2WI_CTRL_GLOBAL_INT_ENB, + p2wi->regs + P2WI_CTRL); + + wait_for_completion(&p2wi->complete); + + if (p2wi->status & P2WI_INTS_LOAD_BSY) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + if (p2wi->status & P2WI_INTS_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 device_node *np = dev->of_node; + struct device_node *childnp; + unsigned long parent_clk_freq; + u32 clk_freq = 100000; + struct resource *r; + struct p2wi *p2wi; + u32 slave_addr; + int clk_div; + int irq; + int ret; + + of_property_read_u32(np, "clock-frequency", &clk_freq); + if (clk_freq > P2WI_MAX_FREQ) { + dev_err(dev, + "required clock-frequency (%u Hz) is too high (max = 6MHz)", + clk_freq); + return -EINVAL; + } + + if (of_get_child_count(np) > 1) { + dev_err(dev, "P2WI only supports one slave device\n"); + return -EINVAL; + } + + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); + if (!p2wi) + return -ENOMEM; + + p2wi->slave_addr = -1; + + /* + * Authorize a p2wi node without any children to be able to use an + * i2c-dev from userpace. + * In this case the slave_addr is set to -1 and won't be checked when + * launching a P2WI transfer. + */ + childnp = of_get_next_available_child(np, NULL); + if (childnp) { + ret = of_property_read_u32(childnp, "reg", &slave_addr); + if (ret) { + dev_err(dev, "invalid slave address on node %s\n", + childnp->full_name); + return -EINVAL; + } + + p2wi->slave_addr = slave_addr; + } + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + p2wi->regs = devm_ioremap_resource(dev, r); + if (IS_ERR(p2wi->regs)) { + ret = PTR_ERR(p2wi->regs); + return ret; + } + + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); + irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_err(dev, "failed to retrieve irq: %d\n", ret); + return irq; + } + + 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; + } + + parent_clk_freq = clk_get_rate(p2wi->clk); + + 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, irq, p2wi_interrupt, 0, pdev->name, p2wi); + if (ret) { + dev_err(dev, "can't register interrupt handler irq%d: %d\n", + irq, ret); + goto err_reset_assert; + } + + writel(P2WI_CTRL_SOFT_RST, p2wi->regs + P2WI_CTRL); + + clk_div = parent_clk_freq / clk_freq; + if (!clk_div) { + dev_warn(dev, + "clock-frequency is too high, setting it to %lu Hz\n", + parent_clk_freq); + clk_div = 1; + } else if (clk_div > P2WI_CCR_MAX_CLK_DIV) { + dev_warn(dev, + "clock-frequency is too low, setting it to %lu Hz\n", + parent_clk_freq / P2WI_CCR_MAX_CLK_DIV); + clk_div = P2WI_CCR_MAX_CLK_DIV; + } + + writel(P2WI_CCR_SDA_OUT_DELAY(1) | P2WI_CCR_CLK_DIV(clk_div), + 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] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-10 13:47 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-10 13:47 UTC (permalink / raw) To: linux-arm-kernel The P2WI controller 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 write followed by a read 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-sun6i-p2wi.c | 356 ++++++++++++++++++++++++++++++++++++ 3 files changed, 369 insertions(+) create mode 100644 drivers/i2c/busses/i2c-sun6i-p2wi.c diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index c94db1c..09bce1c 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_SUN6I_P2WI + tristate "Allwinner sun6i 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..58feeb9 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_SUN6I_P2WI) += i2c-sun6i-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-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c new file mode 100644 index 0000000..4f7b93f --- /dev/null +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c @@ -0,0 +1,356 @@ +/* + * 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. + * + * The P2WI controller 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 write + * followed by a read 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). + * + */ +#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_CTRL_START_TRANS BIT(7) +#define P2WI_CTRL_ABORT_TRANS BIT(6) +#define P2WI_CTRL_GLOBAL_INT_ENB BIT(1) +#define P2WI_CTRL_SOFT_RST BIT(0) + +/* CLK CTRL fields */ +#define P2WI_CCR_SDA_OUT_DELAY(v) (((v) & 0x7) << 8) +#define P2WI_CCR_MAX_CLK_DIV 0xff +#define P2WI_CCR_CLK_DIV(v) ((v) & P2WI_CCR_MAX_CLK_DIV) + +/* STATUS fields */ +#define P2WI_INTS_TRANS_ERR_ID(v) (((v) >> 8) & 0xff) +#define P2WI_INTS_LOAD_BSY BIT(2) +#define P2WI_INTS_TRANS_ERR BIT(1) +#define P2WI_INTS_TRANS_OVER BIT(0) + +/* DATA LENGTH fields*/ +#define P2WI_DLEN_READ BIT(4) +#define P2WI_DLEN_DATA_LENGTH(v) ((v - 1) & 0x7) + +/* LINE CTRL fields*/ +#define P2WI_LCR_SCL_STATE BIT(5) +#define P2WI_LCR_SDA_STATE BIT(4) +#define P2WI_LCR_SCL_CTL BIT(3) +#define P2WI_LCR_SCL_CTL_EN BIT(2) +#define P2WI_LCR_SDA_CTL BIT(1) +#define P2WI_LCR_SDA_CTL_EN BIT(0) + +/* PMU MODE CTRL fields */ +#define P2WI_PMCR_PMU_INIT_SEND BIT(31) +#define P2WI_PMCR_PMU_INIT_DATA(v) (((v) & 0xff) << 16) +#define P2WI_PMCR_PMU_MODE_REG(v) (((v) & 0xff) << 8) +#define P2WI_PMCR_PMU_DEV_ADDR(v) ((v) & 0xff) + +#define P2WI_MAX_FREQ 6000000 + +struct p2wi { + struct i2c_adapter adapter; + struct completion complete; + unsigned int status; + void __iomem *regs; + struct clk *clk; + struct reset_control *rstc; + int slave_addr; +}; + +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_INTS_LOAD_BSY | P2WI_INTS_TRANS_ERR | + P2WI_INTS_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_DLEN_DATA_LENGTH(1); + + if (p2wi->slave_addr >= 0 && addr != p2wi->slave_addr) { + dev_err(&adap->dev, "invalid P2WI address\n"); + return -EINVAL; + } + + if (!data) + return -EINVAL; + + writel(command, p2wi->regs + P2WI_DADDR0); + + if (read_write == I2C_SMBUS_READ) + dlen |= P2WI_DLEN_READ; + else + writel(data->byte, p2wi->regs + P2WI_DATA0); + + writel(dlen, p2wi->regs + P2WI_DLEN); + + if (readl(p2wi->regs + P2WI_CTRL) & P2WI_CTRL_START_TRANS) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + reinit_completion(&p2wi->complete); + + writel(P2WI_INTS_LOAD_BSY | P2WI_INTS_TRANS_ERR | P2WI_INTS_TRANS_OVER, + p2wi->regs + P2WI_INTE); + + writel(P2WI_CTRL_START_TRANS | P2WI_CTRL_GLOBAL_INT_ENB, + p2wi->regs + P2WI_CTRL); + + wait_for_completion(&p2wi->complete); + + if (p2wi->status & P2WI_INTS_LOAD_BSY) { + dev_err(&adap->dev, "P2WI bus busy\n"); + return -EBUSY; + } + + if (p2wi->status & P2WI_INTS_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 device_node *np = dev->of_node; + struct device_node *childnp; + unsigned long parent_clk_freq; + u32 clk_freq = 100000; + struct resource *r; + struct p2wi *p2wi; + u32 slave_addr; + int clk_div; + int irq; + int ret; + + of_property_read_u32(np, "clock-frequency", &clk_freq); + if (clk_freq > P2WI_MAX_FREQ) { + dev_err(dev, + "required clock-frequency (%u Hz) is too high (max = 6MHz)", + clk_freq); + return -EINVAL; + } + + if (of_get_child_count(np) > 1) { + dev_err(dev, "P2WI only supports one slave device\n"); + return -EINVAL; + } + + p2wi = devm_kzalloc(dev, sizeof(struct p2wi), GFP_KERNEL); + if (!p2wi) + return -ENOMEM; + + p2wi->slave_addr = -1; + + /* + * Authorize a p2wi node without any children to be able to use an + * i2c-dev from userpace. + * In this case the slave_addr is set to -1 and won't be checked when + * launching a P2WI transfer. + */ + childnp = of_get_next_available_child(np, NULL); + if (childnp) { + ret = of_property_read_u32(childnp, "reg", &slave_addr); + if (ret) { + dev_err(dev, "invalid slave address on node %s\n", + childnp->full_name); + return -EINVAL; + } + + p2wi->slave_addr = slave_addr; + } + + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); + p2wi->regs = devm_ioremap_resource(dev, r); + if (IS_ERR(p2wi->regs)) { + ret = PTR_ERR(p2wi->regs); + return ret; + } + + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); + irq = platform_get_irq(pdev, 0); + if (irq < 0) { + dev_err(dev, "failed to retrieve irq: %d\n", ret); + return irq; + } + + 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; + } + + parent_clk_freq = clk_get_rate(p2wi->clk); + + 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, irq, p2wi_interrupt, 0, pdev->name, p2wi); + if (ret) { + dev_err(dev, "can't register interrupt handler irq%d: %d\n", + irq, ret); + goto err_reset_assert; + } + + writel(P2WI_CTRL_SOFT_RST, p2wi->regs + P2WI_CTRL); + + clk_div = parent_clk_freq / clk_freq; + if (!clk_div) { + dev_warn(dev, + "clock-frequency is too high, setting it to %lu Hz\n", + parent_clk_freq); + clk_div = 1; + } else if (clk_div > P2WI_CCR_MAX_CLK_DIV) { + dev_warn(dev, + "clock-frequency is too low, setting it to %lu Hz\n", + parent_clk_freq / P2WI_CCR_MAX_CLK_DIV); + clk_div = P2WI_CCR_MAX_CLK_DIV; + } + + writel(P2WI_CCR_SDA_OUT_DELAY(1) | P2WI_CCR_CLK_DIV(clk_div), + 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] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 13:47 ` Boris BREZILLON @ 2014-06-10 13:54 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-10 13:54 UTC (permalink / raw) To: Boris BREZILLON Cc: Wolfram Sang, Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > +config I2C_SUN6I_P2WI > + tristate "Allwinner sun6i 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). > + Sorry for the stupid question, but why is this an i2c driver if the hardware protocol is completely different? I understand that a lot of devices can be driven using either spi or i2c, and we have two sets of {directories,maintainers,bus_types,...} for them. Your description sounds like this is a separate option that isn't any closer to i2c than it is to spi. Would it perhaps be better to expose it only as a regmap rather than an i2c host? Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-10 13:54 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-10 13:54 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > +config I2C_SUN6I_P2WI > + tristate "Allwinner sun6i 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). > + Sorry for the stupid question, but why is this an i2c driver if the hardware protocol is completely different? I understand that a lot of devices can be driven using either spi or i2c, and we have two sets of {directories,maintainers,bus_types,...} for them. Your description sounds like this is a separate option that isn't any closer to i2c than it is to spi. Would it perhaps be better to expose it only as a regmap rather than an i2c host? Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 13:54 ` Arnd Bergmann @ 2014-06-10 14:36 ` Maxime Ripard -1 siblings, 0 replies; 22+ messages in thread From: Maxime Ripard @ 2014-06-10 14:36 UTC (permalink / raw) To: Arnd Bergmann Cc: Boris BREZILLON, Wolfram Sang, Randy Dunlap, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c [-- Attachment #1: Type: text/plain, Size: 1926 bytes --] Hi Arnd, On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: > On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > > > +config I2C_SUN6I_P2WI > > + tristate "Allwinner sun6i 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). > > + > > Sorry for the stupid question, but why is this an i2c driver if the > hardware protocol is completely different? It's not completely different. It deviates, but still looks very similar to i2c, and to be precise, SMBus. You'll have the full discussion that led to do this in i2c here: http://www.spinics.net/lists/linux-i2c/msg15066.html Also, one significant thing to take into account is that the communication with a device starts as I2C, only to switch to this protocol after some initialization sequence. > I understand that a lot of devices can be driven using either spi or > i2c, and we have two sets of {directories,maintainers,bus_types,...} > for them. Your description sounds like this is a separate option > that isn't any closer to i2c than it is to spi. That's not true. It's *much* closer from I2C than it is from SPI. > Would it perhaps be better to expose it only as a regmap rather than > an i2c host? That could be a solution, but is it a common practice to define a bus adapter driver in a regmap driver? Maxime -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-10 14:36 ` Maxime Ripard 0 siblings, 0 replies; 22+ messages in thread From: Maxime Ripard @ 2014-06-10 14:36 UTC (permalink / raw) To: linux-arm-kernel Hi Arnd, On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: > On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > > > +config I2C_SUN6I_P2WI > > + tristate "Allwinner sun6i 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). > > + > > Sorry for the stupid question, but why is this an i2c driver if the > hardware protocol is completely different? It's not completely different. It deviates, but still looks very similar to i2c, and to be precise, SMBus. You'll have the full discussion that led to do this in i2c here: http://www.spinics.net/lists/linux-i2c/msg15066.html Also, one significant thing to take into account is that the communication with a device starts as I2C, only to switch to this protocol after some initialization sequence. > I understand that a lot of devices can be driven using either spi or > i2c, and we have two sets of {directories,maintainers,bus_types,...} > for them. Your description sounds like this is a separate option > that isn't any closer to i2c than it is to spi. That's not true. It's *much* closer from I2C than it is from SPI. > Would it perhaps be better to expose it only as a regmap rather than > an i2c host? That could be a solution, but is it a common practice to define a bus adapter driver in a regmap driver? 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: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140610/45786f14/attachment.sig> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 14:36 ` Maxime Ripard @ 2014-06-10 14:47 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-10 14:47 UTC (permalink / raw) To: Maxime Ripard Cc: Boris BREZILLON, Wolfram Sang, Randy Dunlap, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c On Tuesday 10 June 2014 16:36:04 Maxime Ripard wrote: > On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: > > On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > > > > > +config I2C_SUN6I_P2WI > > > + tristate "Allwinner sun6i 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). > > > + > > > > Sorry for the stupid question, but why is this an i2c driver if the > > hardware protocol is completely different? > > It's not completely different. It deviates, but still looks very > similar to i2c, and to be precise, SMBus. > > You'll have the full discussion that led to do this in i2c here: > http://www.spinics.net/lists/linux-i2c/msg15066.html > > Also, one significant thing to take into account is that the > communication with a device starts as I2C, only to switch to this > protocol after some initialization sequence. Ok, sounds good. > > I understand that a lot of devices can be driven using either spi or > > i2c, and we have two sets of {directories,maintainers,bus_types,...} > > for them. Your description sounds like this is a separate option > > that isn't any closer to i2c than it is to spi. > > That's not true. It's *much* closer from I2C than it is from SPI. Ok. > > Would it perhaps be better to expose it only as a regmap rather than > > an i2c host? > > That could be a solution, but is it a common practice to define a bus > adapter driver in a regmap driver? No, not yet. Maybe Boris can just put an explanation into the changeset description of the driver so other people are able to find it more easily. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-10 14:47 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-10 14:47 UTC (permalink / raw) To: linux-arm-kernel On Tuesday 10 June 2014 16:36:04 Maxime Ripard wrote: > On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: > > On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: > > > > > > +config I2C_SUN6I_P2WI > > > + tristate "Allwinner sun6i 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). > > > + > > > > Sorry for the stupid question, but why is this an i2c driver if the > > hardware protocol is completely different? > > It's not completely different. It deviates, but still looks very > similar to i2c, and to be precise, SMBus. > > You'll have the full discussion that led to do this in i2c here: > http://www.spinics.net/lists/linux-i2c/msg15066.html > > Also, one significant thing to take into account is that the > communication with a device starts as I2C, only to switch to this > protocol after some initialization sequence. Ok, sounds good. > > I understand that a lot of devices can be driven using either spi or > > i2c, and we have two sets of {directories,maintainers,bus_types,...} > > for them. Your description sounds like this is a separate option > > that isn't any closer to i2c than it is to spi. > > That's not true. It's *much* closer from I2C than it is from SPI. Ok. > > Would it perhaps be better to expose it only as a regmap rather than > > an i2c host? > > That could be a solution, but is it a common practice to define a bus > adapter driver in a regmap driver? No, not yet. Maybe Boris can just put an explanation into the changeset description of the driver so other people are able to find it more easily. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 14:47 ` Arnd Bergmann (?) @ 2014-06-11 7:52 ` Boris BREZILLON -1 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:52 UTC (permalink / raw) To: Arnd Bergmann, Maxime Ripard Cc: Wolfram Sang, Randy Dunlap, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c Hello Arnd, On 10/06/2014 16:47, Arnd Bergmann wrote: > On Tuesday 10 June 2014 16:36:04 Maxime Ripard wrote: >> On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: >>> On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: >>>> +config I2C_SUN6I_P2WI >>>> + tristate "Allwinner sun6i 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). >>>> + >>> Sorry for the stupid question, but why is this an i2c driver if the >>> hardware protocol is completely different? >> It's not completely different. It deviates, but still looks very >> similar to i2c, and to be precise, SMBus. >> >> You'll have the full discussion that led to do this in i2c here: >> http://www.spinics.net/lists/linux-i2c/msg15066.html >> >> Also, one significant thing to take into account is that the >> communication with a device starts as I2C, only to switch to this >> protocol after some initialization sequence. > Ok, sounds good. > >>> I understand that a lot of devices can be driven using either spi or >>> i2c, and we have two sets of {directories,maintainers,bus_types,...} >>> for them. Your description sounds like this is a separate option >>> that isn't any closer to i2c than it is to spi. >> That's not true. It's *much* closer from I2C than it is from SPI. > Ok. > >>> Would it perhaps be better to expose it only as a regmap rather than >>> an i2c host? >> That could be a solution, but is it a common practice to define a bus >> adapter driver in a regmap driver? > No, not yet. > > Maybe Boris can just put an explanation into the changeset description > of the driver so other people are able to find it more easily. You mean in my commit message ? I thought it was already explaining the subtle differences between P2WI and the SMBus protocols. What would you like me to add to this explanation ? Something about the I2C to P2WI initialization part ? Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-11 7:52 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:52 UTC (permalink / raw) To: linux-arm-kernel Hello Arnd, On 10/06/2014 16:47, Arnd Bergmann wrote: > On Tuesday 10 June 2014 16:36:04 Maxime Ripard wrote: >> On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: >>> On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: >>>> +config I2C_SUN6I_P2WI >>>> + tristate "Allwinner sun6i 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). >>>> + >>> Sorry for the stupid question, but why is this an i2c driver if the >>> hardware protocol is completely different? >> It's not completely different. It deviates, but still looks very >> similar to i2c, and to be precise, SMBus. >> >> You'll have the full discussion that led to do this in i2c here: >> http://www.spinics.net/lists/linux-i2c/msg15066.html >> >> Also, one significant thing to take into account is that the >> communication with a device starts as I2C, only to switch to this >> protocol after some initialization sequence. > Ok, sounds good. > >>> I understand that a lot of devices can be driven using either spi or >>> i2c, and we have two sets of {directories,maintainers,bus_types,...} >>> for them. Your description sounds like this is a separate option >>> that isn't any closer to i2c than it is to spi. >> That's not true. It's *much* closer from I2C than it is from SPI. > Ok. > >>> Would it perhaps be better to expose it only as a regmap rather than >>> an i2c host? >> That could be a solution, but is it a common practice to define a bus >> adapter driver in a regmap driver? > No, not yet. > > Maybe Boris can just put an explanation into the changeset description > of the driver so other people are able to find it more easily. You mean in my commit message ? I thought it was already explaining the subtle differences between P2WI and the SMBus protocols. What would you like me to add to this explanation ? Something about the I2C to P2WI initialization part ? Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-11 7:52 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:52 UTC (permalink / raw) To: Arnd Bergmann, Maxime Ripard Cc: Wolfram Sang, Randy Dunlap, Hans de Goede, Shuge, kevin-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Hello Arnd, On 10/06/2014 16:47, Arnd Bergmann wrote: > On Tuesday 10 June 2014 16:36:04 Maxime Ripard wrote: >> On Tue, Jun 10, 2014 at 03:54:56PM +0200, Arnd Bergmann wrote: >>> On Tuesday 10 June 2014 15:47:16 Boris BREZILLON wrote: >>>> +config I2C_SUN6I_P2WI >>>> + tristate "Allwinner sun6i 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). >>>> + >>> Sorry for the stupid question, but why is this an i2c driver if the >>> hardware protocol is completely different? >> It's not completely different. It deviates, but still looks very >> similar to i2c, and to be precise, SMBus. >> >> You'll have the full discussion that led to do this in i2c here: >> http://www.spinics.net/lists/linux-i2c/msg15066.html >> >> Also, one significant thing to take into account is that the >> communication with a device starts as I2C, only to switch to this >> protocol after some initialization sequence. > Ok, sounds good. > >>> I understand that a lot of devices can be driven using either spi or >>> i2c, and we have two sets of {directories,maintainers,bus_types,...} >>> for them. Your description sounds like this is a separate option >>> that isn't any closer to i2c than it is to spi. >> That's not true. It's *much* closer from I2C than it is from SPI. > Ok. > >>> Would it perhaps be better to expose it only as a regmap rather than >>> an i2c host? >> That could be a solution, but is it a common practice to define a bus >> adapter driver in a regmap driver? > No, not yet. > > Maybe Boris can just put an explanation into the changeset description > of the driver so other people are able to find it more easily. You mean in my commit message ? I thought it was already explaining the subtle differences between P2WI and the SMBus protocols. What would you like me to add to this explanation ? Something about the I2C to P2WI initialization part ? Best Regards, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-11 7:52 ` Boris BREZILLON @ 2014-06-11 8:07 ` Arnd Bergmann -1 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-11 8:07 UTC (permalink / raw) To: linux-arm-kernel Cc: Boris BREZILLON, Maxime Ripard, devicetree, Wolfram Sang, Randy Dunlap, linux-doc, linux-kernel, Hans de Goede, linux-i2c, Shuge, kevin On Wednesday 11 June 2014 09:52:52 Boris BREZILLON wrote: > You mean in my commit message ? > I thought it was already explaining the subtle differences between P2WI > and the SMBus protocols. > > What would you like me to add to this explanation ? > Something about the I2C to P2WI initialization part ? > What I'd like to see is a mention of previous discussion that concluded that it should be an i2c driver rather than a new type of driver, and why. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-11 8:07 ` Arnd Bergmann 0 siblings, 0 replies; 22+ messages in thread From: Arnd Bergmann @ 2014-06-11 8:07 UTC (permalink / raw) To: linux-arm-kernel On Wednesday 11 June 2014 09:52:52 Boris BREZILLON wrote: > You mean in my commit message ? > I thought it was already explaining the subtle differences between P2WI > and the SMBus protocols. > > What would you like me to add to this explanation ? > Something about the I2C to P2WI initialization part ? > What I'd like to see is a mention of previous discussion that concluded that it should be an i2c driver rather than a new type of driver, and why. Arnd ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 13:47 ` Boris BREZILLON @ 2014-06-10 19:45 ` Wolfram Sang -1 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2014-06-10 19:45 UTC (permalink / raw) To: Boris BREZILLON Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c [-- Attachment #1: Type: text/plain, Size: 2941 bytes --] Hi, compiling this driver gives me: drivers/i2c/busses/i2c-sun6i-p2wi.c: In function 'p2wi_probe': drivers/i2c/busses/i2c-sun6i-p2wi.c:272:2: error: implicit declaration of function 'devm_reset_control_get' [-Werror=implicit-function-declaration] drivers/i2c/busses/i2c-sun6i-p2wi.c:272:13: warning: assignment makes pointer from integer without a cast [enabled by default] > diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c > new file mode 100644 > index 0000000..4f7b93f > --- /dev/null > +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c > @@ -0,0 +1,356 @@ > +/* > + * 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. > + * > + * The P2WI controller 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 write > + * followed by a read 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). > + * > + */ > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/spinlock.h> Unneeded. Please check other includes, too. > +#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> > + ... > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + p2wi->regs = devm_ioremap_resource(dev, r); > + if (IS_ERR(p2wi->regs)) { > + ret = PTR_ERR(p2wi->regs); > + return ret; > + } return PTR_ERR(...)? > + > + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(dev, "failed to retrieve irq: %d\n", ret); > + return irq; > + } > + > + 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); dangling ret. > + return ret; > + } > + ... > +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon@free-electrons.com>"); > +MODULE_DESCRIPTION("Allwinner P2WI driver"); > +MODULE_LICENSE("GPL"); "GPL v2" Thanks, Wolfram [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-10 19:45 ` Wolfram Sang 0 siblings, 0 replies; 22+ messages in thread From: Wolfram Sang @ 2014-06-10 19:45 UTC (permalink / raw) To: linux-arm-kernel Hi, compiling this driver gives me: drivers/i2c/busses/i2c-sun6i-p2wi.c: In function 'p2wi_probe': drivers/i2c/busses/i2c-sun6i-p2wi.c:272:2: error: implicit declaration of function 'devm_reset_control_get' [-Werror=implicit-function-declaration] drivers/i2c/busses/i2c-sun6i-p2wi.c:272:13: warning: assignment makes pointer from integer without a cast [enabled by default] > diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c > new file mode 100644 > index 0000000..4f7b93f > --- /dev/null > +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c > @@ -0,0 +1,356 @@ > +/* > + * 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. > + * > + * The P2WI controller 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 write > + * followed by a read 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). > + * > + */ > +#include <linux/kernel.h> > +#include <linux/slab.h> > +#include <linux/module.h> > +#include <linux/spinlock.h> Unneeded. Please check other includes, too. > +#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> > + ... > + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); > + p2wi->regs = devm_ioremap_resource(dev, r); > + if (IS_ERR(p2wi->regs)) { > + ret = PTR_ERR(p2wi->regs); > + return ret; > + } return PTR_ERR(...)? > + > + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); > + irq = platform_get_irq(pdev, 0); > + if (irq < 0) { > + dev_err(dev, "failed to retrieve irq: %d\n", ret); > + return irq; > + } > + > + 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); dangling ret. > + return ret; > + } > + ... > +MODULE_AUTHOR("Boris BREZILLON <boris.brezillon@free-electrons.com>"); > +MODULE_DESCRIPTION("Allwinner P2WI driver"); > +MODULE_LICENSE("GPL"); "GPL v2" Thanks, Wolfram -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20140610/28722f8e/attachment.sig> ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support 2014-06-10 19:45 ` Wolfram Sang (?) @ 2014-06-11 7:42 ` Boris BREZILLON -1 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:42 UTC (permalink / raw) To: Wolfram Sang Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin, devicetree, linux-doc, linux-arm-kernel, linux-kernel, linux-i2c Hello Wolfram, On 10/06/2014 21:45, Wolfram Sang wrote: > Hi, > > compiling this driver gives me: > > drivers/i2c/busses/i2c-sun6i-p2wi.c: In function 'p2wi_probe': > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:2: error: implicit declaration of function 'devm_reset_control_get' [-Werror=implicit-function-declaration] > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:13: warning: assignment makes pointer from integer without a cast [enabled by default] Yes, there's a missing "depends on RESET_CONTROLLER" in the Kconfig entry, which I didn't notice because this option was selected by the sun6i platform. >> diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> new file mode 100644 >> index 0000000..4f7b93f >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> @@ -0,0 +1,356 @@ >> +/* >> + * 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. >> + * >> + * The P2WI controller 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 write >> + * followed by a read 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). >> + * >> + */ >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/spinlock.h> > Unneeded. Please check other includes, too. Okay, I'll clean it up. > >> +#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> >> + > ... > >> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + p2wi->regs = devm_ioremap_resource(dev, r); >> + if (IS_ERR(p2wi->regs)) { >> + ret = PTR_ERR(p2wi->regs); >> + return ret; >> + } > return PTR_ERR(...)? Oops, some vestige of my previous dev_err call. > >> + >> + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) { >> + dev_err(dev, "failed to retrieve irq: %d\n", ret); >> + return irq; >> + } >> + >> + 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); > dangling ret. I'll fix it. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-11 7:42 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:42 UTC (permalink / raw) To: linux-arm-kernel Hello Wolfram, On 10/06/2014 21:45, Wolfram Sang wrote: > Hi, > > compiling this driver gives me: > > drivers/i2c/busses/i2c-sun6i-p2wi.c: In function 'p2wi_probe': > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:2: error: implicit declaration of function 'devm_reset_control_get' [-Werror=implicit-function-declaration] > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:13: warning: assignment makes pointer from integer without a cast [enabled by default] Yes, there's a missing "depends on RESET_CONTROLLER" in the Kconfig entry, which I didn't notice because this option was selected by the sun6i platform. >> diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> new file mode 100644 >> index 0000000..4f7b93f >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> @@ -0,0 +1,356 @@ >> +/* >> + * 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. >> + * >> + * The P2WI controller 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 write >> + * followed by a read 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). >> + * >> + */ >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/spinlock.h> > Unneeded. Please check other includes, too. Okay, I'll clean it up. > >> +#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> >> + > ... > >> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + p2wi->regs = devm_ioremap_resource(dev, r); >> + if (IS_ERR(p2wi->regs)) { >> + ret = PTR_ERR(p2wi->regs); >> + return ret; >> + } > return PTR_ERR(...)? Oops, some vestige of my previous dev_err call. > >> + >> + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) { >> + dev_err(dev, "failed to retrieve irq: %d\n", ret); >> + return irq; >> + } >> + >> + 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); > dangling ret. I'll fix it. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support @ 2014-06-11 7:42 ` Boris BREZILLON 0 siblings, 0 replies; 22+ messages in thread From: Boris BREZILLON @ 2014-06-11 7:42 UTC (permalink / raw) To: Wolfram Sang Cc: Randy Dunlap, Maxime Ripard, Hans de Goede, Shuge, kevin-0TFLnhJekD6UEPyfVivIlAC/G2K4zDHf, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-doc-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, linux-i2c-u79uwXL29TY76Z2rM5mHXA Hello Wolfram, On 10/06/2014 21:45, Wolfram Sang wrote: > Hi, > > compiling this driver gives me: > > drivers/i2c/busses/i2c-sun6i-p2wi.c: In function 'p2wi_probe': > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:2: error: implicit declaration of function 'devm_reset_control_get' [-Werror=implicit-function-declaration] > drivers/i2c/busses/i2c-sun6i-p2wi.c:272:13: warning: assignment makes pointer from integer without a cast [enabled by default] Yes, there's a missing "depends on RESET_CONTROLLER" in the Kconfig entry, which I didn't notice because this option was selected by the sun6i platform. >> diff --git a/drivers/i2c/busses/i2c-sun6i-p2wi.c b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> new file mode 100644 >> index 0000000..4f7b93f >> --- /dev/null >> +++ b/drivers/i2c/busses/i2c-sun6i-p2wi.c >> @@ -0,0 +1,356 @@ >> +/* >> + * P2WI (Push-Pull Two Wire Interface) bus driver. >> + * >> + * Author: Boris BREZILLON <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> >> + * >> + * 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. >> + * >> + * The P2WI controller 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 write >> + * followed by a read 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). >> + * >> + */ >> +#include <linux/kernel.h> >> +#include <linux/slab.h> >> +#include <linux/module.h> >> +#include <linux/spinlock.h> > Unneeded. Please check other includes, too. Okay, I'll clean it up. > >> +#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> >> + > ... > >> + r = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + p2wi->regs = devm_ioremap_resource(dev, r); >> + if (IS_ERR(p2wi->regs)) { >> + ret = PTR_ERR(p2wi->regs); >> + return ret; >> + } > return PTR_ERR(...)? Oops, some vestige of my previous dev_err call. > >> + >> + snprintf(p2wi->adapter.name, sizeof(p2wi->adapter.name), pdev->name); >> + irq = platform_get_irq(pdev, 0); >> + if (irq < 0) { >> + dev_err(dev, "failed to retrieve irq: %d\n", ret); >> + return irq; >> + } >> + >> + 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); > dangling ret. I'll fix it. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2014-06-11 8:08 UTC | newest] Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-10 13:47 [PATCH v5 0/2] i2c: sunxi: add P2WI controller support Boris BREZILLON 2014-06-10 13:47 ` Boris BREZILLON 2014-06-10 13:47 ` [PATCH v5 1/2] i2c: sunxi: add P2WI DT bindings documentation Boris BREZILLON 2014-06-10 13:47 ` Boris BREZILLON 2014-06-10 13:47 ` [PATCH v5 2/2] i2c: sunxi: add P2WI (Push/Pull 2 Wire Interface) controller support Boris BREZILLON 2014-06-10 13:47 ` Boris BREZILLON 2014-06-10 13:54 ` Arnd Bergmann 2014-06-10 13:54 ` Arnd Bergmann 2014-06-10 14:36 ` Maxime Ripard 2014-06-10 14:36 ` Maxime Ripard 2014-06-10 14:47 ` Arnd Bergmann 2014-06-10 14:47 ` Arnd Bergmann 2014-06-11 7:52 ` Boris BREZILLON 2014-06-11 7:52 ` Boris BREZILLON 2014-06-11 7:52 ` Boris BREZILLON 2014-06-11 8:07 ` Arnd Bergmann 2014-06-11 8:07 ` Arnd Bergmann 2014-06-10 19:45 ` Wolfram Sang 2014-06-10 19:45 ` Wolfram Sang 2014-06-11 7:42 ` Boris BREZILLON 2014-06-11 7:42 ` Boris BREZILLON 2014-06-11 7:42 ` Boris BREZILLON
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.