
Hello Rasmus,
Am 06.07.2020 um 22:01 schrieb Rasmus Villemoes:
I need access to registers other than just the timekeeping ones of the pcf2127, so I wanted to implement ->read8 and ->write8. But for testing these it appeared there was no convenient way to invoke those from the shell, so I also ended up adding such a command.
Also, it seemed more natural to provide array variants that can read or write several registers at once, so rtc_ops is expanded a bit.
Changes in v4:
Add CONFIG_CMD_RTC to sandbox defconfigs (new patch 10/11). Not quite sure exactly which it needed to be added to, but at least sandbox and sandbox_flattree showed CI failures.
Add Heiko's R-B to the 10 v3 patches (1-9 + 11), and Simon's R-B to 6/11.
Fix some checkpatch warnings - I don't really agree with most of
Sorry, I should have mentioned which warnings you should fix ...
them - e.g. having to add an empty line in
int foo = something(); if (foo < 0) return foo; return something_else(foo);
doesn't make the code more readable IMO.
You find this rule all over the source code in U-Boot...
The remaining checkpatch blurps are things I really don't think warrant "fixing", e.g. "WARNING: ENOSYS means 'invalid syscall nr' and nothing else" seems irrelevant in context of U-Boot, and in any case I've only copied existing practice. For "WARNING: please write a paragraph that describes the config symbol fully", that seems to be a false positive, there's certainly a full help text for CMD_RTC.
Yes, this is fine.
Just applied your patches now, there is one warning in patch
"test: dm: rtc: add test of dm_rtc_read, dm_rtc_write"
CHECK: Comparison to NULL could be written "emul" #218: FILE: test/dm/rtc.c:162: + ut_assert(emul != NULL);
I think, this should be fixed! But looking into the source file, there are more such lines, so I let this at it is... may this should be cleaned!
Thanks for your work!
bye, Heiko