ahci_sunxi: Drop AHCI_HFLAG_NO_PMP flag

This is something which we inherited from the Allwinner android kernel sources, and I've always wanted to test if this is really necessary.

So recently I've bought a sata port multiplexer, and I've given this a test spin on both A10 and A20 devices, and it seems to work fine:

[    2.154456] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[    2.161092] ata1.15: Port Multiplier 1.2, 0x197b:0x0325 r0, 5 ports, feat 0x5/0xf
[    2.175511] ata1.00: hard resetting link
[    2.524929] ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 320)
[    2.531430] ata1.01: hard resetting link
[    2.974465] ata1.01: link resume succeeded after 1 retries
[    3.094932] ata1.01: SATA link up 3.0 Gbps (SStatus 123 SControl 300)
[    3.101431] ata1.02: hard resetting link
[    4.174466] ata1.02: failed to resume link (SControl 0)
[    4.180065] ata1.02: SATA link down (SStatus 0 SControl 0)

(and the same for links 3 and 4)

Once the NO_PMP flag is removed it correctly sees the 2 disks which I've attached, and I can mount and use them just fine, so lets drop the flag.

Signed-off-by Hans de Goede <[email protected]>

Quoted 1 line · Hans de Goede

This is something which we inherited from the Allwinner android kernel

sources,

Quoted 3 lines

and I've always wanted to test if this is really necessary.

So recently I've bought a sata port multiplexer, and I've given this a

test

Quoted 4 lines

spin on both A10 and A20 devices, and it seems to work fine:

[    2.154456] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    2.161092] ata1.15: Port Multiplier 1.2, 0x197b:0x0325 r0, 5 ports, 

feat 0x5/0xf

Quoted 10 lines

[    2.175511] ata1.00: hard resetting link 
[    2.524929] ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 320) 
[    2.531430] ata1.01: hard resetting link 
[    2.974465] ata1.01: link resume succeeded after 1 retries 
[    3.094932] ata1.01: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    3.101431] ata1.02: hard resetting link 
[    4.174466] ata1.02: failed to resume link (SControl 0) 
[    4.180065] ata1.02: SATA link down (SStatus 0 SControl 0) 

(and the same for links 3 and 4)

Quoted 4 lines

Once the NO_PMP flag is removed it correctly sees the 2 disks which I've attached, and I can mount and use them just fine, so lets drop the flag.

Signed-off-by Hans de Goede <[email protected] <javascript:>>

Applied to libata/for-3.18-fixes.

Hi,

Quoted 23 lines · Tejun Heo

Quoted 1 line · Hans de Goede

This is something which we inherited from the Allwinner android kernel

sources,

Quoted 3 lines

and I've always wanted to test if this is really necessary.

So recently I've bought a sata port multiplexer, and I've given this a

test

Quoted 4 lines

spin on both A10 and A20 devices, and it seems to work fine:

[    2.154456] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    2.161092] ata1.15: Port Multiplier 1.2, 0x197b:0x0325 r0, 5 ports, 

feat 0x5/0xf

Quoted 10 lines

[    2.175511] ata1.00: hard resetting link 
[    2.524929] ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 320) 
[    2.531430] ata1.01: hard resetting link 
[    2.974465] ata1.01: link resume succeeded after 1 retries 
[    3.094932] ata1.01: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    3.101431] ata1.02: hard resetting link 
[    4.174466] ata1.02: failed to resume link (SControl 0) 
[    4.180065] ata1.02: SATA link down (SStatus 0 SControl 0) 

(and the same for links 3 and 4)

Quoted 7 lines

Quoted 4 lines

Once the NO_PMP flag is removed it correctly sees the 2 disks which I've attached, and I can mount and use them just fine, so lets drop the flag.

Signed-off-by Hans de Goede <[email protected] <javascript:>>

Applied to libata/for-3.18-fixes.

