From: Russell King <rmk@arm.linux.org.uk>
To: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: Haojian Zhuang <haojian.zhuang@gmail.com>,
sameo@linux.intel.com, rpurdie@rpsys.net, bryan.wu@canonical.com,
linux-kernel@vger.kernel.org, Bergmann Arnd <arnd@arndb.de>
Subject: Re: [PATCH 0/5] mfd: replace IORESOURCE_IO by IORESOURCE_MEM
Date: Tue, 7 Aug 2012 13:11:57 +0100 [thread overview]
Message-ID: <20120807121157.GA10166@flint.arm.linux.org.uk> (raw)
In-Reply-To: <20120807114415.GG24257@flint.arm.linux.org.uk>
On Tue, Aug 07, 2012 at 12:44:15PM +0100, Russell King wrote:
> On Tue, Aug 07, 2012 at 12:38:02PM +0100, Mark Brown wrote:
> > On Tue, Aug 07, 2012 at 12:31:21PM +0100, Russell King wrote:
> > > On Tue, Aug 07, 2012 at 12:28:44PM +0100, Mark Brown wrote:
> >
> > > > The changes you're suggesting are extremely invasive for stable
> > > > especially given that we have a simple, driver local, fix available
> >
> > > *Rubbish*.
> >
> > This isn't helpful or constructive...
> >
> > Which bit of the above are you referring to here? If it's the having a
> > fix bit then as pointed out repeatedly now in this and the previous
> > thread we've got the separete resource tree approach implemented in
> > stable right now making actual systems run.
>
> All of your above statement. It is, basically, completely wrong, and
> shows that you haven't thought about the solution I'm proposing at all.
>
> I've shown you in simple steps how easy it is. It is not invasive. It
> is not complex. It is local to the affected drivers. So, all your
> points above are plain wrong. Hence "rubbish".
And, because you WILL NOT tell me which are the affected drivers, I've
assumed in the patch below that you're referring to the wm831x drivers.
So here's the patch in full for this driver. Oh look how simple and
non-invasive it is.
$ git grep IORESOURCE_REG
drivers/mfd/wm831x-core.c: .flags = IORESOURCE_REG,
... repeated ...
drivers/mfd/wm831x-core.c: .flags = IORESOURCE_REG,
drivers/regulator/wm831x-dcdc.c: res = platform_get_resource(pdev, IORESO
drivers/regulator/wm831x-dcdc.c: res = platform_get_resource(pdev, IORESO
drivers/regulator/wm831x-dcdc.c: res = platform_get_resource(pdev, IORESO
drivers/regulator/wm831x-isink.c: res = platform_get_resource(pdev, IORESO
drivers/regulator/wm831x-ldo.c: res = platform_get_resource(pdev, IORESOURCE_REG
drivers/regulator/wm831x-ldo.c: res = platform_get_resource(pdev, IORESOURCE_REG
drivers/regulator/wm831x-ldo.c: res = platform_get_resource(pdev, IORESOURCE_REG
include/linux/ioport.h:#define IORESOURCE_REG 0x00000300 /* Regis
$
So no identifier clashes.
No, I haven't tested it, I don't have the hardware. That's your job
as the driver maintainer.
The only open questions on this are:
1. Is there another driver you are concerned about.
2. Choosing a better name.
But I won't put question marks at the end of those because you never seem
to answer questions. You will just produce yet more justifications why
this approach is not one you're willing to accept. So I really don't know
why I wasted my time to produce this patch.
drivers/mfd/wm831x-core.c | 66 +++++++++++--------------------------
drivers/regulator/wm831x-dcdc.c | 12 +++---
drivers/regulator/wm831x-isink.c | 4 +-
drivers/regulator/wm831x-ldo.c | 12 +++---
include/linux/ioport.h | 1 +
5 files changed, 35 insertions(+), 60 deletions(-)
diff --git a/drivers/mfd/wm831x-core.c b/drivers/mfd/wm831x-core.c
index 946698f..2e5d58e 100644
--- a/drivers/mfd/wm831x-core.c
+++ b/drivers/mfd/wm831x-core.c
@@ -614,18 +614,11 @@ int wm831x_set_bits(struct wm831x *wm831x, unsigned short reg,
}
EXPORT_SYMBOL_GPL(wm831x_set_bits);
-static struct resource wm831x_io_parent = {
- .start = 0,
- .end = 0xffffffff,
- .flags = IORESOURCE_IO,
-};
-
static struct resource wm831x_dcdc1_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_DC1_CONTROL_1,
.end = WM831X_DC1_DVS_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -644,10 +637,9 @@ static struct resource wm831x_dcdc1_resources[] = {
static struct resource wm831x_dcdc2_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_DC2_CONTROL_1,
.end = WM831X_DC2_DVS_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -665,10 +657,9 @@ static struct resource wm831x_dcdc2_resources[] = {
static struct resource wm831x_dcdc3_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_DC3_CONTROL_1,
.end = WM831X_DC3_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -680,10 +671,9 @@ static struct resource wm831x_dcdc3_resources[] = {
static struct resource wm831x_dcdc4_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_DC4_CONTROL,
.end = WM831X_DC4_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -695,10 +685,9 @@ static struct resource wm831x_dcdc4_resources[] = {
static struct resource wm8320_dcdc4_buck_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_DC4_CONTROL,
.end = WM832X_DC4_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -718,10 +707,9 @@ static struct resource wm831x_gpio_resources[] = {
static struct resource wm831x_isink1_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_CURRENT_SINK_1,
.end = WM831X_CURRENT_SINK_1,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.start = WM831X_IRQ_CS1,
@@ -732,10 +720,9 @@ static struct resource wm831x_isink1_resources[] = {
static struct resource wm831x_isink2_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_CURRENT_SINK_2,
.end = WM831X_CURRENT_SINK_2,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.start = WM831X_IRQ_CS2,
@@ -746,10 +733,9 @@ static struct resource wm831x_isink2_resources[] = {
static struct resource wm831x_ldo1_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO1_CONTROL,
.end = WM831X_LDO1_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -761,10 +747,9 @@ static struct resource wm831x_ldo1_resources[] = {
static struct resource wm831x_ldo2_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO2_CONTROL,
.end = WM831X_LDO2_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -776,10 +761,9 @@ static struct resource wm831x_ldo2_resources[] = {
static struct resource wm831x_ldo3_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO3_CONTROL,
.end = WM831X_LDO3_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -791,10 +775,9 @@ static struct resource wm831x_ldo3_resources[] = {
static struct resource wm831x_ldo4_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO4_CONTROL,
.end = WM831X_LDO4_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -806,10 +789,9 @@ static struct resource wm831x_ldo4_resources[] = {
static struct resource wm831x_ldo5_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO5_CONTROL,
.end = WM831X_LDO5_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -821,10 +803,9 @@ static struct resource wm831x_ldo5_resources[] = {
static struct resource wm831x_ldo6_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO6_CONTROL,
.end = WM831X_LDO6_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -836,10 +817,9 @@ static struct resource wm831x_ldo6_resources[] = {
static struct resource wm831x_ldo7_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO7_CONTROL,
.end = WM831X_LDO7_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -851,10 +831,9 @@ static struct resource wm831x_ldo7_resources[] = {
static struct resource wm831x_ldo8_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO8_CONTROL,
.end = WM831X_LDO8_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -866,10 +845,9 @@ static struct resource wm831x_ldo8_resources[] = {
static struct resource wm831x_ldo9_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO9_CONTROL,
.end = WM831X_LDO9_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -881,10 +859,9 @@ static struct resource wm831x_ldo9_resources[] = {
static struct resource wm831x_ldo10_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO10_CONTROL,
.end = WM831X_LDO10_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
{
.name = "UV",
@@ -896,10 +873,9 @@ static struct resource wm831x_ldo10_resources[] = {
static struct resource wm831x_ldo11_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_LDO11_ON_CONTROL,
.end = WM831X_LDO11_SLEEP_CONTROL,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
};
@@ -998,19 +974,17 @@ static struct resource wm831x_rtc_resources[] = {
static struct resource wm831x_status1_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_STATUS_LED_1,
.end = WM831X_STATUS_LED_1,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
};
static struct resource wm831x_status2_resources[] = {
{
- .parent = &wm831x_io_parent,
.start = WM831X_STATUS_LED_2,
.end = WM831X_STATUS_LED_2,
- .flags = IORESOURCE_IO,
+ .flags = IORESOURCE_REG,
},
};
diff --git a/drivers/regulator/wm831x-dcdc.c b/drivers/regulator/wm831x-dcdc.c
index 7413885..64111f4 100644
--- a/drivers/regulator/wm831x-dcdc.c
+++ b/drivers/regulator/wm831x-dcdc.c
@@ -476,9 +476,9 @@ static __devinit int wm831x_buckv_probe(struct platform_device *pdev)
dcdc->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
@@ -651,9 +651,9 @@ static __devinit int wm831x_buckp_probe(struct platform_device *pdev)
dcdc->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
@@ -795,9 +795,9 @@ static __devinit int wm831x_boostp_probe(struct platform_device *pdev)
dcdc->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
diff --git a/drivers/regulator/wm831x-isink.c b/drivers/regulator/wm831x-isink.c
index 0d207c2..2646a19 100644
--- a/drivers/regulator/wm831x-isink.c
+++ b/drivers/regulator/wm831x-isink.c
@@ -172,9 +172,9 @@ static __devinit int wm831x_isink_probe(struct platform_device *pdev)
isink->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
diff --git a/drivers/regulator/wm831x-ldo.c b/drivers/regulator/wm831x-ldo.c
index 5cb70ca..da73daf 100644
--- a/drivers/regulator/wm831x-ldo.c
+++ b/drivers/regulator/wm831x-ldo.c
@@ -269,9 +269,9 @@ static __devinit int wm831x_gp_ldo_probe(struct platform_device *pdev)
ldo->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
@@ -520,9 +520,9 @@ static __devinit int wm831x_aldo_probe(struct platform_device *pdev)
ldo->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
@@ -675,9 +675,9 @@ static __devinit int wm831x_alive_ldo_probe(struct platform_device *pdev)
ldo->wm831x = wm831x;
- res = platform_get_resource(pdev, IORESOURCE_IO, 0);
+ res = platform_get_resource(pdev, IORESOURCE_REG, 0);
if (res == NULL) {
- dev_err(&pdev->dev, "No I/O resource\n");
+ dev_err(&pdev->dev, "No REG resource\n");
ret = -EINVAL;
goto err;
}
diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 589e0e7..bfee885 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -31,6 +31,7 @@ struct resource {
#define IORESOURCE_TYPE_BITS 0x00001f00 /* Resource type */
#define IORESOURCE_IO 0x00000100
#define IORESOURCE_MEM 0x00000200
+#define IORESOURCE_REG 0x00000300 /* Register offsets */
#define IORESOURCE_IRQ 0x00000400
#define IORESOURCE_DMA 0x00000800
#define IORESOURCE_BUS 0x00001000
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of:
next prev parent reply other threads:[~2012-08-07 12:12 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-08-05 16:32 [PATCH 0/5] mfd: replace IORESOURCE_IO by IORESOURCE_MEM Haojian Zhuang
2012-08-05 16:32 ` [PATCH 1/5] mfd: use IORESOURCE_MEM in 88pm860x backlight Haojian Zhuang
2012-08-05 16:32 ` [PATCH 2/5] mfd: use IORESOUCE_MEM in 88pm860x leds driver Haojian Zhuang
2012-08-05 16:32 ` [PATCH 3/5] mfd: use IORESOURCE_MEM in 88pm860x regulator Haojian Zhuang
2012-08-05 16:32 ` [PATCH 4/5] mfd: avoid to return failure in 88pm860x Haojian Zhuang
2012-08-05 16:32 ` [PATCH 5/5] ARM: mmp: enable 88pm860x in ttc dkb Haojian Zhuang
2012-08-06 14:30 ` [PATCH 0/5] mfd: replace IORESOURCE_IO by IORESOURCE_MEM Mark Brown
2012-08-06 14:42 ` Haojian Zhuang
2012-08-06 15:46 ` Mark Brown
2012-08-06 15:56 ` Haojian Zhuang
2012-08-06 15:58 ` Mark Brown
2012-08-06 19:22 ` Russell King
2012-08-06 19:53 ` Mark Brown
2012-08-06 21:31 ` Russell King
2012-08-06 22:00 ` Mark Brown
2012-08-07 1:47 ` Haojian Zhuang
2012-08-07 7:58 ` Russell King
2012-08-07 8:24 ` Benjamin Herrenschmidt
2012-08-07 10:38 ` Mark Brown
2012-08-07 11:13 ` Russell King
2012-08-07 11:28 ` Mark Brown
2012-08-07 11:31 ` Russell King
2012-08-07 11:36 ` Russell King
2012-08-07 11:45 ` Mark Brown
2012-08-07 11:51 ` Russell King
2012-08-07 12:58 ` Mark Brown
2012-08-07 13:47 ` Russell King
2012-08-07 15:54 ` Mark Brown
2012-08-07 15:22 ` Geert Uytterhoeven
2012-08-07 15:44 ` Russell King
2012-08-07 16:48 ` Mark Brown
2012-08-07 20:51 ` Arnd Bergmann
2012-08-07 11:38 ` Mark Brown
2012-08-07 11:44 ` Russell King
2012-08-07 12:11 ` Russell King [this message]
2012-08-07 13:25 ` Mark Brown
2012-08-07 14:28 ` Arnd Bergmann
2012-08-07 14:41 ` Russell King
2012-08-07 16:36 ` Mark Brown
2012-08-07 8:22 ` Benjamin Herrenschmidt
2012-08-07 8:28 ` Russell King
2012-08-07 11:28 ` Arnd Bergmann
2012-08-07 11:32 ` Russell King
2012-08-07 11:34 ` Arnd Bergmann
2012-08-07 11:35 ` Mark Brown
2012-08-07 11:41 ` Russell King
2012-08-07 15:45 ` Alan Cox
2012-08-07 15:50 ` Russell King
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=20120807121157.GA10166@flint.arm.linux.org.uk \
--to=rmk@arm.linux.org.uk \
--cc=arnd@arndb.de \
--cc=broonie@opensource.wolfsonmicro.com \
--cc=bryan.wu@canonical.com \
--cc=haojian.zhuang@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=rpurdie@rpsys.net \
--cc=sameo@linux.intel.com \
/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