Hi Brian,
On Oct 24, 2015, at 6:25 AM, Brian Norris <computersforpeace@xxxxxxxxx> wrote:
>
> 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.shin@xxxxxxxxx>
>> ---
>> 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
>
You are right. This must be changed.
I found the 28nm chipsets have four phy interface control registers. But,
the the 40nm chipsets have only three registers. I did not testing with
two ports at the same time. It seems we should check more.
Thanks.
Jaedon
>> #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 majordomo@xxxxxxxxxxxxxxx
>> 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
