| Message ID | af84a796696f2f366d330595d1a818e8daeb86d8.1469126217.git.tom.ty89@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Hello. On 07/21/2016 09:41 PM, tom.ty89@gmail.com wrote: > From: Tom Yan <tom.ty89@gmail.com> > > 1. Removed a repeated bit masking in ata_mselect_control() > 2. Moved `wce`/`d_sense` assignment below the page validity checks > 3. Added/Removed empty lines where appropriate > > Signed-off-by: Tom Yan <tom.ty89@gmail.com> Perhaps Tejun is OK with that, but if you ask me doing 1 task per patch is a better practice. [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
This patch should cause no behavioral change. Even the (removal of) the redundant bit mask should be a nop. So it seems like a bit of an overkill to split them. On 22 July 2016 at 02:45, Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> wrote: > Hello. > > On 07/21/2016 09:41 PM, tom.ty89@gmail.com wrote: > >> From: Tom Yan <tom.ty89@gmail.com> >> >> 1. Removed a repeated bit masking in ata_mselect_control() >> 2. Moved `wce`/`d_sense` assignment below the page validity checks >> 3. Added/Removed empty lines where appropriate >> >> Signed-off-by: Tom Yan <tom.ty89@gmail.com> > > > Perhaps Tejun is OK with that, but if you ask me doing 1 task per patch > is a better practice. > > [...] > > MBR, Sergei > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index 2bdb5da..eb5e8ff 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -3618,7 +3618,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, * The first two bytes of def_cache_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CACHE_MPAGE_LEN - 2) { if (len < CACHE_MPAGE_LEN - 2) *fp = len; @@ -3627,8 +3626,6 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, return -EINVAL; } - wce = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3642,6 +3639,8 @@ static int ata_mselect_caching(struct ata_queued_cmd *qc, } } + wce = buf[0] & (1 << 2); + tf->flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; tf->protocol = ATA_PROT_NODATA; tf->nsect = 0; @@ -3674,7 +3673,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, * The first two bytes of def_control_mpage are a header, so offsets * in mpage are off by 2 compared to buf. Same for len. */ - if (len != CONTROL_MPAGE_LEN - 2) { if (len < CONTROL_MPAGE_LEN - 2) *fp = len; @@ -3683,8 +3681,6 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } - d_sense = buf[0] & (1 << 2); - /* * Check that read-only bits are not modified. */ @@ -3697,7 +3693,10 @@ static int ata_mselect_control(struct ata_queued_cmd *qc, return -EINVAL; } } - if (d_sense & (1 << 2)) + + d_sense = buf[0] & (1 << 2); + + if (d_sense) dev->flags |= ATA_DFLAG_D_SENSE; else dev->flags &= ~ATA_DFLAG_D_SENSE;