diff mbox

[U-Boot,v6,04/11] dm: i2c: Add a sandbox I2C driver

Message ID 1418226957-7232-5-git-send-email-sjg@chromium.org
State Accepted
Delegated to: Simon Glass
Headers show

Commit Message

Simon Glass Dec. 10, 2014, 3:55 p.m. UTC 
This driver includes some test features such as only supporting certain
bus speeds. It passes its I2C traffic through to an emulator.

Acked-by: Heiko Schocher <hs@denx.de>
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v6:
- Drop a stale comment
- Pass value from i2c_get_chip() to get_emul()
- Use dm_scan_fdt_node() to find emulator

Changes in v5: None
Changes in v4:
- Drop set_offset_len() method

Changes in v3:
- Update for new uclass <=> driver interface

Changes in v2: None

 drivers/i2c/Makefile      |   2 +-
 drivers/i2c/sandbox_i2c.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 drivers/i2c/sandbox_i2c.c

Comments

Masahiro Yamada Dec. 10, 2014, 5:02 p.m. UTC | #1 
Hi Simon,



2014-12-11 0:55 GMT+09:00 Simon Glass <sjg@chromium.org>:
> This driver includes some test features such as only supporting certain
> bus speeds. It passes its I2C traffic through to an emulator.
>
> Acked-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v6:
> - Drop a stale comment
> - Pass value from i2c_get_chip() to get_emul()
> - Use dm_scan_fdt_node() to find emulator



When I run "i2c probe" on a real hardware,
generic-i2c is only added to slave-addresses that have responded.

On sandbox, only 0x2c and 0x59 respond, but generic-i2c is added
to all the slave-addresses.
It looks funny.

Is this a bug?





U-Boot 2015.01-rc3-00010-ge6aa27d-dirty (Dec 11 2014 - 01:53:11)

DRAM:  128 MiB
Using default environment

