sunxi: chip: Enable the DIP auto-detection

Add a bus driver for bitbanging a 1-Wire bus over a GPIO.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/w1/Kconfig   |   6 ++-
 drivers/w1/Makefile  |   1 +-
 drivers/w1/w1-gpio.c | 160 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 167 insertions(+), 0 deletions(-)
 create mode 100644 drivers/w1/w1-gpio.c

drivers/w1/Kconfig +6 -0

index 0c056b4c06a9..ccc3ae15db86 100644
@@ -12,6 +12,12 @@ config W1
 
 if W1
 
+config W1_GPIO
+	bool "Enable 1-Wire GPIO bitbanging"
+	depends on DM_GPIO
+	help
+	  Emulate a 1-Wire bus using a GPIO.
+
 endif
 
 endmenu

drivers/w1/Makefile +1 -0

index 26820fa209e1..7fd8697f8419 100644
@@ -1,2 +1,3 @@
 obj-$(CONFIG_W1) += w1-uclass.o
 
+obj-$(CONFIG_W1_GPIO) += w1-gpio.o

drivers/w1/w1-gpio.c new file +150 -0

new file mode 100644
index 000000000000..091849162533
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2015 Free Electrons
+ * Copyright (c) 2015 NextThing Co
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <w1.h>
+
+#include <asm/gpio.h>
+
+
+#define W1_TIMING_A	6
+#define W1_TIMING_B	64
+#define W1_TIMING_C	60
+#define W1_TIMING_D	10
+#define W1_TIMING_E	9
+#define W1_TIMING_F	55
+#define W1_TIMING_G	0
+#define W1_TIMING_H	480
+#define W1_TIMING_I	70
+#define W1_TIMING_J	410
+
+struct w1_gpio_pdata {
+	struct gpio_desc	gpio;
+	u64			search_id;
+};
+
+static bool w1_gpio_read_bit(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int val;
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT);
+	udelay(W1_TIMING_A);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_IN);
+	udelay(W1_TIMING_E);
+
+	val = dm_gpio_get_value(&pdata->gpio);
+	udelay(W1_TIMING_F);
+
+	return val;
+}
+
+static u8 w1_gpio_read_byte(struct udevice *dev)
+{
+	int i;
+	u8 ret = 0;
+
+	for (i = 0; i < 8; ++i)
+		ret |= (w1_gpio_read_bit(dev) ? 1 : 0) << i;
+
+	return ret;
+}
+
+static void w1_gpio_write_bit(struct udevice *dev, bool bit)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT);
+
+	bit ? udelay(W1_TIMING_A) : udelay(W1_TIMING_C);
+
+	dm_gpio_set_value(&pdata->gpio, 1);
+
+	bit ? udelay(W1_TIMING_B) : udelay(W1_TIMING_D);
+}
+
+static void w1_gpio_write_byte(struct udevice *dev, u8 byte)
+{
+	int i;
+
+	for (i = 0; i < 8; ++i)
+		w1_gpio_write_bit(dev, (byte >> i) & 0x1);
+}
+
+static bool w1_gpio_reset(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int val;
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+	udelay(W1_TIMING_G);
+
+	dm_gpio_set_value(&pdata->gpio, 0);
+	udelay(W1_TIMING_H);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_IN);
+	udelay(W1_TIMING_I);
+
+	val = dm_gpio_get_value(&pdata->gpio);
+	udelay(W1_TIMING_J);
+
+	return val;
+}
+
+static u8 w1_gpio_triplet(struct udevice *dev, bool bdir)
+{
+	u8 id_bit   = w1_gpio_read_bit(dev);
+	u8 comp_bit = w1_gpio_read_bit(dev);
+	u8 retval;
+
+	if (id_bit && comp_bit)
+		return 0x03;  /* error */
+
+	if (!id_bit && !comp_bit) {
+		/* Both bits are valid, take the direction given */
+		retval = bdir ? 0x04 : 0;
+	} else {
+		/* Only one bit is valid, take that direction */
+		bdir = id_bit;
+		retval = id_bit ? 0x05 : 0x02;
+	}
+
+	w1_gpio_write_bit(dev, bdir);
+	return retval;
+}
+
+
+static const struct w1_ops w1_gpio_ops = {
+	.read_byte	= w1_gpio_read_byte,
+	.reset		= w1_gpio_reset,
+	.triplet	= w1_gpio_triplet,
+	.write_byte	= w1_gpio_write_byte,
+};
+
+static int w1_gpio_ofdata_to_platdata(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int ret;
+
+	ret = gpio_request_by_name(dev, "gpios", 0, &pdata->gpio, 0);
+	if (ret < 0)
+		goto error;
+
+	return 0;
+
+error:
+	printf("Error claiming GPIO %d\n", ret);
+	return ret;
+};
+
+static const struct udevice_id w1_gpio_id[] = {
+	{ "w1-gpio", 0 },
+	{ },

Quoted 196 lines · Maxime Ripard

Add a bus driver for bitbanging a 1-Wire bus over a GPIO.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/w1/Kconfig   |   6 ++-
 drivers/w1/Makefile  |   1 +-
 drivers/w1/w1-gpio.c | 160 ++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 167 insertions(+), 0 deletions(-)
 create mode 100644 drivers/w1/w1-gpio.c

drivers/w1/Kconfig +6 -0

index 0c056b4c06a9..ccc3ae15db86 100644
@@ -12,6 +12,12 @@ config W1
 
 if W1
 
+config W1_GPIO
+	bool "Enable 1-Wire GPIO bitbanging"
+	depends on DM_GPIO
+	help
+	  Emulate a 1-Wire bus using a GPIO.
+
 endif
 
 endmenu

drivers/w1/Makefile +1 -0

index 26820fa209e1..7fd8697f8419 100644
@@ -1,2 +1,3 @@
 obj-$(CONFIG_W1) += w1-uclass.o
 
+obj-$(CONFIG_W1_GPIO) += w1-gpio.o

drivers/w1/w1-gpio.c new file +155 -0

new file mode 100644
index 000000000000..091849162533
@@ -0,0 +1,160 @@
+/*
+ * Copyright (c) 2015 Free Electrons
+ * Copyright (c) 2015 NextThing Co
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <w1.h>
+
+#include <asm/gpio.h>
+
+
+#define W1_TIMING_A	6
+#define W1_TIMING_B	64
+#define W1_TIMING_C	60
+#define W1_TIMING_D	10
+#define W1_TIMING_E	9
+#define W1_TIMING_F	55
+#define W1_TIMING_G	0
+#define W1_TIMING_H	480
+#define W1_TIMING_I	70
+#define W1_TIMING_J	410
+
+struct w1_gpio_pdata {
+	struct gpio_desc	gpio;
+	u64			search_id;
+};
+
+static bool w1_gpio_read_bit(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int val;
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT);
+	udelay(W1_TIMING_A);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_IN);
+	udelay(W1_TIMING_E);
+
+	val = dm_gpio_get_value(&pdata->gpio);
+	udelay(W1_TIMING_F);
+
+	return val;
+}
+
+static u8 w1_gpio_read_byte(struct udevice *dev)
+{
+	int i;
+	u8 ret = 0;
+
+	for (i = 0; i < 8; ++i)
+		ret |= (w1_gpio_read_bit(dev) ? 1 : 0) << i;
+
+	return ret;
+}
+
+static void w1_gpio_write_bit(struct udevice *dev, bool bit)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT);
+
+	bit ? udelay(W1_TIMING_A) : udelay(W1_TIMING_C);
+
+	dm_gpio_set_value(&pdata->gpio, 1);
+
+	bit ? udelay(W1_TIMING_B) : udelay(W1_TIMING_D);
+}
+
+static void w1_gpio_write_byte(struct udevice *dev, u8 byte)
+{
+	int i;
+
+	for (i = 0; i < 8; ++i)
+		w1_gpio_write_bit(dev, (byte >> i) & 0x1);
+}
+
+static bool w1_gpio_reset(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int val;
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
+	udelay(W1_TIMING_G);
+
+	dm_gpio_set_value(&pdata->gpio, 0);
+	udelay(W1_TIMING_H);
+
+	dm_gpio_set_dir_flags(&pdata->gpio, GPIOD_IS_IN);
+	udelay(W1_TIMING_I);
+
+	val = dm_gpio_get_value(&pdata->gpio);
+	udelay(W1_TIMING_J);
+
+	return val;
+}
+
+static u8 w1_gpio_triplet(struct udevice *dev, bool bdir)
+{
+	u8 id_bit   = w1_gpio_read_bit(dev);
+	u8 comp_bit = w1_gpio_read_bit(dev);
+	u8 retval;
+
+	if (id_bit && comp_bit)
+		return 0x03;  /* error */
+
+	if (!id_bit && !comp_bit) {
+		/* Both bits are valid, take the direction given */
+		retval = bdir ? 0x04 : 0;
+	} else {
+		/* Only one bit is valid, take that direction */
+		bdir = id_bit;
+		retval = id_bit ? 0x05 : 0x02;
+	}
+
+	w1_gpio_write_bit(dev, bdir);
+	return retval;
+}
+
+
+static const struct w1_ops w1_gpio_ops = {
+	.read_byte	= w1_gpio_read_byte,
+	.reset		= w1_gpio_reset,
+	.triplet	= w1_gpio_triplet,
+	.write_byte	= w1_gpio_write_byte,
+};
+
+static int w1_gpio_ofdata_to_platdata(struct udevice *dev)
+{
+	struct w1_gpio_pdata *pdata = dev_get_platdata(dev);
+	int ret;
+
+	ret = gpio_request_by_name(dev, "gpios", 0, &pdata->gpio, 0);
+	if (ret < 0)
+		goto error;
+
+	return 0;
+
+error:
+	printf("Error claiming GPIO %d\n", ret);
+	return ret;
+};
+
+static const struct udevice_id w1_gpio_id[] = {
+	{ "w1-gpio", 0 },
+	{ },
+};
+
+U_BOOT_DRIVER(w1_gpio_drv) = {
+	.id				= UCLASS_W1,
+	.name				= "w1_gpio_drv",

drivers/w1/w1-gpio.c line 160

Quoted 6 lines

drivers/w1/w1-gpio.c new file +5 -0

+	.of_match			= w1_gpio_id,
+	.ofdata_to_platdata		= w1_gpio_ofdata_to_platdata,
+	.ops				= &w1_gpio_ops,
+	.platdata_auto_alloc_size	= sizeof(struct w1_gpio_pdata),
+};

