Hi Stefano,
On Mon, 2022-04-11 at 17:45 +0200, Stefano Babic wrote:
> Hi Philipp,
>
> On 02.03.22 10:39, Philip Oberfichtner wrote:
> > The Bosch ACC (Air Center Control) Board is based on the i.MX6D.
>
> [snip]
>
> > diff --git a/include/configs/imx6q-acc.h b/include/configs/imx6q-
> > acc.h
> > new file mode 100644
> > index 0000000000..caa11df49f
> > --- /dev/null
> > +++ b/include/configs/imx6q-acc.h
> > @@ -0,0 +1,165 @@
> > +/* SPDX-License-Identifier: GPL-2.0+
> > + *
> > + * Copyright (c) 2017 DENX Software Engineering GmbH, Heiko
> > Schocher <h...@denx.de>
> > + * Copyright (c) 2019 Bosch Thermotechnik GmbH
> > + */
> > +
> > +#ifndef __IMX6Q_ACC_H
> > +#define __IMX6Q_ACC_H
> > +
> > +#include <linux/sizes.h>
> > +#include "mx6_common.h"
> > +
> > +/* Environment settings */
> > +#define CONFIG_ENV_ACCESS_IGNORE_FORCE
> > +
> > +#ifdef CONFIG_SYS_BOOT_EMMC
> > +#define MMC_ROOTFS_DEV 0
> > +#define MMC_ROOTFS_PART 2
> > +#endif
> > +
> > +/* Allow to overwrite serial and ethaddr */
> > +#define CONFIG_ENV_OVERWRITE
> > +
> > +#ifdef CONFIG_SYS_BOOT_EMMC
> > +/* eMMC Boot */
> > +#define ENV_EXTRA \
> > + "mmcdev=" __stringify(MMC_ROOTFS_DEV) "\0" \
> > + "mmcpart=" __stringify(MMC_ROOTFS_PART) "\0" \
> > + "fitpart=1\0" \
> > + "optargs=ro quiet systemd.gpt_auto=false\0" \
> > + "production=1\0" \
> > + "mmcautodetect=yes\0" \
> > + "mmcrootfstype=ext4\0" \
> > + "finduuid=part uuid mmc ${mmcdev}:${mmcpart} uuid\0" \
> > + "mmcargs=run finduuid; setenv bootargs " \
> > + "root=PARTUUID=${uuid} ${optargs}
> > rootfstype=${mmcrootfstype}\0" \
> > + "mmc_mmc_fit=run env_persist; run setbm; run mmcloadfit; "
> > \
> > + "run auth_fit_or_reset; run mmcargs addcon; " \
> > + "bootm ${fit_addr}#${bootconf}\0" \
> > + "bootset=0\0" \
> > + "setbm=if test ${bootset} -eq 1; " \
> > + "then setenv mmcpart 4; setenv fitpart 3; " \
> > + "else; setenv mmcpart 2; setenv fitpart 1; fi\0" \
> > + "handle_ustate=if test ${ustate} -eq 2; then setenv ustate
> > 3; fi\0" \
> > + "switch_bootset=if test ${bootset} -eq 1; then setenv
> > bootset 0; " \
> > + "else; setenv bootset 1;fi\0" \
> > + "env_persisted=0\0" \
> > + "env_persist=if test ${env_persisted} != 1; " \
> > + "then env set env_persisted 1; run save_env; fi;\0"
> > \
> > + "save_env=env save; env save\0" \
> > + "altbootcmd=run handle_ustate; run switch_bootset; run
> > save_env; run bootcmd\0"
> > +
> > +#define CONFIG_ENV_FLAGS_LIST_STATIC \
>
>
> > + "addcon:so," \
> > + "altbootcmd:so," \
> > + "auth_fit_or_reset:so," \
> > + "baudrate:do," \
> > + "bootcmd:so," \
> > + "bootcount:da," \
> > + "bootdelay:do," \
> > + "bootlimit:do," \
> > + "bootset:ba," \
> > + "clone_pending:ba," \
> > + "console:so," \
> > + "endurance_test:ba," \
> > + "env_persist:so," \
> > + "env_persisted:ba," \
> > + "ethaddr:so," \
> > + "factory_reset:ba," \
> > + "fdtcontroladdr:xa," \
> > + "finduuid:so," \
> > + "fit_addr:do," \
> > + "fitpart:da," \
> > + "handle_ustate:so," \
> > + "loadaddr:xo," \
> > + "mmc_mmc_fit:so," \
> > + "mmcargs:so," \
> > + "mmcautodetect:so," \
> > + "mmcdev:do," \
> > + "mmcfit_name:so," \
> > + "mmcloadfit:so," \
> > + "mmcpart:da," \
> > + "mmcrootfstype:so," \
> > + "optargs:so," \
> > + "production:ba," \
> > + "save_env:so," \
> > + "setbm:so," \
> > + "silent:so," \
> > + "switch_bootset:so," \
> > + "ustate:da"
>
> This is the list of the variables that are allowed to be changed.
> Generally, this list is very short to avoid that an attacker can
> change
> a lot. I wonder that on this board the list is very long - sure that
> this is ok ?
>
Thanks for the hint. For V2 I will enable CONFIG_ENV_WRITEABLE_LIST and
shorten this list. This will also involve a 'env_get_location'
implementation in the board file.
>
> > +
> > +#else //CONFIG_SYS_BOOT_EMMC
>
> CONFIG_ are not allowed and they are rejected during build. Any
> CONFIG_
> switch should go into Kbuild. It is not useful here, so just rename
> local defines in this file with something different as CONFIG_
> (ENV_FLAG
>
> Rather the CI hits the comments, too, so please drop them. Not only,
> there are some CONFIG_ that are already defined in Kbuild. Sure that
> you
> have to redefine all of them or you can just add them to
> configs/imx6q_acc_defconfig ?
>
> You can check the error reported here:
>
> https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/420785
For V2 I will migrate CONFIG_MII and CONFIG_SYS_MEMTEST* to Kconfig,
the remainder of CONFIG_* used is whitelisted.
Concerning the CONFIG_SYS_BOOT_EMMC: This is defined in
board/bosch/acc/Kconfig and used to discern SD boot from eMMC boot. Is
it good practice then to leave it like this?
Thanks for the feedback.
Best regards,
Philip
>
> Best regards,
> Stefano
>
>
> > +/* SD Card boot */
> > +#define ENV_EXTRA \
> > + "mmcdev=1\0" \
> > + "fitpart=1\0" \
> > + "rootpart=2\0" \
> > + "optargs=ro systemd.gpt_auto=false\0" \
> > + "finduuid=part uuid mmc ${mmcdev}:${rootpart} uuid\0" \
> > + "mmcargs=run finduuid;setenv bootargs root=PARTUUID=${uuid}
> > ${optargs}\0" \
> > + "mmc_mmc_fit=run mmcloadfit; run auth_fit_or_reset; run
> > mmcargs addcon; " \
> > + "bootm ${fit_addr}#${bootconf}\0"
> > +
> > +#endif //CONFIG_SYS_BOOT_EMMC
> > +
> > +/* Default environment */
> > +#define CONFIG_EXTRA_ENV_SETTINGS \
> > + "bootconf=conf-imx6q-acc.dtb\0"\
> > + "mmcfit_name=fitImage\0" \
> > + "mmcloadfit=ext4load mmc ${mmcdev}:${fitpart} ${fit_addr}
> > ${mmcfit_name}\0" \
> > + "auth_fit_or_reset=hab_auth_img ${fit_addr} ${filesize} ||
> > reset\0" \
> > + "console=ttymxc0\0" \
> > + "addcon=setenv bootargs ${bootargs}
> > console=${console},${baudrate}\0" \
> > + "fit_addr=19000000\0" \
> > + ENV_EXTRA
> > +
> > +/* Miscellaneous configurable options */
> > +#define CONFIG_SYS_MEMTEST_START CONFIG_SYS_SDRAM_BASE
> > +#define CONFIG_SYS_MEMTEST_END (CONFIG_SYS_MEMTEST_START
> > + 5 * SZ_1M)
> > +
> > +/* Physical Memory Map */
> > +#define PHYS_SDRAM MMDC0_ARB_BASE_ADDR
> > +
> > +#define CONFIG_SYS_SDRAM_BASE PHYS_SDRAM
> > +#define CONFIG_SYS_INIT_RAM_ADDR IRAM_BASE_ADDR
> > +#define CONFIG_SYS_INIT_RAM_SIZE IRAM_SIZE
> > +
> > +#define CONFIG_SYS_INIT_SP_OFFSET (CONFIG_SYS_INIT_RAM_SIZE
> > - GENERATED_GBL_DATA_SIZE)
> > +#define CONFIG_SYS_INIT_SP_ADDR (CONFIG_SYS_INIT_RAM_ADDR
> > + CONFIG_SYS_INIT_SP_OFFSET)
> > +
> > +/* Ethernet */
> > +#ifdef CONFIG_FEC_MXC
> > +#define CONFIG_FEC_MXC_PHYADDR 0
> > +#define CONFIG_FEC_XCV_TYPE RMII
> > +#define CONFIG_MII
> > +#endif
> > +
> > +/* SPL */
> > +#ifdef CONFIG_SPL
> > +#include "imx6_spl.h"
> > +
> > +#ifdef CONFIG_SPL_BUILD
> > +#define CONFIG_SYS_FSL_USDHC_NUM 2
> > +
> > +#ifdef CONFIG_SYS_BOOT_EMMC
> > +
> > +/* Boot from eMMC */
> > +#define CONFIG_SYS_FSL_ESDHC_ADDR 1
> > +
> > +#else // CONFIG_SYS_BOOT_EMMC
> > +
> > +/* Boot from SD-card */
> > +# define CONFIG_SYS_FSL_ESDHC_ADDR 0
> > +
> > +#endif // CONFIG_SYS_BOOT_EMMC
> > +
> > +#endif // CONFIG_SPL_BUILD
> > +#endif // CONFIG_SPL
> > +
> > +#define CONFIG_EHCI_HCD_INIT_AFTER_RESET
> > +#define CONFIG_MXC_USB_PORTSC (PORT_PTS_UTMI |
> > PORT_PTS_PTW)
> > +#define CONFIG_MXC_USB_FLAGS 0
> > +#define CONFIG_USB_MAX_CONTROLLER_COUNT 1 /* Enabled USB
> > controller number */
> > +
> > +#endif /* __IMX6Q_ACC_H */
>
>
