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.
