| Message ID | 1415874256-6580-1-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Nov 13, 2014 at 11:24:16AM +0100, Hans de Goede wrote: > 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 <hdegoede@redhat.com> Applied to libata/for-3.18-fixes. Thanks.
Hi, On 11/14/2014 12:56 PM, Hans de Goede wrote: > Hi, > > On Thursday, November 13, 2014 11:29:56 PM UTC+1, Tejun Heo wrote: >> >> On Thu, Nov 13, 2014 at 11:24:16AM +0100, Hans de Goede wrote: >>> 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 <hdeg...@redhat.com <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. 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). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Nov 14, 2014 at 01:49:04PM +0100, Hans de Goede wrote: > > 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 ? I popped the commit from libata/for-3.18-fixes. Thanks.
Hi, On 11/14/2014 01:49 PM, Hans de Goede wrote: > Hi, > > On 11/14/2014 12:56 PM, Hans de Goede wrote: >> Hi, >> >> On Thursday, November 13, 2014 11:29:56 PM UTC+1, Tejun Heo wrote: >>> >>> On Thu, Nov 13, 2014 at 11:24:16AM +0100, Hans de Goede wrote: >>>> 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 <hdeg...@redhat.com <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. > > 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). Just saw your message that you've dropped the troublesome patch, in that case I'll do a completely new patch, rather then a follow on patch, to add a module option for this (unless you've a better idea). Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/ahci_sunxi.c b/drivers/ata/ahci_sunxi.c index e44d675..3dd0224 100644 --- a/drivers/ata/ahci_sunxi.c +++ b/drivers/ata/ahci_sunxi.c @@ -184,7 +184,7 @@ static int ahci_sunxi_probe(struct platform_device *pdev) goto disable_resources; hpriv->flags = AHCI_HFLAG_32BIT_ONLY | AHCI_HFLAG_NO_MSI | - AHCI_HFLAG_NO_PMP | AHCI_HFLAG_YES_NCQ; + AHCI_HFLAG_YES_NCQ; rc = ahci_platform_init_host(pdev, hpriv, &ahci_sunxi_port_info); if (rc)
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 <hdegoede@redhat.com> --- drivers/ata/ahci_sunxi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)