Skip to contentSkip to conversation

ata: propagate return value of scc_wait_after_reset

PATCH In review Arjun Sreedharan 2 participants 5 messages Aug 16, 2014 – Aug 17, 2014 1 message not in the archive libata

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): 53ef6157.qX8Csumbm2zNx/EI%[email protected]

Arjun Sreedharan

author
#1 lore raw

PATCH 1/1 ata: propagate return value of scc_wait_after_reset

scc_bus_softreset not necessarily should return zero. Propagate the error code.

Signed-off-by Arjun Sreedharan <[email protected]>
---
 drivers/ata/pata_scc.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)
drivers/ata/pata_scc.c +7 -8
index 4e006d7..b5013dc 100644
@@ -585,7 +585,7 @@ static int scc_wait_after_reset(struct ata_link *link, unsigned int devmask,
  *	Note: Original code is ata_bus_softreset().
  */
 
-static unsigned int scc_bus_softreset(struct ata_port *ap, unsigned int devmask,
+static int scc_bus_softreset(struct ata_port *ap, unsigned int devmask,
                                       unsigned long deadline)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;

@@ -599,9 +599,7 @@ static unsigned int scc_bus_softreset(struct ata_port *ap, unsigned int devmask,
 	udelay(20);
 	out_be32(ioaddr->ctl_addr, ap->ctl);
 
-	scc_wait_after_reset(&ap->link, devmask, deadline);
-
-	return 0;
+	return scc_wait_after_reset(&ap->link, devmask, deadline);
 }
 
 /**

@@ -618,7 +616,8 @@ static int scc_softreset(struct ata_link *link, unsigned int *classes,
 {
 	struct ata_port *ap = link->ap;
 	unsigned int slave_possible = ap->flags & ATA_FLAG_SLAVE_POSS;
-	unsigned int devmask = 0, err_mask;
+	unsigned int devmask = 0;
+	int rc;
 	u8 err;
 
 	DPRINTK("ENTER\n");

@@ -634,8 +633,8 @@ static int scc_softreset(struct ata_link *link, unsigned int *classes,
 
 	/* issue bus reset */
 	DPRINTK("about to softreset, devmask=%x\n", devmask);
-	err_mask = scc_bus_softreset(ap, devmask, deadline);
-	if (err_mask) {
- 		ata_port_err(ap, "SRST failed (err_mask=0x%x)\n", err_mask);
+	rc = scc_bus_softreset(ap, devmask, deadline);
+	if (rc) {
+ 		ata_port_err(ap, "SRST failed (err_mask=0x%x)\n", rc);
 		return -EIO;
 	}
Signature · 1 line
1.7.11.7
#2 lore raw
Quoted 4 lines · Arjun Sreedharan

scc_bus_softreset not necessarily should return zero. Propagate the error code.

Signed-off-by Arjun Sreedharan <[email protected]>

I reverted the change yesterday but didn't have time to reply. Don't send patches you haven't even compile tested. This is the most basic of your responsibilities when sending out patches. Okay?

Applying the updated version to libata/for-3.17-fixes.

Signature · 1 line
tejun
#3 lore raw
Quoted 11 lines · Tejun Heo
Quoted 4 lines · Arjun Sreedharan

scc_bus_softreset not necessarily should return zero. Propagate the error code.

Signed-off-by Arjun Sreedharan <[email protected]>

I reverted the change yesterday but didn't have time to reply. Don't send patches you haven't even compile tested. This is the most basic of your responsibilities when sending out patches. Okay?

Applying the updated version to libata/for-3.17-fixes.

Gees, and your new patch doesn't even apply. What are you doing?

Signature · 1 line
tejun
author
#4 lore raw
Quoted 10 lines · Tejun Heo
Quoted 9 lines · Tejun Heo
Quoted 4 lines · Arjun Sreedharan

scc_bus_softreset not necessarily should return zero. Propagate the error code.

Signed-off-by Arjun Sreedharan <[email protected]>

I reverted the change yesterday but didn't have time to reply. Don't send patches you haven't even compile tested. This is the most basic of your responsibilities when sending out patches. Okay?

Deepest apologies. I had compile tested the first patch; but with a small variable name change, i overlooked the need to compile test. my fault.

here's the link to the first patch:
https://lkml.org/lkml/diff/2014/8/16/15/1

Also now i feel @err_mask is better.
I rest my case further. Thanks.

Quoted 7 lines
Quoted 2 lines

Applying the updated version to libata/for-3.17-fixes.

Gees, and your new patch doesn't even apply. What are you doing?

--
tejun

#5 lore raw
Quoted 2 lines · Arjun Sreedharan

Also now i feel @err_mask is better.
I rest my case further. Thanks.

The thing is err_mask is used in libata to carry AC_ERR_* values not 0/-errno's, so it's confusing to use it for that. Can you please refresh the patch w/ variable rename?

Thanks.

Signature · 1 line
tejun