| Message ID | 1466422756-126637-5-git-send-email-hare@suse.de |
|---|---|
| State | Not Applicable |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Jun 20, 2016 at 01:39:14PM +0200, Hannes Reinecke wrote: > The taskfile protocol is a numeric value, and can not be ORed. > > Signed-off-by: Hannes Reinecke <hare@suse.de> > --- > drivers/ata/libata-core.c | 4 ++-- > drivers/ata/libata-eh.c | 2 +- > 2 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c > index e798915..d200102 100644 > --- a/drivers/ata/libata-core.c > +++ b/drivers/ata/libata-core.c > @@ -1238,7 +1238,7 @@ static int ata_read_native_max_address(struct ata_device *dev, u64 *max_sectors) > } else > tf.command = ATA_CMD_READ_NATIVE_MAX; > > - tf.protocol |= ATA_PROT_NODATA; > + tf.protocol = ATA_PROT_NODATA; > tf.device |= ATA_LBA; Did the above lead to any actual brekage or was tf.protocol guaranteed to be zero always? I wish the patch description described what the stake of the patch is. Thanks.
On 06/20/2016 05:44 PM, Tejun Heo wrote: > On Mon, Jun 20, 2016 at 01:39:14PM +0200, Hannes Reinecke wrote: >> The taskfile protocol is a numeric value, and can not be ORed. >> >> Signed-off-by: Hannes Reinecke <hare@suse.de> >> --- >> drivers/ata/libata-core.c | 4 ++-- >> drivers/ata/libata-eh.c | 2 +- >> 2 files changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c >> index e798915..d200102 100644 >> --- a/drivers/ata/libata-core.c >> +++ b/drivers/ata/libata-core.c >> @@ -1238,7 +1238,7 @@ static int ata_read_native_max_address(struct ata_device *dev, u64 *max_sectors) >> } else >> tf.command = ATA_CMD_READ_NATIVE_MAX; >> >> - tf.protocol |= ATA_PROT_NODATA; >> + tf.protocol = ATA_PROT_NODATA; >> tf.device |= ATA_LBA; > > Did the above lead to any actual brekage or was tf.protocol guaranteed > to be zero always? I wish the patch description described what the > stake of the patch is. > This is actually a cleanup; if tf.protocl is '0' we won't see any issues here. But if (for some obsure reason) tf.protocol is _not_ zero we'll run into very obscure problems. Plus the OR here might give others the wrong impression, and might lead to a confusion between ATA_PROT_XXX (which is a scalar value and cannot be ORed) and ATA_PROT_FLAG_XXX (which is a bit value and can be ORed). Cheers, Hannes
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c index e798915..d200102 100644 --- a/drivers/ata/libata-core.c +++ b/drivers/ata/libata-core.c @@ -1238,7 +1238,7 @@ static int ata_read_native_max_address(struct ata_device *dev, u64 *max_sectors) } else tf.command = ATA_CMD_READ_NATIVE_MAX; - tf.protocol |= ATA_PROT_NODATA; + tf.protocol = ATA_PROT_NODATA; tf.device |= ATA_LBA; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); @@ -1297,7 +1297,7 @@ static int ata_set_max_sectors(struct ata_device *dev, u64 new_sectors) tf.device |= (new_sectors >> 24) & 0xf; } - tf.protocol |= ATA_PROT_NODATA; + tf.protocol = ATA_PROT_NODATA; tf.device |= ATA_LBA; tf.lbal = (new_sectors >> 0) & 0xff; diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c index 61dc7a9..7832e55 100644 --- a/drivers/ata/libata-eh.c +++ b/drivers/ata/libata-eh.c @@ -3177,7 +3177,7 @@ static void ata_eh_park_issue_cmd(struct ata_device *dev, int park) } tf.flags |= ATA_TFLAG_DEVICE | ATA_TFLAG_ISADDR; - tf.protocol |= ATA_PROT_NODATA; + tf.protocol = ATA_PROT_NODATA; err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0); if (park && (err_mask || tf.lbal != 0xc4)) { ata_dev_err(dev, "head unload failed!\n");
The taskfile protocol is a numeric value, and can not be ORed. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/ata/libata-core.c | 4 ++-- drivers/ata/libata-eh.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)