linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support
@ 2015-02-24 10:29 Chen-Yu Tsai
       [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 10:29 UTC (permalink / raw)
  To: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala
  Cc: Chen-Yu Tsai, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

Hi everyone,

This series adds support for the Reduced Serial Bus (RSB)
controller found on newer Allwinner SoCs, such as the A23
or A80. The RSB is used to communicate with companion ICs,
notably the bundled PMIC.

RSB is an improvement over P2WI that was found on the A31
SoC. The main new feature is support for multiple slave
devices on the same bus, using addresses that are configured
at runtime. The slave devices also have a hardware address,
which is only used when setting the runtime address.

Like the P2WI, the RSB is an SMBus like interface, supporting
byte, word and double-word transfers only. However, the wire
protocol is different so only RSB compatible devices can be
used. This class currently consists of the AXP223, AXP806,
AXP809 PMICs, and the AC100 audio codec, all from X-Powers.

Following the P2WI driver, we hope this driver can be integrated
into the I2C subsystem as well.


Patch 1 adds the driver supporting RSB.

Patch 2 documents the device tree bindings for the driver.

Patch 3 adds a device node for RSB to the A23 dtsi.

Patch 4 enables the RSB for the only supported A23 device.

Only the A23 is enabled at the moment. The A80 uses the same
IP block with no differences, but the related clock/reset/pinctrl
drivers are still WIP.


Regards,
ChenYu

Chen-Yu Tsai (4):
  i2c: sunxi: Add Reduced Serial Bus (RSB) support
  i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
  ARM: dts: sun8i: Add Reduced Serial Bus controller device node to A23
    dtsi
  ARM: dts: sun8i: ippo-q8h-v5: Enable Reduced Serial Bus controller

 .../devicetree/bindings/i2c/i2c-sunxi-rsb.txt      |  50 +++
 arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts        |   4 +
 arch/arm/boot/dts/sun8i-a23.dtsi                   |  21 +
 drivers/i2c/busses/Kconfig                         |  12 +
 drivers/i2c/busses/Makefile                        |   1 +
 drivers/i2c/busses/i2c-sunxi-rsb.c                 | 458 +++++++++++++++++++++
 6 files changed, 546 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-rsb.txt
 create mode 100644 drivers/i2c/busses/i2c-sunxi-rsb.c

-- 
2.1.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support
       [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
@ 2015-02-24 10:29   ` Chen-Yu Tsai
  2015-02-24 10:29   ` [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation Chen-Yu Tsai
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 10:29 UTC (permalink / raw)
  To: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala
  Cc: Chen-Yu Tsai, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

The RSB controller looks like an SMBus controller which only supports byte
and word data transfers. It can also do double-word data transfers, but the
I2C subsystem does not support this, nor have we seen devices using this.

The RSB differs from standard SMBus protocol on several aspects:
- it uses addresses set at runtime to address slaves. Runtime addresses
  are sent to slaves using their 12bit hardware addresses. Up to 15
  runtime addresses are available.
- it adds a parity bit every 8bits of data and address for read and
  write accesses; this replaces the ack bit
- 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 read access

This means this bus cannot be used to interface with standard SMBus
devices (known devices supporting this interface are the AXP223, AXP806,
AXP809 PMICs and the AC100 codec/RTC). However the RSB protocol is an
extension of P2WI, which was close enough to SMBus to be integrated into
the I2C subsystem in commit 3e833490fae5 ("i2c: sunxi: add P2WI (Push/Pull
2 Wire Interface) controller support").

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 drivers/i2c/busses/Kconfig         |  12 +
 drivers/i2c/busses/Makefile        |   1 +
 drivers/i2c/busses/i2c-sunxi-rsb.c | 458 +++++++++++++++++++++++++++++++++++++
 3 files changed, 471 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-sunxi-rsb.c

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 22da9c2ffa22..cf9337877181 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -840,6 +840,18 @@ config I2C_SUN6I_P2WI
 	  This interface is used to connect to specific PMIC devices (like the
 	  AXP221).
 
+config I2C_SUNXI_RSB
+	tristate "Allwinner Reduced Serial Bus controller"
+	depends on RESET_CONTROLLER
+	depends on MACH_SUN8I || MACH_SUN9I || COMPILE_TEST
+	help
+	  If you say yes to this option, support will be included for the
+	  Reduced Serial Bus controller embedded in some sunxi SOCs.
+	  The RSB looks like an SMBus controller (which supports only byte
+	  accesses), but requires setting runtime addresses for slave devices.
+	  This interface is used to connect to specific PMIC devices (like the
+	  AXP223) or peripherals (like the AC100).
+
 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 3638feb6677e..f95d50315003 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -81,6 +81,7 @@ 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_SUNXI_RSB)	+= i2c-sunxi-rsb.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-rsb.c b/drivers/i2c/busses/i2c-sunxi-rsb.c
new file mode 100644
index 000000000000..7e9be3e14b8a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-sunxi-rsb.c
@@ -0,0 +1,458 @@
+/*
+ * RSB (Reduced Serial Bus) driver.
+ *
+ * Author: Chen-Yu Tsai <wens-jdAy2FN1RRM@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 RSB controller looks like an SMBus controller which only supports
+ * byte and word data transfers. But, it differs from standard SMBus
+ * protocol on several aspects:
+ * - it uses addresses set at runtime to address slaves. Runtime addresses
+ *   are sent to slaves using their 12bit hardware addresses. Up to 15
+ *   runtime addresses are available.
+ * - it adds a parity bit every 8bits of data and address for read and
+ *   write accesses; this replaces the ack bit
+ * - 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 read access
+ *
+ * This means this bus cannot be used to interface with standard SMBus
+ * devices. Devices known to support this interface include the AXP223,
+ * AXP809, and AXP806 PMICs, and the AC100 audio codec, all from X-Powers.
+ *
+ * A description of the operation and wire protocol can be found in the
+ * RSB section of Allwinner's A80 user manual, which can be found at
+ *
+ *     https://github.com/allwinner-zh/documents/tree/master/A80
+ *
+ * This document is officially released by Allwinner.
+ *
+ * This driver is based on i2c-sun6i-p2wi.c, the P2WI bus driver.
+ *
+ */
+#include <linux/clk.h>
+#include <linux/i2c.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+
+
+/* RSB registers */
+#define RSB_CTRL	0x0	/* Global control */
+#define RSB_CCR		0x4	/* Clock control */
+#define RSB_INTE	0x8	/* Interrupt controls */
+#define RSB_INTS	0xc	/* Interrupt status */
+#define RSB_ADDR	0x10	/* Address to send with read/write command */
+#define RSB_DATA	0x1c	/* Data to read/write */
+#define RSB_LCR		0x24	/* Line control */
+#define RSB_DMCR	0x28	/* Device mode (init) control */
+#define RSB_CMD		0x2c	/* RSB Command */
+#define RSB_DAR		0x30	/* Device address / runtime address */
+
+/* CTRL fields */
+#define RSB_CTRL_START_TRANS		BIT(7)
+#define RSB_CTRL_ABORT_TRANS		BIT(6)
+#define RSB_CTRL_GLOBAL_INT_ENB		BIT(1)
+#define RSB_CTRL_SOFT_RST		BIT(0)
+
+/* CLK CTRL fields */
+#define RSB_CCR_SDA_OUT_DELAY(v)	(((v) & 0x7) << 8)
+#define RSB_CCR_MAX_CLK_DIV		0xff
+#define RSB_CCR_CLK_DIV(v)		((v) & RSB_CCR_MAX_CLK_DIV)
+
+/* STATUS fields */
+#define RSB_INTS_TRANS_ERR_ACK		BIT(16)
+#define RSB_INTS_TRANS_ERR_ID(v)	(((v) >> 8) & 0xf)
+#define RSB_INTS_LOAD_BSY		BIT(2)
+#define RSB_INTS_TRANS_ERR		BIT(1)
+#define RSB_INTS_TRANS_OVER		BIT(0)
+
+/* LINE CTRL fields*/
+#define RSB_LCR_SCL_STATE		BIT(5)
+#define RSB_LCR_SDA_STATE		BIT(4)
+#define RSB_LCR_SCL_CTL			BIT(3)
+#define RSB_LCR_SCL_CTL_EN		BIT(2)
+#define RSB_LCR_SDA_CTL			BIT(1)
+#define RSB_LCR_SDA_CTL_EN		BIT(0)
+
+/* DEVICE MODE CTRL field values */
+#define RSB_DMCR_DEVICE_START		BIT(31)
+#define RSB_DMCR_MODE_DATA		(0x7c << 16)
+#define RSB_DMCR_MODE_REG		(0x3e << 8)
+#define RSB_DMCR_DEV_ADDR		0x00
+
+/* CMD values */
+#define RSB_CMD_RD8			0x8b
+#define RSB_CMD_RD16			0x9c
+#define RSB_CMD_RD32			0xa6
+#define RSB_CMD_WR8			0x4e
+#define RSB_CMD_WR16			0x59
+#define RSB_CMD_WR32			0x63
+#define RSB_CMD_STRA			0xe8
+
+/* DAR fields */
+#define RSB_DAR_RTA(v)			(((v) & 0xff) << 16)
+#define RSB_DAR_DA(v)			((v) & 0xffff)
+
+#define RSB_MAX_FREQ			20000000
+
+#define RSB_CTRL_NAME			"sunxi-rsb"
+
+struct rsb {
+	struct i2c_adapter adapter;
+	struct completion complete;
+	unsigned int status;
+	void __iomem *regs;
+	struct clk *clk;
+	struct reset_control *rstc;
+};
+
+static irqreturn_t rsb_interrupt(int irq, void *dev_id)
+{
+	struct rsb *rsb = dev_id;
+	unsigned long status;
+
+	status = readl(rsb->regs + RSB_INTS);
+	rsb->status = status;
+
+	/* Clear interrupts */
+	status &= (RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR |
+		   RSB_INTS_TRANS_OVER);
+	writel(status, rsb->regs + RSB_INTS);
+
+	complete(&rsb->complete);
+
+	return IRQ_HANDLED;
+}
+
+/* common code that starts a transfer */
+static int rsb_run_xfer(struct rsb *rsb)
+{
+	if (readl(rsb->regs + RSB_CTRL) & RSB_CTRL_START_TRANS) {
+		dev_dbg(&rsb->adapter.dev, "RSB bus busy\n");
+		return -EBUSY;
+	}
+
+	reinit_completion(&rsb->complete);
+
+	writel(RSB_INTS_LOAD_BSY | RSB_INTS_TRANS_ERR | RSB_INTS_TRANS_OVER,
+	       rsb->regs + RSB_INTE);
+
+	writel(RSB_CTRL_START_TRANS | RSB_CTRL_GLOBAL_INT_ENB,
+	       rsb->regs + RSB_CTRL);
+
+	wait_for_completion(&rsb->complete);
+
+	if (rsb->status & RSB_INTS_LOAD_BSY) {
+		dev_dbg(&rsb->adapter.dev, "RSB bus busy\n");
+		return -EBUSY;
+	}
+
+	if (rsb->status & RSB_INTS_TRANS_ERR) {
+		dev_dbg(&rsb->adapter.dev, "RSB bus xfer error\n");
+		return -ENXIO;
+	}
+
+	return 0;
+}
+
+static int rsb_smbus_xfer(struct i2c_adapter *adap, u16 addr,
+			   unsigned short flags, char read_write,
+			   u8 command, int size, union i2c_smbus_data *data)
+{
+	struct rsb *rsb = i2c_get_adapdata(adap);
+	int ret;
+
+	if (!data)
+		return -EINVAL;
+
+	writel(command, rsb->regs + RSB_ADDR);
+	writel(RSB_DAR_RTA(addr), rsb->regs + RSB_DAR);
+
+	if (read_write == I2C_SMBUS_READ) {
+		if (size == I2C_SMBUS_BYTE_DATA)
+			writel(RSB_CMD_RD8, rsb->regs + RSB_CMD);
+		else
+			writel(RSB_CMD_RD16, rsb->regs + RSB_CMD);
+	} else {
+		if (size == I2C_SMBUS_BYTE_DATA) {
+			writel(RSB_CMD_WR8, rsb->regs + RSB_CMD);
+			writel(data->byte, rsb->regs + RSB_DATA);
+		} else {
+			writel(RSB_CMD_WR16, rsb->regs + RSB_CMD);
+			writel(data->word, rsb->regs + RSB_DATA);
+		}
+	}
+
+	ret = rsb_run_xfer(rsb);
+	if (ret)
+		return ret;
+
+	if (read_write == I2C_SMBUS_READ) {
+		if (size == I2C_SMBUS_BYTE_DATA)
+			data->byte = readl(rsb->regs + RSB_DATA);
+		else
+			data->word = readl(rsb->regs + RSB_DATA);
+	}
+
+	return 0;
+}
+
+static u32 rsb_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA;
+}
+
+static const struct i2c_algorithm rsb_algo = {
+	.smbus_xfer	= rsb_smbus_xfer,
+	.functionality	= rsb_functionality,
+};
+
+/* following functions are used strictly during device probe */
+static void rsb_init_device_mode(struct rsb *rsb)
+{
+	unsigned long expire = jiffies + msecs_to_jiffies(250);
+	u32 reg;
+
+	/* send init sequence */
+	writel(RSB_DMCR_DEVICE_START | RSB_DMCR_MODE_DATA |
+	       RSB_DMCR_MODE_REG | RSB_DMCR_DEV_ADDR, rsb->regs + RSB_DMCR);
+	do {
+		reg = readl(rsb->regs + RSB_DMCR);
+	} while (time_before(jiffies, expire) && (reg & RSB_DMCR_DEVICE_START));
+
+	if (reg & RSB_DMCR_DEVICE_START)
+		dev_warn(&rsb->adapter.dev, "send init sequence timeout\n");
+
+	/* clear interrupt status bits */
+	writel(readl(rsb->regs + RSB_INTS), rsb->regs + RSB_INTS);
+}
+
+/* 15 valid runtime addresses for RSB slaves */
+static const u8 rsb_valid_rtaddr[] = {
+	0x17, 0x2d, 0x3a, 0x4e, 0x59, 0x63, 0x74,
+	/*
+	 * Currently this driver only supports 7-bit addresses.
+	 * The following addresses will be blocked by the i2c core.
+	 */
+	0x8b, 0x9c, 0xa6, 0xb1, 0xc5, 0xd2, 0xe8, 0xff,
+};
+
+static int rsb_check_rt_addr(u8 addr)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(rsb_valid_rtaddr); i++)
+		if (addr == rsb_valid_rtaddr[i])
+			return 0;
+
+	return -EINVAL;
+}
+
+/* Scan the device tree for child nodes and set runtime address for them. */
+static int rsb_set_rt_addrs(struct rsb *rsb)
+{
+	struct device *dev = rsb->adapter.dev.parent;
+	struct device_node *child, *np = dev->of_node;
+	u32 rt_addr, hw_addr;
+	int ret;
+
+	if (!np)
+		return -EINVAL;
+
+	for_each_child_of_node(np, child) {
+		/* get hardware address */
+		ret = of_property_read_u32(child, "allwinner,rsb-hw-addr",
+					   &hw_addr);
+		if (ret) {
+			dev_warn(dev, "runtime address not given for %s\n",
+				 of_node_full_name(child));
+			continue;
+		}
+
+		/* get runtime address */
+		ret = of_property_read_u32(child, "reg", &rt_addr);
+		if (ret) {
+			dev_warn(dev, "runtime address not given for %s\n",
+				 of_node_full_name(child));
+			continue;
+		}
+
+		/* check runtime address */
+		ret = rsb_check_rt_addr(rt_addr);
+		if (ret) {
+			dev_warn(dev, "runtime address for %s is invalid\n",
+				 of_node_full_name(child));
+			continue;
+		}
+
+		/* setup command parameters */
+		writel(RSB_CMD_STRA, rsb->regs + RSB_CMD);
+		writel(RSB_DAR_RTA(rt_addr) | RSB_DAR_DA(hw_addr),
+		       rsb->regs + RSB_DAR);
+
+		/* send command */
+		ret = rsb_run_xfer(rsb);
+		if (ret)
+			dev_warn(dev, "set runtime address failed for %s\n",
+				 of_node_full_name(child));
+	}
+
+	return 0;
+}
+
+
+static const struct of_device_id rsb_of_match_table[] = {
+	{ .compatible = "allwinner,sun8i-a23-rsb" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, rsb_of_match_table);
+
+static int rsb_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	unsigned long parent_clk_freq;
+	u32 clk_freq = 100000;
+	struct resource *r;
+	struct rsb *rsb;
+	int clk_div;
+	int irq;
+	int ret;
+
+	of_property_read_u32(np, "clock-frequency", &clk_freq);
+	if (clk_freq > RSB_MAX_FREQ) {
+		dev_err(dev,
+			"clock-frequency (%u Hz) is too high (max = 20MHz)",
+			clk_freq);
+		return -EINVAL;
+	}
+
+	rsb = devm_kzalloc(dev, sizeof(struct rsb), GFP_KERNEL);
+	if (!rsb)
+		return -ENOMEM;
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	rsb->regs = devm_ioremap_resource(dev, r);
+	if (IS_ERR(rsb->regs))
+		return PTR_ERR(rsb->regs);
+
+	strlcpy(rsb->adapter.name, pdev->name, sizeof(rsb->adapter.name));
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0) {
+		dev_err(dev, "failed to retrieve irq: %d\n", irq);
+		return irq;
+	}
+
+	rsb->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(rsb->clk)) {
+		ret = PTR_ERR(rsb->clk);
+		dev_err(dev, "failed to retrieve clk: %d\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(rsb->clk);
+	if (ret) {
+		dev_err(dev, "failed to enable clk: %d\n", ret);
+		return ret;
+	}
+
+	parent_clk_freq = clk_get_rate(rsb->clk);
+
+	rsb->rstc = devm_reset_control_get(dev, NULL);
+	if (IS_ERR(rsb->rstc)) {
+		ret = PTR_ERR(rsb->rstc);
+		dev_err(dev, "failed to retrieve reset controller: %d\n", ret);
+		goto err_clk_disable;
+	}
+
+	ret = reset_control_deassert(rsb->rstc);
+	if (ret) {
+		dev_err(dev, "failed to deassert reset line: %d\n", ret);
+		goto err_clk_disable;
+	}
+
+	init_completion(&rsb->complete);
+	strlcpy(rsb->adapter.name, RSB_CTRL_NAME, sizeof(rsb->adapter.name));
+	rsb->adapter.dev.parent = dev;
+	rsb->adapter.algo = &rsb_algo;
+	rsb->adapter.owner = THIS_MODULE;
+	rsb->adapter.dev.of_node = pdev->dev.of_node;
+	platform_set_drvdata(pdev, rsb);
+	i2c_set_adapdata(&rsb->adapter, rsb);
+
+	writel(RSB_CTRL_SOFT_RST, rsb->regs + RSB_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 > RSB_CCR_MAX_CLK_DIV) {
+		dev_warn(dev,
+			 "clock-frequency is too low, setting it to %lu Hz\n",
+			 parent_clk_freq / RSB_CCR_MAX_CLK_DIV);
+		clk_div = RSB_CCR_MAX_CLK_DIV;
+	}
+
+	writel(RSB_CCR_SDA_OUT_DELAY(1) | RSB_CCR_CLK_DIV(clk_div),
+	       rsb->regs + RSB_CCR);
+
+	ret = devm_request_irq(dev, irq, rsb_interrupt, 0, RSB_CTRL_NAME, rsb);
+	if (ret) {
+		dev_err(dev, "can't register interrupt handler irq%d: %d\n",
+			irq, ret);
+		goto err_reset_assert;
+	}
+
+	rsb_init_device_mode(rsb);
+
+	ret = rsb_set_rt_addrs(rsb);
+	if (ret)
+		goto err_reset_assert;
+
+	ret = i2c_add_adapter(&rsb->adapter);
+	if (!ret)
+		return 0;
+
+err_reset_assert:
+	reset_control_assert(rsb->rstc);
+
+err_clk_disable:
+	clk_disable_unprepare(rsb->clk);
+
+	return ret;
+}
+
+static int rsb_remove(struct platform_device *dev)
+{
+	struct rsb *rsb = platform_get_drvdata(dev);
+
+	i2c_del_adapter(&rsb->adapter);
+	reset_control_assert(rsb->rstc);
+	clk_disable_unprepare(rsb->clk);
+
+	return 0;
+}
+
+static struct platform_driver rsb_driver = {
+	.probe = rsb_probe,
+	.remove	= rsb_remove,
+	.driver	= {
+		.name = "i2c-sunxi-rsb",
+		.of_match_table = rsb_of_match_table,
+	},
+};
+module_platform_driver(rsb_driver);
+
+MODULE_AUTHOR("Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>");
+MODULE_DESCRIPTION("Allwinner RSB driver");
+MODULE_LICENSE("GPL v2");
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
       [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
  2015-02-24 10:29   ` [PATCH 1/4] " Chen-Yu Tsai
@ 2015-02-24 10:29   ` Chen-Yu Tsai
       [not found]     ` <1424773744-15106-3-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
  2015-02-24 10:29   ` [PATCH 3/4] ARM: dts: sun8i: Add Reduced Serial Bus controller device node to A23 dtsi Chen-Yu Tsai
  2015-02-24 10:29   ` [PATCH 4/4] ARM: dts: sun8i: ippo-q8h-v5: Enable Reduced Serial Bus controller Chen-Yu Tsai
  3 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 10:29 UTC (permalink / raw)
  To: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala
  Cc: Chen-Yu Tsai, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

Reduced Serial Bus (RSB) is an SMBus like bus used to communicate
with some PMICs (like the AXP223) or other peripherals.

The RSB DT bindings are pretty much the same as the one defined for
the marvell's mv64xxx controller, with the additional RSB specific
"allwinner,rsb-hw-addr" property for slave device nodes.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 .../devicetree/bindings/i2c/i2c-sunxi-rsb.txt      | 50 ++++++++++++++++++++++
 1 file changed, 50 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/i2c-sunxi-rsb.txt

diff --git a/Documentation/devicetree/bindings/i2c/i2c-sunxi-rsb.txt b/Documentation/devicetree/bindings/i2c/i2c-sunxi-rsb.txt
new file mode 100644
index 000000000000..90aa5066873c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-sunxi-rsb.txt
@@ -0,0 +1,50 @@
+
+* Allwinner RSB (Reduced Serial Bus) controller
+
+Required properties :
+
+ - reg             : Offset and length of the register set for the device.
+ - compatible      : Should be "allwinner,sun8i-a23-rsb".
+ - interrupts      : The interrupt line connected to the RSB peripheral.
+ - clocks          : The gate clk connected to the RSB peripheral.
+ - resets          : The reset line connected to the RSB peripheral.
+
+Optional properties :
+
+ - clock-frequency : Desired RSB bus clock frequency in Hz. If not set
+		     the default frequency is 100kHz. Maximum is 20MHz.
+
+An RSB device node may contain up to 15 child nodes each encoding an RSB
+slave device.
+
+Slave device properties:
+  Required properties:
+   - reg           : The runtime address used to access the device.
+   - allwinner,rsb-hw-addr : The RSB hardware address for the device. This
+			     is only used when configuring the runtime
+			     address of the device.
+
+  Valid runtime addresses - There are only 15 valid runtime addresses:
+
+      0x17, 0x2d, 0x3a, 0x4e, 0x59, 0x63, 0x74, 0x8b,
+      0x9c, 0xa6, 0xb1, 0xc5, 0xd2, 0xe8, 0xff
+
+
+Example:
+
+	rsb@01f03400 {
+		compatible = "allwinner,sun8i-a23-rsb";
+		reg = <0x01f03400 0x400>;
+		interrupts = <0 39 4>;
+		clocks = <&apb0_gates 3>;
+		clock-frequency = <3000000>;
+		resets = <&apb0_rst 3>;
+
+		axp223: pmic@2d {
+			compatible = "x-powers,axp223", "x-powers,axp221";
+			reg = <0x2d>;
+			allwinner,rsb-hw-addr = <0x3e3>;
+
+			/* ... */
+		};
+	};
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/4] ARM: dts: sun8i: Add Reduced Serial Bus controller device node to A23 dtsi
       [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
  2015-02-24 10:29   ` [PATCH 1/4] " Chen-Yu Tsai
  2015-02-24 10:29   ` [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation Chen-Yu Tsai
@ 2015-02-24 10:29   ` Chen-Yu Tsai
  2015-02-24 10:29   ` [PATCH 4/4] ARM: dts: sun8i: ippo-q8h-v5: Enable Reduced Serial Bus controller Chen-Yu Tsai
  3 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 10:29 UTC (permalink / raw)
  To: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala
  Cc: Chen-Yu Tsai, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

This patch adds a device node for the Reduced Serial Bus (RSB)
controller and the defacto pinmux setting to the A23 dtsi.

Since there is only one possible pinmux setting for RSB, just
set it in the dtsi.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a23.dtsi | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23.dtsi b/arch/arm/boot/dts/sun8i-a23.dtsi
index 382ebd137ee4..fd9b4c811a5f 100644
--- a/arch/arm/boot/dts/sun8i-a23.dtsi
+++ b/arch/arm/boot/dts/sun8i-a23.dtsi
@@ -600,6 +600,13 @@
 			#size-cells = <0>;
 			#gpio-cells = <3>;
 
+			r_rsb_pins: r_rsb {
+				allwinner,pins = "PL0", "PL1";
+				allwinner,function = "s_rsb";
+				allwinner,drive = <SUN4I_PINCTRL_20_MA>;
+				allwinner,pull = <SUN4I_PINCTRL_PULL_UP>;
+			};
+
 			r_uart_pins_a: r_uart@0 {
 				allwinner,pins = "PL2", "PL3";
 				allwinner,function = "s_uart";
@@ -607,5 +614,19 @@
 				allwinner,pull = <SUN4I_PINCTRL_NO_PULL>;
 			};
 		};
+
+		r_rsb: i2c@01f03400 {
+			compatible = "allwinner,sun8i-a23-rsb";
+			reg = <0x01f03400 0x400>;
+			interrupts = <GIC_SPI 39 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&apb0_gates 3>;
+			clock-frequency = <3000000>;
+			resets = <&apb0_rst 3>;
+			pinctrl-names = "default";
+			pinctrl-0 = <&r_rsb_pins>;
+			status = "disabled";
+			#address-cells = <1>;
+			#size-cells = <0>;
+		};
 	};
 };
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/4] ARM: dts: sun8i: ippo-q8h-v5: Enable Reduced Serial Bus controller
       [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
                     ` (2 preceding siblings ...)
  2015-02-24 10:29   ` [PATCH 3/4] ARM: dts: sun8i: Add Reduced Serial Bus controller device node to A23 dtsi Chen-Yu Tsai
@ 2015-02-24 10:29   ` Chen-Yu Tsai
  3 siblings, 0 replies; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 10:29 UTC (permalink / raw)
  To: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala
  Cc: Chen-Yu Tsai, linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

The Reduced Serial Bus controller is used to talk to the onboard PMIC.

Signed-off-by: Chen-Yu Tsai <wens-jdAy2FN1RRM@public.gmane.org>
---
 arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts b/arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts
index 4cb25f8267c8..31882e6e7a38 100644
--- a/arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts
+++ b/arch/arm/boot/dts/sun8i-a23-ippo-q8h-v5.dts
@@ -125,6 +125,10 @@
 	};
 };
 
