> Date: Tue, 25 Feb 2014 15:08:48 -0500
> From: Ted Unangst <[email protected]>
>
> Tangent:
>
> I don't like the style used for the delay loops in azalia.c.
>
> 1. There's an off by one where 0 passes both conditionals. It's
> unlikely this matters, but it's needless inconsistency.
> 2. I think i <= 0 is a dangerous idiom because any code refactor that
> changes the variable in question to unsigned introduces an infinite
> loop.
>
> This can wait til after 5.5, but I made the diff since I was looking
> at the code.
ok kettenis@, but I think you should wait until after release
> Index: azalia.c
> ===================================================================
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.210
> diff -u -p -r1.210 azalia.c
> --- azalia.c 25 Feb 2014 18:40:37 -0000 1.210
> +++ azalia.c 25 Feb 2014 20:02:20 -0000
> @@ -771,26 +771,26 @@ azalia_reset(azalia_t *az)
> DPRINTF(("%s: resetting\n", __func__));
> gctl = AZ_READ_4(az, GCTL);
> AZ_WRITE_4(az, GCTL, gctl & ~HDA_GCTL_CRST);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> if ((AZ_READ_4(az, GCTL) & HDA_GCTL_CRST) == 0)
> break;
> }
> DPRINTF(("%s: reset counter = %d\n", __func__, i));
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: reset failure\n", XNAME(az)));
> return(ETIMEDOUT);
> }
> DELAY(1000);
> gctl = AZ_READ_4(az, GCTL);
> AZ_WRITE_4(az, GCTL, gctl | HDA_GCTL_CRST);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> if (AZ_READ_4(az, GCTL) & HDA_GCTL_CRST)
> break;
> }
> DPRINTF(("%s: reset counter = %d\n", __func__, i));
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: reset-exit failure\n", XNAME(az)));
> return(ETIMEDOUT);
> }
> @@ -1016,13 +1016,13 @@ azalia_halt_corb(azalia_t *az)
> corbctl = AZ_READ_1(az, CORBCTL);
> if (corbctl & HDA_CORBCTL_CORBRUN) { /* running? */
> AZ_WRITE_1(az, CORBCTL, corbctl & ~HDA_CORBCTL_CORBRUN);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> corbctl = AZ_READ_1(az, CORBCTL);
> if ((corbctl & HDA_CORBCTL_CORBRUN) == 0)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: CORB is running\n", XNAME(az)));
> return EBUSY;
> }
> @@ -1061,13 +1061,13 @@ azalia_init_corb(azalia_t *az, int resum
> corbrp = AZ_READ_2(az, CORBRP);
> AZ_WRITE_2(az, CORBRP, corbrp | HDA_CORBRP_CORBRPRST);
> AZ_WRITE_2(az, CORBRP, corbrp & ~HDA_CORBRP_CORBRPRST);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> corbrp = AZ_READ_2(az, CORBRP);
> if ((corbrp & HDA_CORBRP_CORBRPRST) == 0)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: CORBRP reset failure\n", XNAME(az)));
> return -1;
> }
> @@ -1093,13 +1093,13 @@ azalia_halt_rirb(azalia_t *az)
> rirbctl = AZ_READ_1(az, RIRBCTL);
> if (rirbctl & HDA_RIRBCTL_RIRBDMAEN) { /* running? */
> AZ_WRITE_1(az, RIRBCTL, rirbctl & ~HDA_RIRBCTL_RIRBDMAEN);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> rirbctl = AZ_READ_1(az, RIRBCTL);
> if ((rirbctl & HDA_RIRBCTL_RIRBDMAEN) == 0)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: RIRB is running\n", XNAME(az)));
> return(EBUSY);
> }
> @@ -1161,13 +1161,13 @@ azalia_init_rirb(azalia_t *az, int resum
> rirbctl = AZ_READ_1(az, RIRBCTL);
> AZ_WRITE_1(az, RIRBCTL, rirbctl |
> HDA_RIRBCTL_RIRBDMAEN | HDA_RIRBCTL_RINTCTL);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> rirbctl = AZ_READ_1(az, RIRBCTL);
> if (rirbctl & HDA_RIRBCTL_RIRBDMAEN)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: RIRB is not running\n", XNAME(az)));
> return(EBUSY);
> }
> @@ -1244,7 +1244,7 @@ azalia_get_response(azalia_t *az, uint32
> DELAY(10);
> i--;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: RIRB time out\n", XNAME(az)));
> return(ETIMEDOUT);
> }
> @@ -3708,26 +3708,26 @@ azalia_stream_reset(stream_t *this)
> /* Start reset and wait for chip to enter. */
> ctl = STR_READ_2(this, CTL);
> STR_WRITE_2(this, CTL, ctl | HDA_SD_CTL_SRST);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> ctl = STR_READ_2(this, CTL);
> if (ctl & HDA_SD_CTL_SRST)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: stream reset failure 1\n", XNAME(this->az)));
> return -1;
> }
>
> /* Clear reset and wait for chip to finish */
> STR_WRITE_2(this, CTL, ctl & ~HDA_SD_CTL_SRST);
> - for (i = 5000; i >= 0; i--) {
> + for (i = 5000; i > 0; i--) {
> DELAY(10);
> ctl = STR_READ_2(this, CTL);
> if ((ctl & HDA_SD_CTL_SRST) == 0)
> break;
> }
> - if (i <= 0) {
> + if (i == 0) {
> DPRINTF(("%s: stream reset failure 2\n", XNAME(this->az)));
> return -1;
> }
>
>
