Hi
> -----Original Message-----
> From: Simon Glass <[email protected]>
> Sent: Tuesday, December 28, 2021 2:03 PM
> To: Gaurav Jain <[email protected]>
> Cc: U-Boot Mailing List <[email protected]>; Stefano Babic
> <[email protected]>; Fabio Estevam <[email protected]>; Peng Fan
> <[email protected]>; Priyanka Jain <[email protected]>; Ye Li
> <[email protected]>; Horia Geanta <[email protected]>; Ji Luo
> <[email protected]>; Franck Lenormand <[email protected]>; Silvano Di
> Ninno <[email protected]>; Sahil Malhotra <[email protected]>;
> Pankaj Gupta <[email protected]>; Varun Sethi <[email protected]>; dl-
> uboot-imx <[email protected]>; Shengzhou Liu <[email protected]>;
> Mingkai Hu <[email protected]>; Rajesh Bhagat <[email protected]>;
> Meenakshi Aggarwal <[email protected]>; Wasim Khan
> <[email protected]>; Alison Wang <[email protected]>; Pramod
> Kumar <[email protected]>; Andy Tang <[email protected]>;
> Adrian Alonso <[email protected]>; Vladimir Oltean <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
> > Reviewed-by: Ye Li <[email protected]>
> > ---
> > 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
