All of lore.kernel.org
 help / color / mirror / Atom feed
From: swarren@wwwdotorg.org (Stephen Warren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/3] pinctrl: imx: move hard-coding data into device tree
Date: Thu, 21 Feb 2013 12:57:02 -0700	[thread overview]
Message-ID: <51267C0E.6070902@wwwdotorg.org> (raw)
In-Reply-To: <CAP1dx+w2bLcztrRJOYs07xxSqVgo3bggJHMFt5LyL5jXQ-h6Mg@mail.gmail.com>

On 02/21/2013 02:36 AM, Dong Aisheng wrote:
> On 21 February 2013 03:04, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 02/20/2013 12:08 AM, Shawn Guo wrote:
>>> Currently, all imx pinctrl drivers maintain a big array of struct
>>> imx_pin_reg which hard-codes data like register offset and mux mode
>>> setting for each pin function.  Every time a new imx SoC support is
>>> added, we need to add such a big mount of data.  With moving to single
>>> kernel build, it's only matter of time to be blamed on memory consuming.
>>>
>>> With DTC pre-processor support in place, the patch moves all these data
>>> into device tree by redefining the PIN_FUNC_ID in imxXX-pinfunc.h and
>>> changing the PIN_FUNC_ID parsing code a little bit.
...
>> This patch removes a bunch of tables from the kernel. I like having the
>> tables in the kernel, since in theory it could allow a future debugfs or
>> sysfs interface to pinctrl that allows manipulation of the pinctrl HW
>> state at a semantic level.
> 
> Right, that's one of the considerations why i kept the register table in driver
> in the first binding design.
> 
>> This is only possible if the DT binding
>> includes details such as "set this pin to this function", and the driver
>> includes the tables to convert that into details such as register
>> address and value. I don't think such an "API" could be implemented for
>> IMX after this patch.
> 
> Theoretically It can, because device tree contains the pin registers
> informations

> in this way. But the problem is that it can only manipulate the used pins which
> are parsed from devices tree. While for those unused pins, since the driver does
> not have the pin register information, it can not manipulate it.

Right, that's the main issue I was trying to highlight; the driver no
longer provides complete knowledge of the HW, but only learns about
stuff that happens to be used/defined in the board-specific DT.

> But what i'm wondering now is do we really need to manipulate the unused pins
> from sysfs or debugfs?
> I still can not think out a real using case now.
> Anyone else can think one?

For early board bringup, it may be useful to boot with a minimal set of
entries in the DT (enough to support e.g. serial console and some device
for a root filesystem) and then interactively tweak the rest of the
pinmux to define the rest of the board, and perhaps even save the result
somehow, and add it to the next iteration of the DT.

Also, consider a hobbyist-oriented board that has 50 unused pins that
could be GPIO, or mapped to some otherwise unused SD, I2C, SPI, ...
controller. Do we want to force the user to write a new DT and reboot,
or would it be better to provide some runtime API to set up those pins
based on what they had connected to the board, and then instantiate
drivers or use the sysfs GPIO interface, after doing so?

WARNING: multiple messages have this Message-ID (diff)
From: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
To: Dong Aisheng <dong.aisheng-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: "Rob Herring"
	<rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>,
	"Uwe Kleine-König"
	<u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>,
	devicetree-discuss
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Subject: Re: [PATCH 3/3] pinctrl: imx: move hard-coding data into device tree
Date: Thu, 21 Feb 2013 12:57:02 -0700	[thread overview]
Message-ID: <51267C0E.6070902@wwwdotorg.org> (raw)
In-Reply-To: <CAP1dx+w2bLcztrRJOYs07xxSqVgo3bggJHMFt5LyL5jXQ-h6Mg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 02/21/2013 02:36 AM, Dong Aisheng wrote:
> On 21 February 2013 03:04, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 02/20/2013 12:08 AM, Shawn Guo wrote:
>>> Currently, all imx pinctrl drivers maintain a big array of struct
>>> imx_pin_reg which hard-codes data like register offset and mux mode
>>> setting for each pin function.  Every time a new imx SoC support is
>>> added, we need to add such a big mount of data.  With moving to single
>>> kernel build, it's only matter of time to be blamed on memory consuming.
>>>
>>> With DTC pre-processor support in place, the patch moves all these data
>>> into device tree by redefining the PIN_FUNC_ID in imxXX-pinfunc.h and
>>> changing the PIN_FUNC_ID parsing code a little bit.
...
>> This patch removes a bunch of tables from the kernel. I like having the
>> tables in the kernel, since in theory it could allow a future debugfs or
>> sysfs interface to pinctrl that allows manipulation of the pinctrl HW
>> state at a semantic level.
> 
> Right, that's one of the considerations why i kept the register table in driver
> in the first binding design.
> 
>> This is only possible if the DT binding
>> includes details such as "set this pin to this function", and the driver
>> includes the tables to convert that into details such as register
>> address and value. I don't think such an "API" could be implemented for
>> IMX after this patch.
> 
> Theoretically It can, because device tree contains the pin registers
> informations

