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

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

[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.

Signed-off-by: Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>
---
 arch/arm/mach-tegra/board-paz00-pinmux.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
index bdd2627..7de8182 100644
--- a/arch/arm/mach-tegra/board-paz00-pinmux.c
+++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
@@ -27,7 +27,7 @@ static struct tegra_pingroup_config paz00_pinmux[] = {
 	{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_NORMAL,    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},
-- 
1.7.3.4

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

[Marc Dietrich]

Hi Leon,

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. Second, Android has TEGRA_PUPD_PULL_DOWN 
with sound working and I didn't found out yet why this also works.

So there is something strange going on and we should first find out what
it is. Furthermore, there is no codec (ALC5632) and no glue for paz00 yet 
in the mainline kernel (and of course no board support for it) so this can 
still wait IMHO.

I will include your patch in the series which will add sound support once
the fog has lifted a bit.

Thanks

Marc

> Signed-off-by: Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>
> ---
>  arch/arm/mach-tegra/board-paz00-pinmux.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
> index bdd2627..7de8182 100644
> --- a/arch/arm/mach-tegra/board-paz00-pinmux.c
> +++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
> @@ -27,7 +27,7 @@ static struct tegra_pingroup_config paz00_pinmux[] = {
>  	{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_NORMAL,    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},
> 

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

[Leon Romanovsky]

> 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. Second, Android has TEGRA_PUPD_PULL_DOWN
> with sound working and I didn't found out yet why this also works.

I already said before, but will say again. Our board has many commons
to the Harmony board, so our reference is
arch/arm/mach-tegra/board-harmony-pinmux.c
http://git.kernel.org/?p=linux/kernel/git/olof/tegra.git;a=history;f=arch/arm/mach-tegra/board-harmony-pinmux.c;hb=103e1e14441ea86454921cdaf552c9f5c1f7c7c1
In the initial commit (42a7bf4d2686145bea03ff9b87d83868cc514f47)
pushed by Colin, the CDEV1 indeed has TEGRA_PUPD_PULL_DOWN and
TEGRA_TRI_TRISTATE
(http://git.kernel.org/?p=linux/kernel/git/olof/tegra.git;a=blob;f=arch/arm/mach-tegra/board-harmony-pinmux.c;h=50b15d500cacc73817258202178b0b79fc42478b;hb=42a7bf4d2686145bea03ff9b87d83868cc514f47#l28),
but later in the commit (ef2b1a0f1eb299c2d00addfee3a2631395d9bdb5)
pushed by Stephen, the CDEV1 was changed.

I did the same.

>
> So there is something strange going on and we should first find out what
> it is. Furthermore, there is no codec (ALC5632) and no glue for paz00 yet
> in the mainline kernel (and of course no board support for it) so this can
> still wait IMHO.

I have a different view on it. For me, everything that fixes incorrect
behavior and doesn't brake anything, must be included as soon as
possible.possible.

>
> I will include your patch in the series which will add sound support once
> the fog has lifted a bit.
>
> Thanks
>
> Marc
>
> > Signed-off-by: Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org>
> > ---
> >  arch/arm/mach-tegra/board-paz00-pinmux.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/arm/mach-tegra/board-paz00-pinmux.c b/arch/arm/mach-tegra/board-paz00-pinmux.c
> > index bdd2627..7de8182 100644
> > --- a/arch/arm/mach-tegra/board-paz00-pinmux.c
> > +++ b/arch/arm/mach-tegra/board-paz00-pinmux.c
> > @@ -27,7 +27,7 @@ static struct tegra_pingroup_config paz00_pinmux[] = {
> >       {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_NORMAL,    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},
> >
>



--
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon-2ukJVAZIZ/Y@public.gmane.org

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

[Stephen Warren]

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.

> 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.

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

-- 
nvpublic

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

[Marc Dietrich]

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":

--- 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},

 
> > 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.

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

[Stephen Warren]

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.

-- 
nvpublic

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

[Leon Romanovsky]

On Tue, Oct 18, 2011 at 18:10, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 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.

-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon-2ukJVAZIZ/Y@public.gmane.org

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

[Olof Johansson]

Hi,

On Wed, Oct 19, 2011 at 4:29 AM, Leon Romanovsky <leon-2ukJVAZIZ/Y@public.gmane.org> wrote:
> On Tue, Oct 18, 2011 at 18:10, Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> 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

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

[Leon Romanovsky]

On Wed, Oct 19, 2011 at 17:18, Olof Johansson <olof-nZhT3qVonbNeoWH0uzbU5w@public.gmane.org> 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
>



-- 
Leon Romanovsky | Independent Linux Consultant
        www.leon.nu | leon-2ukJVAZIZ/Y@public.gmane.org