+&r_rsb {
+	status = "okay";
+};
+
 &r_uart {
 	pinctrl-names = "default";
 	pinctrl-0 = <&r_uart_pins_a>;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
       [not found]     ` <1424773744-15106-3-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
@ 2015-02-24 10:37       ` Arnd Bergmann
  2015-02-24 14:01         ` Chen-Yu Tsai
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2015-02-24 10:37 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Hans de Goede

On Tuesday 24 February 2015 18:29:02 Chen-Yu Tsai wrote:
> 
> +       rsb@01f03400 {
> +               compatible = "allwinner,sun8i-a23-rsb";
> +               reg = <0x01f03400 0x400>;
> +               interrupts = <0 39 4>;
> +               clocks = <&apb0_gates 3>;
> +               clock-frequency = <3000000>;
> +               resets = <&apb0_rst 3>;
> +
> +               axp223: pmic@2d {
> +                       compatible = "x-powers,axp223", "x-powers,axp221";
> +                       reg = <0x2d>;
> +                       allwinner,rsb-hw-addr = <0x3e3>;
> +
> +                       /* ... */
> +               };
> +       };

The child node cannot have a 'reg' property if the parent does not
have #address-cells/#size-cells. You should add these as mandatory
properties in the list.

I don't really understand the need for having two addresses (runtime
and hardware). Could the runtime address be configured at runtime?
Alternatively, could you use #address-cells=<2> and put both into
'reg'?

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
  2015-02-24 10:37       ` Arnd Bergmann
@ 2015-02-24 14:01         ` Chen-Yu Tsai
       [not found]           ` <CAGb2v66ru6Yurt1CFn-zocAuqen8GWSm8y9dHRf=-OOcc-DR7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 14:01 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Chen-Yu Tsai, Wolfram Sang, Maxime Ripard, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-kernel,
	devicetree, linux-sunxi, Hans de Goede

On Tue, Feb 24, 2015 at 6:37 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Tuesday 24 February 2015 18:29:02 Chen-Yu Tsai wrote:
>>
>> +       rsb@01f03400 {
>> +               compatible = "allwinner,sun8i-a23-rsb";
>> +               reg = <0x01f03400 0x400>;
>> +               interrupts = <0 39 4>;
>> +               clocks = <&apb0_gates 3>;
>> +               clock-frequency = <3000000>;
>> +               resets = <&apb0_rst 3>;
>> +
>> +               axp223: pmic@2d {
>> +                       compatible = "x-powers,axp223", "x-powers,axp221";
>> +                       reg = <0x2d>;
>> +                       allwinner,rsb-hw-addr = <0x3e3>;
>> +
>> +                       /* ... */
>> +               };
>> +       };
>
> The child node cannot have a 'reg' property if the parent does not
> have #address-cells/#size-cells. You should add these as mandatory
> properties in the list.

Oops. I'll add them. A few of the other i2c bindings seem to be missing
it as well. Probably why I missed it.

> I don't really understand the need for having two addresses (runtime
> and hardware). Could the runtime address be configured at runtime?

You can, though the driver doesn't support this. I don't think the
I2C subsystem allows arbitrary device insertion during normal
operation, but maybe i2c-dev? I've tried using different addresses
for devices so they do get changed during the probe phase, just
to be sure that the code works, and it's not just sitting at
the address the bootloader used.

In any case, the distinction is more like burnt-in or hardwired
addresses vs software configurable addresses.

> Alternatively, could you use #address-cells=<2> and put both into
> 'reg'?

I sort of looked into it, and it seems the I2C subsystem won't mind
the extra address cell. Maxime also asked whether we could use 2
addresses, possibly with a reg-names property. Using 2 cells is simpler
though.

I'm open to other options, but I'd like to get some input, especially
from the device tree maintainers, before I change it.


Regards,
ChenYu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
       [not found]           ` <CAGb2v66ru6Yurt1CFn-zocAuqen8GWSm8y9dHRf=-OOcc-DR7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
@ 2015-02-24 14:17             ` Arnd Bergmann
  2015-02-24 14:32               ` Chen-Yu Tsai
  0 siblings, 1 reply; 10+ messages in thread
From: Arnd Bergmann @ 2015-02-24 14:17 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-kernel,
	devicetree, linux-sunxi, Hans de Goede

On Tuesday 24 February 2015 22:01:26 Chen-Yu Tsai wrote:
> On Tue, Feb 24, 2015 at 6:37 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> > On Tuesday 24 February 2015 18:29:02 Chen-Yu Tsai wrote:
> >>
> >> +       rsb@01f03400 {
> >> +               compatible = "allwinner,sun8i-a23-rsb";
> >> +               reg = <0x01f03400 0x400>;
> >> +               interrupts = <0 39 4>;
> >> +               clocks = <&apb0_gates 3>;
> >> +               clock-frequency = <3000000>;
> >> +               resets = <&apb0_rst 3>;
> >> +
> >> +               axp223: pmic@2d {
> >> +                       compatible = "x-powers,axp223", "x-powers,axp221";
> >> +                       reg = <0x2d>;
> >> +                       allwinner,rsb-hw-addr = <0x3e3>;
> >> +
> >> +                       /* ... */
> >> +               };
> >> +       };
> 
> > I don't really understand the need for having two addresses (runtime
> > and hardware). Could the runtime address be configured at runtime?
> 
> You can, though the driver doesn't support this. I don't think the
> I2C subsystem allows arbitrary device insertion during normal
> operation, but maybe i2c-dev? I've tried using different addresses
> for devices so they do get changed during the probe phase, just
> to be sure that the code works, and it's not just sitting at
> the address the bootloader used.
> 
> In any case, the distinction is more like burnt-in or hardwired
> addresses vs software configurable addresses.

The simplest binding would the probably be to only put the
hardware address into the 'reg' property and always assign the
logical addresses dynamically.

Would that add a lot of complexity or does it have any other
downsides?

	arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
  2015-02-24 14:17             ` Arnd Bergmann
