Hi, On 01/10/2015 at 19:20:32 +0200, harald@xxxxxxxxx wrote : > 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. > I'd say one of the main advantage of regmap is that it does proper locking of shared registers and I think that is the main reason of using it when with platform drivers. Obviously, because you have _SET and _CLR register on that platform you don't really care about locking. > 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. > Ok. Like I said, this is not blocking, it was just a cleanup I was suggesting. However, this device is clearly an MFD and that callback feels a bit hackish. Also registering the watchdog would probably better be done from the mfd subsystem (this is the only RTC driver doing so). If you have a look at the history, this was a patch from 2011 taken in 2013 so the kernel had plenty of time to evolve in between ;) We are dealing with legacy and I'm fine with that driver staying that way until it is absolutely necessary to change it. -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html