* [RFC 1/4] i2c: core changes for slave support
2014-09-09 14:54 [RFC 0/4] i2c: slave support framework for Linux devices Wolfram Sang
@ 2014-09-09 14:54 ` Wolfram Sang
2014-09-09 14:54 ` [RFC 2/4] i2c: slave: add eeprom simulator driver Wolfram Sang
` (4 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-09 14:54 UTC (permalink / raw)
To: linux-i2c
Cc: Jean Delvare, Magnus Damm, linux-kernel, linux-arm-kernel,
linux-sh
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/i2c-core.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/i2c.h | 22 ++++++++++++++++++++++
2 files changed, 66 insertions(+)
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index 632057a44615..6713d438539b 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -27,6 +27,7 @@
OF support is copyright (c) 2008 Jochen Friedrich <jochen@scram.de>
(based on a previous patch from Jon Smirl <jonsmirl@gmail.com>) and
(c) 2013 Wolfram Sang <wsa@the-dreams.de>
+ I2C slave support (c) 2014 by Wolfram Sang <wsa@sang-engineering.com>
*/
#include <linux/module.h>
@@ -2536,6 +2537,49 @@ trace:
}
EXPORT_SYMBOL(i2c_smbus_xfer);
+int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb)
+{
+ int ret;
+
+ if (!client || !slave_cb)
+ return -EINVAL;
+
+ if (!(client->flags & I2C_CLIENT_TEN)) {
+ /* Enforce stricter address checking */
+ ret = i2c_check_addr_validity(client->addr);
+ if (ret)
+ return ret;
+ }
+
+ if (!client->adapter->algo->reg_slave)
+ return -EOPNOTSUPP;
+
+ client->slave_cb = slave_cb;
+
+ ret = client->adapter->algo->reg_slave(client);
+ if (ret)
+ client->slave_cb = NULL;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i2c_slave_register);
+
+int i2c_slave_unregister(struct i2c_client *client)
+{
+ int ret;
+
+ if (!client->adapter->algo->unreg_slave)
+ return -EOPNOTSUPP;
+
+ ret = client->adapter->algo->unreg_slave(client);
+
+ if (ret == 0)
+ client->slave_cb = NULL;
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(i2c_slave_unregister);
+
MODULE_AUTHOR("Simon G. Vogl <simon@tk.uni-linz.ac.at>");
MODULE_DESCRIPTION("I2C-Bus main module");
MODULE_LICENSE("GPL");
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index a95efeb53a8b..11d4edb1ab3e 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -121,6 +121,24 @@ extern s32 i2c_smbus_write_i2c_block_data(const struct i2c_client *client,
const u8 *values);
#endif /* I2C */
+/* I2C slave support */
+
+enum i2c_slave_event {
+ I2C_SLAVE_REQ_READ_START,
+ I2C_SLAVE_REQ_READ_END,
+ I2C_SLAVE_REQ_WRITE_START,
+ I2C_SLAVE_REQ_WRITE_END,
+ I2C_SLAVE_STOP,
+};
+
+typedef int (*i2c_slave_cb_t)(struct i2c_client *, enum i2c_slave_event, u8 *);
+
+extern int i2c_slave_register(struct i2c_client *client, i2c_slave_cb_t slave_cb);
+extern int i2c_slave_unregister(struct i2c_client *client);
+
+#define i2c_slave_event(__client, __event, __value) \
+ __client->slave_cb(__client, __event, __value)
+
/**
* struct i2c_driver - represent an I2C device driver
* @class: What kind of i2c device we instantiate (for detect)
@@ -224,6 +242,7 @@ struct i2c_client {
struct device dev; /* the device structure */
int irq; /* irq issued by device */
struct list_head detected;
+ i2c_slave_cb_t slave_cb; /* callback for slave mode */
};
#define to_i2c_client(d) container_of(d, struct i2c_client, dev)
@@ -377,6 +396,9 @@ struct i2c_algorithm {
/* To determine what the adapter supports */
u32 (*functionality) (struct i2c_adapter *);
+
+ int (*reg_slave)(struct i2c_client *client);
+ int (*unreg_slave)(struct i2c_client *client);
};
/**
--
2.0.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC 2/4] i2c: slave: add eeprom simulator driver
2014-09-09 14:54 [RFC 0/4] i2c: slave support framework for Linux devices Wolfram Sang
2014-09-09 14:54 ` [RFC 1/4] i2c: core changes for slave support Wolfram Sang
@ 2014-09-09 14:54 ` Wolfram Sang
2014-09-09 16:34 ` Wolfram Sang
2014-09-09 14:54 ` [RFC 3/4] i2c: rcar: add slave support Wolfram Sang
` (3 subsequent siblings)
5 siblings, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2014-09-09 14:54 UTC (permalink / raw)
To: linux-i2c
Cc: Jean Delvare, Magnus Damm, linux-kernel, linux-arm-kernel,
linux-sh
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/Kconfig | 10 +++
drivers/i2c/Makefile | 1 +
drivers/i2c/i2c-slave-eeprom.c | 152 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 163 insertions(+)
create mode 100644 drivers/i2c/i2c-slave-eeprom.c
diff --git a/drivers/i2c/Kconfig b/drivers/i2c/Kconfig
index b51a402752c4..f7800104b2ff 100644
--- a/drivers/i2c/Kconfig
+++ b/drivers/i2c/Kconfig
@@ -110,6 +110,16 @@ config I2C_STUB
If you don't know what to do here, definitely say N.
+config I2C_SLAVE
+ bool "I2C slave support"
+
+if I2C_SLAVE
+
+config I2C_SLAVE_EEPROM
+ bool "I2C eeprom slave driver"
+
+endif
+
config I2C_DEBUG_CORE
bool "I2C Core debugging messages"
help
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index e0228b228256..d78f81caad8a 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_I2C_CHARDEV) += i2c-dev.o
obj-$(CONFIG_I2C_MUX) += i2c-mux.o
obj-y += algos/ busses/ muxes/
obj-$(CONFIG_I2C_STUB) += i2c-stub.o
+obj-$(CONFIG_I2C_SLAVE_EEPROM) += i2c-slave-eeprom.o
ccflags-$(CONFIG_I2C_DEBUG_CORE) := -DDEBUG
CFLAGS_i2c-core.o := -Wno-deprecated-declarations
diff --git a/drivers/i2c/i2c-slave-eeprom.c b/drivers/i2c/i2c-slave-eeprom.c
new file mode 100644
index 000000000000..e512a40a3c36
--- /dev/null
+++ b/drivers/i2c/i2c-slave-eeprom.c
@@ -0,0 +1,152 @@
+/*
+ * I2C Slave mode EEPROM simulator
+ *
+ * Copyright (C) 2014 by Wolfram Sang, Sang Engineering <wsa@sang-engineering.com>
+ * Copyright (C) 2014 by Renesas Solutions Corp.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; version 2 of the License.
+ */
+
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+
+//TODO: simulate 24c32 as well
+#define EEPROM_SIZE 256
+
+struct at24s_data {
+ //TODO: use lock
+ struct mutex lock;
+ struct bin_attribute bin;
+ u8 buffer[EEPROM_SIZE];
+ u8 eeprom_ptr;
+ bool first_write;
+};
+
+static int at24s_slave_cb(struct i2c_client *client, enum i2c_slave_event event, u8 *val)
+{
+ struct at24s_data *at24s = i2c_get_clientdata(client);
+
+ switch (event) {
+ case I2C_SLAVE_REQ_WRITE_END:
+ if (at24s->first_write) {
+ at24s->eeprom_ptr = *val;
+ at24s->first_write = false;
+ } else {
+ at24s->buffer[at24s->eeprom_ptr++] = *val;
+ }
+ break;
+
+ case I2C_SLAVE_REQ_READ_START:
+ *val = at24s->buffer[at24s->eeprom_ptr];
+ break;
+
+ case I2C_SLAVE_REQ_READ_END:
+ at24s->eeprom_ptr++;
+ break;
+
+ case I2C_SLAVE_STOP:
+ at24s->first_write = true;
+ break;
+
+ default:
+ break;
+ }
+
+ return 0;
+}
+
+static ssize_t at24s_bin_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off, size_t count)
+{
+ struct at24s_data *at24s;
+
+ at24s = dev_get_drvdata(container_of(kobj, struct device, kobj));
+
+ //TODO: boundary checks
+ memcpy(buf, &at24s->buffer[off], count);
+
+ return count;
+}
+
+static ssize_t at24s_bin_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr, char *buf, loff_t off, size_t count)
+{
+ struct at24s_data *at24s;
+
+ if (unlikely(off >= attr->size))
+ return -EFBIG;
+
+ at24s = dev_get_drvdata(container_of(kobj, struct device, kobj));
+
+ //TODO: boundary checks
+ memcpy(&at24s->buffer[off], buf, count);
+
+ return count;
+}
+
+static int i2c_slave_eeprom_probe(struct i2c_client *client, const struct i2c_device_id *id)
+{
+ struct at24s_data *at24s;
+ int ret;
+
+ at24s = devm_kzalloc(&client->dev, sizeof(struct at24s_data), GFP_KERNEL);
+ if (!at24s)
+ return -ENOMEM;
+
+ at24s->first_write = true;
+ i2c_set_clientdata(client, at24s);
+
+ ret = i2c_slave_register(client, at24s_slave_cb);
+ if (ret)
+ return ret;
+
+ sysfs_bin_attr_init(&at24s->bin);
+ at24s->bin.attr.name = "slave-eeprom";
+ at24s->bin.attr.mode = S_IRUSR | S_IWUSR;
+ at24s->bin.read = at24s_bin_read;
+ at24s->bin.write = at24s_bin_write;
+ at24s->bin.size = EEPROM_SIZE;
+
+ ret = sysfs_create_bin_file(&client->dev.kobj, &at24s->bin);
+ if (ret)
+ return ret;
+
+ return 0;
+};
+
+static int i2c_slave_eeprom_remove(struct i2c_client *client)
+{
+ struct at24s_data *at24s;
+
+ at24s = i2c_get_clientdata(client);
+ sysfs_remove_bin_file(&client->dev.kobj, &at24s->bin);
+
+ return i2c_slave_unregister(client);
+}
+
+static const struct i2c_device_id i2c_slave_eeprom_id[] = {
+ { "slave-24c02", 0 },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, i2c_slave_eeprom_id);
+
+static struct i2c_driver i2c_slave_eeprom_driver = {
+ .driver = {
+ .name = "i2c-slave-eeprom",
+ .owner = THIS_MODULE,
+ },
+ .probe = i2c_slave_eeprom_probe,
+ .remove = i2c_slave_eeprom_remove,
+ .id_table = i2c_slave_eeprom_id,
+};
+module_i2c_driver(i2c_slave_eeprom_driver);
+
+MODULE_AUTHOR("Wolfram Sang <wsa@sang-engineering.com>");
+MODULE_DESCRIPTION("I2C slave mode EEPROM simulator");
+MODULE_LICENSE("GPL v2");
--
2.0.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC 3/4] i2c: rcar: add slave support
2014-09-09 14:54 [RFC 0/4] i2c: slave support framework for Linux devices Wolfram Sang
2014-09-09 14:54 ` [RFC 1/4] i2c: core changes for slave support Wolfram Sang
2014-09-09 14:54 ` [RFC 2/4] i2c: slave: add eeprom simulator driver Wolfram Sang
@ 2014-09-09 14:54 ` Wolfram Sang
2014-09-09 14:54 ` [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C " Wolfram Sang
` (2 subsequent siblings)
5 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-09 14:54 UTC (permalink / raw)
To: linux-i2c
Cc: Jean Delvare, Magnus Damm, linux-kernel, linux-arm-kernel,
linux-sh
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
drivers/i2c/busses/i2c-rcar.c | 122 ++++++++++++++++++++++++++++++++++++++----
1 file changed, 113 insertions(+), 9 deletions(-)
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index 1cc146cfc1f3..fe875d994441 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -48,6 +48,12 @@
#define ICMAR 0x20 /* master address */
#define ICRXTX 0x24 /* data port */
+/* ICSCR */
+#define SDBS (1 << 3) /* slave data buffer select */
+#define SIE (1 << 2) /* slave interface enable */
+#define GCAE (1 << 1) /* general call address enable */
+#define FNA (1 << 0) /* forced non acknowledgement */
+
/* ICMCR */
#define MDBS (1 << 7) /* non-fifo mode switch */
#define FSCL (1 << 6) /* override SCL pin */
@@ -58,6 +64,15 @@
#define FSB (1 << 1) /* force stop bit */
#define ESG (1 << 0) /* en startbit gen */
+/* ICSSR (also for ICSIER) */
+#define GCAR (1 << 6) /* general call received */
+#define STM (1 << 5) /* slave transmit mode */
+#define SSR (1 << 4) /* stop received */
+#define SDE (1 << 3) /* slave data empty */
+#define SDT (1 << 2) /* slave data transmitted */
+#define SDR (1 << 1) /* slave data received */
+#define SAR (1 << 0) /* slave addr received */
+
/* ICMSR (also for ICMIE) */
#define MNR (1 << 6) /* nack received */
#define MAL (1 << 5) /* arbitration lost */
@@ -103,6 +118,7 @@ struct rcar_i2c_priv {
u32 icccr;
u32 flags;
enum rcar_i2c_type devtype;
+ struct i2c_client *slave;
};
#define rcar_i2c_priv_to_dev(p) ((p)->adap.dev.parent)
@@ -126,15 +142,6 @@ static u32 rcar_i2c_read(struct rcar_i2c_priv *priv, int reg)
static void rcar_i2c_init(struct rcar_i2c_priv *priv)
{
- /*
- * reset slave mode.
- * slave mode is not used on this driver
- */
- rcar_i2c_write(priv, ICSIER, 0);
- rcar_i2c_write(priv, ICSAR, 0);
- rcar_i2c_write(priv, ICSCR, 0);
- rcar_i2c_write(priv, ICSSR, 0);
-
/* reset master mode */
rcar_i2c_write(priv, ICMIER, 0);
rcar_i2c_write(priv, ICMCR, 0);
@@ -362,6 +369,63 @@ static int rcar_i2c_irq_recv(struct rcar_i2c_priv *priv, u32 msr)
return 0;
}
+static bool rcar_i2c_slave_irq(struct rcar_i2c_priv *priv)
+{
+ u32 ssr_raw, ssr_filtered;
+ u8 value;
+
+ ssr_raw = rcar_i2c_read(priv, ICSSR) & 0xff;
+ ssr_filtered = ssr_raw & rcar_i2c_read(priv, ICSIER);
+
+ if (!ssr_filtered)
+ return false;
+
+ /* address detected */
+ if (ssr_filtered & SAR) {
+ /* read or write request */
+ if (ssr_raw & STM) {
+ i2c_slave_event(priv->slave, I2C_SLAVE_REQ_READ_START, &value);
+ rcar_i2c_write(priv, ICRXTX, value);
+ rcar_i2c_write(priv, ICSIER, SDE | SSR | SAR);
+ } else {
+ i2c_slave_event(priv->slave, I2C_SLAVE_REQ_WRITE_START, NULL);
+ rcar_i2c_read(priv, ICRXTX); /* dummy read */
+ rcar_i2c_write(priv, ICSIER, SDR | SSR | SAR);
+ }
+
+ rcar_i2c_write(priv, ICSSR, ~SAR & 0xff);
+ }
+
+ /* master sent stop */
+ if (ssr_filtered & SSR) {
+ i2c_slave_event(priv->slave, I2C_SLAVE_STOP, NULL);
+ rcar_i2c_write(priv, ICSIER, SAR | SSR);
+ rcar_i2c_write(priv, ICSSR, ~SSR & 0xff);
+ }
+
+ /* master wants to write to us */
+ if (ssr_filtered & SDR) {
+ int ret;
+
+ value = rcar_i2c_read(priv, ICRXTX);
+ ret = i2c_slave_event(priv->slave, I2C_SLAVE_REQ_WRITE_END, &value);
+ /* Send NACK in case of error */
+ rcar_i2c_write(priv, ICSCR, SIE | SDBS | (ret < 0 ? FNA : 0));
+ i2c_slave_event(priv->slave, I2C_SLAVE_REQ_WRITE_START, NULL);
+ rcar_i2c_write(priv, ICSSR, ~SDR & 0xff);
+ }
+
+ /* master wants to read from us */
+ if (ssr_filtered & SDE) {
+ i2c_slave_event(priv->slave, I2C_SLAVE_REQ_READ_END, NULL);
+ i2c_slave_event(priv->slave, I2C_SLAVE_REQ_READ_START, &value);
+ rcar_i2c_write(priv, ICRXTX, value);
+ rcar_i2c_write(priv, ICSSR, ~SDE & 0xff);
+ }
+
+ return true;
+}
+
static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
{
struct rcar_i2c_priv *priv = ptr;
@@ -370,6 +434,9 @@ static irqreturn_t rcar_i2c_irq(int irq, void *ptr)
/*-------------- spin lock -----------------*/
spin_lock(&priv->lock);
+ if (rcar_i2c_slave_irq(priv))
+ goto unlock_out;
+
msr = rcar_i2c_read(priv, ICMSR);
/* Only handle interrupts that are currently enabled */
@@ -408,6 +475,7 @@ out:
wake_up(&priv->wait);
}
+unlock_out:
spin_unlock(&priv->lock);
/*-------------- spin unlock -----------------*/
@@ -498,6 +566,40 @@ out:
return ret;
}
+
+static int rcar_reg_slave(struct i2c_client *slave)
+{
+ struct rcar_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
+
+ if (priv->slave)
+ return -EBUSY;
+
+ if (slave->flags & I2C_CLIENT_TEN)
+ return -EAFNOSUPPORT;
+
+ priv->slave = slave;
+ rcar_i2c_write(priv, ICSAR, slave->addr);
+ rcar_i2c_write(priv, ICSSR, 0);
+ rcar_i2c_write(priv, ICSIER, SAR | SSR);
+ rcar_i2c_write(priv, ICSCR, SIE | SDBS);
+
+ return 0;
+}
+
+static int rcar_unreg_slave(struct i2c_client *slave)
+{
+ struct rcar_i2c_priv *priv = i2c_get_adapdata(slave->adapter);
+
+ WARN_ON(!priv->slave);
+
+ rcar_i2c_write(priv, ICSIER, 0);
+ rcar_i2c_write(priv, ICSCR, 0);
+
+ priv->slave = NULL;
+
+ return 0;
+}
+
static u32 rcar_i2c_func(struct i2c_adapter *adap)
{
/* This HW can't do SMBUS_QUICK and NOSTART */
@@ -507,6 +609,8 @@ static u32 rcar_i2c_func(struct i2c_adapter *adap)
static const struct i2c_algorithm rcar_i2c_algo = {
.master_xfer = rcar_i2c_master_xfer,
.functionality = rcar_i2c_func,
+ .reg_slave = rcar_reg_slave,
+ .unreg_slave = rcar_unreg_slave,
};
static const struct of_device_id rcar_i2c_dt_ids[] = {
--
2.0.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-09 14:54 [RFC 0/4] i2c: slave support framework for Linux devices Wolfram Sang
` (2 preceding siblings ...)
2014-09-09 14:54 ` [RFC 3/4] i2c: rcar: add slave support Wolfram Sang
@ 2014-09-09 14:54 ` Wolfram Sang
2014-09-11 12:17 ` Marc Dietrich
2014-09-11 12:06 ` [RFC 0/4] i2c: slave support framework for Linux devices Marc Dietrich
[not found] ` <1410274470-12712-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
5 siblings, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2014-09-09 14:54 UTC (permalink / raw)
To: linux-i2c
Cc: linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Not for upstream!
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
arch/arm/boot/dts/r8a7790-lager.dts | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/arch/arm/boot/dts/r8a7790-lager.dts b/arch/arm/boot/dts/r8a7790-lager.dts
index 856b4236b674..12aa2f21e6fa 100644
--- a/arch/arm/boot/dts/r8a7790-lager.dts
+++ b/arch/arm/boot/dts/r8a7790-lager.dts
@@ -205,9 +205,9 @@
renesas,function = "msiof1";
};
- iic1_pins: iic1 {
- renesas,groups = "iic1";
- renesas,function = "iic1";
+ i2c1_pins: i2c1 {
+ renesas,groups = "i2c1";
+ renesas,function = "i2c1";
};
iic2_pins: iic2 {
@@ -356,10 +356,15 @@
status = "ok";
};
-&iic1 {
+&i2c1 {
status = "ok";
- pinctrl-0 = <&iic1_pins>;
+ pinctrl-0 = <&i2c1_pins>;
pinctrl-names = "default";
+
+ eeprom@64 {
+ compatible = "slave-24c02";
+ reg = <0x64>;
+ };
};
&iic2 {
--
2.0.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-09 14:54 ` [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C " Wolfram Sang
@ 2014-09-11 12:17 ` Marc Dietrich
2014-09-11 14:12 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Marc Dietrich @ 2014-09-11 12:17 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
cc'ing: devicetree
Am Dienstag, 9. September 2014, 16:54:30 schrieb Wolfram Sang:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Not for upstream!
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> arch/arm/boot/dts/r8a7790-lager.dts | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/boot/dts/r8a7790-lager.dts
> b/arch/arm/boot/dts/r8a7790-lager.dts index 856b4236b674..12aa2f21e6fa
> 100644
> --- a/arch/arm/boot/dts/r8a7790-lager.dts
> +++ b/arch/arm/boot/dts/r8a7790-lager.dts
> @@ -205,9 +205,9 @@
> renesas,function = "msiof1";
> };
>
> - iic1_pins: iic1 {
> - renesas,groups = "iic1";
> - renesas,function = "iic1";
> + i2c1_pins: i2c1 {
> + renesas,groups = "i2c1";
> + renesas,function = "i2c1";
> };
>
> iic2_pins: iic2 {
> @@ -356,10 +356,15 @@
> status = "ok";
> };
>
> -&iic1 {
> +&i2c1 {
> status = "ok";
> - pinctrl-0 = <&iic1_pins>;
> + pinctrl-0 = <&i2c1_pins>;
> pinctrl-names = "default";
> +
> + eeprom@64 {
> + compatible = "slave-24c02";
a "real" compatible value should have vendor,device syntax.
> + reg = <0x64>;
we had some discussions in the past how to represent i2c master devices in
device tree. The outcome was to use to a special representation to distinguish
them from slave devices, e.g. something like reg = <0x1 0x64>, where the 0x1
would mean "master" device (0x0 -> slave). If nothing is added, then it's also
slave. The adapter address cell would also need to be changed if this
extension is used.
An alternative would be to use a special property (e.g. slave address), but
that would encode the same value twice (or even three times).
Marc
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 12:17 ` Marc Dietrich
@ 2014-09-11 14:12 ` Wolfram Sang
2014-09-11 14:40 ` Wolfram Sang
2014-09-11 14:49 ` Marc Dietrich
0 siblings, 2 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-11 14:12 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 2343 bytes --]
On Thu, Sep 11, 2014 at 02:17:16PM +0200, Marc Dietrich wrote:
> cc'ing: devicetree
Thanks, I forgot that!
> > -&iic1 {
> > +&i2c1 {
> > status = "ok";
> > - pinctrl-0 = <&iic1_pins>;
> > + pinctrl-0 = <&i2c1_pins>;
> > pinctrl-names = "default";
> > +
> > + eeprom@64 {
> > + compatible = "slave-24c02";
>
> a "real" compatible value should have vendor,device syntax.
I know. This is maybe a grey line between "configuration" and "hardware
description". If we want an I2C slave, we need to things:
a) HW support in the I2C master driver
b) SW support by giving the slave callback some functionality (like my
eeprom simulator)
a) should be compiled in with the master driver and does not need any DT
binding, so this is easy.
b) could be seen as a configuration thing since the functionality
backend could be changed at runtime even. Think of being a usb-gadget
here. However, since I2C is multi-master and not hot-pluggable, some
other master might be depending on what I2C slave is actually used.
So, it could be seen as hardware description, too? Not entirely sure.
The drawback you also noticed is that there is no vendor for these
functionalities. Maybe "wsa" since I did the functionality driver :)
To be honest, the above DT snipplet was done to get easily started. If
DT maintainers think this is more configuration, not much is lost. The
only difference is that the userspace instantiation method should be
used then:
# echo slave-24c02 0x64 > /sys/bus/i2c/devices/i2c-0/new_device
>
> > + reg = <0x64>;
>
> we had some discussions in the past how to represent i2c master devices in
> device tree. The outcome was to use to a special representation to distinguish
> them from slave devices, e.g. something like reg = <0x1 0x64>, where the 0x1
> would mean "master" device (0x0 -> slave). If nothing is added, then it's also
> slave. The adapter address cell would also need to be changed if this
> extension is used.
Please don't change the reg-layout which is around since forever.
It will break all existing devicetrees. NAK.
> An alternative would be to use a special property (e.g. slave address), but
> that would encode the same value twice (or even three times).
As said, if all breaks we handle it at runtime.
Thanks,
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 14:12 ` Wolfram Sang
@ 2014-09-11 14:40 ` Wolfram Sang
2014-09-11 14:52 ` Marc Dietrich
2014-09-11 14:49 ` Marc Dietrich
1 sibling, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2014-09-11 14:40 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 410 bytes --]
> b) could be seen as a configuration thing since the functionality
> backend could be changed at runtime even.
Come to think of it, not only the functionality, also the address can be
changed at runtime. This makes me think it should really not be in DT
after all.
People will probably find out that it is possible to put it in DT, but
it should not be encouraged and they should do it on their own risk.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 14:40 ` Wolfram Sang
@ 2014-09-11 14:52 ` Marc Dietrich
2014-09-11 14:54 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Marc Dietrich @ 2014-09-11 14:52 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 749 bytes --]
Am Donnerstag, 11. September 2014, 16:40:04 schrieb Wolfram Sang:
> > b) could be seen as a configuration thing since the functionality
> > backend could be changed at runtime even.
>
> Come to think of it, not only the functionality, also the address can be
> changed at runtime. This makes me think it should really not be in DT
> after all.
even worse, there can be multiple masters and slaves changing their role on
the fly AFAIK. So the best dt can do is to provide an initial configuration,
so all drivers know where they are and where to start. Everything else can be
changed during runtime.
> People will probably find out that it is possible to put it in DT, but
> it should not be encouraged and they should do it on their own risk.
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 14:52 ` Marc Dietrich
@ 2014-09-11 14:54 ` Wolfram Sang
2014-09-12 7:51 ` Marc Dietrich
0 siblings, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2014-09-11 14:54 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 751 bytes --]
On Thu, Sep 11, 2014 at 04:52:22PM +0200, Marc Dietrich wrote:
> Am Donnerstag, 11. September 2014, 16:40:04 schrieb Wolfram Sang:
> > > b) could be seen as a configuration thing since the functionality
> > > backend could be changed at runtime even.
> >
> > Come to think of it, not only the functionality, also the address can be
> > changed at runtime. This makes me think it should really not be in DT
> > after all.
>
> even worse, there can be multiple masters and slaves changing their role on
> the fly AFAIK. So the best dt can do is to provide an initial configuration,
> so all drivers know where they are and where to start. Everything else can be
> changed during runtime.
Why do you want DT to be involved at all?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 14:54 ` Wolfram Sang
@ 2014-09-12 7:51 ` Marc Dietrich
2014-09-12 8:08 ` Geert Uytterhoeven
2014-09-12 8:33 ` Wolfram Sang
0 siblings, 2 replies; 24+ messages in thread
From: Marc Dietrich @ 2014-09-12 7:51 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 1494 bytes --]
Am Donnerstag, 11. September 2014, 16:54:22 schrieb Wolfram Sang:
> On Thu, Sep 11, 2014 at 04:52:22PM +0200, Marc Dietrich wrote:
> > Am Donnerstag, 11. September 2014, 16:40:04 schrieb Wolfram Sang:
> > > > b) could be seen as a configuration thing since the functionality
> > > > backend could be changed at runtime even.
> > >
> > > Come to think of it, not only the functionality, also the address can be
> > > changed at runtime. This makes me think it should really not be in DT
> > > after all.
> >
> > even worse, there can be multiple masters and slaves changing their role
> > on
> > the fly AFAIK. So the best dt can do is to provide an initial
> > configuration, so all drivers know where they are and where to start.
> > Everything else can be changed during runtime.
>
> Why do you want DT to be involved at all?
Imagine a device which supports both, slave or master mode. The driver needs
to know in which mode it should operate. This cannot be hard coded, because on
different boards, different modes can be used.
The point is, that if we define a dt binding for master device on slave
adapters it will be there forever. So even if it makes no sense for the
example eeprom simulator (or even our embedded controller), it may make sense
for other or future devices.
On the other hand, we had some painful discussion about that issue in the
past, so if no one else steps up to propose a good alternative binding, I will
be the last to criticize your approach.
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 7:51 ` Marc Dietrich
@ 2014-09-12 8:08 ` Geert Uytterhoeven
2014-09-12 8:33 ` Wolfram Sang
1 sibling, 0 replies; 24+ messages in thread
From: Geert Uytterhoeven @ 2014-09-12 8:08 UTC (permalink / raw)
To: Marc Dietrich
Cc: Wolfram Sang, Linux I2C, Linux-sh list,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Jean Delvare, Magnus Damm,
Andrey Danin, devicetree@vger.kernel.org, Stephen Warren
Hi Marc,
On Fri, Sep 12, 2014 at 9:51 AM, Marc Dietrich <marvin24@gmx.de> wrote:
> Am Donnerstag, 11. September 2014, 16:54:22 schrieb Wolfram Sang:
>> On Thu, Sep 11, 2014 at 04:52:22PM +0200, Marc Dietrich wrote:
>> > Am Donnerstag, 11. September 2014, 16:40:04 schrieb Wolfram Sang:
>> > > > b) could be seen as a configuration thing since the functionality
>> > > > backend could be changed at runtime even.
>> > >
>> > > Come to think of it, not only the functionality, also the address can be
>> > > changed at runtime. This makes me think it should really not be in DT
>> > > after all.
>> >
>> > even worse, there can be multiple masters and slaves changing their role
>> > on
>> > the fly AFAIK. So the best dt can do is to provide an initial
>> > configuration, so all drivers know where they are and where to start.
>> > Everything else can be changed during runtime.
>>
>> Why do you want DT to be involved at all?
>
> Imagine a device which supports both, slave or master mode. The driver needs
> to know in which mode it should operate. This cannot be hard coded, because on
> different boards, different modes can be used.
>
> The point is, that if we define a dt binding for master device on slave
> adapters it will be there forever. So even if it makes no sense for the
> example eeprom simulator (or even our embedded controller), it may make sense
> for other or future devices.
DT describes the hardware. The most you can do is describe an i2c
adapter, and an i2c bus with (physical) i2c slaves, if any, and (other)
i2c masters[*], if any.
If the description contains i2c slaves, the i2c adapter can operate in
(but may be not limited to) master mode.
I2c slaves implemented in Linux are not physical i2c slaves, but just
a piece of software. They can appear/disappear/change at any time.
Hence I think this is something to be configured by userspace, like
"tie the i2c-eeprom-slave driver to address 64 on i2c bus 3".
And then you can have the mixed case, where the i2c adapter is both
a master, talking to the physical slaves, and a slave, being talked to by
another master. But that's just a superposition of the two cases above,
right?
[*] Are there bindings for other masters on an i2c bus?
Just my 0.02€ (as long as those coins still exist).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 7:51 ` Marc Dietrich
2014-09-12 8:08 ` Geert Uytterhoeven
@ 2014-09-12 8:33 ` Wolfram Sang
2014-09-12 9:06 ` Marc Dietrich
1 sibling, 1 reply; 24+ messages in thread
From: Wolfram Sang @ 2014-09-12 8:33 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, linux-sh-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree-u79uwXL29TY76Z2rM5mHXA,
Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]
> > Why do you want DT to be involved at all?
>
> Imagine a device which supports both, slave or master mode. The driver needs
> to know in which mode it should operate. This cannot be hard coded, because on
> different boards, different modes can be used.
Okay, it sounds weird to me that a device is not able to switch between
master and slave, but if you say so. Also, solving this issue would also
handle potential weird IP blocks which can be slave only, right?
What if you use two different adapter drivers or compatibles? One for
master-mode, one for slave-mode (slave could leave algo->master_xfer
empty, so the slave mode driver cannot send packets). I'm brainstorming
here, so while it should work IMO I will probably need a second thought.
So, in the DT you would have a block registering the I2C slave core
which binds to a simple driver providing reg_slave/unreg_slave and
pass on the slave-event. Then you could instantiate slave clients as
said before.
Maybe we need a real world example?
> The point is, that if we define a dt binding for master device on slave
> adapters it will be there forever. So even if it makes no sense for the
> example eeprom simulator (or even our embedded controller), it may make sense
> for other or future devices.
I don't know what you mean here. Again, an example might help?
Thanks,
Wolfram
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 8:33 ` Wolfram Sang
@ 2014-09-12 9:06 ` Marc Dietrich
2014-09-12 9:58 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Marc Dietrich @ 2014-09-12 9:06 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 2785 bytes --]
Am Freitag, 12. September 2014, 10:33:48 schrieb Wolfram Sang:
> > > Why do you want DT to be involved at all?
> >
> > Imagine a device which supports both, slave or master mode. The driver
> > needs to know in which mode it should operate. This cannot be hard coded,
> > because on different boards, different modes can be used.
>
> Okay, it sounds weird to me that a device is not able to switch between
> master and slave, but if you say so. Also, solving this issue would also
> handle potential weird IP blocks which can be slave only, right?
I didn't said device. I meant the driver (software) needs to know if which
mode to operate the device. DT has to provide just enough information to
select the right mode depending on the board implementation.
> What if you use two different adapter drivers or compatibles? One for
> master-mode, one for slave-mode (slave could leave algo->master_xfer
> empty, so the slave mode driver cannot send packets). I'm brainstorming
> here, so while it should work IMO I will probably need a second thought.
>
> So, in the DT you would have a block registering the I2C slave core
> which binds to a simple driver providing reg_slave/unreg_slave and
> pass on the slave-event. Then you could instantiate slave clients as
> said before.
>
> Maybe we need a real world example?
ok, take our embedded controller driver (in staging/nvec) as an example. It's
basicly an MFD connecting keyboard, mouse, power, gpio, and some other stuff
to the soc. The MFD operates in master mode while the SOC is the I2C slave.
Theoretically, these roles could also switch (but that's not defined in the
nvec protocol).
So the i2c client driver sits in between the adapter (in slave or master mode)
and the the MFD children (keyboard, mouse, ...) and provides read/write/cb
functions (nvec uses a side channel gpio to be able to initiate writes on its
own) to its children. The MFD children don't care about how the i2c
communication is done, but the client driver needs to decide in which mode it
should talk to the EC. If this is not autodetectable, the mode needs to be
provided by the device tree.
In this case you cannot use separate drivers for master and slave mode (or you
would need another layer to hide the driver multiplexing).
> > The point is, that if we define a dt binding for master device on slave
> > adapters it will be there forever. So even if it makes no sense for the
> > example eeprom simulator (or even our embedded controller), it may make
> > sense for other or future devices.
>
> I don't know what you mean here. Again, an example might help?
I just wanted to say that the discussion here should find a *generic* way to
distinguish slave and master clients on the i2c bus by using a proper DT
binding.
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 9:06 ` Marc Dietrich
@ 2014-09-12 9:58 ` Wolfram Sang
2014-09-12 10:10 ` Geert Uytterhoeven
2014-09-12 11:42 ` Marc Dietrich
0 siblings, 2 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-12 9:58 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 2024 bytes --]
> ok, take our embedded controller driver (in staging/nvec) as an example. It's
> basicly an MFD connecting keyboard, mouse, power, gpio, and some other stuff
> to the soc. The MFD operates in master mode while the SOC is the I2C slave.
> Theoretically, these roles could also switch (but that's not defined in the
> nvec protocol).
I see these cases currently:
1) my current case
The I2C slave is not needed for board bringup, mainly for development or
playing around. It can have this or that functionality on this or that
address. -> does not belong into DT, should be done in userspace
2) Slave mode is needed for board bringup
Some other components need a specific I2C slave to be present before
userspace is available, otherwise the system is unusable. This is IMO
then a hardware description and justifies DT entries:
DT pseudocode:
i2c {
compatible = "nvidia, tegra-i2c";
ec-slave@42 {
compatible = "nvidia, ax100-ec-slave";
reg = <0x42>;
};
};
Of course, an MFD driver providing "nvidia, ax100-ec-slave" is needed
which uses the I2C slave mode of the tegra controller.
3) Master + slave mode is needed for board bringup:
Again, IMO a hardware description, so we could use:
i2c {
compatible = "nvidia, tegra-i2c";
ec@64 {
compatible = "nvidia, ax100-ec";
reg = <0x64>;
};
};
This is a standard I2C device driver (using the MFD framework) where
i2c-tegra would act as a master on the client for 0x64. However, its
probe function can fill an i2c_board_device (the driver should know the
slave device address because of the protocol), get a new client using
i2c_new_device, and register that as a I2C slave client. It then has an
address where it listens and an address where it can send to. When to do
what is protocol implementation.
Am I missing something? Board properties can be encoded within the
compatible entries ("ax100-ec", "ax200-ec"...). I'd think this means
mostly different protocols, though.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 9:58 ` Wolfram Sang
@ 2014-09-12 10:10 ` Geert Uytterhoeven
2014-09-12 10:26 ` Wolfram Sang
2014-09-12 11:42 ` Marc Dietrich
1 sibling, 1 reply; 24+ messages in thread
From: Geert Uytterhoeven @ 2014-09-12 10:10 UTC (permalink / raw)
To: Wolfram Sang
Cc: Marc Dietrich, Linux I2C, Linux-sh list,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Jean Delvare, Magnus Damm,
Andrey Danin, devicetree@vger.kernel.org, Stephen Warren
Hi Wolfram,
On Fri, Sep 12, 2014 at 11:58 AM, Wolfram Sang <wsa@the-dreams.de> wrote:
> 2) Slave mode is needed for board bringup
>
> Some other components need a specific I2C slave to be present before
> userspace is available, otherwise the system is unusable. This is IMO
> then a hardware description and justifies DT entries:
OK, I didn't think this case, which is similar to an external device requiring
a GPIO to be at a specific level.
> DT pseudocode:
>
> i2c {
> compatible = "nvidia, tegra-i2c";
>
> ec-slave@42 {
> compatible = "nvidia, ax100-ec-slave";
> reg = <0x42>;
> };
> };
So distinguishing between drivers providing slave services (using slave
mode) and drivers driving actual slave hardware (using master mode) is
done based on the compatible-property.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 10:10 ` Geert Uytterhoeven
@ 2014-09-12 10:26 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-12 10:26 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Marc Dietrich, Linux I2C, Linux-sh list,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Jean Delvare, Magnus Damm,
Andrey Danin, devicetree@vger.kernel.org, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 581 bytes --]
> > DT pseudocode:
> >
> > i2c {
> > compatible = "nvidia, tegra-i2c";
> >
> > ec-slave@42 {
> > compatible = "nvidia, ax100-ec-slave";
> > reg = <0x42>;
> > };
> > };
>
> So distinguishing between drivers providing slave services (using slave
> mode) and drivers driving actual slave hardware (using master mode) is
> done based on the compatible-property.
We describe users of one specific I2C bus. And the drivers that go along
with them. No?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 9:58 ` Wolfram Sang
2014-09-12 10:10 ` Geert Uytterhoeven
@ 2014-09-12 11:42 ` Marc Dietrich
2014-09-12 12:15 ` Wolfram Sang
1 sibling, 1 reply; 24+ messages in thread
From: Marc Dietrich @ 2014-09-12 11:42 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 2542 bytes --]
Am Freitag, 12. September 2014, 11:58:21 schrieb Wolfram Sang:
> > ok, take our embedded controller driver (in staging/nvec) as an example.
> > It's basicly an MFD connecting keyboard, mouse, power, gpio, and some
> > other stuff to the soc. The MFD operates in master mode while the SOC is
> > the I2C slave. Theoretically, these roles could also switch (but that's
> > not defined in the nvec protocol).
>
> I see these cases currently:
>
> 1) my current case
>
> The I2C slave is not needed for board bringup, mainly for development or
> playing around. It can have this or that functionality on this or that
> address. -> does not belong into DT, should be done in userspace
>
> 2) Slave mode is needed for board bringup
>
> Some other components need a specific I2C slave to be present before
> userspace is available, otherwise the system is unusable. This is IMO
> then a hardware description and justifies DT entries:
>
> DT pseudocode:
>
> i2c {
> compatible = "nvidia, tegra-i2c";
>
> ec-slave@42 {
> compatible = "nvidia, ax100-ec-slave";
> reg = <0x42>;
> };
> };
>
> Of course, an MFD driver providing "nvidia, ax100-ec-slave" is needed
> which uses the I2C slave mode of the tegra controller.
>
> 3) Master + slave mode is needed for board bringup:
>
> Again, IMO a hardware description, so we could use:
>
> i2c {
> compatible = "nvidia, tegra-i2c";
>
> ec@64 {
> compatible = "nvidia, ax100-ec";
> reg = <0x64>;
> };
> };
>
> This is a standard I2C device driver (using the MFD framework) where
> i2c-tegra would act as a master on the client for 0x64. However, its
> probe function can fill an i2c_board_device (the driver should know the
> slave device address because of the protocol), get a new client using
> i2c_new_device, and register that as a I2C slave client. It then has an
> address where it listens and an address where it can send to. When to do
> what is protocol implementation.
>
> Am I missing something? Board properties can be encoded within the
> compatible entries ("ax100-ec", "ax200-ec"...). I'd think this means
> mostly different protocols, though.
well, ac100 is the name of the board and nvec is the name of the protocol.
Anyway, I'm fine with encoding the mode to use in the compatible property.
Like you said before, a hypothetical driver supporting both modes could also
register two compatibility strings (eg. nvidia,nvec and nvidia,nvec-slave) and
use the mode (and hence the meaning of the reg value) according to this
string.
Thanks,
Marc
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-12 11:42 ` Marc Dietrich
@ 2014-09-12 12:15 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-12 12:15 UTC (permalink / raw)
To: Marc Dietrich
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 456 bytes --]
> > Am I missing something? Board properties can be encoded within the
> > compatible entries ("ax100-ec", "ax200-ec"...). I'd think this means
> > mostly different protocols, though.
>
> well, ac100 is the name of the board and nvec is the name of the protocol.
Yes, the driver could be named nvec, yet the compatible-strings should
be similar to above. From that, the driver can deduce the protocol
variant and possible initial settings.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C slave support
2014-09-11 14:12 ` Wolfram Sang
2014-09-11 14:40 ` Wolfram Sang
@ 2014-09-11 14:49 ` Marc Dietrich
1 sibling, 0 replies; 24+ messages in thread
From: Marc Dietrich @ 2014-09-11 14:49 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin, devicetree, Stephen Warren
[-- Attachment #1: Type: text/plain, Size: 1393 bytes --]
Am Donnerstag, 11. September 2014, 16:12:58 schrieb Wolfram Sang:
> On Thu, Sep 11, 2014 at 02:17:16PM +0200, Marc Dietrich wrote:
> > > + reg = <0x64>;
> >
> > we had some discussions in the past how to represent i2c master devices in
> > device tree. The outcome was to use to a special representation to
> > distinguish them from slave devices, e.g. something like reg = <0x1
> > 0x64>, where the 0x1 would mean "master" device (0x0 -> slave). If
> > nothing is added, then it's also slave. The adapter address cell would
> > also need to be changed if this extension is used.
>
> Please don't change the reg-layout which is around since forever.
> It will break all existing devicetrees. NAK.
it won't break existing device trees. It would only need to be extended for
systems where the adapter uses master and slave modes (or if you add a master
device to the i2c client list). So you need to change the device tree anyway.
All other setups can continue to use a single reg.
Thinking more about it, are there any i2c clients parsing the reg property
directly? In this case I would agree, otherwise, only i2c core needs some
update - right?
Marc
> > An alternative would be to use a special property (e.g. slave address),
> > but
> > that would encode the same value twice (or even three times).
>
> As said, if all breaks we handle it at runtime.
>
> Thanks,
>
> Wolfram
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [RFC 0/4] i2c: slave support framework for Linux devices
2014-09-09 14:54 [RFC 0/4] i2c: slave support framework for Linux devices Wolfram Sang
` (3 preceding siblings ...)
2014-09-09 14:54 ` [RFC 4/4] ARM: shmobile: r8a7790: adapt DTS for I2C " Wolfram Sang
@ 2014-09-11 12:06 ` Marc Dietrich
[not found] ` <1410274470-12712-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
5 siblings, 0 replies; 24+ messages in thread
From: Marc Dietrich @ 2014-09-11 12:06 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c, linux-sh, linux-kernel, linux-arm-kernel, Jean Delvare,
Magnus Damm, Andrey Danin
Hello Wolfram,
Am Dienstag, 9. September 2014, 16:54:26 schrieb Wolfram Sang:
> Finally, here is my take on the often desired feature that Linux can not
> only be an I2C master, but also an I2C slave. Compared to my draft sent out
> last week, this RFC has been tested on hardware (Renesas Lager board) and
> works \o/
first, thanks for taking up this job.
> One big part still missing is documentation, so brave ones need to "use the
> source". However, this approach turned out to be even less intrusive than
> expected, so that is hopefully a good sign.
>
> Basically, an I2C slave is a standard I2C client providing a callback
> function. When registering as a slave, the connection to the I2C adapter is
> made which uses the callback when a slave event happens. That splits the HW
> support (enabling slave mode on the adapter) and SW support (here a generic
> eeprom simulator) nicely IMO.
>From what I can tell, this approach may also work with our driver
(staging/nvec). I especially like the split between the adapter and the client
support.
I only have one question so far regarding the device tree representation
attached patch 4.
Marc
^ permalink raw reply [flat|nested] 24+ messages in thread[parent not found: <1410274470-12712-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>]
* Re: [RFC 0/4] i2c: slave support framework for Linux devices
[not found] ` <1410274470-12712-1-git-send-email-wsa-z923LK4zBo2bacvFa/9K2g@public.gmane.org>
@ 2014-09-12 8:58 ` Uwe Kleine-König
2014-09-12 9:12 ` Wolfram Sang
0 siblings, 1 reply; 24+ messages in thread
From: Uwe Kleine-König @ 2014-09-12 8:58 UTC (permalink / raw)
To: Wolfram Sang
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA, Jean Delvare, Magnus Damm,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sh-u79uwXL29TY76Z2rM5mHXA
Hello Wolfram,
On Tue, Sep 09, 2014 at 04:54:26PM +0200, Wolfram Sang wrote:
> Finally, here is my take on the often desired feature that Linux can not only
> be an I2C master, but also an I2C slave. Compared to my draft sent out last
> week, this RFC has been tested on hardware (Renesas Lager board) and works \o/
>
> One big part still missing is documentation, so brave ones need to "use the
> source". However, this approach turned out to be even less intrusive than
> expected, so that is hopefully a good sign.
Some thoughts from reading through your patch set:
- If I understand correctly you cannot register an i2c slave without
also registering a master, right? I don't think this is troubling in
practice, is it?
For abstraction I would prefer to make these different concepts
though.
- The IMHO most (even only?) useful slave is "i2cslvdev", i.e. a
userspace device. With that you wouldn't need device tree stuff, the
driver would just offer the device if supported. Userspace then could
care about the rest (slave address, functionality, ...).
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [RFC 0/4] i2c: slave support framework for Linux devices
2014-09-12 8:58 ` Uwe Kleine-König
@ 2014-09-12 9:12 ` Wolfram Sang
0 siblings, 0 replies; 24+ messages in thread
From: Wolfram Sang @ 2014-09-12 9:12 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: linux-i2c, Jean Delvare, Magnus Damm, linux-kernel,
linux-arm-kernel, linux-sh
[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]
> - If I understand correctly you cannot register an i2c slave without
> also registering a master, right? I don't think this is troubling in
> practice, is it?
> For abstraction I would prefer to make these different concepts
> though.
You need to register an adapter, yes. It does not need to populate the
callback for sending messages, so it could be slave only. But an adapter
is needed, because it is the glue between a client and the wired lines.
> - The IMHO most (even only?) useful slave is "i2cslvdev", i.e. a
> userspace device. With that you wouldn't need device tree stuff, the
> driver would just offer the device if supported. Userspace then could
> care about the rest (slave address, functionality, ...).
I don't think it is the only useful one. A callback into userspace is
expensive, so stuff like shared memory (my eeprom simulator) or a shared
ringbuffer will come in handy, too. That being said, a slave device like
you described is probably the next user we see using the slave interface
:)
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread