public inbox for linux-arm-kernel@lists.infradead.org 
 help / color / mirror / Atom feed
From: arno@natisbad•org (Arnaud Ebalard)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCHv0 3/3] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver
Date: Fri, 07 Nov 2014 10:37:01 +0100	[thread overview]
Message-ID: <878ujnnygy.fsf@natisbad.org> (raw)
In-Reply-To: 20141107075843.GD10432@pengutronix.de

Hi Uwe,

Thanks for your support.

Uwe Kleine-K?nig <u.kleine-koenig@pengutronix•de> writes:

>> No strong feeling on that point. ISL12008 on which this driver is based
>> has a similar logic.
> Some time ago I already had the feeling that there is much "rank growth"
> in the rtc drivers. I guess this is because maintenance of the rtc
> subsystem isn't optimal and there are no guidelines (at least I'm not
> aware of any) about detail questions.

I had that feeling too when I pushed the ISL12057 driver.

>> > None of the alarm functionality checks to see if there's actually an IRQ
>> > - is that OK? I'd at least expect the alarm interrupt enable call to
>> > check if the interrupt is wired up.
>> 
>> I can add those check BUT I would like some directions in order to
>> support the following use case too.
>> 
>> Current three in-tree users of ISL12057 are NAS devices (Netgear
>> ReadyNAS 102, 104 and 2120). All of them *do not have* the interrupt pin
> I assume you don't have schematics of the devices, right? If so, do you
> think it might be worth to try to get them from Netgear? Just to make
> sure that there really is no pin connected to the SoC.

I'll ask but I am skeptical: I already tested all the MPP of the SoC so
unless I missed something, it's unlikely.


>> of the RTC chip connected to an interrupt line of the SoC. Nonetheless,
>> the IRQ line of the chip being connected to a PMIC on the board (TI
>> TPS65251 [1] on ReadyNAS 102 and 104, I do not know for ReadyNAS
> Looking over the manual it seems there is no way to let the PMIC forward
> the irq either.
>
>> 2120). When the device is off and the alarm rings, the device gets
>> powered on. AFAICT, the IRQ coming on the TPS65251 is not replicated on
>> any line of the SoC.
>> 
>> I think it's possible w/ some soldering to get somewhere where the RTC
>> framework wants me to be and finish the alarm part to have it merged
>> upstream but that's of limited interest if in-tree user cannot use it to
>> fit their need, i.e. configure an alarm value and enable the associated
>> interrupt which is routed externally, i.e. not visible by the SoC.
> Do you need to enable the irq somewhere else (apart from in the RTC
> device)? I guess not.

This is the problem: I just need to enable the interrupt in the ISL12057
(flag in regs) but w/o dealing with SoC/system interrupt, i.e. w/o having
a client->irq passed to the driver. In current RTC framework, alarm
support requires a client->irq and a working interrupt line w/ the SoC.


>> FWIW, we had a similar discussion a while ago, during driver inclusion:
>> 
>>   http://marc.info/?l=devicetree&m=138109313118504&w=2
>> 
>> Uwe, any idea?
> What is the problem of a not-wired-up irq line? Does the rtc framework
> need to change to allow setting an alarm without the irq line being
> hooked up?

yes. 


> Is it "only" that the alarm is missed?

In practice, yes. But RTC framework does not support that. Or maybe
there is some trick a maintainer would be aware of to support that
scenario. Something involving:

 - returning some specific value in alarm_irq_enable handler
 - calling device_init_wakeup(dev, true); w/o having an IRQ line
 - extend workaround started in c9f5c7e7a84f and 4a649903f91232
   and expose those via dt
 - ...

> Irq polling probably isn't sensible?
>
> I assume it's not that unusual that an rtc irq doesn't trigger a system
> irq, so having that supported nicely would be great.

Now, we're two to think that way ;-)


> Looking through the rtc's datasheet, the device is a tad more
> complicated than the current driver handles. There are two irq lines and
> three functions that can be routed through these (alarm1, alarm2 and
> clkout; not all combinations are possible).

Yes, but I wanted to handle the problem at hand before soldering IRQ#1
on my RN102 and add more feature.

a+

  reply	other threads:[~2014-11-07  9:37 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-05 21:42 [PATCHv0 0/3] rtc: rtc-isl12057: fixes and alarm support Arnaud Ebalard
2014-11-05 21:42 ` [PATCHv0 1/3] rtc: rtc-isl12057: fix masking of register values Arnaud Ebalard
2014-11-06  8:29   ` Uwe Kleine-König
2014-11-06 23:34     ` Arnaud Ebalard
2014-11-05 21:42 ` [PATCHv0 2/3] rtc: rtc-isl12057: fix isil vs isl naming for intersil Arnaud Ebalard
2014-11-06  5:32   ` Mark Brown
2014-11-06 22:46     ` Arnaud Ebalard
2014-11-07  6:39       ` Uwe Kleine-König
2014-11-05 21:42 ` [PATCHv0 3/3] rtc: rtc-isl12057: add alarm support to Intersil ISL12057 RTC driver Arnaud Ebalard
2014-11-06  5:50   ` Mark Brown
2014-11-06  5:59     ` Guenter Roeck
2014-11-06  6:07       ` Mark Brown
2014-11-06 23:30     ` Arnaud Ebalard
2014-11-07  7:58       ` Uwe Kleine-König
2014-11-07  9:37         ` Arnaud Ebalard [this message]
2014-11-07  9:53         ` Mark Brown
2014-11-07  9:47       ` Mark Brown
2014-11-06  8:49   ` Uwe Kleine-König
2014-11-06 22:47     ` Arnaud Ebalard

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=878ujnnygy.fsf@natisbad.org \
    --to=arno@natisbad$(echo .)org \
    --cc=linux-arm-kernel@lists$(echo .)infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox