All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tony Lindgren <tony@atomide.com>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Haojian Zhuang <haojian.zhuang@marvell.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Peter Ujfalusi <peter.ujfalusi@ti.com>,
	Prakash Manjunathappa <prakash.pm@ti.com>,
	Haojian Zhuang <haojian.zhuang@linaro.org>,
	Linux-OMAP <linux-omap@vger.kernel.org>,
	Roger Quadros <rogerq@ti.com>
Subject: Re: [PATCH 3/6] pinctrl: single: Prepare for supporting SoC specific features
Date: Tue, 8 Oct 2013 09:21:28 -0700	[thread overview]
Message-ID: <20131008162128.GB7773@atomide.com> (raw)
In-Reply-To: <CACRpkdbx+vJEqz3KQ0EHGt0axRB4JeJh3FF4e+7snbR0HoqKRg@mail.gmail.com>

* Linus Walleij <linus.walleij@linaro.org> [131008 05:03]:
> On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > Hi Linus W,
> >
> > Any comments on the pinctrl patches 3 - 5 in this series?
> 
> I have no problems with this patch #3, as it is just changing syntax,
> not semantics.
> 
> The problems start with patch #4.
> 
> I am tormented with mixed feelings about this, because from one point of
> view I feel it is breaking the promise of pinctrl-single being a
> driver for platforms
> where a pin is controlled by a *single* register.

It is still in that same *single* register. There are interrupt enable
and interrupt status bits for *every* pin register on most omaps.
 
> If this was pinctrl-foo.c I would not have been so much bothered,
> but now it is something that was supposed to be self-contained and
> simple, pertaining to a single register, starting to look like something
> else.
>
> This is a bit like: "oh yeah just one register controls the pins, but under
> some circumstances I also want to mess with this register over here,
> and then this register over there ..." etc.

Not true. If it was some other register I would have set it up as
a separate driver under drivers/irqchip.
 
> I'd like Haojian to ACK this to proceed since he's also using this driver
> now. Then I feel better on continuing down this road ...
> 
> Then I have a lesser comment on patch #4 since it makes it possible
> for this pin controller to support wake-up interrupt, as I don't see how
> this plays out with front-end GPIO controllers, but let's discuss that
> in the context of that patch.

It's completely separate from the GPIO controller wake-up events.

Regards,

Tony


WARNING: multiple messages have this Message-ID (diff)
From: tony@atomide.com (Tony Lindgren)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/6] pinctrl: single: Prepare for supporting SoC specific features
Date: Tue, 8 Oct 2013 09:21:28 -0700	[thread overview]
Message-ID: <20131008162128.GB7773@atomide.com> (raw)
In-Reply-To: <CACRpkdbx+vJEqz3KQ0EHGt0axRB4JeJh3FF4e+7snbR0HoqKRg@mail.gmail.com>

* Linus Walleij <linus.walleij@linaro.org> [131008 05:03]:
> On Mon, Oct 7, 2013 at 7:35 PM, Tony Lindgren <tony@atomide.com> wrote:
> 
> > Hi Linus W,
> >
> > Any comments on the pinctrl patches 3 - 5 in this series?
> 
> I have no problems with this patch #3, as it is just changing syntax,
> not semantics.
> 
> The problems start with patch #4.
> 
> I am tormented with mixed feelings about this, because from one point of
> view I feel it is breaking the promise of pinctrl-single being a
> driver for platforms
> where a pin is controlled by a *single* register.

It is still in that same *single* register. There are interrupt enable
and interrupt status bits for *every* pin register on most omaps.
 
> If this was pinctrl-foo.c I would not have been so much bothered,
> but now it is something that was supposed to be self-contained and
> simple, pertaining to a single register, starting to look like something
> else.
>
> This is a bit like: "oh yeah just one register controls the pins, but under
> some circumstances I also want to mess with this register over here,
> and then this register over there ..." etc.

Not true. If it was some other register I would have set it up as
a separate driver under drivers/irqchip.
 
> I'd like Haojian to ACK this to proceed since he's also using this driver
> now. Then I feel better on continuing down this road ...
> 
> Then I have a lesser comment on patch #4 since it makes it possible
> for this pin controller to support wake-up interrupt, as I don't see how
> this plays out with front-end GPIO controllers, but let's discuss that
> in the context of that patch.

It's completely separate from the GPIO controller wake-up events.

Regards,

Tony

  reply	other threads:[~2013-10-08 16:21 UTC|newest]

Thread overview: 105+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-03  5:42 [PATCH 0/6] add support for omap wake-up interrupts via pinctrl-single, take2 Tony Lindgren
2013-10-03  5:42 ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 1/6] ARM: dts: Fix pinctrl mask for omap3 Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 2/6] ARM: OMAP2+: Add support for auxdata Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 3/6] pinctrl: single: Prepare for supporting SoC specific features Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-07 17:35   ` Tony Lindgren
2013-10-07 17:35     ` Tony Lindgren
2013-10-08 11:55     ` Linus Walleij
2013-10-08 11:55       ` Linus Walleij
2013-10-08 16:21       ` Tony Lindgren [this message]
2013-10-08 16:21         ` Tony Lindgren
2013-10-09  5:03       ` Haojian Zhuang
2013-10-09  5:03         ` Haojian Zhuang
2013-10-09 13:43     ` Linus Walleij
2013-10-09 13:43       ` Linus Walleij
2013-10-09 15:10       ` Tony Lindgren
2013-10-09 15:10         ` Tony Lindgren
2013-10-10 15:35         ` Linus Walleij
2013-10-10 15:35           ` Linus Walleij
2013-10-10 22:41           ` Tony Lindgren
2013-10-10 22:41             ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 4/6] pinctrl: single: Add support for wake-up interrupts Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-03 17:50   ` Tony Lindgren
2013-10-03 17:50     ` Tony Lindgren
2013-10-03 17:50     ` Tony Lindgren
2013-10-08 12:10   ` Linus Walleij
2013-10-08 12:10     ` Linus Walleij
2013-10-08 16:05     ` Tony Lindgren
2013-10-08 16:05       ` Tony Lindgren
2013-10-10 13:24   ` Roger Quadros
2013-10-10 13:24     ` Roger Quadros
2013-10-10 13:24     ` Roger Quadros
     [not found]     ` <5256AA7F.8030005-l0cyMroinI0@public.gmane.org>
