diff mbox series

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

Message ID 20240906-j784s4-esm-enable-v1-2-b83b17d5a744@redhat.com
State RFC
Delegated to: Tom Rini
Headers show
Series k3-j784s4-r5-evm: Enable ESMs and related PMIC | expand

Commit Message

Andrew Halaney Sept. 6, 2024, 9:24 p.m. UTC 
From: Keerthy <j-keerthy@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@ti.com>
Signed-off-by: Neha Malcom Francis <n-francis@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@redhat.com>
---
 board/ti/j784s4/evm.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

Comments

Kumar, Udit Sept. 7, 2024, 5:42 a.m. UTC | #1 
Hi Andrew,

On 9/7/2024 2:54 AM, Andrew Halaney wrote:
> From: Keerthy <j-keerthy@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@ti.com>
> Signed-off-by: Neha Malcom Francis <n-francis@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@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@700000",
> +						&dev);
> +		if (ret)
> +			printf("MISC init for esm@700000 failed: %d\n", ret);
> +
> +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@40800000",
> +						&dev);
> +		if (ret)
> +			printf("MISC init for esm@40800000 failed: %d\n", ret);
> +
> +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@42080000",
> +						&dev);
> +		if (ret)
> +			printf("MISC init for esm@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.

> +		if (ret)
> +			printf("ESM PMIC init failed: %d\n", ret);
> +	}
>   }
>
Andrew Halaney Sept. 10, 2024, 5:22 p.m. UTC | #2 
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@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@ti.com>
> > Signed-off-by: Neha Malcom Francis <n-francis@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@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@700000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm@700000 failed: %d\n", ret);
> > +
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@40800000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm@40800000 failed: %d\n", ret);
> > +
> > +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@42080000",
> > +						&dev);
> > +		if (ret)
> > +			printf("MISC init for esm@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@700000",
						&dev);
		if (ret) {
			printf("MISC init for esm@700000 failed: %d\n", ret);
			return;
		}

		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@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
Kumar, Udit Sept. 11, 2024, 4:18 a.m. UTC | #3 
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@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@ti.com>
>>> Signed-off-by: Neha Malcom Francis <n-francis@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@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@700000",
>>> +						&dev);
>>> +		if (ret)
>>> +			printf("MISC init for esm@700000 failed: %d\n", ret);
>>> +
>>> +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@40800000",
>>> +						&dev);
>>> +		if (ret)
>>> +			printf("MISC init for esm@40800000 failed: %d\n", ret);
>>> +
>>> +		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@42080000",
>>> +						&dev);
>>> +		if (ret)
>>> +			printf("MISC init for esm@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@700000",
> 						&dev);
> 		if (ret) {
> 			printf("MISC init for esm@700000 failed: %d\n", ret);
> 			return;
> 		}


What I have in mind kind of having string array with name , as 
esm@700000, esm@40800000 and esm@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@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
>
diff mbox series

Patch

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@700000",
+						&dev);
+		if (ret)
+			printf("MISC init for esm@700000 failed: %d\n", ret);
+
+		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@40800000",
+						&dev);
+		if (ret)
+			printf("MISC init for esm@40800000 failed: %d\n", ret);
+
+		ret = uclass_get_device_by_name(UCLASS_MISC, "esm@42080000",
+						&dev);
+		if (ret)
+			printf("MISC init for esm@42080000 failed: %d\n", ret);
+	}
+
+	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);
+	}
 }