[PATCH RFC/RFT 2/3] board: ti: j784s4: Initialize the ESM & PMIC ESM

[Andrew Halaney]

On Sat, Sep 07, 2024 at 11:12:05AM GMT, Kumar, Udit wrote:
> Hi Andrew,
> 
> On 9/7/2024 2:54 AM, Andrew Halaney wrote:
> > From: Keerthy <j-keerthy at ti.com>
> > 
> > Initialize the ESM & PMIC ESM. This allows things like
> > the watchdog to reset the board when tripped.
> > 
> > Signed-off-by: Keerthy <j-keerthy at ti.com>
> > Signed-off-by: Neha Malcom Francis <n-francis at ti.com>
> > Link: https://git.ti.com/cgit/ti-u-boot/ti-u-boot/commit/board/ti/j784s4/evm.c?h=ti-u-boot-2024.04&id=9d8b40958ce792808bc571d828197dbc2e7978d6
> > [halaney: add a line to the commit message, include header]
> > Signed-off-by: Andrew Halaney <ahalaney at redhat.com>
> > ---
> >   board/ti/j784s4/evm.c | 28 ++++++++++++++++++++++++++++
> >   1 file changed, 28 insertions(+)
> > 
> > diff --git a/board/ti/j784s4/evm.c b/board/ti/j784s4/evm.c
> > index 548dbd5925d..a0ef3a44536 100644
> > --- a/board/ti/j784s4/evm.c
> > +++ b/board/ti/j784s4/evm.c
> > @@ -7,6 +7,7 @@
> >    *
> >    */
> > +#include <dm.h>
> >   #include <efi_loader.h>
> >   #include <init.h>
> >   #include <spl.h>
> > @@ -72,4 +73,31 @@ int board_late_init(void)
> >   void spl_board_init(void)
> >   {
> > +	struct udevice *dev;
> > +	int ret;
> > +
> > +	if (IS_ENABLED(CONFIG_ESM_K3)) {
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm at 700000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm at 700000 failed: %d\n", ret);
> > +
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm at 40800000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm at 40800000 failed: %d\n", ret);
> > +
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm at 42080000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm at 42080000 failed: %d\n", ret);
> > +	}
> 
> Thanks for patch ,
> 
> IMO, if one of esm is failing then we can skip other esm probe,
> 
> > +
> > +	if (IS_ENABLED(CONFIG_ESM_PMIC)) {
> > +		ret = uclass_get_device_by_driver(UCLASS_MISC,
> > +						  DM_DRIVER_GET(pmic_esm),
> > +						  &dev);
> 
> Same here if esm probe is successful then only we should probe pmic_esm.
> 
> Reason is , if any of probe is failing functionality will not work and
> 
> there is no point to probe devices, if function is not working.

Any preference on how this is handled? I was going to do something like:

	if (IS_ENABLED(CONFIG_ESM_K3)) {
		ret = uclass_get_device_by_name(UCLASS_MISC, "esm at 700000",
						&dev);
		if (ret) {
			printf("MISC init for esm at 700000 failed: %d\n", ret);
			return;
		}

		ret = uclass_get_device_by_name(UCLASS_MISC, "esm at 40800000",
						&dev);
		...
		if (IS_ENABLED(CONFIG_ESM_PMIC)) {
			ret = uclass_get_device_by_driver(UCLASS_MISC,
							  DM_DRIVER_GET(pmic_esm),
							  &dev);
			if (ret) {
				printf("ESM PMIC init failed: %d\n", ret);
				return;
			}
		}
	}

but wasn't sure if a err_esm label and goto would be more in line with
what you imagined? Let me know otherwise I'll just submit this version
tomorrow or so.

Thanks,
Andrew