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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 18 Jun 2013 12:35:39 +0200
From:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:	Magnus Damm <magnus.damm@...il.com>
Cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	SH-Linux <linux-sh@...r.kernel.org>,
	john stultz <johnstul@...ibm.com>,
	"Simon Horman [Horms]" <horms@...ge.net.au>,
	Shinya Kuribayashi <shinya.kuribayashi.px@...esas.com>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH] clocksource: sh_cmt: 32-bit control register support

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@...nsource.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

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