[REGRESSION+BISECT] imx6: 3.8+: sata failure with 24d340ac "ARM i.MX6: Fix ethernet PLL clocks"

[REGRESSION+BISECT] imx6: 3.8+: sata failure with 24d340ac "ARM i.MX6: Fix ethernet PLL clocks"

[Paolo Pisati]

Hi,

sata doesn't work anymore in 3.8rcX (while it was ok in 3.7 + 1 patch[1]) and i've bisected
it down to:

>From 24d340ac43882583024ffcce1cd38b9b2391eb51 Mon Sep 17 00:00:00 2001
From: Sascha Hauer <s.hauer@pengutronix.de>
Date: Wed, 21 Nov 2012 14:42:31 +0100
Subject: [PATCH 132/144] ARM i.MX6: Fix ethernet PLL clocks
In current code the ethernet PLL is not handled correctly. The PLL runs at
500MHz
and has different outputs. Only the enet reference clock is implemented. This
patch changes the PLL so that it outputs 500MHz and adds the additional outputs
as dividers. This now matches the datasheet which says:

...
ahci 2200000.ahci: forcing PORTS_IMPL to 0x1
ahci 2200000.ahci: AHCI 0001.0300 32 slots 1 ports 3 Gbps 0x1 impl platform mode
ahci 2200000.ahci: flags: ncq sntf pm led clo only pmp pio slum part ccc apst
scsi0 : ahci_platform
ata1: SATA max UDMA/133 mmio [mem 0x02200000-0x02203fff] port 0x100 irq 71
...
ata1: link is slow to respond, please be patient (ready=0)
ata1: softreset failed (device not ready)
ata1: SATA link down (SStatus 0 SControl 300)
...

without that patch, it works like a charm.
Here is my kernel config:

http://people.canonical.com/~ppisati/imx6_sata/config

Anyone else seeing this?

1: http://people.canonical.com/~ppisati/imx6_sata/0001-Add-IMX6Q-AHCI-support.patch
-- 
bye,
p.

[REGRESSION+BISECT] imx6: 3.8+: sata failure with 24d340ac "ARM i.MX6: Fix ethernet PLL clocks"

[Shawn Guo]

On Thu, Jan 31, 2013 at 12:36:27PM +0100, Paolo Pisati wrote:
> Hi,
> 
> sata doesn't work anymore in 3.8rcX (while it was ok in 3.7 + 1 patch[1]) and i've bisected
> it down to:
> 
Technically, it's not a regression since vanilla 3.7 kernel does not
have SATA work out of box.  So we do not have to maintain the function
when changing clock code.

Can you please try to see if the following change bring that SATA back
to work?

Shawn

diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
index c0c4e72..8f756af 100644
--- a/arch/arm/mach-imx/clk-imx6q.c
+++ b/arch/arm/mach-imx/clk-imx6q.c
@@ -436,6 +436,9 @@ int __init mx6q_clocks_init(void)
        for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
                clk_prepare_enable(clk[clks_init_on[i]]);

+       if (IS_ENABLED(CONFIG_SATA_AHCI_PLATFORM))
+               clk_prepare_enable(clk[sata_ref_100m]);
+
        /* Set initial power mode */
        imx6q_set_lpm(WAIT_CLOCKED);

[REGRESSION+BISECT] imx6: 3.8+: sata failure with 24d340ac "ARM i.MX6: Fix ethernet PLL clocks"

[Paolo Pisati]

On Thu, Jan 31, 2013 at 09:30:56PM +0800, Shawn Guo wrote:
> 
> Can you please try to see if the following change bring that SATA back
> to work?
> 
> Shawn
> 
> diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> index c0c4e72..8f756af 100644
> --- a/arch/arm/mach-imx/clk-imx6q.c
> +++ b/arch/arm/mach-imx/clk-imx6q.c
> @@ -436,6 +436,9 @@ int __init mx6q_clocks_init(void)
>         for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
>                 clk_prepare_enable(clk[clks_init_on[i]]);
> 
> +       if (IS_ENABLED(CONFIG_SATA_AHCI_PLATFORM))
> +               clk_prepare_enable(clk[sata_ref_100m]);
> +
>         /* Set initial power mode */
>         imx6q_set_lpm(WAIT_CLOCKED);

yes, it fixes SATA.

Tested-by: <p.pisati@gmail.com>
-- 
bye,
p.

[REGRESSION+BISECT] imx6: 3.8+: sata failure with 24d340ac "ARM i.MX6: Fix ethernet PLL clocks"

[Shawn Guo]

On Thu, Jan 31, 2013 at 02:53:51PM +0100, Paolo Pisati wrote:
> On Thu, Jan 31, 2013 at 09:30:56PM +0800, Shawn Guo wrote:
> > 
> > Can you please try to see if the following change bring that SATA back
> > to work?
> > 
> > Shawn
> > 
> > diff --git a/arch/arm/mach-imx/clk-imx6q.c b/arch/arm/mach-imx/clk-imx6q.c
> > index c0c4e72..8f756af 100644
> > --- a/arch/arm/mach-imx/clk-imx6q.c
> > +++ b/arch/arm/mach-imx/clk-imx6q.c
> > @@ -436,6 +436,9 @@ int __init mx6q_clocks_init(void)
> >         for (i = 0; i < ARRAY_SIZE(clks_init_on); i++)
> >                 clk_prepare_enable(clk[clks_init_on[i]]);
> > 
> > +       if (IS_ENABLED(CONFIG_SATA_AHCI_PLATFORM))
> > +               clk_prepare_enable(clk[sata_ref_100m]);
> > +
> >         /* Set initial power mode */
> >         imx6q_set_lpm(WAIT_CLOCKED);
> 
> yes, it fixes SATA.
> 
> Tested-by: <p.pisati@gmail.com>

Just to be clear, I'm not sending this change to mainline, as I expect
someone who cares about SATA function adds this code when he submits
imx6q SATA support (that out of tree patch).

Shawn