Samsung / TizenRT

TizenRT is a lightweight RTOS-based platform to support low-end IoT devices
Apache License 2.0
561 stars 560 forks source link

os/arch/arm/src/amebasmart: Revise BSP to follow PM new structure #6247

Closed edwakuwaku closed 1 week ago

edwakuwaku commented 2 weeks ago
ritesh55555 commented 1 week ago

hi @edwakuwaku , there are some comments related to this PR in my another PR (https://github.com/Samsung/TizenRT/pull/6259) , which i have taken patch from here , Can you check it?

edwakuwaku commented 1 week ago

hi @edwakuwaku , there are some comments related to this PR in my another PR (#6259) , which i have taken patch from here , Can you check it?

Hi @ritesh55555 , as I have discussed with @ewoodev , he suggested another way such as store domain id for each peripheral driver, so that we don't need to invoke pm_domain_register() every time. It is not a logic error. But, I think current implementation is also fine. Since the interface is able for us to retrieve the domain id according to each driver.

sunghan-chang commented 1 week ago

@edwakuwaku As far as I know, our concept is

  1. register new domain (get an integer value with return)
  2. control pm using the integer value instead of domain name string

But you use a string at every pm function instead of an integer value. It's not good at performance. Could you let use know if you feel difficult to use an integer?

@ewoodev @gSahitya-samsung Please confirm above.

gSahitya-samsung commented 1 week ago

@edwakuwaku As far as I know, our concept is

  1. register new domain (get an integer value with return)
  2. control pm using the integer value instead of domain name string

But you use a string at every pm function instead of an integer value. It's not good at performance. Could you let use know if you feel difficult to use an integer?

@ewoodev @gSahitya-samsung Please confirm above.

@sunghan-chang You are right, we need to use domain_id instead of domain_name to control pm operations for performance improvement. @edwakuwaku It is advisable to register domain once and use corresponding domain_id multiple times.

edwakuwaku commented 1 week ago

@edwakuwaku As far as I know, our concept is

  1. register new domain (get an integer value with return)
  2. control pm using the integer value instead of domain name string

But you use a string at every pm function instead of an integer value. It's not good at performance. Could you let use know if you feel difficult to use an integer?

@ewoodev @gSahitya-samsung Please confirm above.

No, there is no difficulties to apply int domain_id. Initially, I think that it is unnecessary to store the domain_id since the pm_domain_register can search and return accordingly. But as you mentioned, it might cause performance drop since we have to loop through the pm domain buffer everytime. I have refine it accordingly.

sunghan-chang commented 1 week ago

@edwakuwaku Could you check a build break?

CC:  chip/amebasmart_pmhelpers.c
chip/amebasmart_pmhelpers.c: In function 'bsp_pm_domain_register':
chip/amebasmart_pmhelpers.c:130:6: error: 'domain' undeclared (first use in this function)
  if (domain < 0) {
      ^~~~~~