All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tomasz Figa <tomasz.figa@gmail.com>
To: Mark Brown <broonie@kernel.org>, Padmavathi Venna <padma.v@samsung.com>
Cc: Tomasz Figa <t.figa@samsung.com>,
	linux-samsung-soc@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
	padma.kvr@gmail.com, kgene.kim@samsung.com,
	abrestic@chromium.org
Subject: Re: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
Date: Mon, 12 Aug 2013 19:02:20 +0200	[thread overview]
Message-ID: <1427610.pB8BGbMKTL@flatron> (raw)
In-Reply-To: <20130812131235.GX6427@sirena.org.uk>

On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> > > I'd expect that to interact badly with the pinmuxing - unless the
> > > device is disabled it'll try to grab its pins on probe which is not
> > > going to be a good idea unless it is actually wired up for use in
> > > the system.  Or is there some other mechanism for handling that?
> > 
> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> > considered board-specific and specified in board-level dts instead?
> 
> It seems a bit cleaner to use the current mechanism in that it stops the
> device appearing at all and hence repeated efforts to probe, plus a
> simple enable is less error prone, the way these SoCs are designed you
> don't have to pick which pinmux is in use for most of the IPs.  Where
> there are multiple options it does seem like a good approach though.
> 
> Tastes may differ though.

Right, if this SoC has only one pinmux setting for this IP, then it's 
fine.

Padmavathi, this was the only issue I spotted, so have my:

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz

WARNING: multiple messages have this Message-ID (diff)
From: tomasz.figa@gmail.com (Tomasz Figa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers
Date: Mon, 12 Aug 2013 19:02:20 +0200	[thread overview]
Message-ID: <1427610.pB8BGbMKTL@flatron> (raw)
In-Reply-To: <20130812131235.GX6427@sirena.org.uk>

On Monday 12 of August 2013 14:12:36 Mark Brown wrote:
> On Mon, Aug 12, 2013 at 01:41:23PM +0200, Tomasz Figa wrote:
> > On Monday 12 of August 2013 12:34:48 Mark Brown wrote:
> > > I'd expect that to interact badly with the pinmuxing - unless the
> > > device is disabled it'll try to grab its pins on probe which is not
> > > going to be a good idea unless it is actually wired up for use in
> > > the system.  Or is there some other mechanism for handling that?
> > 
> > Ah, good point. Now I wonder whether pinctrl nodes shouldn't be
> > considered board-specific and specified in board-level dts instead?
> 
> It seems a bit cleaner to use the current mechanism in that it stops the
> device appearing at all and hence repeated efforts to probe, plus a
> simple enable is less error prone, the way these SoCs are designed you
> don't have to pick which pinmux is in use for most of the IPs.  Where
> there are multiple options it does seem like a good approach though.
> 
> Tastes may differ though.

Right, if this SoC has only one pinmux setting for this IP, then it's 
fine.

Padmavathi, this was the only issue I spotted, so have my:

Reviewed-by: Tomasz Figa <t.figa@samsung.com>

Best regards,
Tomasz

  reply	other threads:[~2013-08-12 17:02 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-08-12 10:07 [PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420 Padmavathi Venna
2013-08-12 10:07 ` Padmavathi Venna
2013-08-12 10:07 ` [PATCH V4 1/4] ARM: dts: exynos5420: add i2s controllers Padmavathi Venna
2013-08-12 10:07   ` Padmavathi Venna
2013-08-12 11:14   ` Tomasz Figa
2013-08-12 11:14     ` Tomasz Figa
2013-08-12 11:34     ` Mark Brown
2013-08-12 11:34       ` Mark Brown
2013-08-12 11:41       ` Tomasz Figa
2013-08-12 11:41         ` Tomasz Figa
2013-08-12 13:12         ` Mark Brown
2013-08-12 13:12           ` Mark Brown
2013-08-12 17:02           ` Tomasz Figa [this message]
2013-08-12 17:02             ` Tomasz Figa
2013-08-14  5:51             ` Padma Venkat
2013-08-14  5:51               ` Padma Venkat
2013-08-14  8:19               ` Tomasz Figa
2013-08-14  8:19                 ` Tomasz Figa
2013-08-12 10:07 ` [PATCH V4 2/4] ARM: dts: Add i2c bus 1 and it's audio codec child node on smdk5420 Padmavathi Venna
2013-08-12 10:07   ` Padmavathi Venna
2013-08-12 10:07 ` [PATCH V4 3/4] ARM: dts: Add osc clock " Padmavathi Venna
2013-08-12 10:07   ` Padmavathi Venna
2013-08-12 10:07 ` [PATCH V4 4/4] ARM: dts: Enable sound support " Padmavathi Venna
2013-08-12 10:07   ` Padmavathi Venna
2013-08-12 11:17   ` Tomasz Figa
2013-08-12 11:17     ` Tomasz Figa
2013-08-12 11:18 ` [PATCH V4 0/4] Add i2s nodes on Exynos5420 and enable sound support on sdmk5420 Tomasz Figa
2013-08-12 11:18   ` Tomasz Figa

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=1427610.pB8BGbMKTL@flatron \
    --to=tomasz.figa@gmail.com \
    --cc=abrestic@chromium.org \
    --cc=broonie@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=kgene.kim@samsung.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=padma.kvr@gmail.com \
    --cc=padma.v@samsung.com \
    --cc=t.figa@samsung.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.