This prepares for the inclusion of the current PDMA callback in the migration
stream since the callback is referenced by an integer instead of a function
pointer.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
hw/scsi/esp.c | 47 ++++++++++++++++++++++++++++++-------------
include/hw/scsi/esp.h | 11 +++++++++-
2 files changed, 43 insertions(+), 15 deletions(-)
diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 035fb0d1f6..e8b6f25dbb 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -195,14 +195,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
esp_set_tc(s, dmalen);
}
-static void esp_set_pdma_cb(ESPState *s, void (*cb)(ESPState *))
+static void esp_set_pdma_cb(ESPState *s, int pdma_cb)
{
- s->pdma_cb = cb;
-}
-
-static void esp_pdma_cb(ESPState *s)
-{
- s->pdma_cb(s);
+ assert(pdma_cb >= 0 && pdma_cb < PDMA_NUM_CBS);
+ s->pdma_cb = pdma_cb;
}
static int esp_select(ESPState *s)
@@ -366,7 +362,7 @@ static void handle_satn(ESPState *s)
s->dma_cb = handle_satn;
return;
}
- esp_set_pdma_cb(s, satn_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_SATN_PDMA_CB);
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 1;
@@ -397,7 +393,7 @@ static void handle_s_without_atn(ESPState *s)
s->dma_cb = handle_s_without_atn;
return;
}
- esp_set_pdma_cb(s, s_without_satn_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_S_WITHOUT_SATN_PDMA_CB);
cmdlen = get_cmd(s, ESP_CMDFIFO_SZ);
if (cmdlen > 0) {
s->cmdfifo_cdb_offset = 0;
@@ -432,7 +428,7 @@ static void handle_satn_stop(ESPState *s)
s->dma_cb = handle_satn_stop;
return;
}
- esp_set_pdma_cb(s, satn_stop_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_SATN_STOP_PDMA_CB);
cmdlen = get_cmd(s, 1);
if (cmdlen > 0) {
trace_esp_handle_satn_stop(fifo8_num_used(&s->cmdfifo));
@@ -474,7 +470,7 @@ static void write_response(ESPState *s)
s->rregs[ESP_RINTR] |= INTR_BS | INTR_FC;
s->rregs[ESP_RSEQ] = SEQ_CD;
} else {
- esp_set_pdma_cb(s, write_response_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_WRITE_RESPONSE_PDMA_CB);
esp_raise_drq(s);
return;
}
@@ -614,7 +610,7 @@ static void esp_do_dma(ESPState *s)
s->dma_memory_read(s->dma_opaque, buf, len);
fifo8_push_all(&s->cmdfifo, buf, len);
} else {
- esp_set_pdma_cb(s, do_dma_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_DO_DMA_PDMA_CB);
esp_raise_drq(s);
return;
}
@@ -656,7 +652,7 @@ static void esp_do_dma(ESPState *s)
if (s->dma_memory_read) {
s->dma_memory_read(s->dma_opaque, s->async_buf, len);
} else {
- esp_set_pdma_cb(s, do_dma_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_DO_DMA_PDMA_CB);
esp_raise_drq(s);
return;
}
@@ -688,7 +684,7 @@ static void esp_do_dma(ESPState *s)
}
esp_set_tc(s, esp_get_tc(s) - len);
- esp_set_pdma_cb(s, do_dma_pdma_cb);
+ esp_set_pdma_cb(s, PDMA_DO_DMA_PDMA_CB);
esp_raise_drq(s);
/* Indicate transfer to FIFO is complete */
@@ -787,6 +783,29 @@ static void esp_do_nodma(ESPState *s)
esp_raise_irq(s);
}
+static void esp_pdma_cb(ESPState *s)
+{
+ switch (s->pdma_cb) {
+ case PDMA_SATN_PDMA_CB:
+ satn_pdma_cb(s);
+ break;
+ case PDMA_S_WITHOUT_SATN_PDMA_CB:
+ s_without_satn_pdma_cb(s);
+ break;
+ case PDMA_SATN_STOP_PDMA_CB:
+ satn_stop_pdma_cb(s);
+ break;
+ case PDMA_WRITE_RESPONSE_PDMA_CB:
+ write_response_pdma_cb(s);
+ break;
+ case PDMA_DO_DMA_PDMA_CB:
+ do_dma_pdma_cb(s);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+}
+
void esp_command_complete(SCSIRequest *req, size_t resid)
{
ESPState *s = req->hba_private;
diff --git a/include/hw/scsi/esp.h b/include/hw/scsi/esp.h
index b1ec27612f..885ccf4660 100644
--- a/include/hw/scsi/esp.h
+++ b/include/hw/scsi/esp.h
@@ -51,7 +51,7 @@ struct ESPState {
ESPDMAMemoryReadWriteFunc dma_memory_write;
void *dma_opaque;
void (*dma_cb)(ESPState *s);
- void (*pdma_cb)(ESPState *s);
+ uint8_t pdma_cb;
uint8_t mig_version_id;
@@ -150,6 +150,15 @@ struct SysBusESPState {
#define TCHI_FAS100A 0x4
#define TCHI_AM53C974 0x12
+/* PDMA callbacks */
+#define PDMA_SATN_PDMA_CB 0
+#define PDMA_S_WITHOUT_SATN_PDMA_CB 1
+#define PDMA_SATN_STOP_PDMA_CB 2
+#define PDMA_WRITE_RESPONSE_PDMA_CB 3
+#define PDMA_DO_DMA_PDMA_CB 4
+
+#define PDMA_NUM_CBS 5
+
void esp_dma_enable(ESPState *s, int irq, int level);
void esp_request_cancelled(SCSIRequest *req);
void esp_command_complete(SCSIRequest *req, size_t resid);
--
2.20.1On 28/2/22 23:25, Mark Cave-Ayland wrote:
> This prepares for the inclusion of the current PDMA callback in the migration
> stream since the callback is referenced by an integer instead of a function
> pointer.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
> hw/scsi/esp.c | 47 ++++++++++++++++++++++++++++++-------------
> include/hw/scsi/esp.h | 11 +++++++++-
> 2 files changed, 43 insertions(+), 15 deletions(-)
>
> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
> index 035fb0d1f6..e8b6f25dbb 100644
> --- a/hw/scsi/esp.c
> +++ b/hw/scsi/esp.c
> @@ -195,14 +195,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
> esp_set_tc(s, dmalen);
> }
>
> -static void esp_set_pdma_cb(ESPState *s, void (*cb)(ESPState *))
> +static void esp_set_pdma_cb(ESPState *s, int pdma_cb)
Why signed?
> {
> - s->pdma_cb = cb;
> -}
> -
> -static void esp_pdma_cb(ESPState *s)
> -{
> - s->pdma_cb(s);
> + assert(pdma_cb >= 0 && pdma_cb < PDMA_NUM_CBS);
No need to check >=0 if using unsigned.
> + s->pdma_cb = pdma_cb;
> }
> +static void esp_pdma_cb(ESPState *s)
> +{
> + switch (s->pdma_cb) {
> + case PDMA_SATN_PDMA_CB:
> + satn_pdma_cb(s);
> + break;
> + case PDMA_S_WITHOUT_SATN_PDMA_CB:
> + s_without_satn_pdma_cb(s);
> + break;
> + case PDMA_SATN_STOP_PDMA_CB:
> + satn_stop_pdma_cb(s);
> + break;
> + case PDMA_WRITE_RESPONSE_PDMA_CB:
> + write_response_pdma_cb(s);
> + break;
> + case PDMA_DO_DMA_PDMA_CB:
> + do_dma_pdma_cb(s);
> + break;
> + default:
> + g_assert_not_reached();
> + }
> +}
> +
> index b1ec27612f..885ccf4660 100644
> --- a/include/hw/scsi/esp.h
> +++ b/include/hw/scsi/esp.h
> @@ -51,7 +51,7 @@ struct ESPState {
> ESPDMAMemoryReadWriteFunc dma_memory_write;
> void *dma_opaque;
> void (*dma_cb)(ESPState *s);
> - void (*pdma_cb)(ESPState *s);
> + uint8_t pdma_cb;
And here you use unsigned :)
> uint8_t mig_version_id;
>
> @@ -150,6 +150,15 @@ struct SysBusESPState {
> #define TCHI_FAS100A 0x4
> #define TCHI_AM53C974 0x12
>
> +/* PDMA callbacks */
> +#define PDMA_SATN_PDMA_CB 0
> +#define PDMA_S_WITHOUT_SATN_PDMA_CB 1
> +#define PDMA_SATN_STOP_PDMA_CB 2
> +#define PDMA_WRITE_RESPONSE_PDMA_CB 3
> +#define PDMA_DO_DMA_PDMA_CB 4
I'd rather use an enum (and in esp_pdma_cb's switch).
> +#define PDMA_NUM_CBS 5
> +
> void esp_dma_enable(ESPState *s, int irq, int level);
> void esp_request_cancelled(SCSIRequest *req);
> void esp_command_complete(SCSIRequest *req, size_t resid);
Preferrably using unsigned:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
On 28/02/2022 23:10, Philippe Mathieu-Daudé wrote:
> On 28/2/22 23:25, Mark Cave-Ayland wrote:
>> This prepares for the inclusion of the current PDMA callback in the migration
>> stream since the callback is referenced by an integer instead of a function
>> pointer.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>> hw/scsi/esp.c | 47 ++++++++++++++++++++++++++++++-------------
>> include/hw/scsi/esp.h | 11 +++++++++-
>> 2 files changed, 43 insertions(+), 15 deletions(-)
>>
>> diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
>> index 035fb0d1f6..e8b6f25dbb 100644
>> --- a/hw/scsi/esp.c
>> +++ b/hw/scsi/esp.c
>> @@ -195,14 +195,10 @@ static void esp_pdma_write(ESPState *s, uint8_t val)
>> esp_set_tc(s, dmalen);
>> }
>> -static void esp_set_pdma_cb(ESPState *s, void (*cb)(ESPState *))
>> +static void esp_set_pdma_cb(ESPState *s, int pdma_cb)
>
> Why signed?
Ooops.
>> {
>> - s->pdma_cb = cb;
>> -}
>> -
>> -static void esp_pdma_cb(ESPState *s)
>> -{
>> - s->pdma_cb(s);
>> + assert(pdma_cb >= 0 && pdma_cb < PDMA_NUM_CBS);
>
> No need to check >=0 if using unsigned.
Agreed.
>> + s->pdma_cb = pdma_cb;
>> }
>
>> +static void esp_pdma_cb(ESPState *s)
>> +{
>> + switch (s->pdma_cb) {
>> + case PDMA_SATN_PDMA_CB:
>> + satn_pdma_cb(s);
>> + break;
>> + case PDMA_S_WITHOUT_SATN_PDMA_CB:
>> + s_without_satn_pdma_cb(s);
>> + break;
>> + case PDMA_SATN_STOP_PDMA_CB:
>> + satn_stop_pdma_cb(s);
>> + break;
>> + case PDMA_WRITE_RESPONSE_PDMA_CB:
>> + write_response_pdma_cb(s);
>> + break;
>> + case PDMA_DO_DMA_PDMA_CB:
>> + do_dma_pdma_cb(s);
>> + break;
>> + default:
>> + g_assert_not_reached();
>> + }
>> +}
>> +
>
>> index b1ec27612f..885ccf4660 100644
>> --- a/include/hw/scsi/esp.h
>> +++ b/include/hw/scsi/esp.h
>> @@ -51,7 +51,7 @@ struct ESPState {
>> ESPDMAMemoryReadWriteFunc dma_memory_write;
>> void *dma_opaque;
>> void (*dma_cb)(ESPState *s);
>> - void (*pdma_cb)(ESPState *s);
>> + uint8_t pdma_cb;
>
> And here you use unsigned :)
>
>> uint8_t mig_version_id;
>> @@ -150,6 +150,15 @@ struct SysBusESPState {
>> #define TCHI_FAS100A 0x4
>> #define TCHI_AM53C974 0x12
>> +/* PDMA callbacks */
>> +#define PDMA_SATN_PDMA_CB 0
>> +#define PDMA_S_WITHOUT_SATN_PDMA_CB 1
>> +#define PDMA_SATN_STOP_PDMA_CB 2
>> +#define PDMA_WRITE_RESPONSE_PDMA_CB 3
>> +#define PDMA_DO_DMA_PDMA_CB 4
>
> I'd rather use an enum (and in esp_pdma_cb's switch).
>
>> +#define PDMA_NUM_CBS 5
>> +
>> void esp_dma_enable(ESPState *s, int irq, int level);
>> void esp_request_cancelled(SCSIRequest *req);
>> void esp_command_complete(SCSIRequest *req, size_t resid);
>
> Preferrably using unsigned:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
I'll aim to keep the unsigned type in ESPState, and experiment with changing the
#defines to an enum. If it looks good, I'll include the enum version in v2.
ATB,
Mark.
© 2016 - 2026 Red Hat, Inc.