On 2018/11/01 23:00, Frederic Cambus wrote:
> Hi tech@,
>
> Remove unnecessary if/else block, both branches are identical. We can
> in fact use the ATA_DELAY macro directly.
>
> Coverity CID 1453008.
>
> Comments? OK?
>
> Index: dev/ata/ata_wdc.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/ata/ata_wdc.c,v
> retrieving revision 1.51
> diff -u -p -r1.51 ata_wdc.c
> --- dev/ata/ata_wdc.c 30 Dec 2017 20:46:59 -0000 1.51
> +++ dev/ata/ata_wdc.c 1 Nov 2018 12:34:17 -0000
> @@ -240,7 +240,6 @@ _wdc_ata_bio_start(struct channel_softc
> u_int16_t cyl;
> u_int8_t head, sect, cmd = 0;
> int nblks;
> - int ata_delay;
> int error, dma_flags = 0;
>
> WDCDEBUG_PRINT(("_wdc_ata_bio_start %s:%d:%d\n",
> @@ -283,10 +282,6 @@ _wdc_ata_bio_start(struct channel_softc
> if (ata_bio->flags & ATA_LBA48)
> dma_flags |= WDC_DMA_LBA48;
> }
> - if (ata_bio->flags & ATA_SINGLE)
> - ata_delay = ATA_DELAY;
> - else
> - ata_delay = ATA_DELAY;
There since r1.1 :-)
The indirection could possibly have been done to allow tweaking the
delay with ddb for debug, but I don't think that really matters for wdc
any more..
OK with me
> again:
> /*
> *
> @@ -345,7 +340,7 @@ again:
> }
> /* Initiate command */
> wdc_set_drive(chp, xfer->drive);
> - if (wait_for_ready(chp, ata_delay) < 0)
> + if (wait_for_ready(chp, ATA_DELAY) < 0)
> goto timeout;
>
> /* start the DMA channel (before) */
> @@ -391,7 +386,7 @@ again:
> }
> /* Initiate command! */
> wdc_set_drive(chp, xfer->drive);
> - if (wait_for_ready(chp, ata_delay) < 0)
> + if (wait_for_ready(chp, ATA_DELAY) < 0)
> goto timeout;
> if (ata_bio->flags & ATA_LBA48) {
> wdccommandext(chp, xfer->drive, cmd,
> @@ -410,7 +405,7 @@ again:
> }
> /* If this was a write and not using DMA, push the data. */
> if ((ata_bio->flags & ATA_READ) == 0) {
> - if (wait_for_drq(chp, ata_delay) != 0) {
> + if (wait_for_drq(chp, ATA_DELAY) != 0) {
> printf("%s:%d:%d: timeout waiting for DRQ, "
> "st=0x%b, err=0x%02x\n",
> chp->wdc->sc_dev.dv_xname, chp->channel,
>