[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay' URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-589976655 @klmchp and @patacongo , we don't need add any config here: if some chipset decide to use arch_timer.c, the chipset just need remove up_delay.c from it's Make.defs. I added arch_timer.c/arch_rtc.c/arch_alarm.c just because NuttX define two similar timer/rtc inteface: 1.One set come from include/nuttx/arch.h which all start with up_ 2.One set come from include/nuttx/timers/[oneshot.h|timer.h|rtc.h] It doesn't make sense to let developer learn and implement two interface for one hardware. Since the driver interface is bigger(multiple instance, alarm, /dev/xxx ...) than arch interface, it make sense to implement up_ timer/rtc API on top of driver interface, then the developer just need write timer/rtc/oneshot driver and let arch_timer.c/arch_rtc.c/arch_alarm.c do the conversion(remove up_delay.c from Make.defs since the implementation provide by arch_timer.c now). Of course, the devloper can still implement up_xxx directly if they like. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay' URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-590031621 But, arch code don't need reference any specific config for arch_timer.c/arch_rtc.c/arch_oneshot.c since this is design decision whether to use the implemetnetation inside these files: 1.Provide up_ timer/rtc API implmentation in arch/ or 2.Implement timer/oneshot/rtc driver and enable arch_timer.c/arch_rtc.c/arch_oneshot.c from defconfig I don't think the developer want to implement both and let the user select from defconfig. We don't need apply the change provided by @klmchp at all. @klmchp if you want to use arch_timer.c, please: 1.Remove up_delay.c from your chipset Make.defs directly 2.Enable CONFIG_ALARM_ARCH in your defconfig This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay' URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-590074752 Not a whole architecture, different chipset in the same architecture can select the different approach because each chipset has own Make.defs and can include/exclude up_delay.c as needed, but the same chipset must share the same decision. The code under drivers/timers/arch_*.c just want to simplify the chipset developer work, supporting both approach just make their work hard than before without any benefit. So I don't think the last issue is a real limitation, the developer just need decide which method he want to use and forget another one totally before writing the code. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-590080225
> @patacongo @xiaoxiang781216 , also find multi definition of
up_rtc_getdatetime, up_rtc_settime and so on. I list the code from
nuttx/drivers/timers/arch_rtc.c for reference.
> `int up_rtc_getdatetime(FAR struct tm *tp)
> {
> if (g_rtc_lower != NULL)
> {
> struct rtc_time rtctime;
>
> ```
> ret = g_rtc_lower->ops->rdtime(g_rtc_lower, &rtctime);
> ```
>
> There are many the implementations of rdtime in different platform.
> In arch/arm/src/stm32/stm32_rtc_lowerhalf.c
>
> static int stm32_rdtime(FAR struct rtc_lowerhalf_s *lower,
> FAR struct rtc_time *rtctime)
> {
> #if defined(CONFIG_RTC_DATETIME)
> return **up_rtc_getdatetime**((FAR struct tm *)rtctime);
> `
> BR
Yes, all code inside drivers/timer/arch_*.c is to implement up_timer_
up_rtc_ on top of timer/oneshot/rtc driver interface. As I said before, if you
want to use these files:
1.Remove up_rtc and up_timer_ up_alarm_ from your chipset
2.Implement the standard timer/oneshot/rtc driver interface
3.Let these file implement these function for you
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay' URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-590082027 > > Not a whole architecture, different chipset in the same architecture can select the different approach because each chipset has own Make.defs and can include/exclude up_delay.c as needed, but the same chipset must share the same decision. > > Isn't that just an abritrary limitation? Why should implementation on any architecture have this option? Why limit it to just certain, pre-determined architectures? So you want to add an option to let user select? 1.Use up_rtc_*, up_timer_* and up_alarm_* in arch/ or 2.Use up_rtc_*, up_timer_* and up_alarm_* in drivers/timers/ There isn't difference outside the implementatin, the user get the same functionality with either selection. Actually, up_timer_/up_rtc_/up_alarm_ API is duplicated with oneshot_operations_s/rtc_ops_s/timer_ops_s and make the chpset developer write many duplication without any benefit. The best solution is: 1.Remove up_timer_, up_alarm_, up_rtc from nuttx/arch.h 2.Modify the code under sched/ to call oneshot_operations_s/rtc_ops_s/timer_ops_s instead of up_timer_, up_alarm_, up_rtc. 3.Developer just need implement oneshot_operations_s/rtc_ops_s/timer_ops_s for their hardware This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay'
xiaoxiang781216 commented on issue #353: multiple definition of `up_mdelay' URL: https://github.com/apache/incubator-nuttx/issues/353#issuecomment-590087053 Ok, maybe it is better to change CONFIG_ARCH_[TIMER|RTC|ALARM] to CONFIG_ARCH_HAVE_[TIMER|RTC|ALARM] without prompt string, than each chipset could select these option from Kconfig base on their implementation decision. So it is impossible to make the wrong selection from defconfig. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
