diff mbox

[1/1] ARM: tegra: paz00: Fix board pinmux table.

Message ID 9849440.cSOtOCBmUT@ax5200p
State Not Applicable, archived
Headers show

Commit Message

Marc Dietrich Oct. 18, 2011, 3:50 p.m. UTC 
On Monday 17 October 2011 08:29:01 Stephen Warren wrote:
> Marc Dietrich wrote at Saturday, October 15, 2011 10:34 AM:
> > Am Samstag 15 Oktober 2011, 17:18:33 schrieb Leon Romanovsky:
> > > This fix updates the CDEV1 pinmux for the paz00 board to be as in the
> > > Harmony board. Paz00 board is originally based on Harmony design.
> > 
> > the fact that this patch makes sound work on paz00 does not necessary mean
> > that this is the "right thing" to do.
> 
> > First, the initial state should be
> > TRISTATE, because sound layer should switch between NORMAL/TRISTATE
> > automaticly to save some power.
> 
> None of the other boards do that, and the drivers don't support it. I
> believe the clock would need to be started as soon as the audio driver
> module were loaded, and kept running as long as it was loaded, so there's
> not much chance of optimizing power here at the moment.

Which driver? I guess this could be done by the board specific code e.g. in
in sound/soc/tegra/paz00.c I haven't checked if the clock is disabled, because
the whole clock setup is a bit confusing...

Here is androids pinmux diff "without and with music":


 
> > Second, Android has TEGRA_PUPD_PULL_DOWN
> > with sound working and I didn't found out yet why this also works.
> 
> I don't know why there's a pull-down there in the Android kernel; there's
> no need as far as I know, since it's a clock signal. I'd go so far as to
> believe that a pull-down would distort the clock signal slightly.

Is there some docu somewhere about what PUPD_NORMAL and POPD_PULL_DOWN means? 
I know the meaning of TRISTATE and PULL_DOWN/UP from electronics, but
that does not define what the NORMAL state is.

