Messages in this thread | | | From | Laurent Pinchart <> | Subject | Re: [PATCH] clocksource: sh_cmt: 32-bit control register support | Date | Tue, 18 Jun 2013 12:35:39 +0200 |
| |
Hi Magnus,
On Tuesday 18 June 2013 14:39:38 Magnus Damm wrote: > On Tue, Jun 18, 2013 at 3:37 AM, Laurent Pinchart wrote: > > On Monday 17 June 2013 15:40:52 Magnus Damm wrote: > >> From: Magnus Damm <damm@opensource.se> > >> > >> Add support for CMT hardware with 32-bit control and counter > >> registers, as found on r8a73a4 and r8a7790. To use the CMT > >> with 32-bit hardware a second I/O memory resource needs to > >> point out the CMSTR register and it needs to be 32 bit wide. > > > > Is a memory second resource required ? Can't we use a single resource that > > will contain all the registers ? > > The CMT hardware block comes with a shared timer start stop register > that historically has been left out of the resource. The location of > this register has so far been pointed out by the "channel offset" > platform data member, together with information about which bit that > happens to be assigned to the timer channel. This start stop register > has happened to be kept in the same page of I/O memory as the main > timer channel resource, so at this point we're sort of "lucky" that a > single ioremap() has covered all cases. > > With this patch it becomes optional to instead of platform data use a > second resource to point out the timer start/stop register. While we > do that we can also use the size of that resource to determine the I/O > access width, which happens to be something that is needed to enable > the driver on certain SoCs.
OK, I get it now. I've had a quick look at the documentation, and I'm wondering whether we shouldn't register a single platform device that span all the channels contained in the CMT, instead of registering one platform device per channel.
> > Time to switch to devm_* managed functions ? :-) > > Yes, indeed. That among other things, like converting the driver to in > a more optimal way support clock source only or clock event only > configurations. Also, some more modern CMT hardware versions have > extended registers with 48-bit counters, and we can also often use > more high frequency clocks to improve timer resolution. > > As you can tell, in general there are many things that can be improved > with this driver. I thought a first shot could be to make it actually > work on more recent CMT hardware with 32-bit only registers. So that's > what this patch does!
-- Regards,
Laurent Pinchart
| |