From: nico@fluxnic.net (Nicolas Pitre)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/5] [orion] Move address map setup out of the drivers and into platform.
Date: Mon, 14 Nov 2011 19:06:47 -0500 (EST) [thread overview]
Message-ID: <alpine.LFD.2.02.1111141855340.3307@xanadu.home> (raw)
In-Reply-To: <1321128007-3520-4-git-send-email-andrew@lunn.ch>
On Sat, 12 Nov 2011, Andrew Lunn wrote:
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> ---
> arch/arm/mach-dove/pcie.c | 3 +-
> arch/arm/mach-kirkwood/mpp.c | 1 -
> arch/arm/mach-kirkwood/pcie.c | 3 +-
> arch/arm/mach-mv78xx0/mpp.c | 1 -
> arch/arm/mach-mv78xx0/pcie.c | 3 +-
> arch/arm/mach-orion5x/mpp.c | 1 -
> arch/arm/mach-orion5x/pci.c | 2 +-
> arch/arm/plat-orion/addr-map.c | 182 +++++++++++++++++++++++++++
> arch/arm/plat-orion/include/plat/addr-map.h | 8 ++
> arch/arm/plat-orion/include/plat/pcie.h | 3 +-
> arch/arm/plat-orion/pcie.c | 6 +-
> drivers/ata/sata_mv.c | 36 +-----
> drivers/dma/mv_xor.c | 37 +-----
> drivers/dma/mv_xor.h | 5 -
> drivers/mmc/host/mvsdio.c | 28 +----
> drivers/mmc/host/mvsdio.h | 4 -
> drivers/net/mv643xx_eth.c | 45 +------
> drivers/usb/host/ehci-orion.c | 30 +----
> sound/soc/kirkwood/kirkwood-dma.c | 30 +----
> sound/soc/kirkwood/kirkwood.h | 4 -
> 20 files changed, 218 insertions(+), 214 deletions(-)
I have a conceptual concern with this patch. Initially, we moved all
the device specific mbus initializations from the core SOC code into
their respective drivers before submitting this code to mainline, simply
because that was the right thing to do. Because the corresponding
registers are highly device/peripheral specific, it makes sense to put
that info in the drivers.
Now you are moving it all back into a centralized place, meaning that a
lot of different peripheral knowledge has to be gathered up into that
single place. And if a new driver is ever added for a new peripheral,
then that central place will have to learn about it too. This doesn't
look that pretty to me.
Furthermore, this is not providing any sort of code saving either.
I understand that you might want to get rid of the annoying platform
data pointer for the mbus values. But I think that this could be done
some other way though. IMHO the mbus initialization code for
peripherals should remain in the drivers, and a different way to query
the needed information be elaborated instead. Otherwise this is like
throwing out the baby with the bath water.
Nicolas
next prev parent reply other threads:[~2011-11-15 0:06 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-11-12 20:00 [PATCH 0/5] Move mbus setup code out of drivers and into platform Andrew Lunn
2011-11-12 20:00 ` [PATCH 1/5] [orion] Consolidate the address map setup on Orion based platforms Andrew Lunn
2011-11-12 20:00 ` [PATCH 2/5] [orion] Move the *_mbus_dram_info structure into the orion platform and call it orion_mbus_dram_info everywhere Andrew Lunn
2011-11-12 20:00 ` [PATCH 3/5] [orion] Move address map setup out of the drivers and into platform Andrew Lunn
2011-11-13 20:53 ` Michael Walle
2011-11-15 0:06 ` Nicolas Pitre [this message]
2011-11-15 7:41 ` Andrew Lunn
2011-11-15 22:14 ` Nicolas Pitre
2011-11-16 6:59 ` Andrew Lunn
2011-11-16 16:08 ` Nicolas Pitre
2011-11-16 16:49 ` Andrew Lunn
2011-11-16 17:18 ` Michael Walle
2011-11-16 17:25 ` Nicolas Pitre
2011-11-12 20:00 ` [PATCH 4/5] [orion] Remove address map info from all platform data strucutures Andrew Lunn
2011-11-12 20:00 ` [PATCH 5/5] [orion] Consolidate the address map setup code Andrew Lunn
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=alpine.LFD.2.02.1111141855340.3307@xanadu.home \
--to=nico@fluxnic.net \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).