In:    cros-ec-keyb
Out:   lcd
Err:   lcd
=> i2c dev 0
Setting bus to 0
=> i2c probe
Valid chip addresses: 2C 59
=> dm tree
ROOT 05c60098
- * root_driver @ 05c60098
|-   demo_shape_drv @ 05c60178
|-   demo_simple_drv @ 05c60218
|-   demo_shape_drv @ 05c602b8
|-   demo_simple_drv @ 05c60358
|-   demo_shape_drv @ 05c603f8
|-   test_drv @ 05c604f8
|-   test_drv @ 05c60598
|-   test_drv @ 05c60638
|-   gpio_sandbox @ 05c60718
|-   serial_sandbox @ 05c607f8
|- * serial @ 05c60898
|-   triangle @ 05c60938
|-   square @ 05c609d8
|-   hexagon @ 05c60a78
|-   gpios @ 05c60b18
|- * i2c@0 @ 05c60bf8, 0
||- * eeprom@2c @ 05c60cd8, 44
||`- * emul @ 05c654b8
||- * generic_0 @ 05c62df8
||- * generic_1 @ 05c62ed8
||- * generic_2 @ 05c62fb8
||- * generic_3 @ 05c63098
||- * generic_4 @ 05c63178
||- * generic_5 @ 05c63258
||- * generic_6 @ 05c63338
||- * generic_7 @ 05c63418
||- * generic_8 @ 05c634f8
||- * generic_9 @ 05c635d8
||- * generic_a @ 05c636b8
||- * generic_b @ 05c63798
||- * generic_c @ 05c63878
||- * generic_d @ 05c63958
||- * generic_e @ 05c63a38
||- * generic_f @ 05c63b18
||- * generic_10 @ 05c63bf8
||- * generic_11 @ 05c63cd8
||- * generic_12 @ 05c63db8
||- * generic_13 @ 05c63e98
||- * generic_14 @ 05c63f78
||- * generic_15 @ 05c64058
||- * generic_16 @ 05c64138
||- * generic_17 @ 05c64218
||- * generic_18 @ 05c642f8
||- * generic_19 @ 05c643d8
||- * generic_1a @ 05c644b8
||- * generic_1b @ 05c64598
||- * generic_1c @ 05c64678
||- * generic_1d @ 05c64758
||- * generic_1e @ 05c64838
||- * generic_1f @ 05c64918
||- * generic_20 @ 05c649f8
||- * generic_21 @ 05c64ad8
||- * generic_22 @ 05c64bb8
||- * generic_23 @ 05c64c98
||- * generic_24 @ 05c64d78
||- * generic_25 @ 05c64e58
||- * generic_26 @ 05c64f38
||- * generic_27 @ 05c65018
||- * generic_28 @ 05c650f8
||- * generic_29 @ 05c651d8
||- * generic_2a @ 05c652b8
||- * generic_2b @ 05c65398
||- * generic_2d @ 05c65648
||- * generic_2e @ 05c65728
||- * generic_2f @ 05c65808
||- * generic_30 @ 05c658e8
||- * generic_31 @ 05c659c8
||- * generic_32 @ 05c65aa8
||- * generic_33 @ 05c65b88
||- * generic_34 @ 05c65c68
||- * generic_35 @ 05c65d48
||- * generic_36 @ 05c65e28
||- * generic_37 @ 05c65f08
||- * generic_38 @ 05c65fe8
||- * generic_39 @ 05c660c8
||- * generic_3a @ 05c661a8
||- * generic_3b @ 05c66288
||- * generic_3c @ 05c66368
||- * generic_3d @ 05c66448
||- * generic_3e @ 05c66528
||- * generic_3f @ 05c66608
||- * generic_40 @ 05c666e8
||- * generic_41 @ 05c667c8
||- * generic_42 @ 05c668a8
||- * generic_43 @ 05c66988
||- * generic_44 @ 05c66a68
||- * generic_45 @ 05c66b48
||- * generic_46 @ 05c66c28
||- * generic_47 @ 05c66d08
||- * generic_48 @ 05c66de8
||- * generic_49 @ 05c66ec8
||- * generic_4a @ 05c66fa8
||- * generic_4b @ 05c67088
||- * generic_4c @ 05c67168
||- * generic_4d @ 05c67248
||- * generic_4e @ 05c67328
||- * generic_4f @ 05c67408
||- * generic_50 @ 05c674e8
||- * generic_51 @ 05c675c8
||- * generic_52 @ 05c676a8
||- * generic_53 @ 05c67788
||- * generic_54 @ 05c67868
||- * generic_55 @ 05c67948
||- * generic_56 @ 05c67a28
||- * generic_57 @ 05c67b08
||- * generic_58 @ 05c67be8
||- * generic_59 @ 05c67cc8
||- * generic_5a @ 05c67da8
||- * generic_5b @ 05c67e88
||- * generic_5c @ 05c67f68
||- * generic_5d @ 05c68048
||- * generic_5e @ 05c68128
||- * generic_5f @ 05c68208
||- * generic_60 @ 05c682e8
||- * generic_61 @ 05c683c8
||- * generic_62 @ 05c684a8
||- * generic_63 @ 05c68588
||- * generic_64 @ 05c68668
||- * generic_65 @ 05c68748
||- * generic_66 @ 05c68828
||- * generic_67 @ 05c68908
||- * generic_68 @ 05c689e8
||- * generic_69 @ 05c68ac8
||- * generic_6a @ 05c68ba8
||- * generic_6b @ 05c68c88
||- * generic_6c @ 05c68d68
||- * generic_6d @ 05c68e48
||- * generic_6e @ 05c68f28
||- * generic_6f @ 05c69008
||- * generic_70 @ 05c690e8
||- * generic_71 @ 05c691c8
||- * generic_72 @ 05c692a8
||- * generic_73 @ 05c69388
||- * generic_74 @ 05c69468
||- * generic_75 @ 05c69548
||- * generic_76 @ 05c69628
||- * generic_77 @ 05c69708
||- * generic_78 @ 05c697e8
||- * generic_79 @ 05c698c8
||- * generic_7a @ 05c699a8
||- * generic_7b @ 05c69a88
||- * generic_7c @ 05c69b68
||- * generic_7d @ 05c69c48
||- * generic_7e @ 05c69d28
|`- * generic_7f @ 05c69e08
|-   spi@0 @ 05c60db8, 0
|`-   flash@0 @ 05c60e98, 0
`- * cros-ec@0 @ 05c60f78
Simon Glass Dec. 10, 2014, 5:16 p.m. UTC | #2 
Hi Masahiro,

On 10 December 2014 at 10:02, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> 2014-12-11 0:55 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> This driver includes some test features such as only supporting certain
>> bus speeds. It passes its I2C traffic through to an emulator.
>>
>> Acked-by: Heiko Schocher <hs@denx.de>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>> Changes in v6:
>> - Drop a stale comment
>> - Pass value from i2c_get_chip() to get_emul()
>> - Use dm_scan_fdt_node() to find emulator
>
>
>
> When I run "i2c probe" on a real hardware,
> generic-i2c is only added to slave-addresses that have responded.
>
> On sandbox, only 0x2c and 0x59 respond, but generic-i2c is added
> to all the slave-addresses.
> It looks funny.
>
> Is this a bug?