Has someone test this on different HW?

As I went through the series will be good to have cmd/w1.c to be able to test that devices.

We might want to access data stored onto EEPROMs. Create a framework to provide a consistent API.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/Kconfig                |  2 +-
 drivers/Makefile               |  1 +-
 drivers/eeprom/Kconfig         | 17 ++++++++++-
 drivers/eeprom/Makefile        |  2 +-
 drivers/eeprom/eeprom-uclass.c | 57 +++++++++++++++++++++++++++++++++++-
 include/dm/uclass-id.h         |  1 +-
 include/eeprom.h               | 21 +++++++++++++-
 7 files changed, 101 insertions(+), 0 deletions(-)
 create mode 100644 drivers/eeprom/Kconfig
 create mode 100644 drivers/eeprom/Makefile
 create mode 100644 drivers/eeprom/eeprom-uclass.c
 create mode 100644 include/eeprom.h

drivers/Kconfig +2 -0

index 74194b0d6f7f..e518752eae1a 100644
@@ -20,6 +20,8 @@ source "drivers/dfu/Kconfig"
 
 source "drivers/dma/Kconfig"
 
+source "drivers/eeprom/Kconfig"
+
 source "drivers/fpga/Kconfig"
 
 source "drivers/gpio/Kconfig"

drivers/Makefile +1 -0

