linux-kernel - Re: [PATCH v3 04/12] of: add J-Core timer bindings

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 for Android: free password hash cracker in your pocket

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

Date:	Wed, 1 Jun 2016 17:36:19 -0500
From:	Rob Herring <robh@...nel.org>
To:	Rich Felker <dalias@...c.org>
Cc:	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	SH-Linux <linux-sh@...r.kernel.org>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Mark Rutland <mark.rutland@....com>,
	Pawel Moll <pawel.moll@....com>
Subject: Re: [PATCH v3 04/12] of: add J-Core timer bindings

On Wed, Jun 1, 2016 at 12:53 PM, Rich Felker <dalias@...c.org> wrote:
> On Wed, Jun 01, 2016 at 08:58:52AM -0500, Rob Herring wrote:
>> On Wed, May 25, 2016 at 05:43:03AM +0000, Rich Felker wrote:
>> > Signed-off-by: Rich Felker <dalias@...c.org>
>> > ---
>> >  .../devicetree/bindings/timer/jcore,pit.txt        | 28 ++++++++++++++++++++++
>> >  1 file changed, 28 insertions(+)
>> >  create mode 100644 Documentation/devicetree/bindings/timer/jcore,pit.txt
>> >
>> > diff --git a/Documentation/devicetree/bindings/timer/jcore,pit.txt b/Documentation/devicetree/bindings/timer/jcore,pit.txt
>> > new file mode 100644
>> > index 0000000..96c6815
>> > --- /dev/null
>> > +++ b/Documentation/devicetree/bindings/timer/jcore,pit.txt
>> > @@ -0,0 +1,28 @@
>> > +J-Core Programmable Interval Timer and Clocksource
>> > +
>> > +Required properties:
>> > +
>> > +- compatible: Must be "jcore,pit".
>> > +
>> > +- reg: Memory region for timer/clocksource registers.
>> > +
>> > +- interrupts: An interrupt to assign for the timer. The actual pit
>> > +  core is integrated with the aic and allows the timer interrupt
>> > +  assignment to be programmed by software, but this property is
>> > +  required in order to reserve an interrupt number that doesn't
>> > +  conflict with other devices.
>> > +
>> > +Optional properties:
>> > +
>> > +- cpu-offset: For SMP, the per-cpu offset to the local timer
>> > +  programming memory range.
>> > +
>> > +
>> > +Example:
>> > +
>> > +timer@200 {
>> > +   compatible = "jcore,pit";
>> > +   reg = < 0x200 0x30 >;
>> > +   cpu-offset = < 0x300 >;
>>
>> This is outside the reg range. Perhaps reg should include each range of
>> per cpu registers.
>
> In the hardware, each timer instance is mapped independently so
> there's no fundamental reason they need to be mapped sufficiently
> close that it would make sense for a single virtual mapping to cover
> them all. This doesn't matter for nommu but it would with mmu in the
> future. In the driver I've updated it to ioremap each percpu instance
> separately (as its own memory range) using the cpu-offset applied to
> the range obtained from "reg". Is this acceptable (in which case I
> suppose the binding needs to be documented that "reg" just covers the
> cpu0 instance's range)? Do you think it would be preferable to have
> multiple "reg" ranges indexed by cpu instead of cpu-offset?

Yes. Either reg should cover the full range of addresses or you should
have multiple reg values with one for each cpu. I prefer the latter as
it doesn't create a custom property for expressing register addresses.

> In theory it would even be possible to just require a DT node per
> cpulocal timer, but I didn't see a good way to make the bindings
> represent the relationship to cpus or to make the driver handle irqs
> correctly for such a setup, so I'd need a viable proposal for how that
> could be done to even consider such an approach.

Yeah, there's not really a standard way to map per cpu blocks to cpus.
We could, but doesn't really seem necessary here.

For the irqs, percpu irqs doesn't help you?

Rob