* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
@ 2013-03-23 12:56 Sebastian Hesselbarth
2013-03-23 15:17 ` Thomas Petazzoni
2013-03-28 17:00 ` Jason Cooper
0 siblings, 2 replies; 6+ messages in thread
From: Sebastian Hesselbarth @ 2013-03-23 12:56 UTC (permalink / raw)
To: linux-arm-kernel
mvsdio_platform_data allows to pass card detect and write protect gpio
numbers to the driver. Some kirkwood boards don't use both pins as they
are not connected, and don't set the corresponding value in platform_data.
This will leave the unset values in platform_data initialized as 0, which
is in fact a valid gpio pin. mvsdio will grab that pin and configure it as
gpio, which in turn breaks nand controller as mpp0 also carries nand_io2.
This patch fixes the above by initializing unused gpio functions in the
platform_data with an invalid (-1) value.
Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Reported-by: Soeren Moch <smoch@web.de>
---
Cc: Soeren Moch <smoch@web.de>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: Andrew Lunn <andrew@lunn.ch>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
arch/arm/mach-kirkwood/guruplug-setup.c | 2 ++
arch/arm/mach-kirkwood/openrd-setup.c | 1 +
arch/arm/mach-kirkwood/rd88f6281-setup.c | 1 +
3 files changed, 4 insertions(+)
diff --git a/arch/arm/mach-kirkwood/guruplug-setup.c b/arch/arm/mach-kirkwood/guruplug-setup.c
index 1c6e736..08dd739 100644
--- a/arch/arm/mach-kirkwood/guruplug-setup.c
+++ b/arch/arm/mach-kirkwood/guruplug-setup.c
@@ -53,6 +53,8 @@ static struct mv_sata_platform_data guruplug_sata_data = {
static struct mvsdio_platform_data guruplug_mvsdio_data = {
/* unfortunately the CD signal has not been connected */
+ .gpio_card_detect = -1,
+ .gpio_write_protect = -1,
};
static struct gpio_led guruplug_led_pins[] = {
diff --git a/arch/arm/mach-kirkwood/openrd-setup.c b/arch/arm/mach-kirkwood/openrd-setup.c
index 8ddd69f..6a6eb54 100644
--- a/arch/arm/mach-kirkwood/openrd-setup.c
+++ b/arch/arm/mach-kirkwood/openrd-setup.c
@@ -55,6 +55,7 @@ static struct mv_sata_platform_data openrd_sata_data = {
static struct mvsdio_platform_data openrd_mvsdio_data = {
.gpio_card_detect = 29, /* MPP29 used as SD card detect */
+ .gpio_write_protect = -1,
};
static unsigned int openrd_mpp_config[] __initdata = {
diff --git a/arch/arm/mach-kirkwood/rd88f6281-setup.c b/arch/arm/mach-kirkwood/rd88f6281-setup.c
index c7d93b4..d242231 100644
--- a/arch/arm/mach-kirkwood/rd88f6281-setup.c
+++ b/arch/arm/mach-kirkwood/rd88f6281-setup.c
@@ -69,6 +69,7 @@ static struct mv_sata_platform_data rd88f6281_sata_data = {
static struct mvsdio_platform_data rd88f6281_mvsdio_data = {
.gpio_card_detect = 28,
+ .gpio_write_protect = -1,
};
static unsigned int rd88f6281_mpp_config[] __initdata = {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
2013-03-23 12:56 [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins Sebastian Hesselbarth
@ 2013-03-23 15:17 ` Thomas Petazzoni
2013-03-23 15:25 ` Sebastian Hesselbarth
2013-03-28 17:00 ` Jason Cooper
1 sibling, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2013-03-23 15:17 UTC (permalink / raw)
To: linux-arm-kernel
Dear Sebastian Hesselbarth,
On Sat, 23 Mar 2013 13:56:58 +0100, Sebastian Hesselbarth wrote:
> mvsdio_platform_data allows to pass card detect and write protect gpio
> numbers to the driver. Some kirkwood boards don't use both pins as
> they are not connected, and don't set the corresponding value in
> platform_data.
>
> This will leave the unset values in platform_data initialized as 0,
> which is in fact a valid gpio pin. mvsdio will grab that pin and
> configure it as gpio, which in turn breaks nand controller as mpp0
> also carries nand_io2.
>
> This patch fixes the above by initializing unused gpio functions in
> the platform_data with an invalid (-1) value.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Reported-by: Soeren Moch <smoch@web.de>
This somewhat "conflicts" with the patch I've submitted on the mvsdio
driver to exclude 0 as a valid GPIO, see
http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/157157.html.
This patch ensures that the mvsdio driver behaves as it was behaving
before 3.9 as far as legacy probing is concerned.
That said, I have nothing against explicitly setting those GPIO values
to an invalid value. Maybe -EINVAL would make more sense than just -1 ?
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
2013-03-23 15:17 ` Thomas Petazzoni
@ 2013-03-23 15:25 ` Sebastian Hesselbarth
2013-03-23 16:30 ` Thomas Petazzoni
0 siblings, 1 reply; 6+ messages in thread
From: Sebastian Hesselbarth @ 2013-03-23 15:25 UTC (permalink / raw)
To: linux-arm-kernel
On 03/23/2013 04:17 PM, Thomas Petazzoni wrote:
> On Sat, 23 Mar 2013 13:56:58 +0100, Sebastian Hesselbarth wrote:
>> mvsdio_platform_data allows to pass card detect and write protect gpio
>> numbers to the driver. Some kirkwood boards don't use both pins as
>> they are not connected, and don't set the corresponding value in
>> platform_data.
>>
>> This will leave the unset values in platform_data initialized as 0,
>> which is in fact a valid gpio pin. mvsdio will grab that pin and
>> configure it as gpio, which in turn breaks nand controller as mpp0
>> also carries nand_io2.
>>
>> This patch fixes the above by initializing unused gpio functions in
>> the platform_data with an invalid (-1) value.
>>
>> Signed-off-by: Sebastian Hesselbarth<sebastian.hesselbarth@gmail.com>
>> Reported-by: Soeren Moch<smoch@web.de>
>
> This somewhat "conflicts" with the patch I've submitted on the mvsdio
> driver to exclude 0 as a valid GPIO, see
> http://lists.infradead.org/pipermail/linux-arm-kernel/2013-March/157157.html.
> This patch ensures that the mvsdio driver behaves as it was behaving
> before 3.9 as far as legacy probing is concerned.
Thomas,
I understand that you proposed patch fixes mvsdio grab mpp0 by accident.
But what if you have a kirkwood board where cd-gpio _is_ connected to mpp0?
Not that there is one I know of, but IMHO the only useful patch is to
set passed values to an invalid gpio number.
> That said, I have nothing against explicitly setting those GPIO values
> to an invalid value. Maybe -EINVAL would make more sense than just -1 ?
Every invalid gpio number will be sufficient. But -EINVAL doesn't make
more sense than -1 does. Having no cd-gpio is not an "Invalid argument".
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
2013-03-23 15:25 ` Sebastian Hesselbarth
@ 2013-03-23 16:30 ` Thomas Petazzoni
2013-03-23 17:33 ` Sebastian Hesselbarth
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Petazzoni @ 2013-03-23 16:30 UTC (permalink / raw)
To: linux-arm-kernel
Dear Sebastian Hesselbarth,
On Sat, 23 Mar 2013 16:25:54 +0100, Sebastian Hesselbarth wrote:
> I understand that you proposed patch fixes mvsdio grab mpp0 by accident.
> But what if you have a kirkwood board where cd-gpio _is_ connected to mpp0?
It didn't work with the existing mvsdio driver, so the purpose of my
patch was merely to restore the old behavior, in order to avoid having
to change all the instances of mvsdio_platform_data, knowing that those
would anyway go away as we convert boards to the Device Tree.
> Not that there is one I know of, but IMHO the only useful patch is to
> set passed values to an invalid gpio number.
To me, it remains a fragile way of doing things. Let's say tomorrow you
add a new "int foo_gpio" field in mvsdio_platform_data. The whole
purpose of C99 struct initializers is that you don't have to change all
instances of the structure because all fields that are not initialized
with .<field> = <value> are guaranteed to be zero. If you need to set
foo_gpio to -1 everywhere when you add this field, it becomes quite
annoying.
> > That said, I have nothing against explicitly setting those GPIO values
> > to an invalid value. Maybe -EINVAL would make more sense than just -1 ?
>
> Every invalid gpio number will be sufficient. But -EINVAL doesn't make
> more sense than -1 does. Having no cd-gpio is not an "Invalid argument".
It's just that I've seen -EINVAL being used on some other platforms, at
least mach-at91/, but I agree it's not an invalid argument per se.
Best regards,
Thomas
--
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
2013-03-23 16:30 ` Thomas Petazzoni
@ 2013-03-23 17:33 ` Sebastian Hesselbarth
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Hesselbarth @ 2013-03-23 17:33 UTC (permalink / raw)
To: linux-arm-kernel
On 03/23/2013 05:30 PM, Thomas Petazzoni wrote:
> Dear Sebastian Hesselbarth,
>
> On Sat, 23 Mar 2013 16:25:54 +0100, Sebastian Hesselbarth wrote:
>
>> I understand that you proposed patch fixes mvsdio grab mpp0 by accident.
>> But what if you have a kirkwood board where cd-gpio _is_ connected to mpp0?
>
> It didn't work with the existing mvsdio driver, so the purpose of my
> patch was merely to restore the old behavior, in order to avoid having
> to change all the instances of mvsdio_platform_data, knowing that those
> would anyway go away as we convert boards to the Device Tree.
Thomas,
I agree both approaches fix the same issue. I haven't seen your patch
earlier and was just proposing a patch for the same issue. You or Jason
are free to choose whatever patch you like.
>> Not that there is one I know of, but IMHO the only useful patch is to
>> set passed values to an invalid gpio number.
>
> To me, it remains a fragile way of doing things. Let's say tomorrow you
> add a new "int foo_gpio" field in mvsdio_platform_data. The whole
> purpose of C99 struct initializers is that you don't have to change all
> instances of the structure because all fields that are not initialized
> with .<field> =<value> are guaranteed to be zero. If you need to set
> foo_gpio to -1 everywhere when you add this field, it becomes quite
> annoying.
I understand that struct initialization with zero is done for a purpose.
But gpio = 0 is a valid gpio number and engineers will always count from
zero ;)
Given the fact that .gpio = 0 is valid, you would have to add some
.really_use_that_number_i_was_too_lazy_to_set = 1.. but mvsdio_plaform_data
won't stay long as you already pointed out, and I am happy with any fix
for the issue.
>>> That said, I have nothing against explicitly setting those GPIO values
>>> to an invalid value. Maybe -EINVAL would make more sense than just -1 ?
>>
>> Every invalid gpio number will be sufficient. But -EINVAL doesn't make
>> more sense than -1 does. Having no cd-gpio is not an "Invalid argument".
>
> It's just that I've seen -EINVAL being used on some other platforms, at
> least mach-at91/, but I agree it's not an invalid argument per se.
If you don't like -1, you can choose -ENOENT. But -1 has been used for ages
as "invalid" or "unused" so it was my first guess.
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
2013-03-23 12:56 [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins Sebastian Hesselbarth
2013-03-23 15:17 ` Thomas Petazzoni
@ 2013-03-28 17:00 ` Jason Cooper
1 sibling, 0 replies; 6+ messages in thread
From: Jason Cooper @ 2013-03-28 17:00 UTC (permalink / raw)
To: linux-arm-kernel
On Sat, Mar 23, 2013 at 01:56:58PM +0100, Sebastian Hesselbarth wrote:
> mvsdio_platform_data allows to pass card detect and write protect gpio
> numbers to the driver. Some kirkwood boards don't use both pins as they
> are not connected, and don't set the corresponding value in platform_data.
>
> This will leave the unset values in platform_data initialized as 0, which
> is in fact a valid gpio pin. mvsdio will grab that pin and configure it as
> gpio, which in turn breaks nand controller as mpp0 also carries nand_io2.
>
> This patch fixes the above by initializing unused gpio functions in the
> platform_data with an invalid (-1) value.
>
> Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
> Reported-by: Soeren Moch <smoch@web.de>
> ---
> Cc: Soeren Moch <smoch@web.de>
> Cc: Jason Cooper <jason@lakedaemon.net>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: linux-arm-kernel at lists.infradead.org
> Cc: linux-kernel at vger.kernel.org
> ---
> arch/arm/mach-kirkwood/guruplug-setup.c | 2 ++
> arch/arm/mach-kirkwood/openrd-setup.c | 1 +
> arch/arm/mach-kirkwood/rd88f6281-setup.c | 1 +
> 3 files changed, 4 insertions(+)
Applied to mvebu/fixes
thx,
Jason.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-28 17:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-23 12:56 [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins Sebastian Hesselbarth
2013-03-23 15:17 ` Thomas Petazzoni
2013-03-23 15:25 ` Sebastian Hesselbarth
2013-03-23 16:30 ` Thomas Petazzoni
2013-03-23 17:33 ` Sebastian Hesselbarth
2013-03-28 17:00 ` Jason Cooper
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).