On Tue, 2017-10-17 at 18:17 -0400, Douglas Gilbert wrote:
> On 2017-10-17 05:03 PM, Bart Van Assche wrote:
> > @@ -1025,7 +1025,6 @@ static int sd_setup_read_write_cmnd(struct scsi_cmnd 
> > *SCpnt)
> >     struct gendisk *disk = rq->rq_disk;
> >     struct scsi_disk *sdkp = scsi_disk(disk);
> >     sector_t block = blk_rq_pos(rq);
> 
> s/block/lba/          # use the well understood SCSI abbreviation

Since blk_rq_pos() returns the offset in units of 512 bytes, how about renaming
'block' into 'sector' and using the name 'lba' for the number obtained after the
shift operation?

> 
> > -   sector_t threshold;
> >     unsigned int this_count = blk_rq_sectors(rq);
> >     unsigned int dif, dix;
> >     bool zoned_write = sd_is_zoned(sdkp) && rq_data_dir(rq) == WRITE;
> > @@ -1071,20 +1070,22 @@ static int sd_setup_read_write_cmnd(struct 
> > scsi_cmnd *SCpnt)
> >             goto out;
> >     }
> >   
> > -   /*
> > -    * Some SD card readers can't handle multi-sector accesses which touch
> > -    * the last one or two hardware sectors.  Split accesses as needed.
> > -    */
> > -   threshold = get_capacity(disk) - SD_LAST_BUGGY_SECTORS *
> > -           (sdp->sector_size / 512);
> > +   if (unlikely(sdp->last_sector_bug)) {
> > +           sector_t threshold;
> 
> s/threshold/threshold_lba/    # a bit long but more precise

A similar comment applies here - shouldn't this be called 'threshold_sector'?

> >             }
> >     }
> >     if (rq_data_dir(rq) == WRITE) {
> > @@ -1173,56 +1157,26 @@ static int sd_setup_read_write_cmnd(struct 
> > scsi_cmnd *SCpnt)
> >             SCpnt->cmnd[7] = 0x18;
> >             SCpnt->cmnd[9] = (rq_data_dir(rq) == READ) ? READ_32 : WRITE_32;
> 
> Perhaps rq_data_dir(rq) could be placed in a local variable

I will keep that for a separate patch.

Bart.

Reply via email to