Re: [PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
On 13/3/24 22:08, Mark Cave-Ayland wrote:
On 13/03/2024 11: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_command_phase()
use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
- uint32_t cmdlen;
+ uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
- esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+ memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
'n' is unused, use NULL?
I was sure I had tried that before and it had failed, but I see that you
made it work with NULL in commit cd04033dbe ("util/fifo8: Allow
fifo8_pop_buf() to not populate popped length") - thanks!
Ah!
So using NULL in patches 1 and 2:
Reviewed-by: Philippe Mathieu-Daudé
I'll make the change for v3, but I'll wait a couple of days first to see
if there are any further comments, in particular R-B tags for patches 10
and 11.
I still have them in my TOREVIEW queue and need to digest them.
current_lun = scsi_device_find(>bus, 0, s->current_dev->id,
s->lun);
if (!current_lun) {
ATB,
Mark.
Re: [PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
On 13/03/2024 11: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_command_phase() use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
- uint32_t cmdlen;
+ uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
- esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+ memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
'n' is unused, use NULL?
I was sure I had tried that before and it had failed, but I see that you made it work
with NULL in commit cd04033dbe ("util/fifo8: Allow fifo8_pop_buf() to not populate
popped length") - thanks!
I'll make the change for v3, but I'll wait a couple of days first to see if there are
any further comments, in particular R-B tags for patches 10 and 11.
current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
if (!current_lun) {
ATB,
Mark.
Re: [PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
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_command_phase() use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
-uint32_t cmdlen;
+uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
-esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
'n' is unused, use NULL?
current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
if (!current_lun) {
[PATCH v2 01/16] esp.c: replace cmdfifo use of esp_fifo_pop_buf() in do_command_phase()
The aim is to restrict the esp_fifo_*() functions so that they only operate on
the hardware FIFO. When reading from cmdfifo in do_command_phase() use the
underlying Fifo8 functions directly.
Signed-off-by: Mark Cave-Ayland
---
hw/scsi/esp.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 590ff99744..f8230c74b3 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -265,7 +265,7 @@ static void esp_do_nodma(ESPState *s);
static void do_command_phase(ESPState *s)
{
-uint32_t cmdlen;
+uint32_t cmdlen, n;
int32_t datalen;
SCSIDevice *current_lun;
uint8_t buf[ESP_CMDFIFO_SZ];
@@ -275,7 +275,7 @@ static void do_command_phase(ESPState *s)
if (!cmdlen || !s->current_dev) {
return;
}
-esp_fifo_pop_buf(>cmdfifo, buf, cmdlen);
+memcpy(buf, fifo8_pop_buf(>cmdfifo, cmdlen, ), cmdlen);
current_lun = scsi_device_find(>bus, 0, s->current_dev->id, s->lun);
if (!current_lun) {
--
2.39.2