index 543c43bb0d23..c8f285f66bb3 100644
@@ -64,6 +64,7 @@ obj-y += block/
 obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/
 obj-$(CONFIG_CPU) += cpu/
 obj-y += crypto/
+obj-y += eeprom/
 obj-$(CONFIG_FPGA) += fpga/
 obj-y += hwmon/
 obj-y += misc/

drivers/eeprom/Kconfig new file +1 -0

new file mode 100644
index 000000000000..8dc597a8d894
@@ -0,0 +1,17 @@
+#

drivers/eeprom/Kconfig new file +6 -0

+
+if EEPROM
+
+endif
+
+endmenu

drivers/eeprom/Makefile new file +2 -0

new file mode 100644
index 000000000000..147dba5ec4b8
@@ -0,0 +1,2 @@
+obj-$(CONFIG_EEPROM) += eeprom-uclass.o
+

drivers/eeprom/eeprom-uclass.c new file +36 -0

new file mode 100644
index 000000000000..020b0087d22c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2015 Free Electrons
+ * Copyright (c) 2015 NextThing Co.
+ *
+ * Maxime Ripard <[email protected]>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <eeprom.h>
+
+#include <dm/device-internal.h>
+
+int eeprom_read_buf(struct udevice *dev, unsigned offset,
+                   u8 *buf, unsigned count)
+{
+       const struct eeprom_ops *ops = device_get_ops(dev);
+
+       if (!ops->read_buf)
+               return -ENOSYS;
+
+       return ops->read_buf(dev, offset, buf, count);
+}
+
+
+UCLASS_DRIVER(eeprom) = {
+       .name           = "eeprom",
+       .id             = UCLASS_EEPROM,
+};
+
+int eeprom_dm_init(void)
+{
+       struct udevice *bus;
+       struct uclass *uc;

drivers/eeprom/eeprom-uclass.c new file +6 -0

+                       continue;
+               }
+       }
+
+       return 0;
+}

include/dm/uclass-id.h +0 -0

index b88adcbe802f..909a32404259 100644

include/dm/uclass-id.h +0 -0

        UCLASS_COUNT,
        UCLASS_INVALID = -1,

include/eeprom.h new file +1 -0

new file mode 100644
index 000000000000..648c8606c35a
@@ -0,0 +1,21 @@
+/*

include/eeprom.h new file +8 -1

+       int     (*read_buf)(struct udevice *dev, unsigned offset,
+                           u8 *buf, unsigned count);
+};
+
+int eeprom_read_buf(struct udevice *dev, unsigned offset,
+                   u8 *buf, unsigned count);
+
+#endif
--

git-series 0.8.11

Hi Maxime,

Quoted 2 lines · Maxime Ripard

We might want to access data stored onto EEPROMs. Create a framework to provide a consistent API.

We have UCLASS_I2C_EEPROM. Can we unify these? If not, please add a sandbox driver and test.

Hi Maxime,

Quoted 9 lines · Simon Glass

Hi Maxime,

Quoted 2 lines · Maxime Ripard

We might want to access data stored onto EEPROMs. Create a framework to provide a consistent API.

We have UCLASS_I2C_EEPROM. Can we unify these? If not, please add a sandbox driver and test.

I've been working on something very similar (the API looks the same obviously, since the ops are pretty trivial, modulo function names)
In my opinion it should be working as follows:

UCLASS_EEPROM
\... I2C_EEPROM
\....SPI_EEPROM (AT25)

Add a driver for the Maxim DS2431 1-Wire EEPROM

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/eeprom/Kconfig  |  6 ++++++
 drivers/eeprom/Makefile |  1 +
 drivers/eeprom/ds2431.c | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)
 create mode 100644 drivers/eeprom/ds2431.c

drivers/eeprom/Kconfig +6 -0

index 8dc597a8d894..98bbd67ba579 100644
@@ -12,6 +12,12 @@ config EEPROM
 
 if EEPROM
 
+config EEPROM_DS2431
+       bool "Enable Maxim DS2431 EEPROM support"
+       depends on W1
+       help
+         Maxim DS2431 1-Wire EEPROM support
+
 endif
 
 endmenu

drivers/eeprom/Makefile +1 -0

index 147dba5ec4b8..93dae0bf5d6d 100644
@@ -1,2 +1,3 @@
 obj-$(CONFIG_EEPROM) += eeprom-uclass.o
 
+obj-$(CONFIG_EEPROM_DS2431) += ds2431.o

drivers/eeprom/ds2431.c new file +25 -0

new file mode 100644
index 000000000000..84c1a126c339
@@ -0,0 +1,38 @@
+/*
+ * Copyright (c) 2015 Free Electrons
+ * Copyright (c) 2015 NextThing Co
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ */
+
+#include <common.h>
+#include <linux/err.h>
+#include <dm.h>
+#include <eeprom.h>
+#include <w1.h>
+
+#define W1_F2D_READ_EEPROM     0xf0
+
+static int ds2431_read_buf(struct udevice *dev, unsigned offset,
+                          u8 *buf, unsigned count)
+{
+       w1_reset_select(dev);
+
+       w1_write_byte(dev, W1_F2D_READ_EEPROM);
+       w1_write_byte(dev, offset & 0xff);
+       w1_write_byte(dev, offset >> 8);
+
+       return w1_read_buf(dev, buf, count);

drivers/eeprom/ds2431.c new file +3 -1

+};
+
+U_BOOT_W1_DEVICE(ds2431, W1_FAMILY_DS2431);
--

