Re: [PATCH 2/2] samsung: common: only load CROS_EC if enabled in config

Henrik Grimler
2023-02-13T07:23:30Z

Hi Simon,

On Wed, 2023-02-08 at 18:08 -0700, Simon Glass wrote:
> On Wed, 8 Feb 2023 at 12:04, Henrik Grimler <henrik@grimler.se>
> wrote:
> >
> > Otherwise non-ChromeOS samsung devices, like the odroid boards, are
> > stuck in a bootloop if CONFIG_CROS_EC is not enabled:
> >
> >     <...>
> >     MMC: SAMSUNG SDHCI: 2, EXYNOS DWMMC: 0
> >     Loading Environment from MMC... *** Warning - bad CRC, using
> > default environment
> >
> >     cros-ec communications failure -96
> >
> >     Please reset with Power+Refresh
> >
> >     Cannot init cros-ec device
> >     resetting ...
> >
> > Issue started after commit e44d7e73fe0d ("dm: core: Switch
> > uclass_*_device_err to use uclass_*_device_check").
> >
> > Signed-off-by: Henrik Grimler <henrik@grimler.se>
> > ---
> >  board/samsung/common/board.c | 27 ++++++++++++++-------------
> >  1 file changed, 14 insertions(+), 13 deletions(-)
> >
> > diff --git a/board/samsung/common/board.c
> > b/board/samsung/common/board.c
> > index 16ce5cb89253..78161b807bac 100644
> > --- a/board/samsung/common/board.c
> > +++ b/board/samsung/common/board.c
> > @@ -217,26 +217,27 @@ int checkboard(void)
> >  #ifdef CONFIG_BOARD_LATE_INIT
> >  int board_late_init(void)
> >  {
> > -       struct udevice *dev;
> > -       int ret;
> >         int mmcbootdev = get_boot_mmc_dev();
> >         char mmcbootdev_str[16];
> >
> > -       ret = uclass_first_device_err(UCLASS_CROS_EC, &dev);
> > -       if (ret && ret != -ENODEV) {
> > -               /* Force console on */
> > -               gd->flags &= ~GD_FLG_SILENT;
> > -
> > -               printf("cros-ec communications failure %d\n", ret);
> > -               puts("\nPlease reset with Power+Refresh\n\n");
> > -               panic("Cannot init cros-ec device");
> > -               return -1;
> > -       }
> > -
> >         printf("Boot device: MMC(%u)\n", mmcbootdev);
> >         sprintf(mmcbootdev_str, "%u", mmcbootdev);
> >         env_set("mmcbootdev", mmcbootdev_str);
> >
> > +       if (IS_ENABLED(CONFIG_CROS_EC)) {
> > +               struct udevice *dev;
> > +               int ret = uclass_first_device_err(UCLASS_CROS_EC,
> > &dev);
> > +               if (ret && ret != -ENODEV) {
> > +                       /* Force console on */
> > +                       gd->flags &= ~GD_FLG_SILENT;
> > +
> > +                       printf("cros-ec communications failure
> > %d\n", ret);
> > +                       puts("\nPlease reset with
> > Power+Refresh\n\n");
> > +                       panic("Cannot init cros-ec device");
> > +                       return -1;
> > +               }
> > +       }
> > +
> >         return 0;
> >  }
> >  #endif
> > --
> > 2.30.2
> >
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> Another option would be to change the check for -ENODEV to also check
> for -EEPFNOSUPPORT (missing uclass).

Thanks for reviewing! Checking for that return code works as well, and
would require a smaller patch, I will send a v2 and use that suggestion
instead.

Best regards,
Henrik Grimler