[PATCH] libata-sff: Don't scan disabled ports when checking for legacy mode.

[PATCH] libata-sff: Don't scan disabled ports when checking for legacy mode.

[Darren Stevens]

libata-sff.c checks for legacy mode by testing if both primary
and secondary ports on a controller are in legacy mode and selects
legacy if either one is. However on some southbridge chips (e.g
AMD SB600/SB700) the secondary port is not wired, and when it is 
disabled by setting the disable bit in the PCI header it appears
as a fixed legacy port.
Prevent incorrect detection by not testing ports that are marked
as 'dummy'

Signed-off-by: Darren Stevens <darren@stevens-zone.net>
---

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 051b615..05f688a 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -2429,9 +2429,16 @@ int ata_pci_sff_activate_host(struct ata_host *host,
    if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
        u8 tmp8, mask;
 
-       /* TODO: What if one channel is in native mode ... */
+       /*
+        * ATA spec says we should use legacy mode when one
+        * port is in legacy mode, but disabled ports on some
+        * PCI hosts appear as fixed legacy ports, e.g SB600/700
+        * on which the secondary port is not wired, so
+        * ignore ports that we've marked as 'dummy' during
+        * this check
+        */
        pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
-       mask = (1 << 2) | (1 << 0);
+       mask = (! ata_port_is_dummy(host->ports[1]) << 2) | (!
ata_port_is_dummy(host->ports[0]) << 0);
        if ((tmp8 & mask) != mask)
            legacy_mode = 1;
    }

Re: [PATCH] libata-sff: Don't scan disabled ports when checking for legacy mode.

[Tejun Heo]

Hello,

On Fri, Jan 20, 2017 at 05:58:21PM +0000, Darren Stevens wrote:
> @@ -2429,9 +2429,16 @@ int ata_pci_sff_activate_host(struct ata_host *host,
>     if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
>         u8 tmp8, mask;
>  
> -       /* TODO: What if one channel is in native mode ... */
> +       /*
> +        * ATA spec says we should use legacy mode when one
> +        * port is in legacy mode, but disabled ports on some
> +        * PCI hosts appear as fixed legacy ports, e.g SB600/700
> +        * on which the secondary port is not wired, so
> +        * ignore ports that we've marked as 'dummy' during
> +        * this check
> +        */
>         pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
> -       mask = (1 << 2) | (1 << 0);
> +       mask = (! ata_port_is_dummy(host->ports[1]) << 2) | (!
> ata_port_is_dummy(host->ports[0]) << 0);

The patch is damaged and maybe just using two if statements would read
easier?

Thanks.

-- 
tejun

Re: libata-sff: Don't scan disabled ports when checking for legacy mode.

[Darren Stevens]

Hello Tejun

On 20/01/2017, Tejun Heo wrote:
> The patch is damaged and maybe just using two if statements would read
> easier?

Thanks for reviewing, I've made the changes you suggested, and resumbitted,
this time I have attached the patches so my emailer won't mangle them.

Both have come from my git tree and are tested as far as I can.

Regards
Darren

Re: [PATCH] libata-sff: Don't scan disabled ports when checking for legacy mode.

[Sergei Shtylyov]

Hello!

On 1/20/2017 8:58 PM, Darren Stevens wrote:

> libata-sff.c checks for legacy mode by testing if both primary
> and secondary ports on a controller are in legacy mode and selects
> legacy if either one is. However on some southbridge chips (e.g

    South bridge.

> AMD SB600/SB700) the secondary port is not wired, and when it is
> disabled by setting the disable bit in the PCI header it appears
> as a fixed legacy port.
> Prevent incorrect detection by not testing ports that are marked
> as 'dummy'
>
> Signed-off-by: Darren Stevens <darren@stevens-zone.net>
> ---
>
> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> index 051b615..05f688a 100644
> --- a/drivers/ata/libata-sff.c
> +++ b/drivers/ata/libata-sff.c
> @@ -2429,9 +2429,16 @@ int ata_pci_sff_activate_host(struct ata_host *host,
>     if ((pdev->class >> 8) == PCI_CLASS_STORAGE_IDE) {
>         u8 tmp8, mask;
>
> -       /* TODO: What if one channel is in native mode ... */
> +       /*
> +        * ATA spec says we should use legacy mode when one
> +        * port is in legacy mode, but disabled ports on some
> +        * PCI hosts appear as fixed legacy ports, e.g SB600/700
> +        * on which the secondary port is not wired, so
> +        * ignore ports that we've marked as 'dummy' during
> +        * this check
> +        */
>         pci_read_config_byte(pdev, PCI_CLASS_PROG, &tmp8);
> -       mask = (1 << 2) | (1 << 0);
> +       mask = (! ata_port_is_dummy(host->ports[1]) << 2) | (!
> ata_port_is_dummy(host->ports[0]) << 0);

    In addition to what Tejun said: no need for spaces after !.

[...]

MBR, Sergei