git-series 0.8.11
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

drivers/eeprom/ds2431.c line 35

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

Hi,

Quoted 5 lines · Moritz Fischer

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Quoted 1 line

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Quoted 8 lines · Maxime Ripard

Hi,

Quoted 5 lines · Moritz Fischer

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Quoted 6 lines

Quoted 1 line

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Bah. Do you have a pointer to some discussion about this handy?

Quoted 16 lines · Tom Rini

Quoted 14 lines · Maxime Ripard

Hi,

Quoted 6 lines · Moritz Fischer

drivers/eeprom/ds2431.c line 35

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Quoted 1 line

Bah. Do you have a pointer to some discussion about this handy?

Rob said this multiple times, but here is an example:
http://lkml.iu.edu/hypermail/linux/kernel/1609.1/00653.html

Hi Maxime,

Quoted 17 lines · Maxime Ripard

Quoted 16 lines · Tom Rini

Quoted 14 lines · Maxime Ripard

Hi,

Quoted 6 lines · Moritz Fischer

drivers/eeprom/ds2431.c line 35

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Quoted 4 lines

Quoted 1 line

Bah. Do you have a pointer to some discussion about this handy?

Rob said this multiple times, but here is an example:
http://lkml.iu.edu/hypermail/linux/kernel/1609.1/00653.html

Well that's not really an explanation!

Quoted 26 lines · Simon Glass

Hi Maxime,

Quoted 21 lines · Maxime Ripard

Quoted 9 lines · Tom Rini

Quoted 8 lines · Maxime Ripard

Hi,

Quoted 5 lines · Moritz Fischer

Quoted 4 lines

(quoted diff) +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Quoted 8 lines

Quoted 6 lines

Quoted 1 line

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Bah. Do you have a pointer to some discussion about this handy?

Rob said this multiple times, but here is an example:
http://lkml.iu.edu/hypermail/linux/kernel/1609.1/00653.html

Quoted 1 line

Well that's not really an explanation!

Don't shoot the messenger :)

I think I found some other discussion where they discuss it at length:
https://patchwork.kernel.org/patch/9133903/

The interesting part is when Rob Herring is involved.

Hi,

Quoted 5 lines · Moritz Fischer

Quoted 4 lines

drivers/eeprom/ds2431.c new file +4 -0

+U_BOOT_DRIVER(ds2431) = {
+       .name           = "ds2431",
+       .id             = UCLASS_EEPROM,
+       .ops            = &ds2431_ops,

Quoted 1 line

Do you want to add a .flags = DM_UC_FLAG_SEQ_ALIAS here?

I don't know. I was kind of wondering why U-Boot relies on aliases so much, especially when the Linux DT maintainers are saying that aliases should be avoided entirely, and we'll won't be able to upstream those changes.

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Signed-off-by Maxime Ripard <[email protected]>

---
 drivers/video/cfb_console.c |  9 +++++++++
 1 file changed, 9 insertions(+), 0 deletions(-)

drivers/video/cfb_console.c +9 -1

index c0b1b8dc17e2..724ae16c5a9b 100644
@@ -2119,6 +2119,11 @@ __weak int board_video_skip(void)
        return 0;
 }
 