2013-10-10 14:04       ` Linus Walleij
2013-10-10 14:04         ` Linus Walleij
2013-10-10 14:04         ` Linus Walleij
     [not found]         ` <CACRpkdZx5+OCcQytkpjd7_nz4hw6VXoXOS0LByKgJ8_cQjbA4g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-10 14:35           ` Roger Quadros
2013-10-10 14:35             ` Roger Quadros
2013-10-10 14:35             ` Roger Quadros
2013-10-10 15:32             ` Linus Walleij
2013-10-10 15:32               ` Linus Walleij
     [not found]               ` <CACRpkdY6NSR7gSc6=_nEr_BdTiEM0Mt9t4GoBSoP26ceVphDhA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2013-10-10 16:15                 ` Tony Lindgren
2013-10-10 16:15                   ` Tony Lindgren
2013-10-10 16:15                   ` Tony Lindgren
2013-10-10 16:00     ` Tony Lindgren
2013-10-10 16:00       ` Tony Lindgren
2013-10-10 16:11       ` Linus Walleij
2013-10-10 16:11         ` Linus Walleij
2013-10-10 16:20         ` Tony Lindgren
2013-10-10 16:20           ` Tony Lindgren
2013-10-11  8:00           ` Linus Walleij
2013-10-11  8:00             ` Linus Walleij
2013-10-11  8:56             ` Roger Quadros
2013-10-11  8:56               ` Roger Quadros
2013-10-11  8:56               ` Roger Quadros
2013-10-11 10:32               ` Linus Walleij
2013-10-11 10:32                 ` Linus Walleij
2013-10-11 15:43                 ` Tony Lindgren
2013-10-11 15:43                   ` Tony Lindgren
2013-10-11 15:56                   ` Linus Walleij
2013-10-11 15:56                     ` Linus Walleij
2013-10-11 16:01                     ` Tony Lindgren
2013-10-11 16:01                       ` Tony Lindgren
2013-10-18  7:40                       ` Balaji T K
2013-10-18  7:40                         ` Balaji T K
2013-10-18 15:35                         ` Tony Lindgren
2013-10-18 15:35                           ` Tony Lindgren
2013-10-18 15:59                           ` Balaji T K
2013-10-18 15:59                             ` Balaji T K
2013-10-18 15:59                             ` Balaji T K
     [not found]                             ` <52615AD4.7040306-l0cyMroinI0@public.gmane.org>
2013-10-18 16:06                               ` Tony Lindgren
2013-10-18 16:06                                 ` Tony Lindgren
2013-10-18 16:06                                 ` Tony Lindgren
2013-10-11 15:36               ` Tony Lindgren
2013-10-11 15:36                 ` Tony Lindgren
2013-10-11 15:43                 ` Balaji T K
2013-10-11 15:43                   ` Balaji T K
2013-10-11 15:43                   ` Balaji T K
2013-10-11 15:48                   ` Tony Lindgren
2013-10-11 15:48                     ` Tony Lindgren
2013-10-10 16:23       ` Tony Lindgren
2013-10-10 16:23         ` Tony Lindgren
2013-10-11  8:45         ` Roger Quadros
2013-10-11  8:45           ` Roger Quadros
2013-10-11  8:49       ` Roger Quadros
2013-10-11  8:49         ` Roger Quadros
2013-10-11  8:49         ` Roger Quadros
     [not found]         ` <5257BB8C.5080001-l0cyMroinI0@public.gmane.org>
2013-10-11 13:59           ` Roger Quadros
2013-10-11 13:59             ` Roger Quadros
2013-10-11 13:59             ` Roger Quadros
     [not found]             ` <52580438.4040002-l0cyMroinI0@public.gmane.org>
2013-10-11 15:18               ` Tony Lindgren
2013-10-11 15:18                 ` Tony Lindgren
2013-10-11 15:18                 ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 5/6] pinctrl: single: Add support for auxdata Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-03  5:42 ` [PATCH 6/6] ARM: OMAP: Move DT wake-up event handling over to use pinctrl-single-omap Tony Lindgren
2013-10-03  5:42   ` Tony Lindgren
2013-10-10 21:47 ` [PATCH 0/6] add support for omap wake-up interrupts via pinctrl-single, take2 Kevin Hilman
2013-10-10 21:47   ` Kevin Hilman
2013-10-10 22:47   ` Tony Lindgren
2013-10-10 22:47     ` Tony Lindgren

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=20131008162128.GB7773@atomide.com \
    --to=tony@atomide.com \
    --cc=grygorii.strashko@ti.com \
    --cc=haojian.zhuang@linaro.org \
    --cc=haojian.zhuang@marvell.com \
    --cc=linus.walleij@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=peter.ujfalusi@ti.com \
    --cc=prakash.pm@ti.com \
    --cc=rogerq@ti.com \
    /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.