On Tue, Jun 18, 2013 at 10:27:44PM +0900, Magnus Damm wrote: > Hi Laurent, > > On Tue, Jun 18, 2013 at 9:30 PM, Laurent Pinchart > <laurent.pinch...@ideasonboard.com> wrote: > > Hi Magnus, > > > > On Tuesday 18 June 2013 20:54:47 Magnus Damm wrote: > >> On Tue, Jun 18, 2013 at 7:35 PM, Laurent Pinchart wrote: > >> > 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 <d...@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. > >> > >> I both agree with you and disagree because of the current state of timers > >> in > >> the linux kernel. I would have liked a single platform device with all > >> channles if this would be a generic timer driver that from user space could > >> be configured to associate channels with various subsystems like PWM, > >> clocksource, clockevent. > >> > >> At this point the driver is doing clockevent and clocksource only, and no > >> sane user wants 84 channels of equivalent hardware blocks for those two. > > > > Of course, but we could always select which channels to register clockevents > > and clocksources for in platform data. That won't fix the overall problem, > > but > > it's one step forward. > > But that's pretty much what we're doing, but only listing timer > channels that will be used. Of course, moving around things can be > done but I can't see why we want to do that if we have no selection of > drivers for the actual timer channels. Also, each timer channel may > have it's own unique set of possible parent clocks. That's something > we want to tie in to DT together with CCF. Solving those things > together makes sense IMO. > > >> So based on that I'd rather do it like today and let people write custom > >> drivers for whatever applications they may use the other channels for. > >> > >> So if you're in hacking mode, why don't you figure out some way timers can > >> be configured from user space? =) > > > > I don't have *that* much free time at the moment I'm afraid, and I'm sure > > you > > know why :-) > > Yes I do, and that's why I asked. =) > > >> If so then we can use DT to describe the actual hardware and let the > >> software policy be decided via some configuration mechanism. > > > > Don't we also need timers during early boot, when userspace isn't available > > yet ? > > It depends on the rest of the system. It is possible to boot to user > space without a timer, but I don't recommend it. =)
Hi, I am holding off on this patch until some consensus is reached. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