+__weak int board_video_pre_init(void)
+{
+       return 0;
+}
+
 int drv_video_init(void)
 {
        struct stdio_dev console_dev;

@@ -2129,6 +2134,10 @@ int drv_video_init(void)
        if (board_video_skip())
                return 0;
 
+       /* Allow the board to setup a few things */
+       if (board_video_pre_init())
+               return 0;
+
        /* Init video chip - returns with framebuffer cleared */
        if (cfg_video_init() == -1)
                return 0;
--

git-series 0.8.11

Hi Maxime,

Quoted 4 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Instead of this, can you use driver model (UCLASS_VIDEO)?

Hi Simon,

Quoted 7 lines · Simon Glass

Hi Maxime,

Quoted 3 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Quoted 3 lines

Quoted 1 line

Add some hook to allow that.

Instead of this, can you use driver model (UCLASS_VIDEO)?

I don't really know the device model that well, hence 'm not really sure how would that help. Can a board register a hook to be called before a driver is probed?

Hi Maxime,

Quoted 12 lines · Maxime Ripard

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Maxime,

Quoted 4 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Quoted 5 lines

Quoted 1 line

Instead of this, can you use driver model (UCLASS_VIDEO)?

I don't really know the device model that well, hence 'm not really sure how would that help. Can a board register a hook to be called before a driver is probed?

My suggest would be that the driver can do whatever is required. What is the board-specific code actually wanting to do?

Hi Simon,

Quoted 16 lines · Simon Glass

Hi Maxime,

Quoted 12 lines · Maxime Ripard

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Maxime,

Quoted 4 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Instead of this, can you use driver model (UCLASS_VIDEO)?

I don't really know the device model that well, hence 'm not really sure how would that help. Can a board register a hook to be called before a driver is probed?

My suggest would be that the driver can do whatever is required. What is the board-specific code actually wanting to do?

We need to adjust the video-mode environment variable that is going to be used in the driver's initialisation. We're doing that by identifying which daughter board is attached, and then setting that value according to the display output we want to use.

So we need some hook that would run between the environment initialisation and the video driver's init.

Hi Maxime,

Quoted 33 lines · Maxime Ripard

Hi Simon,

Quoted 16 lines · Simon Glass

Hi Maxime,

Quoted 12 lines · Maxime Ripard

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Maxime,

Quoted 4 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Instead of this, can you use driver model (UCLASS_VIDEO)?

I don't really know the device model that well, hence 'm not really sure how would that help. Can a board register a hook to be called before a driver is probed?

My suggest would be that the driver can do whatever is required. What is the board-specific code actually wanting to do?

We need to adjust the video-mode environment variable that is going to be used in the driver's initialisation. We're doing that by identifying which daughter board is attached, and then setting that value according to the display output we want to use.

Quoted 2 lines

So we need some hook that would run between the environment initialisation and the video driver's init.

As an example of how you might pass platform data to the video driver, see board_init() in gurnard.c.

Quoted 40 lines · Simon Glass

Hi Maxime,

Quoted 35 lines · Maxime Ripard

Hi Simon,

Quoted 16 lines · Simon Glass

Hi Maxime,

Quoted 12 lines · Maxime Ripard

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Maxime,

Quoted 4 lines · Maxime Ripard

Some boards might need to some additional setup right before initialising the video console.

Add some hook to allow that.

Quoted 8 lines

Quoted 5 lines

Quoted 1 line

Instead of this, can you use driver model (UCLASS_VIDEO)?

I don't really know the device model that well, hence 'm not really sure how would that help. Can a board register a hook to be called before a driver is probed?

My suggest would be that the driver can do whatever is required. What is the board-specific code actually wanting to do?

We need to adjust the video-mode environment variable that is going to be used in the driver's initialisation. We're doing that by identifying which daughter board is attached, and then setting that value according to the display output we want to use.

So we need some hook that would run between the environment initialisation and the video driver's init.

Quoted 2 lines

As an example of how you might pass platform data to the video driver, see board_init() in gurnard.c.

That's very useful, thanks!

Can you also postpone the probe of one driver, or at least defer a driver's probe until some other is done?

The way we identify the daughter boards also rely on some driver in the device model (a 1-Wire EEPROM, connected on a GPIO, both of them also being part of the device model), so we need to defer the video driver probe until both of them have been probed and accessed.

I think the biggest drawback at the moment is that we maintain a list of DIPs and the actions needed directly into the C code, which will make it quite hard to customise for end users and tedious to maintain in the long term. I couldn't really get my head around a better solution, so feel free to suggest alternative approaches.

On Tue, Nov 08, 2016 at 11:19:20AM +0100, Maxime Ripard wrote:

[snip]

Quoted 5 lines

I think the biggest drawback at the moment is that we maintain a list of DIPs and the actions needed directly into the C code, which will make it quite hard to customise for end users and tedious to maintain in the long term. I couldn't really get my head around a better solution, so feel free to suggest alternative approaches.

A thought I had after reading over 8/9 in the series was, could we try something like:
- In C, loop over everything in w1 and set environment variables based on each thing we find.
- In a CONFIG_PREBOOT load a U-Boot shell script in that will look for the right env variables to be set for a given DIP and then do whatever is needed to load in the overlay.

Does that make sense?

Hi Tom,

Quoted 17 lines · Tom Rini

On Tue, Nov 08, 2016 at 11:19:20AM +0100, Maxime Ripard wrote:

[snip]

Quoted 5 lines

I think the biggest drawback at the moment is that we maintain a list of DIPs and the actions needed directly into the C code, which will make it quite hard to customise for end users and tedious to maintain in the long term. I couldn't really get my head around a better solution, so feel free to suggest alternative approaches.

A thought I had after reading over 8/9 in the series was, could we try something like:
- In C, loop over everything in w1 and set environment variables based on each thing we find.
- In a CONFIG_PREBOOT load a U-Boot shell script in that will look for the right env variables to be set for a given DIP and then do whatever is needed to load in the overlay.

Quoted 1 line

Does that make sense?

I thought about an environment-based solution too, but I was pretty worried about its scalability. But I guesse that's also the easiest thing to modify by end-users.

However, we have basically, three things to do (so far)
1) Modify the U-Boot environment before the display is initialized
2) Modify the kernel command line
3) Apply an overlay