> in this way. But the problem is that it can only manipulate the used pins which
> are parsed from devices tree. While for those unused pins, since the driver does
> not have the pin register information, it can not manipulate it.

Right, that's the main issue I was trying to highlight; the driver no
longer provides complete knowledge of the HW, but only learns about
stuff that happens to be used/defined in the board-specific DT.

> But what i'm wondering now is do we really need to manipulate the unused pins
> from sysfs or debugfs?
> I still can not think out a real using case now.
> Anyone else can think one?

For early board bringup, it may be useful to boot with a minimal set of
entries in the DT (enough to support e.g. serial console and some device
for a root filesystem) and then interactively tweak the rest of the
pinmux to define the rest of the board, and perhaps even save the result
somehow, and add it to the next iteration of the DT.

Also, consider a hobbyist-oriented board that has 50 unused pins that
could be GPIO, or mapped to some otherwise unused SD, I2C, SPI, ...
controller. Do we want to force the user to write a new DT and reboot,
or would it be better to provide some runtime API to set up those pins
based on what they had connected to the board, and then instantiate
drivers or use the sysfs GPIO interface, after doing so?

  reply	other threads:[~2013-02-21 19:57 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-20  7:08 [PATCH 0/3] Get rid of big array from imx pinctrl driver Shawn Guo
2013-02-20  7:08 ` [PATCH 1/3] ARM: dts: imx: use pre-processor for device trees Shawn Guo
2013-02-20  9:26   ` Dong Aisheng
2013-02-20  9:25 ` [PATCH 0/3] Get rid of big array from imx pinctrl driver Dong Aisheng
     [not found] ` <1361344089-16804-3-git-send-email-shawn.guo@linaro.org>
2013-02-20  9:30   ` [PATCH 2/3] ARM: dts: imx: replace magic number with pin function name Dong Aisheng
2013-02-20 18:46   ` Stephen Warren
2013-02-20 18:46     ` Stephen Warren
2013-02-21  0:03     ` Matt Sealey
2013-02-21  0:03       ` Matt Sealey
2013-02-21  0:34       ` Stephen Warren
2013-02-21  0:34         ` Stephen Warren
2013-02-21  5:02       ` Shawn Guo
2013-02-21  5:02         ` Shawn Guo
2013-02-21 17:36         ` Matt Sealey
2013-02-21 17:36           ` Matt Sealey
2013-02-21 17:57           ` Matt Sealey
2013-02-21 17:57             ` Matt Sealey
2013-02-21 21:43           ` Sascha Hauer
2013-02-21 21:43             ` Sascha Hauer
2013-02-22  7:58             ` Shawn Guo
2013-02-22  7:58               ` Shawn Guo
2013-02-22  5:52           ` Shawn Guo
2013-02-22  5:52             ` Shawn Guo
2013-02-22  7:27             ` Sascha Hauer
2013-02-22  7:27               ` Sascha Hauer
2013-02-22  7:36               ` Shawn Guo
2013-02-22  7:36                 ` Shawn Guo
2013-02-22  8:12                 ` Sascha Hauer
2013-02-22  8:12                   ` Sascha Hauer
2013-02-27  6:51             ` Matt Sealey
2013-02-27  6:51               ` Matt Sealey
2013-02-27  7:44               ` Sascha Hauer
2013-02-27  7:44                 ` Sascha Hauer
2013-02-27 18:16                 ` Matt Sealey
2013-02-27 18:16                   ` Matt Sealey
2013-02-27 20:00                   ` Sascha Hauer
2013-02-27 20:00                     ` Sascha Hauer
2013-02-28  3:06                   ` Shawn Guo
2013-02-28  3:06                     ` Shawn Guo
2013-02-21  4:59     ` Shawn Guo
2013-02-21  4:59       ` Shawn Guo
     [not found] ` <1361344089-16804-4-git-send-email-shawn.guo@linaro.org>
2013-02-20  9:44   ` [PATCH 3/3] pinctrl: imx: move hard-coding data into device tree Dong Aisheng
2013-02-21  6:04     ` Shawn Guo
2013-02-20 19:04   ` Stephen Warren
2013-02-20 19:04     ` Stephen Warren
2013-02-21  5:30     ` Shawn Guo
2013-02-21  5:30       ` Shawn Guo
2013-02-21  7:55       ` Sascha Hauer
2013-02-21  7:55         ` Sascha Hauer
2013-02-21  9:36     ` Dong Aisheng
2013-02-21  9:36       ` Dong Aisheng
2013-02-21 19:57       ` Stephen Warren [this message]
2013-02-21 19:57         ` Stephen Warren
2013-02-26  8:02         ` Dong Aisheng
2013-02-26  8:02           ` Dong Aisheng

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=51267C0E.6070902@wwwdotorg.org \
    --to=swarren@wwwdotorg.org \
    --cc=linux-arm-kernel@lists.infradead.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.