@ 2015-02-24 14:32               ` Chen-Yu Tsai
  2015-02-25 16:10                 ` Arnd Bergmann
  0 siblings, 1 reply; 10+ messages in thread
From: Chen-Yu Tsai @ 2015-02-24 14:32 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Chen-Yu Tsai, Wolfram Sang, Maxime Ripard, Rob Herring,
	Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-arm-kernel, linux-kernel,
	devicetree, linux-sunxi, Hans de Goede

On Tue, Feb 24, 2015 at 10:17 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Tuesday 24 February 2015 22:01:26 Chen-Yu Tsai wrote:
>> On Tue, Feb 24, 2015 at 6:37 PM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>> > On Tuesday 24 February 2015 18:29:02 Chen-Yu Tsai wrote:
>> >>
>> >> +       rsb@01f03400 {
>> >> +               compatible = "allwinner,sun8i-a23-rsb";
>> >> +               reg = <0x01f03400 0x400>;
>> >> +               interrupts = <0 39 4>;
>> >> +               clocks = <&apb0_gates 3>;
>> >> +               clock-frequency = <3000000>;
>> >> +               resets = <&apb0_rst 3>;
>> >> +
>> >> +               axp223: pmic@2d {
>> >> +                       compatible = "x-powers,axp223", "x-powers,axp221";
>> >> +                       reg = <0x2d>;
>> >> +                       allwinner,rsb-hw-addr = <0x3e3>;
>> >> +
>> >> +                       /* ... */
>> >> +               };
>> >> +       };
>>
>> > I don't really understand the need for having two addresses (runtime
>> > and hardware). Could the runtime address be configured at runtime?
>>
>> You can, though the driver doesn't support this. I don't think the
>> I2C subsystem allows arbitrary device insertion during normal
>> operation, but maybe i2c-dev? I've tried using different addresses
>> for devices so they do get changed during the probe phase, just
>> to be sure that the code works, and it's not just sitting at
>> the address the bootloader used.
>>
>> In any case, the distinction is more like burnt-in or hardwired
>> addresses vs software configurable addresses.
>
> The simplest binding would the probably be to only put the
> hardware address into the 'reg' property and always assign the
> logical addresses dynamically.
>
> Would that add a lot of complexity or does it have any other
> downsides?

