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

[Kumar, Udit]

On 9/10/2024 10:52 PM, Andrew Halaney wrote:
> 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;
> 		}


What I have in mind kind of having string array with name , as 
esm at 700000, esm at 40800000 and esm at 42080000

if (IS_ENABLED(CONFIG_ESM_K3)) {

     for(i = 0 ; i< 2; i++) {
             ret = uclass_get_device_by_name(UCLASS_MISC, STRING[i],
					&dev);
            if(ret) {
                    printf("MISC init for %s failed: %d\n", ret, STRING[i]);
                    break;
             }
     }

}

if (IS_ENABLED(CONFIG_ESM_PMIC) && ret == 0) {
			ret = uclass_get_device_by_driver(UCLASS_MISC,
							  DM_DRIVER_GET(pmic_esm),
							  &dev);
....
}
...
....

> 		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
>