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

[Mike Looijmans]

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@xxxxxxxx>
> >
> > ---
> >
> >   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@xxxxxxxx
W: www.topic.nl