All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Warren <swarren@wwwdotorg.org>
To: Tomasz Figa <t.figa@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org, kgene.kim@samsung.com,
	devicetree-discuss@lists.ozlabs.org, kyungmin.park@samsung.com,
	linux-samsung-soc@vger.kernel.org, thomas.abraham@linaro.org,
	linus.walleij@linaro.org, m.szyprowski@samsung.com
Subject: Re: [RFC 0/6] pinctrl: samsung: Remove static platform-specific data
Date: Fri, 21 Sep 2012 12:40:35 -0600	[thread overview]
Message-ID: <505CB4A3.9050707@wwwdotorg.org> (raw)
In-Reply-To: <1348131197-25506-1-git-send-email-t.figa@samsung.com>

On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> This RFC series is a work on replacing static platform-specific data in
> pinctrl-samsung driver with data dynamically parsed from device tree.

Hmm. I tend to think this is exactly the opposite of the correct
direction; you end up wasting a whole ton of time during the boot
process parsing data out of the device tree only to end up with exactly
the same tables that you would have just put into the kernel anyway. Is
it really likely that future SoCs will change information such as the
width of the pullup/pulldown bitfield, but not change anything else
that's not already in this binding. If that isn't the case, the binding
won't be complete enough to describe any new features on future SoCs anyway.

> It aims at reducing the SoC-specific part of the driver and thus the
> amount of modifications to driver sources when adding support for next
> SoCs (like Exynos4x12).
> 
> Furthermore, moving definitions of pin banks to device tree will allow
> to simplify GPIO and GEINT specification to a format similar to used
> previously by gpiolib-based implementation, using a phandle to the bank
> and pin index inside the bank, e.g.
> 	gpios = <&gpa1 4 0>;
> 	interrupt-parent = <&gpa1>;
> 	interrupts = <4 0>;

I don't think those two are correlated; the GPIO specifier format could
just as easily be <bank pin> irrespective of whether the pinctrl driver
contains SoC-specific tables or not.

> Any comments are welcome.
> 
> TODO:
>  - bindings documentation

That's unfortunate; it would be the most interesting part to review. I
guess I'll try to work out the binding from the examples in patch 6.

WARNING: multiple messages have this Message-ID (diff)
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC 0/6] pinctrl: samsung: Remove static platform-specific data
Date: Fri, 21 Sep 2012 12:40:35 -0600	[thread overview]
Message-ID: <505CB4A3.9050707@wwwdotorg.org> (raw)
In-Reply-To: <1348131197-25506-1-git-send-email-t.figa@samsung.com>

On 09/20/2012 02:53 AM, Tomasz Figa wrote:
> This RFC series is a work on replacing static platform-specific data in
> pinctrl-samsung driver with data dynamically parsed from device tree.

Hmm. I tend to think this is exactly the opposite of the correct
direction; you end up wasting a whole ton of time during the boot
process parsing data out of the device tree only to end up with exactly
the same tables that you would have just put into the kernel anyway. Is
it really likely that future SoCs will change information such as the
width of the pullup/pulldown bitfield, but not change anything else
that's not already in this binding. If that isn't the case, the binding
won't be complete enough to describe any new features on future SoCs anyway.

> It aims at reducing the SoC-specific part of the driver and thus the
> amount of modifications to driver sources when adding support for next
> SoCs (like Exynos4x12).
> 
> Furthermore, moving definitions of pin banks to device tree will allow
> to simplify GPIO and GEINT specification to a format similar to used
> previously by gpiolib-based implementation, using a phandle to the bank
> and pin index inside the bank, e.g.
> 	gpios = <&gpa1 4 0>;
> 	interrupt-parent = <&gpa1>;
> 	interrupts = <4 0>;

I don't think those two are correlated; the GPIO specifier format could
just as easily be <bank pin> irrespective of whether the pinctrl driver
contains SoC-specific tables or not.

> Any comments are welcome.
> 
> TODO:
>  - bindings documentation

That's unfortunate; it would be the most interesting part to review. I
guess I'll try to work out the binding from the examples in patch 6.

  parent reply	other threads:[~2012-09-21 18:40 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-20  8:53 [RFC 0/6] pinctrl: samsung: Remove static platform-specific data Tomasz Figa
2012-09-20  8:53 ` Tomasz Figa
2012-09-20  8:53 ` [RFC 1/6] pinctrl: exynos: Parse wakeup-eint parameters from DT Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-20  8:53 ` [RFC 2/6] pinctrl: samsung: Parse pin banks " Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-20  8:53 ` [RFC 3/6] pinctrl: exynos: Remove static platform-specific data Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-20  8:53 ` [RFC 4/6] pinctrl: samsung: Parse bank-specific eint offset from DT Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-20  8:53 ` [RFC 5/6] ARM: dts: exynos4210: Remove legacy gpio nodes Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-20  8:53 ` [RFC 6/6] ARM: dts: exynos4210: Add platform-specific descriptions for pin controllers Tomasz Figa
2012-09-20  8:53   ` Tomasz Figa
2012-09-21 18:56   ` Stephen Warren
2012-09-21 18:56     ` Stephen Warren
2012-09-21 19:54     ` Tomasz Figa
2012-09-21 19:54       ` Tomasz Figa
2012-09-24 17:42       ` Stephen Warren
2012-09-24 17:42         ` Stephen Warren
2012-09-24 21:31         ` Tomasz Figa
2012-09-24 21:31           ` Tomasz Figa
2012-09-24 23:14           ` Stephen Warren
2012-09-24 23:14             ` Stephen Warren
2012-09-25  9:37             ` Tomasz Figa
2012-09-25  9:37               ` Tomasz Figa
2012-09-25 16:49               ` Stephen Warren
2012-09-25 16:49                 ` Stephen Warren
2012-09-25 17:41                 ` Tomasz Figa
2012-09-25 17:41                   ` Tomasz Figa
2012-09-25 18:22                   ` Stephen Warren
2012-09-25 18:22                     ` Stephen Warren
2012-09-25 18:35                     ` Tomasz Figa
2012-09-25 18:35                       ` Tomasz Figa
2012-09-25 22:52                       ` Stephen Warren
2012-09-25 22:52                         ` Stephen Warren
2012-09-20 10:27 ` [RFC 0/6] pinctrl: samsung: Remove static platform-specific data Linus Walleij
2012-09-20 10:27   ` Linus Walleij
2012-09-21 18:40 ` Stephen Warren [this message]
2012-09-21 18:40   ` Stephen Warren
2012-09-21 19:31   ` Tomasz Figa
2012-09-21 19:31     ` Tomasz Figa
2012-09-24 17:34     ` Stephen Warren
2012-09-24 17:34       ` Stephen Warren

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=505CB4A3.9050707@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=devicetree-discuss@lists.ozlabs.org \
    --cc=kgene.kim@samsung.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=m.szyprowski@samsung.com \
    --cc=t.figa@samsung.com \
    --cc=thomas.abraham@linaro.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.