Re: [PATCH] clocksource: sh_cmt: 32-bit control register support

[Laurent Pinchart]

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 <damm@xxxxxxxxxxxxx>
> >> >> 
> >> >> 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.

> 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 :-)

> 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 ?

-- 
Regards,

Laurent Pinchart

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/