sata_sis: SCR accessors should return -EINVAL when the requested SCR isn't available

sis_scr_cfg_read() can't access SError and was incorrectly returning -1 instead of -EINVAL. This went unnoticed because SError used to be cleared in @postreset() and it didn't care about how scr_read() failed but commit ac371987 moved SError clearing into sata_link_resume() and SCR access failure other than -EINVAL is considered an error condition and exposes the incorrect return value bug as detection failure. Fix it.

Also, scsi_scr_cfg_write() was incorrectly returning 0 after it ignored the request to write to SError. Make it also return -EINVAL.

This was bisected and reported by Patrick McHardy.

Signed-off-by Tejun Heo <[email protected]>

Cc Patrick McHardy <[email protected]>

sis_scr_cfg_read() can't access SError and was incorrectly returning -1 instead of -EINVAL. This went unnoticed because SError used to be cleared in @postreset() and it didn't care about how scr_read() failed but commit ac371987 moved SError clearing into sata_link_resume() and SCR access failure other than -EINVAL is considered an error condition and exposes the incorrect return value bug as detection failure. Fix it.

Also, scsi_scr_cfg_write() was incorrectly returning 0 after it ignored the request to write to SError. Make it also return -EINVAL.

This was bisected and reported by Patrick McHardy.

Signed-off-by Tejun Heo <[email protected]>

Cc Patrick McHardy <[email protected]>

---
drivers/ata/sata_sis.c | 12 +++++++-----

1 file changed, 7 insertions(+), 5 deletions(-)

drivers/ata/sata_sis.c +3 -3

index 6b8e45b..1010b30 100644
@@ -142,7 +142,7 @@ static u32 sis_scr_cfg_read(struct ata_port *ap, 
 unsigned int sc_reg, u32 *val)
    u8 pmr;
 
    if (sc_reg == SCR_ERROR) /* doesn't exist in PCI cfg space */
-        return 0xffffffff;
+        return -EINVAL;
 
    pci_read_config_byte(pdev, SIS_PMR, &pmr);

@@ -158,14 +158,14 @@ static u32 sis_scr_cfg_read(struct ata_port *ap, 
 unsigned int sc_reg, u32 *val)
    return 0;
 }
 
-static void sis_scr_cfg_write(struct ata_port *ap, unsigned int sc_reg, 
 u32 val)
+static int sis_scr_cfg_write(struct ata_port *ap, unsigned int sc_reg, 
 u32 val)
 {
    struct pci_dev *pdev = to_pci_dev(ap->host->dev);
    unsigned int cfg_addr = get_scr_cfg_addr(ap, sc_reg);
    u8 pmr;
 
    if (sc_reg == SCR_ERROR) /* doesn't exist in PCI cfg space */
-        return;
+        return -EINVAL;

    pci_read_config_byte(pdev, SIS_PMR, &pmr);

drivers/ata/sata_sis.c +2 -0

@@ -174,6 +174,8 @@ static void sis_scr_cfg_write(struct ata_port *ap, 
 unsigned int sc_reg, u32 val)
    if ((pdev->device == 0x0182) || (pdev->device == 0x0183) ||
        (pdev->device == 0x1182) || (pmr & SIS_PMR_COMBINED))
        pci_write_config_dword(pdev, cfg_addr+0x10, val);
+
+    return 0;
 }
 

static int sis_scr_read(struct ata_port *ap, unsigned int sc_reg, u32 *val)

drivers/ata/sata_sis.c +1 -1

@@ -211,14 +213,14 @@ static int sis_scr_write(struct ata_port *ap, 
 unsigned int sc_reg, u32 val)
    pci_read_config_byte(pdev, SIS_PMR, &pmr);
 
    if (ap->flags & SIS_FLAG_CFGSCR)
-        sis_scr_cfg_write(ap, sc_reg, val);
+        return sis_scr_cfg_write(ap, sc_reg, val);