Not really - the driver is set to allow a probe on any chip. I suppose
we could change it, but then we would have to change the tests to add
the ability to tell it which things to respond to.

Regards,
Simon
Masahiro Yamada Dec. 11, 2014, 12:48 a.m. UTC | #3 
Hi Simon,



On Wed, 10 Dec 2014 10:16:30 -0700
Simon Glass <sjg@chromium.org> wrote:

> Hi Masahiro,
> 
> On 10 December 2014 at 10:02, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote:
> > Hi Simon,
> >
> >
> >
> > 2014-12-11 0:55 GMT+09:00 Simon Glass <sjg@chromium.org>:
> >> This driver includes some test features such as only supporting certain
> >> bus speeds. It passes its I2C traffic through to an emulator.
> >>
> >> Acked-by: Heiko Schocher <hs@denx.de>
> >> Signed-off-by: Simon Glass <sjg@chromium.org>
> >> ---
> >>
> >> Changes in v6:
> >> - Drop a stale comment
> >> - Pass value from i2c_get_chip() to get_emul()
> >> - Use dm_scan_fdt_node() to find emulator
> >
> >
> >
> > When I run "i2c probe" on a real hardware,
> > generic-i2c is only added to slave-addresses that have responded.
> >
> > On sandbox, only 0x2c and 0x59 respond, but generic-i2c is added
> > to all the slave-addresses.
> > It looks funny.
> >
> > Is this a bug?
> 
> Not really - the driver is set to allow a probe on any chip. I suppose
> we could change it, but then we would have to change the tests to add
> the ability to tell it which things to respond to.
> 


OK, then

Reviewed-by: Masahiro Yamada <yamada.m@jp.panasonic.com>


Now I am satisfied with the whole of this series
and agree to get it in.

Thanks for your hard work!





Best Regards
Masahiro Yamada
Simon Glass Dec. 11, 2014, 3:35 a.m. UTC | #4 
Hi Masahiro,

On 10 December 2014 at 17:48, Masahiro Yamada <yamada.m@jp.panasonic.com> wrote:
> Hi Simon,
>
>
>
> On Wed, 10 Dec 2014 10:16:30 -0700
> Simon Glass <sjg@chromium.org> wrote:
>
>> Hi Masahiro,
>>
>> On 10 December 2014 at 10:02, Masahiro YAMADA <yamada.m@jp.panasonic.com> wrote:
>> > Hi Simon,
>> >
>> >
>> >
>> > 2014-12-11 0:55 GMT+09:00 Simon Glass <sjg@chromium.org>:
>> >> This driver includes some test features such as only supporting certain
>> >> bus speeds. It passes its I2C traffic through to an emulator.
>> >>
>> >> Acked-by: Heiko Schocher <hs@denx.de>
>> >> Signed-off-by: Simon Glass <sjg@chromium.org>
>> >> ---
>> >>
>> >> Changes in v6:
>> >> - Drop a stale comment
>> >> - Pass value from i2c_get_chip() to get_emul()
>> >> - Use dm_scan_fdt_node() to find emulator
>> >
>> >
>> >
>> > When I run "i2c probe" on a real hardware,
>> > generic-i2c is only added to slave-addresses that have responded.
>> >
>> > On sandbox, only 0x2c and 0x59 respond, but generic-i2c is added
>> > to all the slave-addresses.
>> > It looks funny.
>> >
>> > Is this a bug?
>>
>> Not really - the driver is set to allow a probe on any chip. I suppose
>> we could change it, but then we would have to change the tests to add
>> the ability to tell it which things to respond to.
>>
>
>
> OK, then
>
> Reviewed-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
>
>
> Now I am satisfied with the whole of this series
> and agree to get it in.

OK thanks for reviewing this. You probably understand driver better
than anyone now. If you have time please look at the DM Kconfig patch
as I hope soon to be able to make that official.

>
> Thanks for your hard work!

And you also!

Regards,
Simon
Simon Glass Dec. 11, 2014, 1:22 p.m. UTC | #5 
On 10 December 2014 at 08:55, Simon Glass <sjg@chromium.org> wrote:
> This driver includes some test features such as only supporting certain
> bus speeds. It passes its I2C traffic through to an emulator.
>
> Acked-by: Heiko Schocher <hs@denx.de>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v6:
> - Drop a stale comment
> - Pass value from i2c_get_chip() to get_emul()
> - Use dm_scan_fdt_node() to find emulator

