On Mon, Oct 31, 2022 at 11:26:44AM +0800, Tzung-Bi Shih wrote: > On Fri, Oct 28, 2022 at 05:54:00PM -0700, Guenter Roeck wrote: > > Drop error messages in cros_ec_rtc_get() and cros_ec_rtc_set() since the > > calling code also logs an error and to avoid spurious error messages if > > setting the alarm ultimately succeeds. > > It only retries for cros_ec_rtc_set(). cros_ec_rtc_get() doesn't emit > spurious error messages. All of cros_ec_rtc_get()'s callers were also logging the same message. So it was redundant. I think the general strategy here was to log the error(s) in callers (last point before we "exit" the driver), to have the best chance at context-relevant error messages, or ignoring them where proper. It's already a bit dubious to log kernel messages at all in response to normal sysfs operations. We probably want them in some cases, when things are particularly unexpected, but it shouldn't be a regular occurrence, and we certainly don't need *two* log lines for each error. Technically, if one wants to be super-nitpicky about one purpose per patch, then maybe a patch to trim the logging, and a patch to fix the alarm range issues... ...but I think that would be a little silly, and perhaps even harmful. They are related concerns that should be patched (and probably backported) together. Brian
