From: arnd@arndb•de (Arnd Bergmann)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)
Date: Thu, 28 Mar 2013 19:27:34 +0000 [thread overview]
Message-ID: <201303281927.34327.arnd@arndb.de> (raw)
In-Reply-To: <5154848D.7010400@linaro.org>
On Thursday 28 March 2013, John Stultz wrote:
> On 03/28/2013 09:03 AM, Christian Daudt wrote:
> >>> Maybe we should move the ARM specific ones into
> >>> drivers/clocksource/arm/ ?
> >> About half the IP blocks we use on ARM are also used on at least
> >> one ARM64/AVR32/MIPS/PowerPC/x86/SH/Hexagon/c6x/etc part. Grouping them
> >> by which CPU architecture first starts using them or happens to be
> >> more popular at the time does not seem too helpful here.
> >>
> >> Maybe it's better to have a subdirectory for those clock sources
> >> that are used on any SoC, or have subdirectories based on the
> >> company that created that part, as we do for ethernet drivers.
> >> I wouldn't bother with that until there are a couple of dozen
> >> different clock source drivers.
>
> So having had a few days to think about this, I think what usually rubs
> me the wrong way when I get driver/clocksource submissions, is that for
> 99% of it, they *aren't clocksource drivers*. Most of the code is
> *clockevent* driver logic, and then maybe 1-5 lines of actual
> clocksource code.
>
> Now, I know the reason for this is often the clocksource and clockevent
> drivers are backed by the same hardware, and since there's no clockevent
> directory, might as well have it all in a single file somewhere. But
> mixing the different subsystem drivers together causes some of the
> maintenance confusion here.
>
> So instead of creating drivers/clocksource/arch/ directories, what I'd
> propose is we create a drivers/clockevent directory to handle the actual
> clockevent code. I think this would better delineate the lines of
> responsibility on the gatekeeper side (that being Thomas or maybe
> someone else who has an interest in the subtleties of how various
> hardware timers are be broken-by-design ;), and I'd be much happier
> taking clocksource code where I felt I had a reasonable chance of
> noticing bugs.
Yes, this sounds like a good idea.
> Thomas: Not that you need more to maintain, but does this seem
> semi-reasonable? Do we need to find someone else to help here?
>
> That said, at the end of the day, if I take a bad drivers/clocksource
> patch, what breaks won't be the timekeeping core, it will be an SoC
> board. So I'll have to really rely on the original clocksource driver
> authors to help vet incoming patches. This is where I think having the
> SoC tree as a central point for SoC patches has and advantage, as its
> less likely breakage will sneak upstream via a subsystem tree. But
> understanding the need for review help, I think I'm ok with taking on
> more clocksource specific review.
I think the important point for you to realize is that you don't need to
worry about breaking a specific SoC. The patches will come from someone
who is using that hardware in the end. What we need from clock{source,event}
subsystem maintainers is perspective of how things fit into the subsystem
and to provide review like:
- this driver is not using CLOCKSOURCE_OF_DECLARE when it should
- that driver only handles clockevent, don't put it into drivers/clocksource
- I don't like your coding style, it doesn't fit in with the other
drivers in this subsystem.
- what you do can be implemented much simpler using the generic infrastructure
introduced by that earlier driver.
Arnd
next prev parent reply other threads:[~2013-03-28 19:27 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-13 21:27 [PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion) Christian Daudt
2013-03-13 21:27 ` [PATCH V7 2/2] ARM: bcm281xx: Add timer driver (DT portion) Christian Daudt
2013-03-28 16:07 ` Christian Daudt
2013-04-02 20:43 ` Olof Johansson
2013-04-02 20:41 ` Olof Johansson
2013-03-13 23:29 ` [PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion) John Stultz
2013-03-14 9:16 ` Thomas Gleixner
2013-03-14 16:03 ` Arnd Bergmann
2013-03-28 16:03 ` Christian Daudt
2013-03-28 17:57 ` John Stultz
2013-03-28 19:27 ` Arnd Bergmann [this message]
2013-03-28 18:01 ` John Stultz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201303281927.34327.arnd@arndb.de \
--to=arnd@arndb$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox