public inbox for linux-scsi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ipr: qc_fill_rtf() method should not store alternate status register
@ 2013-05-21 19:24 Sergei Shtylyov
  2013-05-31 12:46 ` Brian King
  0 siblings, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2013-05-21 19:24 UTC (permalink / raw)
  To: brking, JBottomley, linux-scsi; +Cc: linux-ide, tj

The 'ctl' field of  the 'struct ata_taskfile' is not really dual purpose, i.e.
it is not intended  for storing the alternate status register (which is mapped
at the same address in the legacy IDE controllers) in the qc_fill_rtf() method.
No other 'libata' driver except 'drivers/scsi/ipr.c' stores the alternate status
register's value in the 'ctl' field of 'qc->result_tf', hence this driver should
not do this as well...

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
The patch is against the recent Linus' tree.

 drivers/scsi/ipr.c |    1 -
 1 file changed, 1 deletion(-)

Index: linux/drivers/scsi/ipr.c
===================================================================
--- linux.orig/drivers/scsi/ipr.c
+++ linux/drivers/scsi/ipr.c
@@ -6662,7 +6662,6 @@ static bool ipr_qc_fill_rtf(struct ata_q
 	tf->hob_lbal = g->hob_lbal;
 	tf->hob_lbam = g->hob_lbam;
 	tf->hob_lbah = g->hob_lbah;
-	tf->ctl = g->alt_status;
 
 	return true;
 }

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ipr: qc_fill_rtf() method should not store alternate status register
  2013-05-21 19:24 [PATCH] ipr: qc_fill_rtf() method should not store alternate status register Sergei Shtylyov
@ 2013-05-31 12:46 ` Brian King
  2013-05-31 22:37   ` Sergei Shtylyov
  0 siblings, 1 reply; 4+ messages in thread
From: Brian King @ 2013-05-31 12:46 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: brking, JBottomley, linux-scsi, linux-ide, tj

On 05/21/2013 02:24 PM, Sergei Shtylyov wrote:
> The 'ctl' field of  the 'struct ata_taskfile' is not really dual purpose, i.e.
> it is not intended  for storing the alternate status register (which is mapped
> at the same address in the legacy IDE controllers) in the qc_fill_rtf() method.
> No other 'libata' driver except 'drivers/scsi/ipr.c' stores the alternate status
> register's value in the 'ctl' field of 'qc->result_tf', hence this driver should
> not do this as well...
> 
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

Acked-by: Brian King <brking@linux.vnet.ibm.com>

-- 
Brian King
Power Linux I/O
IBM Linux Technology Center



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ipr: qc_fill_rtf() method should not store alternate status register
  2013-05-31 12:46 ` Brian King
@ 2013-05-31 22:37   ` Sergei Shtylyov
  2013-06-02  8:15     ` Tejun Heo
  0 siblings, 1 reply; 4+ messages in thread
From: Sergei Shtylyov @ 2013-05-31 22:37 UTC (permalink / raw)
  To: Brian King, tj; +Cc: JBottomley, linux-scsi, linux-ide

Hello.

On 05/31/2013 04:46 PM, Brian King wrote:

>
>> The 'ctl' field of  the 'struct ata_taskfile' is not really dual purpose, i.e.
>> it is not intended  for storing the alternate status register (which is mapped
>> at the same address in the legacy IDE controllers) in the qc_fill_rtf() method.
>> No other 'libata' driver except 'drivers/scsi/ipr.c' stores the alternate status
>> register's value in the 'ctl' field of 'qc->result_tf', hence this driver should
>> not do this as well...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> Acked-by: Brian King <brking@linux.vnet.ibm.com>
>

     Tejun, I would prefer that this patch be merged thru the libata tree
as my future cleanup patches depend on it.

MBR, Sergei


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ipr: qc_fill_rtf() method should not store alternate status register
  2013-05-31 22:37   ` Sergei Shtylyov
@ 2013-06-02  8:15     ` Tejun Heo
  0 siblings, 0 replies; 4+ messages in thread
From: Tejun Heo @ 2013-06-02  8:15 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: Brian King, JBottomley, linux-scsi, linux-ide

On Sat, Jun 01, 2013 at 02:37:00AM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/31/2013 04:46 PM, Brian King wrote:
> 
> >
> >>The 'ctl' field of  the 'struct ata_taskfile' is not really dual purpose, i.e.
> >>it is not intended  for storing the alternate status register (which is mapped
> >>at the same address in the legacy IDE controllers) in the qc_fill_rtf() method.
> >>No other 'libata' driver except 'drivers/scsi/ipr.c' stores the alternate status
> >>register's value in the 'ctl' field of 'qc->result_tf', hence this driver should
> >>not do this as well...
> >>
> >>Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
> >Acked-by: Brian King <brking@linux.vnet.ibm.com>
> >
> 
>     Tejun, I would prefer that this patch be merged thru the libata tree
> as my future cleanup patches depend on it.

Applied to libata/for-3.11 w/ Brian's ack added.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-06-02  8:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-21 19:24 [PATCH] ipr: qc_fill_rtf() method should not store alternate status register Sergei Shtylyov
2013-05-31 12:46 ` Brian King
2013-05-31 22:37   ` Sergei Shtylyov
2013-06-02  8:15     ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox