On Sun, 4 Mar 2018, I wrote:

> > +   } else {
> > +           scsi_esp_cmd(esp, ESP_CMD_FLUSH);
> > +
> > +           if (esp_count >= ZORRO_ESP_FIFO_SIZE) {
> > +                   ZORRO_ESP_PIO_FILL("%0@+,%2@", esp_count);
> > +           } else {
> > +                   ZORRO_ESP_PIO_LOOP("%0@+,%2@", esp_count);
> > +           }
> > +
> > +           scsi_esp_cmd(esp, cmd);
> > +
> > +           while (esp_count) {
> > +                   unsigned int n;
> > +
> > +                   if (zorro_esp_wait_for_intr(esp))
> > +                           break;
> > +
> > +                   if ((esp->sreg & ESP_STAT_PMASK) != phase)
> > +                           break;
> > +
> > +                   esp->ireg = zorro_esp_read8(esp, ESP_INTRPT);
> > +                   /* This is tricky - ~ESP_INTR_BSERV is the wrong mask
> > +                    * a ESP_CMD_SELAS command which also generates
> > +                    * ESP_INTR_FDONE! Use ~(ESP_INTR_BSERV|ESP_INTR_FDONE)
> > +                    * since ESP_INTR_FDONE is not raised by any other
> > +                    * error condition. Unfortunately, this is still not
> > +                    * sufficient to make the single message byte transfer
> > +                    * of ESP_CMD_SELAS work ...
> > +                    */
> 
> Is that comment correct? This is the !write branch, that is, read 
> acccess from memory. But tag bytes move in the other direction for 
> ESP_CMD_SELAS.
> 

Nevermind -- I got the direction wrong there.

But if your comment is correct and the mask is wrong, that problem should 
show up in mac_esp too, right?

The whole TCQ problem showed up, and was fixed by checking ESP_INTR_FDONE 
in the Message In transfer.

> > +                   if (esp->ireg & ~(ESP_INTR_BSERV | ESP_INTR_FDONE)) {
> > +                           zep->error = 1;
> > +                           break;
> > +                   }

If you really do get ESP_INTR_FDONE instead of ESP_INTR_BSERV for Message 
Out, wouldn't it be safer (and more consistent) to do,

                        if (esp->ireg & mask) {
                                mep->error = 1;
                                break;
                        }

and initialize mask as,

        u8 mask = ~(phase == ESP_MIP || phase == ESP_MOP ? ESP_INTR_FDONE
                                                         : ESP_INTR_BSERV);

to cover both !write and write?

-- 

Reply via email to