Hello,
On 28/10/2022 17:54:00-0700, Guenter Roeck wrote:
> RTC chips on some older Chromebooks can only handle alarms less than 24
> hours in the future. Attempts to set an alarm beyond that range fails.
> The most severe impact of this limitation is that suspend requests fail
> if alarmtimer_suspend() tries to set an alarm for more than 24 hours
> in the future.
>
> Try to set the real-time alarm to just below 24 hours if setting it to
> a larger value fails to work around the problem. While not perfect, it
> is better than just failing the call. A similar workaround is already
> implemented in the rtc-tps6586x driver.
I'm not super convinced this is actually better than failing the call
because your are implementing policy in the driver which is bad from a
user point of view. It would be way better to return -ERANGE and let
userspace select a better alarm time.
Do you have to know in advance which are the "older" chromebooks that
are affected?
>
> 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.
>
> Cc: Brian Norris <briannorris@xxxxxxxxxxxx>
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> drivers/rtc/rtc-cros-ec.c | 35 ++++++++++++++++++++---------------
> 1 file changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
> index 887f5193e253..a3ec066d8066 100644
> --- a/drivers/rtc/rtc-cros-ec.c
> +++ b/drivers/rtc/rtc-cros-ec.c
> @@ -14,6 +14,8 @@
>
> #define DRV_NAME "cros-ec-rtc"
>
> +#define SECS_PER_DAY (24 * 60 * 60)
> +
> /**
> * struct cros_ec_rtc - Driver data for EC RTC
> *
> @@ -43,13 +45,8 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
> msg.msg.insize = sizeof(msg.data);
>
> ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
> - if (ret < 0) {
> - dev_err(cros_ec->dev,
> - "error getting %s from EC: %d\n",
> - command == EC_CMD_RTC_GET_VALUE ? "time" : "alarm",
> - ret);
> + if (ret < 0)
> return ret;
> - }
>
> *response = msg.data.time;
>
> @@ -59,7 +56,7 @@ static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
> static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
> u32 param)
> {
> - int ret = 0;
> + int ret;
> struct {
> struct cros_ec_command msg;
> struct ec_response_rtc data;
> @@ -71,13 +68,8 @@ static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
> msg.data.time = param;
>
> ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
> - if (ret < 0) {
> - dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
> - command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
> - ret);
> + if (ret < 0)
> return ret;
> - }
> -
> return 0;
> }
>
> @@ -190,8 +182,21 @@ static int cros_ec_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
>
> ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM, alarm_offset);
> if (ret < 0) {
> - dev_err(dev, "error setting alarm: %d\n", ret);
> - return ret;
> + if (ret == -EINVAL && alarm_offset >= SECS_PER_DAY) {
> + /*
> + * RTC chips on some older Chromebooks can only handle
> + * alarms up to 24h in the future. Try to set an alarm
> + * below that limit to avoid suspend failures.
> + */
> + ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
> + SECS_PER_DAY - 1);
> + }
> +
> + if (ret < 0) {
> + dev_err(dev, "error setting alarm in %u seconds: %d\n",
> + alarm_offset, ret);
> + return ret;
> + }
> }
>
> return 0;
> --
> 2.36.2
>
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