I think CONFIG_PREBOOT is too late for 1, because the display is already initialized, and too early for 2 and 3, because the command line and DT will probably be set / loaded at bootcmd time.

I think we can decouple 1 and 2 from 3, since doing 2 too early is not an issue, because we might have boards that require to load some overlay but wouldn't need to make environment modifications (like an MMC DIP), or the other way around (even though I fail to see a use case for that at the moment).

So we're left with basically two things:
- Do a bunch of modifications in the environment early
- And apply the overlay very late (basically right before booting the kernel)

So we could do something like having an environment script called dip-<vid>-<pid>-setup called when a DIP is detected (if it exists), and keep the DT overlay logic in its current form.

What do you think?

The NextThing's C.H.I.P. can have expansion boards called DIPs. Those DIPs are connected through the external headers, and comes with an identification mechanism based on 1-Wire EEPROMs.

That auto-detection works great, because 1-Wire allows the enumeration, and the EEPROMs are guaranteed to have different addresses, which allows us to stack as many DIPs as possible, without any constraints.

Since those DIPs can be close to anything, ranging from a simple PWM-based buzzer to a full featured device such as the PocketCHIP (which comes with a touchscreen, a keyboard, a battery, etc), some adjustments might be needed in U-Boot to be able to present something that just works to the user.

In particular, we need to:
- Apply an overlay if the device is something that should be dealt with early in the boot process (display, storage device)
- Adjust the U-Boot environment if the DIP has a display to change the default video output
- Adjust the kernel command line in previous case for Linux to have the same default

In order to achieve that, we introduced some logic optionally hooked into the sunxi board, two new uclasses for 1-Wire and EEPROMs, and a bunch of new environment variables:
- dip-auto-video to control the automatic video set up (default to yes)
- dip_overlay_cmd that is a script to load the overlay $dip_overlay_name to $dip_addr_r, from whatever place it was stored in, and later apply it.
- kernelarg_video to host the default kernel output in the kernel command line

I think the biggest drawback at the moment is that we maintain a list of DIPs and the actions needed directly into the C code, which will make it quite hard to customise for end users and tedious to maintain in the long term. I couldn't really get my head around a better solution, so feel free to suggest alternative approaches.

Let me know what you think,