From: swarren@wwwdotorg•org (Stephen Warren)
To: linux-arm-kernel@lists•infradead.org
Subject: [PATCH] pinctrl: gpio: vt8500: Add pin control driver for Wondermedia SoCs
Date: Fri, 01 Mar 2013 11:24:54 -0700 [thread overview]
Message-ID: <5130F276.8000904@wwwdotorg.org> (raw)
In-Reply-To: <1362032757.32211.10.camel@gitbox>
On 02/27/2013 11:25 PM, Tony Prisk wrote:
> Thanks for the review Stephen,
>
> I have posted replies to most of your points/questions inline.
>
> The review was slightly more in-depth than I expected and as you
> noticed, the patch is poor in quality. I was more 'testing the water'
> that people were happy with the basic design of the code, rather than
> the specifics as I have 4 SoC's worth of data to add, and didn't want to
> have to redo it all if I had to make a data format change.
>
> It seems that on the whole the design is ok, so I will post a proper
> patch series for this to be reviewed (after addressing the issues you
> pointed out).
Oh, OK. It might have been a good idea to mention this in the patch
description, or have posted it as [PATCH RFC].
>>> diff --git a/drivers/pinctrl/pinctrl-wm8850.c b/drivers/pinctrl/pinctrl-wm8850.c
>>
>>> +/* Please keep sorted by bank/bit */
>>> +#define WMT_PIN_EXTGPIO0 WMT_PIN(0, 0)
>>> +#define WMT_PIN_EXTGPIO1 WMT_PIN(0, 1)
>> ...
>>> +#define WMT_PIN_I2C2_SCL WMT_PIN(5, 12)
>>> +#define WMT_PIN_I2C2_SDA WMT_PIN(5, 13)
>>
>> There are a lot of gaps in that list. Does the HW really not support pin
>> muxing on the rest of the bits in the registers?
>
> Nobody who knows is willing to say :). Most of the code for Wondermedia
> SoCs is based of the vendor source that has come out, and we therefore
> only know what the vendor has included support for.
>
> No doubt there will be other pins which have valid functions, but we
> don't know what they are and don't have support for them at the moment.
> The reason I went with the bank/pin encoding is so that if/when we add
> other pins, it won't affect any pin numbering.
Doing the pin numbering that way was a good choice.
So, there's an idea that when first defining a DT binding for HW, that
binding should be complete and accurate, rather than being minimal, with
a plan to incrementally expand it later. I would take that to mean that
for a pinctrl binding, the list of pins/groups/function/config-options
that the binding supports is complete. Obviously this isn't possible if
there's no complete documentation for the HW:-(
I hope this case will be allowed though, since any modifications to
support new pins/groups/functions should be possible to add in a
backwards-compatible way, especially with a forward-looking number
scheme for the pins.
We should consider this kind of issue if/when we actually implement
stricter rules for modifications to DT bindings, so as not to disallow
solving this problem.
prev parent reply other threads:[~2013-03-01 18:24 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-15 2:48 [RFC PATCH] Add pin control driver for Wondermedia SoCS Tony Prisk
2013-02-15 2:48 ` [PATCH] pinctrl: gpio: vt8500: Add pin control driver for Wondermedia SoCs Tony Prisk
2013-02-15 20:27 ` Linus Walleij
2013-02-15 22:26 ` Tony Prisk
2013-02-27 22:21 ` Stephen Warren
2013-02-28 6:25 ` Tony Prisk
2013-03-01 18:24 ` Stephen Warren [this message]
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=5130F276.8000904@wwwdotorg.org \
--to=swarren@wwwdotorg$(echo .)org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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