[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5113ab6e-881b-fa6b-00c4-118b32be4a4c@topic.nl>
Date: Thu, 17 Aug 2023 11:02:43 +0200
From: Mike Looijmans <mike.looijmans@...ic.nl>
To: Alexandre Belloni <alexandre.belloni@...tlin.com>
CC: linux-rtc@...r.kernel.org, Alessandro Zummo <a.zummo@...ertech.it>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rtc: pcf85363: Allow to wake up system without IRQ
Met vriendelijke groet / kind regards,
Mike Looijmans
System Expert
TOPIC Embedded Products B.V.
Materiaalweg 4, 5681 RJ Best
The Netherlands
T: +31 (0) 499 33 69 69
E: mike.looijmans@...ic.nl
W: www.topic.nl
Please consider the environment before printing this e-mail
On 17-08-2023 10:17, Alexandre Belloni wrote:
> On 17/08/2023 07:42:25+0200, Mike Looijmans wrote:
>> On 16-08-2023 23:40, Alexandre Belloni wrote:
>>> Hello Mike,
>>>
>>> Sorry for the very late review, there is a small change that is needed:
>>
>> Better late than never
>>
>>>
>>> On 02/05/2023 07:54:58+0200, Mike Looijmans wrote:
>>>> When wakeup-source is set in the devicetree, set up the device for
>>>> using the output as interrupt instead of clock. This is similar to
>>>> how other RTC devices handle this.
>>>>
>>>> This allows the clock chip to turn on the board when wired to do
>>>> so in hardware.
>>>>
>>>> Signed-off-by: Mike Looijmans <mike.looijmans@...ic.nl>
>>>>
>>>> ---
>>>>
>>>> drivers/rtc/rtc-pcf85363.c | 17 ++++++++++-------
>>>> 1 file changed, 10 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/rtc/rtc-pcf85363.c b/drivers/rtc/rtc-pcf85363.c
>>>> index 8958eadf1c3e..b1543bcdc530 100644
>>>> --- a/drivers/rtc/rtc-pcf85363.c
>>>> +++ b/drivers/rtc/rtc-pcf85363.c
>>>> @@ -434,23 +434,26 @@ static int pcf85363_probe(struct i2c_client *client)
>>>> pcf85363->rtc->range_max = RTC_TIMESTAMP_END_2099;
>>>> clear_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
>>>> + if (client->irq > 0 || device_property_read_bool(&client->dev,
>>>> + "wakeup-source")) {
>>>> + regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
>>>> + regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
>>>> + PIN_IO_INTA_OUT, PIN_IO_INTAPM);
>>>> + device_init_wakeup(&client->dev, true);
>>>> + set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
>>>
>>> This should not be done when devm_request_threaded_irq fails.
>>
>> Yeah, easiest would be to just clear the feature flag when that happens.
>
> My comment was also for device_init_wakeup(&client->dev, true);. I think
> the easiest would be to move this block later on and set client->irq to
> 0 when devm_request_threaded_irq fails.
>
Ah, clear. That also properly handles things when IRQ fails but wakeup-source
was present. In table form:
IRQ wakeup-source Call "device_init_wakeup" etc.
N/A No No
N/A Yes Yes
OK No Yes
OK Yes Yes
FAIL No No
FAIL Yes Yes
We still have to program the registers before registering the IRQ I think.
Powered by blists - more mailing lists