Hi, Torsten, Matthias,
On Wed, Apr 19, 2023 at 02:34:03PM +0200, Matthias Brugger wrote:
>
>
> On 19/04/2023 13:28, Torsten Duwe wrote:
> > U-Boot already has a mechanism to fix up the DT before OS boot.
> > This avoids the excessive duplication of data and work proposed
> > by the explicit separation of 1.2a and 1.3b board revisions. It
> > will also, to a good degree, improve the user experience, as
> > pointed out by Matthias.
> >
> > The defconfig changes required (in diffconfig format) are
> >
> > -I2C n
> > -NET_RANDOM_ETHADDR y
> > +CMD_I2C y
> > +CMD_MISC y
> > +DM_I2C y
> > +I2C_EEPROM y
> > +MISC y
> > +MISC_INIT_R y
> > +OF_BOARD_SETUP y
> > +SPL_DM_I2C n
> > +SPL_MISC n
> > +SYS_I2C_DW y
> > +SYS_I2C_EEPROM_ADDR 0x0
> >
> > along with the patch below. It has the neat side effect of providing
> > the network with the proper MAC addresses ;)
> >
> > I take advantage of the fact that I²C-5 is also required to talk to the
> > PMIC, so it must already be initialised by OpenSBI. All that's required
> > is to declare the EEPROM and to pull in the drivers.
> >
> > This is only a proof of concept; let me know if you like it and I can
> > add the other 12 DT patches to adjust_for_rev13b(), or maybe start with
> > 1.3b as the default and go the other way, or something in between.
> >
> > The last hunk, to the i2c Makefile, is IMHO an independent fix, because
> > the implication PCI => ACPI in designware_i2c_pci is invalid, and the
> > VisionFive2 config proves it. Use this quick hack for now.
> >
>
> Looks like a neat approach to enable a signle U-Boot binary to boot on both
> platforms.
>
> Thanks for investigating this.
LGTM as well!
Best regards,
Leo
>
> > Signed-off-by: Torsten Duwe <d...@suse.de>
> >
> > ---
> > diff --git a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > index ff9df56ec2..fd3a1d057a 100644
> > --- a/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > +++ b/arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi
> > @@ -119,6 +119,12 @@
> > pinctrl-names = "default";
> > pinctrl-0 = <&i2c5_pins>;
> > status = "okay";
> > +
> > + eeprom@50 {
> > + compatible = "atmel,24c04";
> > + reg = <0x50>;
> > + pagesize = <0x10>;
> > + };
> > };
> > &i2c6 {
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c
> > b/board/starfive/visionfive2/starfive_visionfive2.c
> > index 613fe793c4..d7f846a357 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -7,6 +7,10 @@
> > #include <common.h>
> > #include <asm/io.h>
> > #include <cpu_func.h>
> > +#include <dm/uclass.h>
> > +#include <fdt_support.h>
> > +#include <i2c_eeprom.h>
> > +#include <net.h>
> > #include <linux/bitops.h>
> > #define JH7110_L2_PREFETCHER_BASE_ADDR 0x2030000
> > @@ -38,3 +42,62 @@ int board_init(void)
> > return 0;
> > }
> > +
> > +#ifdef CONFIG_MISC_INIT_R
>
> As this is will be enabled for the board config I think we don't need the
> ifedef.
>
> > +int misc_init_r(void)
> > +{
> > + int ret = 0;
> > +
> > +#ifdef CONFIG_I2C_EEPROM
> > + struct udevice *dev;
> > + char mac_addr[6];
> > + unsigned char pcb_rev, BOM;
> > +
> > + ret = uclass_first_device_err(UCLASS_I2C_EEPROM, &dev);
> > + if (ret)
> > + goto out;
> > +
> > + if (eth_env_get_enetaddr("ethaddr", mac_addr) == 0) {
> > + int i;
> > + for (i=0; i<2; i++) {
> > + ret = i2c_eeprom_read(dev, 0x78+6*i, mac_addr, 6);
> > + if (!ret && is_valid_ethaddr(mac_addr))
> > + eth_env_set_enetaddr_by_index("eth", i,
> > mac_addr);
> > + }
> > + }
> > +
> > + ret = i2c_eeprom_read(dev, 0x76, &pcb_rev, 1);
> > + if (!ret)
> > + env_set_hex("board_revision", pcb_rev);
> > +
> > + ret = i2c_eeprom_read(dev, 0x77, &BOM, 1);
> > +
> > + out:
> > +#endif
> > + return ret;
> > +}
> > +#endif
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
>
> Same here.
>
> Regards,
> Matthias
>
> > +static void adjust_for_rev13b(void * fdt)
> > +{
> > + do_fixup_by_path(fdt, "/soc/ethernet@16040000",
> > + "phy-mode", "rgmii-id", 9, 0);
> > + /*
> > + ... other fixups ...
> > +
> > + */
> > +}
> > +
> > +int ft_board_setup(void *fdt, struct bd_info *bdip)
> > +{
> > + unsigned char pcb_rev = 0;
> > +
> > + pcb_rev = env_get_hex("board_revision", pcb_rev);
> > + if (pcb_rev >= 0xB2) {
> > + printf("Adjusting FDT for v1.3B board rev\n");
> > + adjust_for_rev13b(fdt);
> > + }
> > + return 0;
> > +}
> > +#endif
> > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
> > index 99545df2e5..828856e40d 100644
> > --- a/drivers/i2c/Makefile
> > +++ b/drivers/i2c/Makefile
> > @@ -19,8 +19,10 @@ obj-$(CONFIG_SYS_I2C_CA) += i2c-cortina.o
> > obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o
> > obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o
> > ifdef CONFIG_PCI
> > +ifdef CONFIG_ACPIGEN
> > obj-$(CONFIG_SYS_I2C_DW) += designware_i2c_pci.o
> > endif
> > +endif
> > obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o
> > obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o
> > obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o
