From: pawel.moll@arm.com (Pawel Moll)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH v3 0/2] drivers: mfd: Versatile Express SPC support
Date: Thu, 13 Jun 2013 10:45:57 +0100 [thread overview]
Message-ID: <1371116757.3307.13.camel@hornet> (raw)
In-Reply-To: <20130613001311.GC4567@zurbaran>
On Thu, 2013-06-13 at 01:13 +0100, Samuel Ortiz wrote:
> Now, about the driver itself, besides the really odd code design, the
> static variables all over the place, the nasty init hacks and the
> unneeded long function names, someone should refresh my memory and explain
> to me why is this guy under mfd. I can see it somehow supports IP blocks
> providing different functions, but the design is not sharing anything with
> most of the rest of the mfd drivers.
I belive the vexpress-sysreg.c is a Multi Function Device by all means.
It does so many things that only a water fountain is missing ;-)
If you feel strongly about it, I'm ready to split it into mfd_cells and
move the gpio and leds code into separate drivers, however I'm not
convinced that it's worth the effort.
Now, as to the vexpress-config.c... The first time I've posted the
series, all parts lived in "driver/misc(/vexpress)", but (if I remember
correctly) Arnd had some feelings about "misc" existence at all... I was
thinking about a separate directory for random "system/platform/machine
configuration" drivers, but the idea didn't get any traction.
> Not only that, but the whole vexpress-config code design is not the
> nicest piece of code I've ever seen. And I'm usually not picky. e.g. the
> whole vexpress-config ad-hoc API is awkward and I wonder if it could be
> implemented as a bus instead.
Funny you mention this :-) Again, the first version actually was a
vexpress-config bus. The feedback was - a whole bus_type is over the top
(I'm simplifying the letter slightly but this was the spirit).
> FWIW I take the blame here for not reviewing the initial driver
> submission that Arnd kindly sent to me...But now that I'm looking at it,
> I think it really is on the edge of being staging material. Any thought
> on that ?
I'm more than happy to improve it. The infrastructure (as in: the
hardware) itself is slightly strange and the code pretty much reflects
the situation. There is also a very good reason for some of the oddities
like static bridges array etc - the infrastructure must be functional
very early, long before slab is available (this also caused a lot of
issues with the bus-based implementation, as the device model does
kmalloc all over the place).
So to summarize - I'm open to any suggestions and ready to spend time on
this stuff.
Regards and thanks for your time!
Pawel
WARNING: multiple messages have this Message-ID (diff)
From: Pawel Moll <pawel.moll@arm.com>
To: Samuel Ortiz <sameo@linux.intel.com>
Cc: Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"devicetree-discuss@lists.ozlabs.org"
<devicetree-discuss@lists.ozlabs.org>,
Nicolas Pitre <nicolas.pitre@linaro.org>,
Amit Kucheria <amit.kucheria@linaro.org>,
Jon Medhurst <tixy@linaro.org>, Achin Gupta <Achin.Gupta@arm.com>,
Sudeep KarkadaNagesha <Sudeep.KarkadaNagesha@arm.com>,
Arnd Bergmann <arnd@arndb.de>
Subject: Re: [RFC PATCH v3 0/2] drivers: mfd: Versatile Express SPC support
Date: Thu, 13 Jun 2013 10:45:57 +0100 [thread overview]
Message-ID: <1371116757.3307.13.camel@hornet> (raw)
In-Reply-To: <20130613001311.GC4567@zurbaran>
On Thu, 2013-06-13 at 01:13 +0100, Samuel Ortiz wrote:
> Now, about the driver itself, besides the really odd code design, the
> static variables all over the place, the nasty init hacks and the
> unneeded long function names, someone should refresh my memory and explain
> to me why is this guy under mfd. I can see it somehow supports IP blocks
> providing different functions, but the design is not sharing anything with
> most of the rest of the mfd drivers.
I belive the vexpress-sysreg.c is a Multi Function Device by all means.
It does so many things that only a water fountain is missing ;-)
If you feel strongly about it, I'm ready to split it into mfd_cells and
move the gpio and leds code into separate drivers, however I'm not
convinced that it's worth the effort.
Now, as to the vexpress-config.c... The first time I've posted the
series, all parts lived in "driver/misc(/vexpress)", but (if I remember
correctly) Arnd had some feelings about "misc" existence at all... I was
thinking about a separate directory for random "system/platform/machine
configuration" drivers, but the idea didn't get any traction.
> Not only that, but the whole vexpress-config code design is not the
> nicest piece of code I've ever seen. And I'm usually not picky. e.g. the
> whole vexpress-config ad-hoc API is awkward and I wonder if it could be
> implemented as a bus instead.
Funny you mention this :-) Again, the first version actually was a
vexpress-config bus. The feedback was - a whole bus_type is over the top
(I'm simplifying the letter slightly but this was the spirit).
> FWIW I take the blame here for not reviewing the initial driver
> submission that Arnd kindly sent to me...But now that I'm looking at it,
> I think it really is on the edge of being staging material. Any thought
> on that ?
I'm more than happy to improve it. The infrastructure (as in: the
hardware) itself is slightly strange and the code pretty much reflects
the situation. There is also a very good reason for some of the oddities
like static bridges array etc - the infrastructure must be functional
very early, long before slab is available (this also caused a lot of
issues with the bus-based implementation, as the device model does
kmalloc all over the place).
So to summarize - I'm open to any suggestions and ready to spend time on
this stuff.
Regards and thanks for your time!
Pawel
next prev parent reply other threads:[~2013-06-13 9:45 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-06 9:59 [RFC PATCH v3 0/2] drivers: mfd: Versatile Express SPC support Lorenzo Pieralisi
2013-06-06 9:59 ` Lorenzo Pieralisi
2013-06-06 9:59 ` Lorenzo Pieralisi
2013-06-06 9:59 ` [RFC PATCH v3 1/2] drivers: mfd: refactor the vexpress config bridge API Lorenzo Pieralisi
2013-06-06 9:59 ` Lorenzo Pieralisi
2013-06-06 9:59 ` [RFC PATCH v3 2/2] drivers: mfd: vexpress: add Serial Power Controller (SPC) support Lorenzo Pieralisi
2013-06-06 9:59 ` Lorenzo Pieralisi
2013-06-06 9:59 ` Lorenzo Pieralisi
2013-06-13 0:01 ` Samuel Ortiz
2013-06-13 0:01 ` Samuel Ortiz
2013-06-13 3:26 ` Nicolas Pitre
2013-06-13 3:26 ` Nicolas Pitre
2013-06-13 9:54 ` Lorenzo Pieralisi
2013-06-13 9:54 ` Lorenzo Pieralisi
2013-06-13 22:52 ` Olof Johansson
2013-06-13 22:52 ` Olof Johansson
2013-06-14 0:21 ` Nicolas Pitre
2013-06-14 0:21 ` Nicolas Pitre
2013-06-14 0:21 ` Nicolas Pitre
2013-06-14 12:19 ` Lorenzo Pieralisi
2013-06-14 12:19 ` Lorenzo Pieralisi
2013-06-14 13:04 ` Pawel Moll
2013-06-14 13:04 ` Pawel Moll
2013-06-14 13:04 ` Pawel Moll
2013-06-14 17:49 ` Olof Johansson
2013-06-14 17:49 ` Olof Johansson
2013-06-11 9:05 ` [RFC PATCH v3 0/2] drivers: mfd: Versatile Express SPC support Lorenzo Pieralisi
2013-06-11 9:05 ` Lorenzo Pieralisi
2013-06-13 0:13 ` Samuel Ortiz
2013-06-13 0:13 ` Samuel Ortiz
2013-06-13 9:45 ` Pawel Moll [this message]
2013-06-13 9:45 ` Pawel Moll
2013-06-18 9:09 ` Samuel Ortiz
2013-06-18 9:09 ` Samuel Ortiz
2013-06-18 9:29 ` Pawel Moll
2013-06-18 9:29 ` Pawel Moll
2013-06-19 9:30 ` Samuel Ortiz
2013-06-19 9:30 ` Samuel Ortiz
2013-06-19 12:37 ` Arnd Bergmann
2013-06-19 12:37 ` Arnd Bergmann
2013-06-19 12:55 ` Pawel Moll
2013-06-19 12:55 ` Pawel Moll
2013-06-19 15:03 ` Arnd Bergmann
2013-06-19 15:03 ` Arnd Bergmann
2013-06-19 15:14 ` Pawel Moll
2013-06-19 15:14 ` Pawel Moll
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=1371116757.3307.13.camel@hornet \
--to=pawel.moll@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.