I also don't see why you can't get a nice clock signal out of a pull down pin. 
If inactive, the pin is high (e.g. +3.3V), while when active it is pulled to
zero (ok, it's logical inverted). 

Marc


> But yes, it probably makes sense to include this patch in a series that
> enables audio on PAZ00 whenever that appears.




--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Stephen Warren Oct. 18, 2011, 4:10 p.m. UTC | #1 
Marc Dietrich wrote at Tuesday, October 18, 2011 9:50 AM:
> On Monday 17 October 2011 08:29:01 Stephen Warren wrote:
> > Marc Dietrich wrote at Saturday, October 15, 2011 10:34 AM:
> > > Am Samstag 15 Oktober 2011, 17:18:33 schrieb Leon Romanovsky:
> > > > This fix updates the CDEV1 pinmux for the paz00 board to be as in the
> > > > Harmony board. Paz00 board is originally based on Harmony design.
> > >
> > > the fact that this patch makes sound work on paz00 does not necessary mean
> > > that this is the "right thing" to do.
> >
> > > First, the initial state should be
> > > TRISTATE, because sound layer should switch between NORMAL/TRISTATE
> > > automaticly to save some power.
> >
> > None of the other boards do that, and the drivers don't support it. I
> > believe the clock would need to be started as soon as the audio driver
> > module were loaded, and kept running as long as it was loaded, so there's
> > not much chance of optimizing power here at the moment.
> 
> Which driver?

The whole ASoC driver stack. It may depend on the codec, but I assume that
CDEV1 clock output should be running the whole time the ASoC codec driver
is active. Alsa-devel would be the best place to ask about that kind of
thing.

...
> > I don't know why there's a pull-down there in the Android kernel; there's
> > no need as far as I know, since it's a clock signal. I'd go so far as to
> > believe that a pull-down would distort the clock signal slightly.
> 
> Is there some docu somewhere about what PUPD_NORMAL and POPD_PULL_DOWN means?
> I know the meaning of TRISTATE and PULL_DOWN/UP from electronics, but
> that does not define what the NORMAL state is.

"NORMAL" means no pull-up or pull-down, or not tri-stated. It's badly
named.

> I also don't see why you can't get a nice clock signal out of a pull down pin.
> If inactive, the pin is high (e.g. +3.3V), while when active it is pulled to
> zero (ok, it's logical inverted).

Well, I may be wrong, but the pin will attempt to drive a nice symmetric
signal, yet with a pull-up/down, there will be some slight drive on the
wire that'll tend to pull the signal towards 0v/3.3v the whole time which
will distort it. Whether this is a problem for this particular signal,
pull-up/down strength, etc. is something I don't know. Certainly, the
pull isn't actually needed AFAIK, so it may as well be removed.
Leon Romanovsky Oct. 19, 2011, 11:29 a.m. UTC | #2 
On Tue, Oct 18, 2011 at 18:10, Stephen Warren <swarren@nvidia.com> wrote:
> Marc Dietrich wrote at Tuesday, October 18, 2011 9:50 AM:
>> On Monday 17 October 2011 08:29:01 Stephen Warren wrote:
>> > Marc Dietrich wrote at Saturday, October 15, 2011 10:34 AM:
>> > > Am Samstag 15 Oktober 2011, 17:18:33 schrieb Leon Romanovsky:
>> > > > This fix updates the CDEV1 pinmux for the paz00 board to be as in the
>> > > > Harmony board. Paz00 board is originally based on Harmony design.
>> > >
>> > > the fact that this patch makes sound work on paz00 does not necessary mean
>> > > that this is the "right thing" to do.
>> > None of the other boards do that, and the drivers don't support it. I
>> > believe the clock would need to be started as soon as the audio driver
>> > module were loaded, and kept running as long as it was loaded, so there's
>> > not much chance of optimizing power here at the moment.
>>
>> Which driver?
>
> The whole ASoC driver stack. It may depend on the codec, but I assume that
> CDEV1 clock output should be running the whole time the ASoC codec driver
> is active. Alsa-devel would be the best place to ask about that kind of
> thing.

Let's summarize, we agree that it is a correct way to handle CDEV1,
and it is a correct patch.
I believe it is a good time to merge the patch before we start to
merge the codec itself.
Olof Johansson Oct. 19, 2011, 3:18 p.m. UTC | #3 
Hi,

On Wed, Oct 19, 2011 at 4:29 AM, Leon Romanovsky <leon@leon.nu> wrote:
> On Tue, Oct 18, 2011 at 18:10, Stephen Warren <swarren@nvidia.com> wrote:
>> Marc Dietrich wrote at Tuesday, October 18, 2011 9:50 AM:
>>> On Monday 17 October 2011 08:29:01 Stephen Warren wrote:
>>> > Marc Dietrich wrote at Saturday, October 15, 2011 10:34 AM:
>>> > > Am Samstag 15 Oktober 2011, 17:18:33 schrieb Leon Romanovsky:
>>> > > > This fix updates the CDEV1 pinmux for the paz00 board to be as in the
>>> > > > Harmony board. Paz00 board is originally based on Harmony design.
>>> > >
>>> > > the fact that this patch makes sound work on paz00 does not necessary mean
>>> > > that this is the "right thing" to do.
>>> > None of the other boards do that, and the drivers don't support it. I
>>> > believe the clock would need to be started as soon as the audio driver
>>> > module were loaded, and kept running as long as it was loaded, so there's
>>> > not much chance of optimizing power here at the moment.
>>>
>>> Which driver?
>>
>> The whole ASoC driver stack. It may depend on the codec, but I assume that
>> CDEV1 clock output should be running the whole time the ASoC codec driver
>> is active. Alsa-devel would be the best place to ask about that kind of
>> thing.
>
> Let's summarize, we agree that it is a correct way to handle CDEV1,
> and it is a correct patch.
> I believe it is a good time to merge the patch before we start to
> merge the codec itself.

It'll be staged for 3.3 when I start those branches, which should be soon.

We're close to the merge window, it's not a regression fix and you
need other changes on top of it anyway to get anything useful out of
it -- so it can wait.

Thank you for posting it early though, and I'll make sure it makes it
into linux-next quickly so you can use that as a base for your codec
work.


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Leon Romanovsky Oct. 19, 2011, 9:05 p.m. UTC | #4 
On Wed, Oct 19, 2011 at 17:18, Olof Johansson <olof@lixom.net> wrote:
> Thank you for posting it early though, and I'll make sure it makes it
> into linux-next quickly so you can use that as a base for your codec
> work.
Thanks

> -Olof
>
diff mbox

Patch

--- pinmux_no_music.txt
+++ pinmux_with_music.txt
@@ -3,11 +3,11 @@ 
        {TEGRA_PINGROUP_ATC,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_ATD,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_ATE,   TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
-       {TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
+       {TEGRA_PINGROUP_CDEV1, TEGRA_MUX_PLLA_OUT,      TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_CDEV2, TEGRA_MUX_PLLP_OUT4,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_CRTP,  TEGRA_MUX_CRT,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},
        {TEGRA_PINGROUP_CSUS,  TEGRA_MUX_PLLC_OUT1,     TEGRA_PUPD_PULL_DOWN, TEGRA_TRI_TRISTATE},
-       {TEGRA_PINGROUP_DAP1,  TEGRA_MUX_DAP1,          TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},
+       {TEGRA_PINGROUP_DAP1,  TEGRA_MUX_DAP1,          TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_DAP2,  TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_NORMAL},
        {TEGRA_PINGROUP_DAP3,  TEGRA_MUX_DAP3,          TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},
        {TEGRA_PINGROUP_DAP4,  TEGRA_MUX_GMI,           TEGRA_PUPD_NORMAL,    TEGRA_TRI_TRISTATE},