linux-kernel - Re: [PATCH] rtc: pcf85363: Allow to wake up system without IRQ

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ec2d7ef3-3dcc-aebb-1a4f-c2e01a09f750@topic.nl>
Date:   Thu, 17 Aug 2023 07:42:25 +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

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.

> 
>> +	}
>> +
>>   	if (client->irq > 0) {
>>   		unsigned long irqflags = IRQF_TRIGGER_LOW;
>>   
>>   		if (dev_fwnode(&client->dev))
>>   			irqflags = 0;
>> -
>> -		regmap_write(pcf85363->regmap, CTRL_FLAGS, 0);
>> -		regmap_update_bits(pcf85363->regmap, CTRL_PIN_IO,
>> -				   PIN_IO_INTA_OUT, PIN_IO_INTAPM);
>>   		ret = devm_request_threaded_irq(&client->dev, client->irq,
>>   						NULL, pcf85363_rtc_handle_irq,
>>   						irqflags | IRQF_ONESHOT,
>>   						"pcf85363", client);
>>   		if (ret)
>> -			dev_warn(&client->dev, "unable to request IRQ, alarms disabled\n");
>> -		else
>> -			set_bit(RTC_FEATURE_ALARM, pcf85363->rtc->features);
>> +			dev_warn(&client->dev, "unable to request IRQ, alarm not functional\n");
>>   	}
>>   
>>   	ret = devm_rtc_register_device(pcf85363->rtc);
>> -- 
>> 2.17.1
>>
>>

-- 
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