The hardware address is 12 bits wide. Any address higher than
0x3ff will be rejected by the I2C core. The AC100 is at 0xe89.

Assigning addresses dynamically means the driver has to keep
a lookup table to map the hardware address to the logical
address to issue the command to.

There's also the issue of dynamically assigned address colliding
with unlisted devices, though I think this would only happen in
the development / bring up phase of the device.

I think the first issue pretty much rules out putting the
hardware address into 'reg'.

Putting the logical address in the 'reg' property allows the
user to poke unlisted devices using i2c-tools, though this
is not so useful to the average user.


Regards,
ChenYu

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation
  2015-02-24 14:32               ` Chen-Yu Tsai
@ 2015-02-25 16:10                 ` Arnd Bergmann
  0 siblings, 0 replies; 10+ messages in thread
From: Arnd Bergmann @ 2015-02-25 16:10 UTC (permalink / raw)
  To: Chen-Yu Tsai
  Cc: Wolfram Sang, Maxime Ripard, Rob Herring, Pawel Moll,
	Mark Rutland, Ian Campbell, Kumar Gala, linux-i2c,
	linux-arm-kernel, linux-kernel, devicetree, linux-sunxi,
	Hans de Goede

On Tuesday 24 February 2015 22:32:57 Chen-Yu Tsai wrote:
> On Tue, Feb 24, 2015 at 10:17 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Tuesday 24 February 2015 22:01:26 Chen-Yu Tsai wrote:
> >> On Tue, Feb 24, 2015 at 6:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> >> > On Tuesday 24 February 2015 18:29:02 Chen-Yu Tsai wrote:
> >> >>
> >> >> +       rsb@01f03400 {
> >> >> +               compatible = "allwinner,sun8i-a23-rsb";
> >> >> +               reg = <0x01f03400 0x400>;
> >> >> +               interrupts = <0 39 4>;
> >> >> +               clocks = <&apb0_gates 3>;
> >> >> +               clock-frequency = <3000000>;
> >> >> +               resets = <&apb0_rst 3>;
> >> >> +
> >> >> +               axp223: pmic@2d {
> >> >> +                       compatible = "x-powers,axp223", "x-powers,axp221";
> >> >> +                       reg = <0x2d>;
> >> >> +                       allwinner,rsb-hw-addr = <0x3e3>;
> >> >> +
> >> >> +                       /* ... */
> >> >> +               };
> >> >> +       };
> >>
> >> > I don't really understand the need for having two addresses (runtime
> >> > and hardware). Could the runtime address be configured at runtime?
> >>
> >> You can, though the driver doesn't support this. I don't think the
> >> I2C subsystem allows arbitrary device insertion during normal
> >> operation, but maybe i2c-dev? I've tried using different addresses
> >> for devices so they do get changed during the probe phase, just
> >> to be sure that the code works, and it's not just sitting at
> >> the address the bootloader used.
> >>
> >> In any case, the distinction is more like burnt-in or hardwired
> >> addresses vs software configurable addresses.
> >
> > The simplest binding would the probably be to only put the
> > hardware address into the 'reg' property and always assign the
> > logical addresses dynamically.
> >
> > Would that add a lot of complexity or does it have any other
> > downsides?
> 
> The hardware address is 12 bits wide. Any address higher than
> 0x3ff will be rejected by the I2C core. The AC100 is at 0xe89.
> 
> Assigning addresses dynamically means the driver has to keep
> a lookup table to map the hardware address to the logical
> address to issue the command to.
> 
> There's also the issue of dynamically assigned address colliding
> with unlisted devices, though I think this would only happen in
> the development / bring up phase of the device.
> 
> I think the first issue pretty much rules out putting the
> hardware address into 'reg'.
> 
> Putting the logical address in the 'reg' property allows the
> user to poke unlisted devices using i2c-tools, though this
> is not so useful to the average user.

