Hi Jadeon,

Hmm, my suspicions about the PHY driver are probably meant to be applied
here. I don't think this change is sufficient.

On Fri, Oct 23, 2015 at 10:44:16AM +0900, Jaedon Shin wrote:
> Add offsets for 40nm BMIPS based set-top box platforms.
> 
> Signed-off-by: Jaedon Shin <jaedon.s...@gmail.com>
> ---
>  drivers/ata/ahci_brcmstb.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/ahci_brcmstb.c b/drivers/ata/ahci_brcmstb.c
> index 8cf6f7d4798f..59eb526cf4f6 100644
> --- a/drivers/ata/ahci_brcmstb.c
> +++ b/drivers/ata/ahci_brcmstb.c
> @@ -50,7 +50,8 @@
>    #define SATA_TOP_CTRL_2_SW_RST_RX                  BIT(2)
>    #define SATA_TOP_CTRL_2_SW_RST_TX                  BIT(3)
>    #define SATA_TOP_CTRL_2_PHY_GLOBAL_RESET           BIT(14)
> - #define SATA_TOP_CTRL_PHY_OFFS                              0x8
> + #define SATA_TOP_CTRL_28NM_PHY_OFFS                 0x8
> + #define SATA_TOP_CTRL_40NM_PHY_OFFS                 0x4

I don't remember the exact 40nm vs. 28nm map that well, but judging by
the code-is-the-documentation, the 28nm layout is like this:

base + 0x0C = port 0, phy control 1
base + 0x10 = port 0, phy control 2
base + 0x14 = port 1, phy control 1
base + 0x18 = port 1, phy control 2

but the 40nm layout is differnt, where the ports are interleaved:

base + 0x0C = port 0, phy control 1
base + 0x10 = port 1, phy control 1
base + 0x14 = port 0, phy control 2
base + 0x18 = port 1, phy control 2

So, your patch gets phy control 1 correct for both ports, but it doesn't
get phy control 2 correct. (Or at least, even if my guess at the 40nm
layout is wrong, your patch makes "port 0, phy control 2" collide with
"port 1, phy control 1", which is most certainly a bug.)

Are you sure you're testing this properly? Did you try using both ports
at the same time? And please, apply the same scrutiny to the PHY driver.
(e.g., did you test SSC? did you test both ports?)

Brian

>   #define SATA_TOP_MAX_PHYS                           2
>  #define SATA_TOP_CTRL_SATA_TP_OUT                    0x1c
>  #define SATA_TOP_CTRL_CLIENT_INIT_CTRL                       0x20
> @@ -237,7 +238,13 @@ static int brcm_ahci_resume(struct device *dev)
>  
>  static const struct of_device_id ahci_of_match[] = {
>       {.compatible = "brcm,bcm7445-ahci",
> -                     .data = (void *)SATA_TOP_CTRL_PHY_OFFS},
> +                     .data = (void *)SATA_TOP_CTRL_28NM_PHY_OFFS},
> +     {.compatible = "brcm,bcm7346-ahci",
> +                     .data = (void *)SATA_TOP_CTRL_40NM_PHY_OFFS},
> +     {.compatible = "brcm,bcm7360-ahci",
> +                     .data = (void *)SATA_TOP_CTRL_40NM_PHY_OFFS},
> +     {.compatible = "brcm,bcm7362-ahci",
> +                     .data = (void *)SATA_TOP_CTRL_40NM_PHY_OFFS},
>       {},
>  };
>  MODULE_DEVICE_TABLE(of, ahci_of_match);
> -- 
> 2.6.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to