Hello.

On 06/30/2014 07:44 PM, Antoine Ténart wrote:

quoted

quoted

quoted

quoted

+	/* set the controller speed */
+	writel(0x31, ctrl_reg + PORT_SCR_CTL);

quoted

quoted

quoted

    Value undocumented? Or is this the SATA SControl register by chance?

quoted

quoted

Some magic is still there...

quoted

I guess Sergei was referring to AHCI spec here.

    Actually, even to the SATA specs. :-)

quoted

PORT_SCR bits are documented in AHCI spec as:

quoted

7:4 = 0x3 Limit speed negotiation to a rate not greater than Gen3
           communication rate.

quoted

3:0 = 0x1 Perform interface communication sequence [...]. This is
           functionally equivalent to a hard reset [...].

quoted

So, the question is: Should we really need to reset controller in the
PHY driver or is it already done in AHCI common code? At least we
should change the comment to something like
/* set Gen3 controller speed and perform hard reset */

I just checked, the AHCI common code has a function to do the reset:
ahci_reset_controller(). As of the max speed negociation rate, I did not
see it in the common AHCI functions.

    You've looked in a wrong place -- since SControl is a standard *SATA* 
register, it gets read/written by the libata core. The low-level driver only 
provides scr_{read|write}() methods.

The eSATA port on the Berlin2Q works without this line, but it may be a
good idea to keep the max speed negociation rate.

    It's usually libata's task to negotiate the SATA speed.

Anyway, we can remove the reset part. Nice catch!

    Thanks.

Antoine

WBR, Sergei