Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-12-03 Thread Mikael Pettersson
On Mon, 03 Dec 2007 13:49:36 +0900, Tejun Heo wrote:
 Mikael Pettersson wrote:
  what was the outcome of this discussion?
 
  I haven't looked over the Promise datasheet nor checked my brain for 
  details, hoping Mikael would do that for me ;-)
  
  I've now tested this on top of 2.6.24-rc3, with no observable
  regressions. Blanking, writing, and mounting/reading CD-RWs
  on both SATAPI and PATAPI works (tested on a 300 TX2plus card).
  
  I can't find anything in Promise's public docs or reference driver
  about non-standard requirements on lbam/lbah in ATAPI packets.
 
 The values set by core layer should be good enough.  The only thing I'm
 worried about is setting transfer chunk size when protocol is DMA.  As
 setting this value hasn't caused any problem for other controllers and
 it seems sata_promise doesn't seem to have problem with it either, I'm
 leaning toward keeping this value but if setting this value to zero is
 the right thing to do, we can definitely change that in the core layer.
  One way or the other, I'd really like to keep sata_promise's behavior
 in line with other libata drivers.
 
 So, Mikael, do you think it would be okay to include the patch for
 #upstream and see how it works in -mm?

Yes

/Mikael
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-12-02 Thread Mikael Pettersson
On Sat, 01 Dec 2007 18:23:44 -0500, Jeff Garzik wrote:
 Tejun Heo wrote:
  Make pdc_atapi_pkt() use values from qc-tf instead of creating its
  own.  This is to ease future ATAPI handling changes.
  
  DONT APPLY YET
  ---
  Mikael, would this work?  Values other than lbam and lbah remain the
  same.  Does sata_promise have strict requirements for lbam and lbah?
  
  Thanks.
  
   drivers/ata/sata_promise.c |   34 +-
   1 file changed, 13 insertions(+), 21 deletions(-)
 
 what was the outcome of this discussion?
 
 I haven't looked over the Promise datasheet nor checked my brain for 
 details, hoping Mikael would do that for me ;-)

I've now tested this on top of 2.6.24-rc3, with no observable
regressions. Blanking, writing, and mounting/reading CD-RWs
on both SATAPI and PATAPI works (tested on a 300 TX2plus card).

I can't find anything in Promise's public docs or reference driver
about non-standard requirements on lbam/lbah in ATAPI packets.

/Mikael
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-12-02 Thread Tejun Heo
Mikael Pettersson wrote:
 what was the outcome of this discussion?

 I haven't looked over the Promise datasheet nor checked my brain for 
 details, hoping Mikael would do that for me ;-)
 
 I've now tested this on top of 2.6.24-rc3, with no observable
 regressions. Blanking, writing, and mounting/reading CD-RWs
 on both SATAPI and PATAPI works (tested on a 300 TX2plus card).
 
 I can't find anything in Promise's public docs or reference driver
 about non-standard requirements on lbam/lbah in ATAPI packets.

The values set by core layer should be good enough.  The only thing I'm
worried about is setting transfer chunk size when protocol is DMA.  As
setting this value hasn't caused any problem for other controllers and
it seems sata_promise doesn't seem to have problem with it either, I'm
leaning toward keeping this value but if setting this value to zero is
the right thing to do, we can definitely change that in the core layer.
 One way or the other, I'd really like to keep sata_promise's behavior
in line with other libata drivers.

So, Mikael, do you think it would be okay to include the patch for
#upstream and see how it works in -mm?

Thanks.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-12-01 Thread Jeff Garzik

Tejun Heo wrote:

Make pdc_atapi_pkt() use values from qc-tf instead of creating its
own.  This is to ease future ATAPI handling changes.

DONT APPLY YET
---
Mikael, would this work?  Values other than lbam and lbah remain the
same.  Does sata_promise have strict requirements for lbam and lbah?

Thanks.

 drivers/ata/sata_promise.c |   34 +-
 1 file changed, 13 insertions(+), 21 deletions(-)


what was the outcome of this discussion?

I haven't looked over the Promise datasheet nor checked my brain for 
details, hoping Mikael would do that for me ;-)



-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-11-26 Thread Mikael Pettersson
On Mon, 26 Nov 2007 20:34:38 +0900, Tejun Heo wrote:
 Make pdc_atapi_pkt() use values from qc-tf instead of creating its
 own.  This is to ease future ATAPI handling changes.
 
 DONT APPLY YET
 ---
 Mikael, would this work?  Values other than lbam and lbah remain the
 same.  Does sata_promise have strict requirements for lbam and lbah?

...

   /* set feature and byte counter registers */
 - if (qc-tf.protocol != ATA_PROT_ATAPI_DMA) {
 + if (qc-tf.protocol != ATA_PROT_ATAPI_DMA)
   feature = PDC_FEATURE_ATAPI_PIO;
 - /* set byte counter register to real transfer byte count */
 - nbytes = qc-nbytes;
 - if (nbytes  0x)
 - nbytes = 0x;
 - } else {
 + else
   feature = PDC_FEATURE_ATAPI_DMA;
 - /* set byte counter register to 0 */
 - nbytes = 0;
 - }
 +
   buf[20] = (1  5) | ATA_REG_FEATURE;
   buf[21] = feature;
   buf[22] = (1  5) | ATA_REG_BYTEL;
 - buf[23] = nbytes  0xFF;
 + buf[23] = qc-tf.lbam;
   buf[24] = (1  5) | ATA_REG_BYTEH;
 - buf[25] = (nbytes  8)  0xFF;
 + buf[25] = qc-tf.lbah;

The original code matches what Promise' own driver does, including
the set byte counter register to real transfer byte count comment.
It's certainly possible that if lbah/lbam don't match -nbytes,
the HW will go nuts. Their data sheets are very quiet about ATAPI.

I can test your proposed change next weekend when I'm back to where
my sata_promise test equipment is.

/Mikael
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC] sata_promise: make pdc_atapi_pkt() use values from qc-tf

2007-11-26 Thread Tejun Heo
Hello,

Mikael Pettersson wrote:
  buf[20] = (1  5) | ATA_REG_FEATURE;
  buf[21] = feature;
  buf[22] = (1  5) | ATA_REG_BYTEL;
 -buf[23] = nbytes  0xFF;
 +buf[23] = qc-tf.lbam;
  buf[24] = (1  5) | ATA_REG_BYTEH;
 -buf[25] = (nbytes  8)  0xFF;
 +buf[25] = qc-tf.lbah;
 
 The original code matches what Promise' own driver does, including
 the set byte counter register to real transfer byte count comment.
 It's certainly possible that if lbah/lbam don't match -nbytes,
 the HW will go nuts. Their data sheets are very quiet about ATAPI.

Yeah, now core code sets the value properly and I just posted a patch to
massage transfer chunk size a bit so I was curious whether now
sata_promise can use the result of core layer instead.

 I can test your proposed change next weekend when I'm back to where
 my sata_promise test equipment is.

Thank you.

-- 
tejun
-
To unsubscribe from this list: send the line unsubscribe linux-ide in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html