Message ID | 20210218132938.2168-9-kory.maincent@bootlin.com |
---|---|
State | Superseded |
Delegated to: | Tom Rini |
Headers | show |
Series | Add support for extension boards detection and DT overlays application | expand |
On Thu, Feb 18, 2021 at 02:29:36PM +0100, Kory Maincent wrote: > Add the extension_board_scan specific function to scan the information > of the EEPROM on one-wire and fill the extension struct. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- > arch/arm/mach-sunxi/Kconfig | 2 + > board/sunxi/Makefile | 1 + > board/sunxi/chip.c | 104 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 107 insertions(+) > create mode 100644 board/sunxi/chip.c > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > index 37a4294d88..3bb8a08b34 100644 > --- a/arch/arm/mach-sunxi/Kconfig > +++ b/arch/arm/mach-sunxi/Kconfig > @@ -6,10 +6,12 @@ choice > > config TARGET_CHIP > bool "CHIP board" > + select SUPPORT_EXTENSION_SCAN > select W1 > select W1_GPIO > select W1_EEPROM > select W1_EEPROM_DS24XXX > + imply CMD_EXTENSION And then based on my comment in the previous patch, similar to PINEPHONE_DT_SELECTION and other board-specific options, add a CHIP_DIP_SCAN option or similar.
On Fri, 19 Feb 2021 11:54:54 -0500 Tom Rini <trini@konsulko.com> wrote: Hi, > On Thu, Feb 18, 2021 at 02:29:36PM +0100, Kory Maincent wrote: > > > Add the extension_board_scan specific function to scan the information > > of the EEPROM on one-wire and fill the extension struct. > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > --- > > arch/arm/mach-sunxi/Kconfig | 2 + > > board/sunxi/Makefile | 1 + > > board/sunxi/chip.c | 104 ++++++++++++++++++++++++++++++++++++ > > 3 files changed, 107 insertions(+) > > create mode 100644 board/sunxi/chip.c > > > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > > index 37a4294d88..3bb8a08b34 100644 > > --- a/arch/arm/mach-sunxi/Kconfig > > +++ b/arch/arm/mach-sunxi/Kconfig > > @@ -6,10 +6,12 @@ choice > > > > config TARGET_CHIP > > bool "CHIP board" > > + select SUPPORT_EXTENSION_SCAN > > select W1 > > select W1_GPIO > > select W1_EEPROM > > select W1_EEPROM_DS24XXX > > + imply CMD_EXTENSION > > And then based on my comment in the previous patch, similar to > PINEPHONE_DT_SELECTION and other board-specific options, add a > CHIP_DIP_SCAN option or similar. Yes, indeed, thanks Tom. The idea of making this extension scheme generic is great, it's just that sunxi is using a different approach to board specific code here. Normally we expect U-Boot-proper board specific code to be DT driven, and where this does not really work, use those symbols that Tom pointed to. Thanks, Andre
Hi Tom, Andre Thanks for your reviews. On Fri, 19 Feb 2021 17:29:26 +0000 Andre Przywara <andre.przywara@arm.com> wrote: > > > > And then based on my comment in the previous patch, similar to > > PINEPHONE_DT_SELECTION and other board-specific options, add a > > CHIP_DIP_SCAN option or similar. > > Yes, indeed, thanks Tom. > > The idea of making this extension scheme generic is great, it's just > that sunxi is using a different approach to board specific code here. > Normally we expect U-Boot-proper board specific code to be DT driven, > and where this does not really work, use those symbols that Tom pointed > to. For my extension_board_scan board specific function, would you prefer if I move to callback like below instead of Kconfig? if (of_machine_is_compatible("nextthing,chip")) extension_board_register_callback(chip_extension_board_scan); Regards, Köry
On Mon, 22 Feb 2021 15:23:08 +0100 Köry Maincent <kory.maincent@bootlin.com> wrote: Hi Köry, > Thanks for your reviews. > > On Fri, 19 Feb 2021 17:29:26 +0000 > Andre Przywara <andre.przywara@arm.com> wrote: > > > > > > > And then based on my comment in the previous patch, similar to > > > PINEPHONE_DT_SELECTION and other board-specific options, add a > > > CHIP_DIP_SCAN option or similar. > > > > Yes, indeed, thanks Tom. > > > > The idea of making this extension scheme generic is great, it's just > > that sunxi is using a different approach to board specific code here. > > Normally we expect U-Boot-proper board specific code to be DT driven, > > and where this does not really work, use those symbols that Tom pointed > > to. > > For my extension_board_scan board specific function, would you prefer if I move > to callback like below instead of Kconfig? > > if (of_machine_is_compatible("nextthing,chip")) > extension_board_register_callback(chip_extension_board_scan); Well, the CHIP Pro uses a different compatible string, so you would need to check for that too. I am not fully decided if checking for the machine compatible is the right approach. The more traditional U-Boot way would be to define a config symbol (as Tom already pointed out), that would also keep the code out of other board builds. We do this already with CONFIG_PINE64_DT_SELECTION and CONFIG_PINEPHONE_DT_SELECTION. Cheers, Andre
Hello Andre, On Tue, 23 Feb 2021 13:30:38 +0000 Andre Przywara <andre.przywara@arm.com> wrote: y extension_board_scan board specific function, would you prefer if I > > move to callback like below instead of Kconfig? > > > > if (of_machine_is_compatible("nextthing,chip")) > > extension_board_register_callback(chip_extension_board_scan); > > Well, the CHIP Pro uses a different compatible string, so you would > need to check for that too. > I am not fully decided if checking for the machine compatible is the > right approach. The more traditional U-Boot way would be to define a > config symbol (as Tom already pointed out), that would also keep the > code out of other board builds. > We do this already with CONFIG_PINE64_DT_SELECTION and > CONFIG_PINEPHONE_DT_SELECTION. Okay, I will move on to the traditional config symbol. Thanks. Köry
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig index 37a4294d88..3bb8a08b34 100644 --- a/arch/arm/mach-sunxi/Kconfig +++ b/arch/arm/mach-sunxi/Kconfig @@ -6,10 +6,12 @@ choice config TARGET_CHIP bool "CHIP board" + select SUPPORT_EXTENSION_SCAN select W1 select W1_GPIO select W1_EEPROM select W1_EEPROM_DS24XXX + imply CMD_EXTENSION endchoice diff --git a/board/sunxi/Makefile b/board/sunxi/Makefile index c4e13f8c38..e6dcc9fca5 100644 --- a/board/sunxi/Makefile +++ b/board/sunxi/Makefile @@ -11,3 +11,4 @@ obj-$(CONFIG_SUN7I_GMAC) += gmac.o obj-$(CONFIG_MACH_SUN4I) += dram_sun4i_auto.o obj-$(CONFIG_MACH_SUN5I) += dram_sun5i_auto.o obj-$(CONFIG_MACH_SUN7I) += dram_sun5i_auto.o +obj-$(CONFIG_TARGET_CHIP) += chip.o diff --git a/board/sunxi/chip.c b/board/sunxi/chip.c new file mode 100644 index 0000000000..fc3d14e497 --- /dev/null +++ b/board/sunxi/chip.c @@ -0,0 +1,104 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * (C) Copyright 2021 + * Köry Maincent, Bootlin, <kory.maincent@bootlin.com> + * Based on initial code from Maxime Ripard + */ + +#include <common.h> +#include <malloc.h> +#include <dm.h> +#include <w1.h> +#include <w1-eeprom.h> +#include <dm/device-internal.h> + +#include <asm/arch/gpio.h> + +#include <extension_board.h> + +#ifdef CONFIG_CMD_EXTENSION + +#define for_each_1w_device(b, d) \ + for (device_find_first_child(b, d); *d; device_find_next_child(d)) + +#define dip_convert(field) \ + ( \ + (sizeof(field) == 1) ? field : \ + (sizeof(field) == 2) ? be16_to_cpu(field) : \ + (sizeof(field) == 4) ? be32_to_cpu(field) : \ + -1 \ + ) + +#define DIP_MAGIC 0x50494843 /* CHIP */ + +struct dip_w1_header { + u32 magic; /* CHIP */ + u8 version; /* spec version */ + u32 vendor_id; + u16 product_id; + u8 product_version; + char vendor_name[32]; + char product_name[32]; + u8 rsvd[36]; /* rsvd for future spec versions */ + u8 data[16]; /* user data, per-dip specific */ +} __packed; + +int extension_board_scan(struct list_head *extension_list) +{ + struct extension *dip; + struct dip_w1_header w1_header; + struct udevice *bus, *dev; + u32 vid; + u16 pid; + int ret; + + int num_dip = 0; + + sunxi_gpio_set_pull(SUNXI_GPD(2), SUNXI_GPIO_PULL_UP); + + ret = w1_get_bus(0, &bus); + if (ret) { + printf("one wire interface not found\n"); + return 0; + } + + for_each_1w_device(bus, &dev) { + if (w1_get_device_family(dev) != W1_FAMILY_DS2431) + continue; + + ret = device_probe(dev); + if (ret) { + printf("Couldn't probe device %s: error %d", + dev->name, ret); + continue; + } + + w1_eeprom_read_buf(dev, 0, (u8 *)&w1_header, sizeof(w1_header)); + + if (w1_header.magic != DIP_MAGIC) + continue; + + vid = dip_convert(w1_header.vendor_id); + pid = dip_convert(w1_header.product_id); + + printf("DIP: %s (0x%x) from %s (0x%x)\n", + w1_header.product_name, pid, + w1_header.vendor_name, vid); + + dip = calloc(1, sizeof(struct extension)); + if (!dip) { + printf("Error in memory allocation\n"); + return num_dip; + } + + snprintf(dip->overlay, sizeof(dip->overlay), "dip-%x-%x.dtbo", + vid, pid); + strncpy(dip->name, w1_header.product_name, 32); + strncpy(dip->owner, w1_header.vendor_name, 32); + list_add_tail(&dip->list, extension_list); + num_dip++; + } + return num_dip; +} + +#endif /* CONFIG_CMD_EXTENSION */
Add the extension_board_scan specific function to scan the information of the EEPROM on one-wire and fill the extension struct. Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> --- arch/arm/mach-sunxi/Kconfig | 2 + board/sunxi/Makefile | 1 + board/sunxi/chip.c | 104 ++++++++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+) create mode 100644 board/sunxi/chip.c