Applied to u-boot-dm.
diff mbox

Patch

diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index 5a93473..6f3c86c 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -27,7 +27,7 @@  obj-$(CONFIG_SYS_I2C_OMAP34XX) += omap24xx_i2c.o
 obj-$(CONFIG_SYS_I2C_PPC4XX) += ppc4xx_i2c.o
 obj-$(CONFIG_SYS_I2C_RCAR) += rcar_i2c.o
 obj-$(CONFIG_SYS_I2C_S3C24X0) += s3c24x0_i2c.o
-obj-$(CONFIG_SYS_I2C_SANDBOX) += i2c-emul-uclass.o
+obj-$(CONFIG_SYS_I2C_SANDBOX) += sandbox_i2c.o i2c-emul-uclass.o
 obj-$(CONFIG_SYS_I2C_SH) += sh_i2c.o
 obj-$(CONFIG_SYS_I2C_SOFT) += soft_i2c.o
 obj-$(CONFIG_SYS_I2C_TEGRA) += tegra_i2c.o
diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c
new file mode 100644
index 0000000..f0e9f51
--- /dev/null
+++ b/drivers/i2c/sandbox_i2c.c
@@ -0,0 +1,111 @@ 
+/*
+ * Simulate an I2C port
+ *
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <i2c.h>
+#include <asm/test.h>
+#include <dm/lists.h>
+#include <dm/device-internal.h>
+#include <dm/root.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+struct dm_sandbox_i2c_emul_priv {
+	struct udevice *emul;
+};
+
+static int get_emul(struct udevice *dev, struct udevice **devp,
+		    struct dm_i2c_ops **opsp)
+{
+	struct dm_i2c_chip *priv;
+	int ret;
+
+	*devp = NULL;
+	*opsp = NULL;
+	priv = dev_get_parentdata(dev);
+	if (!priv->emul) {
+		ret = dm_scan_fdt_node(dev, gd->fdt_blob, dev->of_offset,
+				       false);
+		if (ret)
+			return ret;
+
+		ret = device_get_child(dev, 0, &priv->emul);
+		if (ret)
+			return ret;
+	}
+	*devp = priv->emul;
+	*opsp = i2c_get_ops(priv->emul);
+
+	return 0;
+}
+
+static int sandbox_i2c_xfer(struct udevice *bus, struct i2c_msg *msg,
+			    int nmsgs)
+{
+	struct dm_i2c_bus *i2c = bus->uclass_priv;
+	struct dm_i2c_ops *ops;
+	struct udevice *emul, *dev;
+	bool is_read;
+	int ret;
+
+	/* Special test code to return success but with no emulation */
+	if (msg->addr == SANDBOX_I2C_TEST_ADDR)
+		return 0;
+
+	ret = i2c_get_chip(bus, msg->addr, &dev);
+	if (ret)
+		return ret;
+
+	ret = get_emul(dev, &emul, &ops);
+	if (ret)
+		return ret;
+
+	/*
+	 * For testing, don't allow writing above 100KHz for writes and
+	 * 400KHz for reads
+	 */
+	is_read = nmsgs > 1;
+	if (i2c->speed_hz > (is_read ? 400000 : 100000))
+		return -EINVAL;
+	return ops->xfer(emul, msg, nmsgs);
+}
+
+static const struct dm_i2c_ops sandbox_i2c_ops = {
+	.xfer		= sandbox_i2c_xfer,
+};
+
+static int sandbox_i2c_child_pre_probe(struct udevice *dev)
+{
+	struct dm_i2c_chip *i2c_chip = dev_get_parentdata(dev);
+
+	/* Ignore our test address */
+	if (i2c_chip->chip_addr == SANDBOX_I2C_TEST_ADDR)
+		return 0;
+	if (dev->of_offset == -1)
+		return 0;
+
+	return i2c_chip_ofdata_to_platdata(gd->fdt_blob, dev->of_offset,
+					   i2c_chip);
+}
+
+static const struct udevice_id sandbox_i2c_ids[] = {
+	{ .compatible = "sandbox,i2c" },
+	{ }
+};
+
+U_BOOT_DRIVER(i2c_sandbox) = {
+	.name	= "i2c_sandbox",
+	.id	= UCLASS_I2C,
+	.of_match = sandbox_i2c_ids,
+	.per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
+	.child_pre_probe = sandbox_i2c_child_pre_probe,
+	.ops	= &sandbox_i2c_ops,
+};