Re: [PATCH 23/27] clocksource: sh_cmt: Add DT support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Magnus,

On Saturday 15 February 2014 02:22:00 Magnus Damm wrote:
> On Sat, Feb 15, 2014 at 1:12 AM, Laurent Pinchart wrote:
> > On Saturday 15 February 2014 01:01:30 Magnus Damm wrote:
> >> On Sat, Feb 15, 2014 at 12:53 AM, Laurent Pinchart wrote:
> >> > On Friday 14 February 2014 10:58:22 Mark Rutland wrote:
> >> >> On Fri, Feb 14, 2014 at 01:00:01AM +0000, Laurent Pinchart wrote:
> >> >> > +Channels Optional Properties:
> >> >> > +
> >> >> > +  - clock-source-rating: rating of the timer as a clock source
> >> >> > device.
> >> >> > +  - clock-event-rating: rating of the timer as a clock event
> >> >> > device.
> >> >> 
> >> >> This feels like a leak of Linux internals. Why do you need this?
> >> > 
> >> > You're right, it is. The clock source and clock event ratings are
> >> > currently configured through platform data, I'll need to find a way to
> >> > compute them in the driver instead.
> >> 
> >> That would be very good!
> > 
> > Any pointer would be appreciated :-) How did you compute the various
> > ratings used in platform data all over the place ?
> 
> Historically we used the rating to select between CMT and TMU. For
> clock sources I suppose you also have the jiffy rating to consider.
> And for the SMP parts we have ARM IP for TWD and arch timers that have
> their ratings too. So you need to check all the timers on a particular
> system and consider what you want to have operating by default. The
> ARM IP timers should be preferred if available. For clock sources the
> rule is probably the higher resolution the better.
> 
> >> > There's still one piece of Linux-specific data I need though, as I need
> >> > to specify for each channel whether to use it as a clock source device,
> >> > a clock event device, both of them or none. That's configuration
> >> > information that needs to be provided somehow.
> >> 
> >> I think you can decide clock source or clock event assignment based on
> >> number of channels available. If you have only a single channel then both
> >> clock event and clock source need to be supported. Otherwise use one
> >> channel for clock source and the rest for clock events.
> > 
> > That won't match the current situation. Look at CMT0 in r8a7790 for
> > instance. There's two hardware channels available, and we only use the
> > first one, for clock events only.
> 
> You are correct. The reason for that is that the CMT driver today is
> optimized for combined clock event and clock source operation.
> 
> Historically the hardware it initially was written for (sh-mobile on
> the SH arch) only had a single timer channel so combined operation was
> required for tickless to work. But since you're asking how to allocate
> channels then I propose checking numbers of channels available and go
> from there. With that the r8a7790 support can only get better. =)
> 
> >> This is probably out of scope for this DT conversion, but it would be
> >> neat if you somehow could specify the CPU affinity for a channel to tie a
> >> clock event to an individual CPU core. This would make a a per-cpu timer
> >> unless I'm mistaken. But that's more of a software policy than anything
> >> else.
> > 
> > Yes, that's a configuration that needs to be specified somewhere. I don't
> > know where though.
> 
> As long as you have per-channel interrupts described in DT you can
> probably handle this in a generic way in the driver.

But how do we decide whether to use a single timer channel or one channel per 
CPU ? Will the kernel use one clock event device per CPU automatically ? I 
have to confess I have no idea how this works.

-- 
Regards,

Laurent Pinchart