New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 758245 link

Starred by 1 user

Issue metadata

Status: Closed
Owner:
Last visit 15 days ago
Closed: Oct 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

ec: cleanup rtc code for different chips

Project Member Reported by philipchen@chromium.org, Aug 23 2017

Issue description

Recently we added code to support rtc console/host commands for stm32:
https://chromium-review.googlesource.com/#/c/chromiumos/platform/ec/+/627637/

But as the discussion in that CL, we should change the naming / API to be more consistent with npcx (eg. use system_set_rtc()).

Moreover, we can move some common code (eg. host command implementation) to common/rtc.c and delete the duplicate npcx + stm32 copies.
 
Owner: gwink@chromium.org
Status: Assigned (was: Untriaged)
It looks like the smt32/rtc functions, such as rtc_read_sec, referred to in this bug have been removed. Does this bug still make sense?
It's still around.
e.g. system_set_rtc() in npcx is equivalent to rtc_set() in stm32.

But given the HW difference between stm32 and npcx rtc, I don't know how much we can factor out to common code.

rtc_set v.s. system_set_rtc might be the only function with a common signature. Most (all) others take different parameters as far as I can tell. Maybe regularizing the function names still make sense. Sharing the code looks like an iffy proposition.
Yeah, I agree.
Actually, even renaming the functions gets into a nasty tangle:

stm32/clock_f.h includes include/system.h, which has the definitions for the system_rtc* function. Renaming the stm32 functions to match npcx causes name conflicts, for the most part with functions that have different signatures.

It looks like we can't do this without major refactoring. I think it's beyond the scope of this bug. 
ok, thanks for looking into this.
Feel free to close this bug.
Status: Closed (was: Assigned)

Sign in to add a comment