From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
Russell King <linux@arm.linux.org.uk>,
Jason Cooper <jason@lakedaemon.net>,
linux-kernel@vger.kernel.org, Soeren Moch <smoch@web.de>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] ARM: Kirkwood: fix unused mvsdio gpio pins
Date: Sat, 23 Mar 2013 17:30:52 +0100 [thread overview]
Message-ID: <20130323173052.10ecc4cb@skate> (raw)
In-Reply-To: <514DC982.3010706@gmail.com>
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
next prev parent reply other threads:[~2013-03-23 16:30 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2013-03-23 17:33 ` Sebastian Hesselbarth
2013-03-28 17:00 ` Jason Cooper
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=20130323173052.10ecc4cb@skate \
--to=thomas.petazzoni@free-electrons.com \
--cc=andrew@lunn.ch \
--cc=jason@lakedaemon.net \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=sebastian.hesselbarth@gmail.com \
--cc=smoch@web.de \
/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