Re: [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins

From: Sebastian Hesselbarth
Date: Sat Mar 23 2013 - 13:33:50 EST

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.


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

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.

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at