RE: [PATCH v2] ahci: implement aggressive SATA device sleep support

To avoid attempting to enable on older devices, you will need an 
appropriate test (ata_id_has_ncq perhaps?)

So far I don't find better method to exclude old devices than yours.

As you figured out in the other email, I was referring to sata_settings

I'm going to modify both sata_settings and ID[78].

I just do not like programming the device, when power policy may 
indicate otherwise.

Most conservative is to leave devslp feature in reset state and not 
touch device or host until power policy dictates it is time to program 
host + device.

OK, I'll move setting feature back to ahci_set_aggressive_devslp().
I'm trying to avoid setting feature each time for every sleep to
improve performance.

Adding pp->device_devslp_enabled with the code below does not make
much difference because it is still called for every sleep.
Do you have good suggestion?

Any should we also call setting feature to disable device DevSlp
for each !sleep?


static void ahci_set_aggressive_devslp(struct ata_port *ap, bool sleep)
{
...
        /* disable device sleep */
        if (!sleep) {
                if (devslp & PORT_DEVSLP_ADSE)
                        writel(devslp & ~PORT_DEVSLP_ADSE,
                               port_mmio + PORT_DEVSLP);
                if (pp->device_devslp_enabled)
                        pp->device_devslp_enabled = false;
                return;
        }
...
        ahci_start_engine(ap);

        /* enable device sleep feature for the drive */
        if (!pp->device_devslp_enabled) {
                err_mask = ata_dev_set_feature(ap->link.device,
                                               SETFEATURES_SATA_ENABLE,
                                               SATA_DEVSLP);
                if (err_mask && err_mask != AC_ERR_DEV)
                        ata_dev_warn(ap->link.device,
                                     "failed to enable DEVSLP, Emask 0x%x\n", err_mask);
                else
                        pp->device_devslp_enabled = true;
        }
}


Thanks,
Shane