From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-oa1-f54.google.com (mail-oa1-f54.google.com [209.85.160.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 5E2B08F76 for ; Mon, 31 Oct 2022 23:07:52 +0000 (UTC) Received: by mail-oa1-f54.google.com with SMTP id 586e51a60fabf-1322d768ba7so15121017fac.5 for ; Mon, 31 Oct 2022 16:07:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:from:to:cc:subject:date:message-id :reply-to; bh=iIKNfIS9DJZ+OreDnkhjQEbBfQ5lKHnR+23wu5eNP+4=; b=CFRw2KUOfNI618M5wfGr3VzUsoE6+rWut09ICvW0xZsQBJAlb5iRyuOIVZyW+cxhPN FiLdQd+HvwAGMBpFpKXdwkY3UgbdJU8qQlttbtfn0+pDpMzG4GHywbVRKilsOiPyrXxn jcjICLAvDvkydW2h6PGFJWcSFhkyv2KSUz/YaFzZlaNzZFV56/mIRyP7ZTnwk64y4n9q Lv/zcPwIMiaDw5ylE2aXisFQ4lVq5UAkb7fCyQ0f6vcACiTcAaNvyZCiGr2a/snQEkUu PHpymBsb+z8NIseqAyoBNWRvJ+pKanJFHgU007rivRmdTB0G5MNZ1TIjvu+SQCf5/HGt az/Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:sender:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=iIKNfIS9DJZ+OreDnkhjQEbBfQ5lKHnR+23wu5eNP+4=; b=JsVzuO6GFOKbKtUCHw+It12iwJg+kcJOiY3YQjiCap3LYAq0JX995Wf5QzJahtZigj HDd0X0Y4FaW6ffthfB9pYhh09oTmy8w32riyXosobEA2UDtQIHb3E9NqdSBa1k0Gjcsu xk4gNBES9iQIDYtgv8w/IEAmG5caQ/c8kJcR+G5q6k5/ayozYcaTsC87vrP3M6zEdYW/ GCDf1uc+pPDBKQdWRZuUUyzzqdYygzmQmh/Cm26zkXXzNopYAjVP619cl171e2CcKIJw /7175DioVnyqsT395Ce3YGv4JmsSWnfqMYw0RZzVwjrNXe0x/U62SrSEzslx4YKzQi4J MblQ== X-Gm-Message-State: ACrzQf3Yt8Jvw4HrzsICgLST8au5RSbbbCbvMVlbsnODarbFeVo3ehOU iuN9nFusvRiD4UFOLS3/f00= X-Google-Smtp-Source: AMsMyM4jMrEtQxRZyChRGb9EEzYfviGe38pgCfAYpjEgP7S2SnvOLdFtOazIJKW1VuMSGW3gIp9RUQ== X-Received: by 2002:a05:6870:5894:b0:131:b25b:413e with SMTP id be20-20020a056870589400b00131b25b413emr18653062oab.192.1667257671404; Mon, 31 Oct 2022 16:07:51 -0700 (PDT) Received: from server.roeck-us.net ([2600:1700:e321:62f0:329c:23ff:fee3:9d7c]) by smtp.gmail.com with ESMTPSA id a13-20020a4ad5cd000000b0049427725e62sm2844817oot.19.2022.10.31.16.07.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 31 Oct 2022 16:07:50 -0700 (PDT) Sender: Guenter Roeck Date: Mon, 31 Oct 2022 16:07:49 -0700 From: Guenter Roeck To: Alexandre Belloni Cc: Alessandro Zummo , Benson Leung , linux-rtc@vger.kernel.org, chrome-platform@lists.linux.dev, linux-kernel@vger.kernel.org, Brian Norris Subject: Re: [PATCH] rtc: cros-ec: Limit RTC alarm range if needed Message-ID: <20221031230749.GB2082109@roeck-us.net> References: <20221029005400.2712577-1-linux@roeck-us.net> <20221031181913.GA3841664@roeck-us.net> Precedence: bulk X-Mailing-List: chrome-platform@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Mon, Oct 31, 2022 at 11:14:23PM +0100, Alexandre Belloni wrote: > On 31/10/2022 11:19:13-0700, Guenter Roeck wrote: > > On Mon, Oct 31, 2022 at 06:10:53PM +0100, Alexandre Belloni wrote: > > > 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. > > > > The failing call is from alarmtimer_suspend() which is called during suspend. > > It is not from userspace, and userspace has no chance to intervene. > > > > It is also not just one userspace application which could request a large > > timeout, it is a variety of userspace applications, and not all of them are > > written by Google. Some are Android applications. I don't see how it would be > > realistic to expect all such applications to fix their code (if that is even > > possible - there might be an application which called sleep(100000) or > > something equivalent, which works just fine as long as the system is not > > suspended. > > > > > Do you have to know in advance which are the "older" chromebooks that > > > are affected? > > > > Not sure I understand the question. Technically we know, but the cros_ec > > rtc driver doesn't know because the EC doesn't have an API to report the > > maximum timeout to the Linux driver. Even if that existed, it would not > > help because the rtc API only supports absolute maximum clock values, > > not clock offsets relative to the current time. So ultimately there is no > > means for an RTC driver to tell the maximum possible alarm timer offset to > > the RTC subsystem, and there is no means for a user such as > > alarmtimer_suspend() to obtain the maximum time offset. Does that answer > > your question ? > > Yes, my question was missing a few words, sorry I wanted to know if you > had *a way* to know. > See below. It is doable, but there is no real good solution, or at least I don't see one right now. > > > > On a side note, I tried an alternate implementation by adding a retry into > > alarmtimer_suspend(), where it would request a smaller timeout if the > > requested timeout failed. I did not pursue/submit this since it seemed > > hacky. To solve that problem, I'd rather discuss extending the RTC API > > to provide a maximum offset to its users. Such a solution would probably > > be desirable, but that it more longer term and would not solve the > > immediate problem. > > Yes, this is what I was aiming for. This is something that is indeed > missing in the RTC API and that I already thought about. But indeed, it > would be great to have a way to set the alarm range separately from the > time keeping range. This would indeed have to be a range relative to the > current time. > > alarmtimer_suspend() can then get the allowed alarm range for the RTC, > and set the alarm to max(alarm range, timer value) and loop until the > timer has expired. Once we have this API, userspace can do the same. > > I guess that ultimately, this doesn't help your driver unless you are > wanting to wakeup all the chromebooks at least once a day regardless of > their EC. That is a no-go. It would reduce battery lifetime on all Chromebooks, including those not affected by the problem (that is, almost all of them). To implement reporting the maximum supported offset, I'd probably either try to identify affected Chromebooks using devicetree information, or by sending am alarm request > 24h in the future in the probe function and setting the maximum offset just below 24h if that request fails. We'd have to discuss the best approach internally. Either case, that doesn't help with the short term problem that we have to solve now and that can be backported to older kernels. It also won't help userspace - userspace alarm requests, as Brian has pointed out, are separate from limits supported by the RTC hardware. We can not change the API for CLOCK_xxx_ALARM to userspace, and doing so would not make sense anyway since it works just fine as long as the system isn't suspended. Besides, changing alarmtimer_suspend() as you suggest above would solve the problem for userspace, so I don't see a need for a userspace API/ABI change unless I am missing something. Thanks, Guenter