linux-kernel - Re: [PATCH] clocksource: moxart: Add AST2500 compatible string

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]

Message-ID: <1495761138.7794.11.camel@aj.id.au>
Date:   Fri, 26 May 2017 11:12:18 +1000
From:   Andrew Jeffery <andrew@...id.au>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     linux-kernel@...r.kernel.org, robh+dt@...nel.org,
        mark.rutland@....com, tglx@...utronix.de, joel@....id.au,
        devicetree@...r.kernel.org, ryan_chen@...eedtech.com,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Subject: Re: [PATCH] clocksource: moxart: Add AST2500 compatible string

On Thu, 2017-05-25 at 22:28 +0200, Daniel Lezcano wrote:
> On Tue, May 16, 2017 at 03:58:40PM +0800, Andrew Jeffery wrote:
> > Also clean up space-before-tab issues in the documentation.
> 
> Andrew,
> 
> I reworked the patch to apply to the changes Linus did recently to convert to
> the fttrm010 driver.
> 
> Please have a look at:
> 
> https://git.linaro.org/people/daniel.lezcano/linux.git/commit/?h=clockevents/4.13&id=3ca904162ffdd72f4fad3ab731fc94a12c50f682
> 

I think we're going to run into trouble here:

https://git.linaro.org/people/daniel.lezcano/linux.git/tree/drivers/clocksource/timer-fttmr010.c?h=clockevents/4.13&id=3ca904162ffdd72f4fad3ab731fc94a12c50f682#n260

As it stands if a aspeed,ast2500-timer compatible is provided we'll
take the else branch and hit the issues Joel found with Linus' original
series counting up on the Aspeed hardware.

My change was somewhat cosmetic - Ben (now Cc'ed) didn't seemed too
concerned about using the the aspeed,ast2400-timer compatible string
for ast2500 dts. My motivation for the patch was that by describing the
aspeed,ast2500-timer compatible it signals that someone had taken a
look and judged it so. However, my point is maybe one solution is
simply to drop the patch and continue to use aspeed,ast2400-timer
compatible where we need.

Another is to rework your change to switch to
of_device_compatible_match() in drivers/clocksource/timer-fttmr010.c
and also check against aspeed,ast2500-timer.

What direction should we go?

> Shouldn't the compatible string be:
> 
> 	"aspeed,ast2400-timer", "faraday,fttmr010"
> 	"aspeed,ast2500-timer", "faraday,fttmr010"
> 

Does it makes sense in the face of the Aspeed quirks? If so it seems
reasonable, but falling back to the faraday,fttmr010 compatible could
lead to failures (if the compatible driver counted up).

Cheers,

Andrew

> 
>   -- Daniel
> 
Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)