[EXT] Re: [PATCH v7 05/15] i.MX6: Enable Job ring driver model in U-Boot.

[Gaurav Jain]

Hi

> -----Original Message-----
> From: Simon Glass <sjg at chromium.org>
> Sent: Tuesday, December 28, 2021 2:03 PM
> To: Gaurav Jain <gaurav.jain at nxp.com>
> Cc: U-Boot Mailing List <u-boot at lists.denx.de>; Stefano Babic
> <sbabic at denx.de>; Fabio Estevam <festevam at gmail.com>; Peng Fan
> <peng.fan at nxp.com>; Priyanka Jain <priyanka.jain at nxp.com>; Ye Li
> <ye.li at nxp.com>; Horia Geanta <horia.geanta at nxp.com>; Ji Luo
> <ji.luo at nxp.com>; Franck Lenormand <franck.lenormand at nxp.com>; Silvano Di
> Ninno <silvano.dininno at nxp.com>; Sahil Malhotra <sahil.malhotra at nxp.com>;
> Pankaj Gupta <pankaj.gupta at nxp.com>; Varun Sethi <V.Sethi at nxp.com>; dl-
> uboot-imx <uboot-imx at nxp.com>; Shengzhou Liu <shengzhou.liu at nxp.com>;
> Mingkai Hu <mingkai.hu at nxp.com>; Rajesh Bhagat <rajesh.bhagat at nxp.com>;
> Meenakshi Aggarwal <meenakshi.aggarwal at nxp.com>; Wasim Khan
> <wasim.khan at nxp.com>; Alison Wang <alison.wang at nxp.com>; Pramod
> Kumar <pramod.kumar_1 at nxp.com>; Andy Tang <andy.tang at nxp.com>;
> Adrian Alonso <adrian.alonso at nxp.com>; Vladimir Oltean <olteanv at gmail.com>
> Subject: [EXT] Re: [PATCH v7 05/15] i.MX6: Enable Job ring driver model in U-
> Boot.
> 
> Caution: EXT Email
> 
> Hi,
> 
> On Tue, 7 Dec 2021 at 00:42, Gaurav Jain <gaurav.jain at nxp.com> wrote:
> >
> > i.MX6,i.MX6SX,i.MX6UL - added support for JR driver model.
> >
> > removed sec_init() call, sec is initialized based on job ring
> > information processed from device tree.
> >
> > Signed-off-by: Gaurav Jain <gaurav.jain at nxp.com>
> > Reviewed-by: Ye Li <ye.li at nxp.com>
> > ---
> >  arch/arm/mach-imx/mx6/Kconfig | 15 +++++++++++++++
> >  arch/arm/mach-imx/mx6/soc.c   | 12 ++++++++----
> >  2 files changed, 23 insertions(+), 4 deletions(-)
> >
> 
> Tiny nit...we know this is U-Boot so you don't really need
Ok..

> > diff --git a/arch/arm/mach-imx/mx6/Kconfig
> > b/arch/arm/mach-imx/mx6/Kconfig index 62de942a32..8ccb3b6e35 100644
> > --- a/arch/arm/mach-imx/mx6/Kconfig
> > +++ b/arch/arm/mach-imx/mx6/Kconfig
> > @@ -354,6 +354,9 @@ config TARGET_MX6SABREAUTO
> >         select DM_THERMAL
> >         select SUPPORT_SPL
> >         imply CMD_DM
> > +       select FSL_CAAM
> > +       select MISC
> > +       select ARCH_MISC_INIT
> >
> >  config TARGET_MX6SABRESD
> >         bool "mx6sabresd"
> > @@ -364,6 +367,9 @@ config TARGET_MX6SABRESD
> >         select DM_THERMAL
> >         select SUPPORT_SPL
> >         imply CMD_DM
> > +       select FSL_CAAM
> > +       select MISC
> > +       select ARCH_MISC_INIT
> >
> >  config TARGET_MX6SLEVK
> >         bool "mx6slevk"
> > @@ -386,6 +392,9 @@ config TARGET_MX6SXSABRESD
> >         select DM
> >         select DM_THERMAL
> >         select SUPPORT_SPL
> > +       select FSL_CAAM
> > +       select MISC
> > +       select ARCH_MISC_INIT
> >
> >  config TARGET_MX6SXSABREAUTO
> >         bool "mx6sxsabreauto"
> > @@ -404,6 +413,9 @@ config TARGET_MX6UL_9X9_EVK
> >         select DM_THERMAL
> >         select SUPPORT_SPL
> >         imply CMD_DM
> > +       select FSL_CAAM
> > +       select MISC
> > +       select ARCH_MISC_INIT
> >
> >  config TARGET_MX6UL_14X14_EVK
> >         bool "mx6ul_14x14_evk"
> > @@ -413,6 +425,9 @@ config TARGET_MX6UL_14X14_EVK
> >         select DM_THERMAL
> >         select SUPPORT_SPL
> >         imply CMD_DM
> > +       select FSL_CAAM
> > +       select MISC
> > +       select ARCH_MISC_INIT
> >
> >  config TARGET_MX6UL_ENGICAM
> >         bool "Support Engicam GEAM6UL/Is.IoT"
> > diff --git a/arch/arm/mach-imx/mx6/soc.c b/arch/arm/mach-imx/mx6/soc.c
> > index aacfc854a2..fa6c3778bb 100644
> > --- a/arch/arm/mach-imx/mx6/soc.c
> > +++ b/arch/arm/mach-imx/mx6/soc.c
> > @@ -4,6 +4,7 @@
> >   * Sascha Hauer, Pengutronix
> >   *
> >   * (C) Copyright 2009 Freescale Semiconductor, Inc.
> > + * Copyright 2021 NXP
> >   */
> >
> >  #include <common.h>
> > @@ -23,7 +24,6 @@
> >  #include <asm/arch/mxc_hdmi.h>
> >  #include <asm/arch/crm_regs.h>
> >  #include <dm.h>
> > -#include <fsl_sec.h>
> >  #include <imx_thermal.h>
> >  #include <mmc.h>
> >
> > @@ -734,9 +734,13 @@ static void setup_serial_number(void)
> >
> >  int arch_misc_init(void)
> >  {
> > -#ifdef CONFIG_FSL_CAAM
> > -       sec_init();
> > -#endif
> > +       struct udevice *dev;
> > +       int ret;
> > +
> > +       ret = uclass_get_device_by_driver(UCLASS_MISC,
> > + DM_DRIVER_GET(caam_jr), &dev);
> 
> You could use a board driver perhaps, to specify the phandle of the device you
> want to access. Or, if you used a separate uclass, you could just get the first
> device.
Probe is also called from this API which is satisfying driver requirement.

> 
> > +       if (ret)
> > +               printf("Failed to initialize %s: %d\n", dev->name,
> > + ret);
> 
> Shouldn't you return the error here?
It is done intentionally as we do not want to report error but to print the fail log only.

Regards
Gaurav Jain
> 
> > +
> >         setup_serial_number();
> >         return 0;
> >  }
> > --
> > 2.17.1
> >
> 
> Regards,
> Simon