| Message ID | 20240313085810.2655062-3-mark.cave-ayland@ilande.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | esp: avoid explicit setting of DRQ within ESP state machine | expand |
On 13/3/24 09:57, Mark Cave-Ayland wrote: > The aim is to restrict the esp_fifo_*() functions so that they only operate on > the hardware FIFO. When reading from cmdfifo in do_message_phase() use the > underlying Fifo8 functions directly. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/scsi/esp.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c > index f8230c74b3..100560244b 100644 > --- a/hw/scsi/esp.c > +++ b/hw/scsi/esp.c > @@ -309,6 +309,8 @@ static void do_command_phase(ESPState *s) > > static void do_message_phase(ESPState *s) > { > + uint32_t n; > + > if (s->cmdfifo_cdb_offset) { > uint8_t message = esp_fifo_pop(&s->cmdfifo); > > @@ -320,7 +322,10 @@ static void do_message_phase(ESPState *s) > /* Ignore extended messages for now */ > if (s->cmdfifo_cdb_offset) { > int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); > - esp_fifo_pop_buf(&s->cmdfifo, NULL, len); > + > + if (len) { > + fifo8_pop_buf(&s->cmdfifo, len, &n); 'n' is unused, use NULL? > + } > s->cmdfifo_cdb_offset = 0; > } > }
On 13/3/24 12:03, Philippe Mathieu-Daudé wrote: > On 13/3/24 09:57, Mark Cave-Ayland wrote: >> The aim is to restrict the esp_fifo_*() functions so that they only >> operate on >> the hardware FIFO. When reading from cmdfifo in do_message_phase() use >> the >> underlying Fifo8 functions directly. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/scsi/esp.c | 7 ++++++- >> 1 file changed, 6 insertions(+), 1 deletion(-) >> >> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c >> index f8230c74b3..100560244b 100644 >> --- a/hw/scsi/esp.c >> +++ b/hw/scsi/esp.c >> @@ -309,6 +309,8 @@ static void do_command_phase(ESPState *s) >> static void do_message_phase(ESPState *s) >> { >> + uint32_t n; >> + >> if (s->cmdfifo_cdb_offset) { >> uint8_t message = esp_fifo_pop(&s->cmdfifo); >> @@ -320,7 +322,10 @@ static void do_message_phase(ESPState *s) >> /* Ignore extended messages for now */ >> if (s->cmdfifo_cdb_offset) { >> int len = MIN(s->cmdfifo_cdb_offset, >> fifo8_num_used(&s->cmdfifo)); >> - esp_fifo_pop_buf(&s->cmdfifo, NULL, len); >> + >> + if (len) { >> + fifo8_pop_buf(&s->cmdfifo, len, &n); > > 'n' is unused, use NULL? Using NULL: Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c index f8230c74b3..100560244b 100644 --- a/hw/scsi/esp.c +++ b/hw/scsi/esp.c @@ -309,6 +309,8 @@ static void do_command_phase(ESPState *s) static void do_message_phase(ESPState *s) { + uint32_t n; + if (s->cmdfifo_cdb_offset) { uint8_t message = esp_fifo_pop(&s->cmdfifo); @@ -320,7 +322,10 @@ static void do_message_phase(ESPState *s) /* Ignore extended messages for now */ if (s->cmdfifo_cdb_offset) { int len = MIN(s->cmdfifo_cdb_offset, fifo8_num_used(&s->cmdfifo)); - esp_fifo_pop_buf(&s->cmdfifo, NULL, len); + + if (len) { + fifo8_pop_buf(&s->cmdfifo, len, &n); + } s->cmdfifo_cdb_offset = 0; } }
The aim is to restrict the esp_fifo_*() functions so that they only operate on the hardware FIFO. When reading from cmdfifo in do_message_phase() use the underlying Fifo8 functions directly. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/scsi/esp.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)