Re: [rtc-linux] [PATCH] watchdog: stmp3xxx: Implement GETBOOTSTATUS

[<harald@ccbib.org>]

Hi Alexandre,

sorry for the very long delay. 

I tried your suggestion to switch the watchdog driver to regmap several
times but couldn't come up with something that wasn't either very ugly
or completely pointless - i.e. no better then passing a pointer to the
register block. See below.

On Tue, 5 May 2015 11:40:31 +0200, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:
> Hi,
> 
> On 20/04/2015 at 09:11:02 +0200, harald@ccbib.org wrote :
>> Hi Alexandre,
>> 
>> On Sun, 19 Apr 2015 17:37:43 +0200, Alexandre Belloni
>> <alexandre.belloni@free-electrons.com> wrote:
>> > On 19/04/2015 at 15:41:42 +0200, harald@ccbib.org wrote :
>> >> >  - use syscon instead of passing callbacks to the watchdog driver
>> >> 
>> >> I will put looking into this onto my todo list, but I'm not familiar
>> >> with syscon yet and it is not obvious to me how to port the rtc
driver
>> >> to syscon without breaking devicetree backwards compatibility. So
I'd
>> >> like to defer this for later unless somebody gives me some guidance.
>> > 
>> > As the rtc driver is the one instantiating the watchdog driver, there
>> > is
>> > no reason why the DT ABI would be broken. You simply have to create
>> > the syscon in the RTC driver and then pass it to the watchdog driver,
>> > the same way it is done with the callbacks.
>> 
>> Ok, this is less invasive than what I have been thinking. This makes
>> sense. 
> 
> I realized I confused syscon with regmap. So indeed, switching to syscon
> will require a not nice change to the DT. But what I really meant was
> creating a regmap from the rtc driver and pass it to the watchdog
> driver.

Since the rtc and watchdog drivers need different bits of the same control
register, we would need to use regmap_fields to pass them around.
Unfortunately this means we can't use the _SET and _CLR registers, which
means we need to use regmap everywhere to get proper locking. A lot of
overhead for no benefit.

After reading lots of messages around the genesis and usage of
regmap_mmio,
it seems to me the intended usecases are to fix some endianness issues
and to have a register cache available during suspend - both of which
don't
apply in this case.

If you want to get rid of the callbacks we should just pass the pointer
to the register block to the child device, I think. Which way is
preferable
is probably only a matter of taste so I won't override the driver authors
decision unless there is some clear statement that this is the preferred
style in the rtc subsystem.

I will send the other patch (unify register access macros) you asked about
shortly.

Harald
--
To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html