devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Cc: Grant Likely
	<grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org>,
	"Olof Johansson (olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org)"
	<olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org>,
	"Colin Cross (ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org)"
	<ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org>,
	"linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org"
	<devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>
Subject: Re: [PATCH 6/6] gpio: tegra: Parameterize the number of banks
Date: Sat, 14 Jan 2012 09:15:54 -0600	[thread overview]
Message-ID: <4F119C2A.6020107@gmail.com> (raw)
In-Reply-To: <74CDBE0F657A3D45AFBB94109FB122FF17801D2039-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>

On 01/13/2012 02:55 PM, Stephen Warren wrote:
> Rob Herring wrote at Thursday, January 05, 2012 10:25 AM:
>> On 01/04/2012 04:00 PM, Grant Likely wrote:
>>> On Wed, Jan 4, 2012 at 1:00 PM, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>>>> Rob Herring wrote at Wednesday, January 04, 2012 12:54 PM:
>>>>> On 01/04/2012 12:39 PM, Stephen Warren wrote:
>>>>>> Tegra20's GPIO controller has 7 banks, and Tegra30's controller has 8
>>>>>> banks. Allow the number of banks to be configured at run-time by the
>>>>>> device tree.
>>>> ...
>>>>>> diff --git a/Documentation/devicetree/bindings/gpio/gpio_nvidia.txt
>>>> ...
>>>>>>  Required properties:
>>>>>>  - compatible : "nvidia,tegra20-gpio"
>>>>>>  - reg : Physical base address and length of the controller's registers.
>>>>>> +- nvidia,num-banks : The number of GPIO banks. This should be 7 for
>>>>>> +  Tegra20 and 8 for Tegra30. This must match the number of interrupt
>>>>>> +  specifiers in the interrupts property.
>>>>>
>>>>> You can determine the number of banks based on the compatible property
>>>>> rather than needing an additional property.
>>>>
>>>> That's certainly possible.
>>>>
>>>> However, if say nvidia,tegraNNN-gpio has 9 banks, we then have to
>>>> explicitly edit the driver to know that, whereas by using a property,
>>>> we wouldn't have to change the driver at all to support a future GPIO
>>>> controller. So, isn't it better to explicitly represent this in DT?
>>>>
>>>> Note that I have no idea how many GPIO banks our future chips will have,
>>>> so this might not turn out to save any work at all, but perhaps.
>>>
>>> It's an engineering/design decision that requires taste and instinct.
>>> Either approach is fine, you decide which one will be the best in the
>>> long term.
>>
>> Agreed. I'm really fine with it either way.
>>
>> Trying to predict future h/w is a bit pointless IMO. H/w designers
>> always find new ways to do things differently. i.MX family has gpio
>> interrupts hooked up 3 different ways for example. How would you handle
>> the case that the banks are sparsely implemented?
>>
>> Is adding support for a different number of banks every couple of years
>> really an issue? It's much more important to have properties for which
>> change with every board.
> 
> Thinking about this some more, I'm tempted to rework this patch to remove
> the extra DT property and just "detect" the number of banks based on the
> length of the interrupts property, (well, actually the number of IRQ
> resources that the platform device has) since each bank has its own
> interrupt. Does anyone disagree with doing that?

Sounds fine too me.

Rob

  parent reply	other threads:[~2012-01-14 15:15 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-04 18:39 [PATCH 1/6] ARM: tegra: Remove use of TEGRA_GPIO_TO_IRQ Stephen Warren
     [not found] ` <1325702378-20863-1-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-01-04 18:39   ` [PATCH 2/6] dt: tegra gpio: Flesh out binding documentation Stephen Warren
2012-01-04 18:39   ` [PATCH 3/6] ARM: dt: tegra30.dtsi: Reformat gpio's interrupts property Stephen Warren
     [not found]     ` <1325702378-20863-3-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-01-04 19:50       ` Grant Likely
2012-01-04 18:39   ` [PATCH 4/6] ARM: dt: tegra30.dtsi: Add extra GPIO interrupt Stephen Warren
2012-01-04 18:39   ` [PATCH 5/6] gpio: tegra: Dynamically allocate IRQ base, and support DT Stephen Warren
     [not found]     ` <1325702378-20863-5-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-01-05  7:23       ` Thierry Reding
     [not found]         ` <20120105072306.GA3980-RM9K5IK7kjIQXX3q8xo1gnVAuStQJXxyR5q1nwbD4aMs9pC9oP6+/A@public.gmane.org>
2012-01-05 17:47           ` Stephen Warren
     [not found]             ` <74CDBE0F657A3D45AFBB94109FB122FF17761F16C2-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2012-01-05 17:56               ` Grant Likely
2012-01-05 13:17       ` Jamie Iles
2012-01-05 16:47         ` Stephen Warren
     [not found]           ` <74CDBE0F657A3D45AFBB94109FB122FF17761F1683-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2012-01-05 16:48             ` Jamie Iles
2012-01-04 18:39   ` [PATCH 6/6] gpio: tegra: Parameterize the number of banks Stephen Warren
     [not found]     ` <1325702378-20863-6-git-send-email-swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2012-01-04 19:54       ` Rob Herring
     [not found]         ` <4F04AE71.2090203-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-01-04 20:00           ` Stephen Warren
     [not found]             ` <74CDBE0F657A3D45AFBB94109FB122FF17761F145E-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2012-01-04 22:00               ` Grant Likely
     [not found]                 ` <CACxGe6tyZKW37sGiDauvikU70ZVLHFVrXjBPJk2Kgee1Rj3sbg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2012-01-05 17:24                   ` Rob Herring
     [not found]                     ` <4F05DCDB.40806-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2012-01-13 20:55                       ` Stephen Warren
     [not found]                         ` <74CDBE0F657A3D45AFBB94109FB122FF17801D2039-C7FfzLzN0UxDw2glCA4ptUEOCMrvLtNR@public.gmane.org>
2012-01-14 15:15                           ` Rob Herring [this message]
2012-01-04 19:52   ` [PATCH 1/6] ARM: tegra: Remove use of TEGRA_GPIO_TO_IRQ Grant Likely
     [not found]     ` <20120104195218.GF15503-e0URQFbLeQY2iJbIjFUEsiwD8/FfD2ys@public.gmane.org>
2012-01-24  8:34       ` Olof Johansson

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=4F119C2A.6020107@gmail.com \
    --to=robherring2-re5jqeeqqe8avxtiumwx3w@public.gmane.org \
    --cc=ccross-z5hGa2qSFaRBDgjK7y7TUQ@public.gmane.org \
    --cc=devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org \
    --cc=grant.likely-s3s/WqlpOiPyB63q8FvJNQ@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org \
    --cc=swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).