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

[Alexandre Belloni <alexandre.belloni@bootlin.com>]

Hello Mike,

Sorry for the very late review, there is a small change that is needed:

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@topic.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.

> +	}
> +
>  	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
> 
> 
> 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@topicproducts.com
> W: www.topic.nl
> 
> Please consider the environment before printing this e-mail

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com