I just found out why Allwinner added that AHCI_HFLAG_NO_PMP flag, after putting the system I used to test the port multiplexer back together this morning, it seems that probing for a port multiplexer being present breaks the no-port-multiplexer (so direct attached disk) case :( I guess I should have tested for that before sending the patch, but the thought of it breaking normal usage did not cross my mind.

So self NACK. If it is not too late, can you please drop this from your tree ?

Let me know if you want me to send a revert instead.

I'll look into fixing this another way, maybe with a quirk + an extra reset after failed pmp testing, and it that does not work a module option.

Quoted 9 lines

I just found out why Allwinner added that AHCI_HFLAG_NO_PMP flag, after putting the system I used to test the port multiplexer back together this morning, it seems that probing for a port multiplexer being present breaks the no-port-multiplexer (so direct attached disk) case :( I guess I should have tested for that before sending the patch, but the thought of it breaking normal usage did not cross my mind.

So self NACK. If it is not too late, can you please drop this from your tree ?

Hi,

Quoted 41 lines · Hans de Goede

Hi,

Quoted 23 lines · Tejun Heo

Quoted 1 line · Hans de Goede

This is something which we inherited from the Allwinner android kernel

sources,

Quoted 3 lines

and I've always wanted to test if this is really necessary.

So recently I've bought a sata port multiplexer, and I've given this a

test

Quoted 4 lines

spin on both A10 and A20 devices, and it seems to work fine:

[    2.154456] ata1: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    2.161092] ata1.15: Port Multiplier 1.2, 0x197b:0x0325 r0, 5 ports, 

feat 0x5/0xf

Quoted 10 lines

[    2.175511] ata1.00: hard resetting link 
[    2.524929] ata1.00: SATA link up 3.0 Gbps (SStatus 123 SControl 320) 
[    2.531430] ata1.01: hard resetting link 
[    2.974465] ata1.01: link resume succeeded after 1 retries 
[    3.094932] ata1.01: SATA link up 3.0 Gbps (SStatus 123 SControl 300) 
[    3.101431] ata1.02: hard resetting link 
[    4.174466] ata1.02: failed to resume link (SControl 0) 
[    4.180065] ata1.02: SATA link down (SStatus 0 SControl 0) 

(and the same for links 3 and 4)

Quoted 7 lines

Quoted 4 lines

Once the NO_PMP flag is removed it correctly sees the 2 disks which I've attached, and I can mount and use them just fine, so lets drop the flag.

Signed-off-by Hans de Goede <[email protected] <javascript:>>

Applied to libata/for-3.18-fixes.

I just found out why Allwinner added that AHCI_HFLAG_NO_PMP flag, after putting the system I used to test the port multiplexer back together this morning, it seems that probing for a port multiplexer being present breaks the no-port-multiplexer (so direct attached disk) case :( I guess I should have tested for that before sending the patch, but the thought of it breaking normal usage did not cross my mind.

Quoted 7 lines

So self NACK. If it is not too late, can you please drop this from your tree ?

Let me know if you want me to send a revert instead.

I'll look into fixing this another way, maybe with a quirk + an extra reset after failed pmp testing, and it that does not work a module option.

Ok, so the problem seems to be that the controller fails to do a soft-reset when attached directly to a normal device rather then to a pmp.

If I comment out this block of code in libata-core.c:

        if (sata_pmp_supported(link->ap) && ata_is_host_link(link)) {
                /* If PMP is supported, we have to do follow-up SRST.
                 * Some PMPs don't send D2H Reg FIS after hardreset if
                 * the first port is empty.  Wait only for
                 * ATA_TMOUT_PMP_SRST_WAIT.
                 */
                if (check_ready) {
                        unsigned long pmp_deadline;

                        pmp_deadline = ata_deadline(jiffies,
                                                    ATA_TMOUT_PMP_SRST_WAIT);
                        if (time_after(pmp_deadline, deadline))
                                pmp_deadline = deadline;
                        ata_wait_ready(link, pmp_deadline, check_ready);
                }
                rc = -EAGAIN;
                goto out;
        }

Then things work again. This code looks like it is a must have for pmp use, so I'll do a follow up patch to my initial patch dropping the AHCI_HFLAG_NO_PMP flag, which re-adds it based on a module option (defaulting to the non-pmp case).

This way you don't have to do a forced-push to your tree to drop the original patch (or revert it).