Ok, fair enough. Your approach seems reasonable then, but it
might be helpful to add your explanation to the changelog of the
patch that introduces the binding.

	Arnd

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-02-25 16:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-24 10:29 [PATCH 0/4] i2c: sunxi: Add Reduced Serial Bus (RSB) support Chen-Yu Tsai
     [not found] ` <1424773744-15106-1-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
2015-02-24 10:29   ` [PATCH 1/4] " Chen-Yu Tsai
2015-02-24 10:29   ` [PATCH 2/4] i2c: sunxi: Add Reduced Serial Bus (RSB) DT bindings documentation Chen-Yu Tsai
     [not found]     ` <1424773744-15106-3-git-send-email-wens-jdAy2FN1RRM@public.gmane.org>
2015-02-24 10:37       ` Arnd Bergmann
2015-02-24 14:01         ` Chen-Yu Tsai
     [not found]           ` <CAGb2v66ru6Yurt1CFn-zocAuqen8GWSm8y9dHRf=-OOcc-DR7g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2015-02-24 14:17             ` Arnd Bergmann
2015-02-24 14:32               ` Chen-Yu Tsai
2015-02-25 16:10                 ` Arnd Bergmann
2015-02-24 10:29   ` [PATCH 3/4] ARM: dts: sun8i: Add Reduced Serial Bus controller device node to A23 dtsi Chen-Yu Tsai
2015-02-24 10:29   ` [PATCH 4/4] ARM: dts: sun8i: ippo-q8h-v5: Enable Reduced Serial Bus controller Chen-Yu Tsai

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