Re: [PATCH 03/10] ata: ahci_brcmstb: add support 40nm platforms

[Brian Norris <computersforpeace@gmail.com>]

Hi Jaedon,

On Sat, Oct 24, 2015 at 01:50:54PM +0900, Jaedon Shin wrote:
> On Oct 24, 2015, at 6:25 AM, Brian Norris <computersforpeace@gmail.com> wrote:
> > On Fri, Oct 23, 2015 at 10:44:16AM +0900, Jaedon Shin wrote:
> > 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.

OK. So you don't just need more testing, you need to make sure the code
actually matches the documentation at all. If there are only 3 PHY
control registers for 40nm, then this driver (as patched by you) doesn't
make any sense. It will need a much different patch than what you gave.

Brian