All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Dunn <mikedunn@newsguy.com>
To: Igor Grinberg <grinberg@compulab.co.il>
Cc: Marek Vasut <marex@denx.de>,
	alsa-devel@alsa-project.org, Eric Miao <eric.y.miao@gmail.com>,
	Mark Brown <broonie@opensource.wolfsonmicro.com>,
	Robert Jarzmik <robert.jarzmik@free.fr>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 3/4] ALSA: pxa27x: ac97 controller driver requests gpio
Date: Mon, 07 Jan 2013 08:40:01 -0800	[thread overview]
Message-ID: <50EAFA61.7000209@newsguy.com> (raw)
In-Reply-To: <50EAEBD8.40002@compulab.co.il>

On 01/07/2013 07:38 AM, Igor Grinberg wrote:
> On 01/07/13 16:10, Mike Dunn wrote:


[..]


>>
>> Well, I won't make a fuss on this point, but with code that runs very
>> infrequently, I thought that the cost of a call to gpio_direction_output() was
>> worth the clarity and insurance.
> 
> This is not about how frequent the code runs, but the fact that the arch code
> relies on sound code (outside of arch) to request the GPIO for it.
> I would recommend to place the GPIO request call along with the
> direction/value calls - in the same subsystem.


OK.  This is in some shared pxa code called by the ac97 driver during
initialization, where I earlier placed the gpio_request() calls only.


> 
> Also, the patch ordering is not good:
> patch 2 drives the GPIO, and patch 3 requests it,
> whereas it should be the other way around, so I would recommend to
> combine both into one.


Agreed.

Thanks,
Mike

WARNING: multiple messages have this Message-ID (diff)
From: mikedunn@newsguy.com (Mike Dunn)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 3/4] ALSA: pxa27x: ac97 controller driver requests gpio
Date: Mon, 07 Jan 2013 08:40:01 -0800	[thread overview]
Message-ID: <50EAFA61.7000209@newsguy.com> (raw)
In-Reply-To: <50EAEBD8.40002@compulab.co.il>

On 01/07/2013 07:38 AM, Igor Grinberg wrote:
> On 01/07/13 16:10, Mike Dunn wrote:


[..]


>>
>> Well, I won't make a fuss on this point, but with code that runs very
>> infrequently, I thought that the cost of a call to gpio_direction_output() was
>> worth the clarity and insurance.
> 
> This is not about how frequent the code runs, but the fact that the arch code
> relies on sound code (outside of arch) to request the GPIO for it.
> I would recommend to place the GPIO request call along with the
> direction/value calls - in the same subsystem.


OK.  This is in some shared pxa code called by the ac97 driver during
initialization, where I earlier placed the gpio_request() calls only.


> 
> Also, the patch ordering is not good:
> patch 2 drives the GPIO, and patch 3 requests it,
> whereas it should be the other way around, so I would recommend to
> combine both into one.


Agreed.

Thanks,
Mike

  reply	other threads:[~2013-01-07 16:39 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-06 19:13 [PATCH 0/4] ALSA: pxa27x: ac97 reset fixes Mike Dunn
2013-01-06 19:13 ` Mike Dunn
2013-01-06 19:13 ` [PATCH 1/4] ALSA: pxa2xx: fix ac97 cold reset Mike Dunn
2013-01-06 19:13   ` Mike Dunn
2013-01-07  9:16   ` Igor Grinberg
2013-01-07  9:16     ` Igor Grinberg
2013-01-07 13:36     ` Mike Dunn
2013-01-07 13:36       ` Mike Dunn
2013-01-07 13:57       ` Igor Grinberg
2013-01-07 13:57         ` Igor Grinberg
2013-01-07 14:19         ` Mike Dunn
2013-01-07 14:19           ` Mike Dunn
2013-01-07 15:28           ` Igor Grinberg
2013-01-07 15:28             ` Igor Grinberg
2013-01-06 19:13 ` [PATCH 2/4] ALSA: pxa27x: fix ac97 warm reset bug work-around code Mike Dunn
2013-01-06 19:13   ` Mike Dunn
2013-01-07  9:31   ` Igor Grinberg
2013-01-07  9:31     ` Igor Grinberg
2013-01-07 11:06     ` Russell King - ARM Linux
2013-01-07 11:06       ` Russell King - ARM Linux
2013-01-07 13:40     ` Mike Dunn
2013-01-07 13:40       ` Mike Dunn
2013-01-06 19:13 ` [PATCH 3/4] ALSA: pxa27x: ac97 controller driver requests gpio Mike Dunn
2013-01-06 19:13   ` Mike Dunn
2013-01-07  9:36   ` Igor Grinberg
2013-01-07  9:36     ` Igor Grinberg
2013-01-07  9:42     ` Igor Grinberg
2013-01-07  9:42       ` Igor Grinberg
2013-01-07 14:10     ` Mike Dunn
2013-01-07 14:10       ` Mike Dunn
2013-01-07 15:38       ` Igor Grinberg
2013-01-07 15:38         ` Igor Grinberg
2013-01-07 16:40         ` Mike Dunn [this message]
2013-01-07 16:40           ` Mike Dunn
2013-01-06 19:14 ` [PATCH 4/4] ALSA: pxa27x: rename pxa27x_assert_ac97() Mike Dunn
2013-01-06 19:14   ` Mike Dunn
2013-01-07  9:39   ` Igor Grinberg
2013-01-07  9:39     ` Igor Grinberg
2013-01-07 14:34     ` Mike Dunn
2013-01-07 14:34       ` Mike Dunn
2013-01-07 15:41       ` Igor Grinberg
2013-01-07 15:41         ` Igor Grinberg
2013-01-06 20:33 ` [PATCH 0/4] ALSA: pxa27x: ac97 reset fixes Robert Jarzmik
2013-01-06 20:33   ` Robert Jarzmik

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=50EAFA61.7000209@newsguy.com \
    --to=mikedunn@newsguy.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@opensource.wolfsonmicro.com \
    --cc=eric.y.miao@gmail.com \
    --cc=grinberg@compulab.co.il \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=robert.jarzmik@free.fr \
    /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.