[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/5] lsi: use enum type for s->waiting
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [Qemu-devel] [PATCH 2/5] lsi: use enum type for s->waiting |
Date: |
Tue, 5 Mar 2019 00:18:01 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 |
On 3/4/19 7:09 PM, Sven Schnelle wrote:
> This makes the code easier to read - no functional change.
>
> Signed-off-by: Sven Schnelle <address@hidden>
> ---
> hw/scsi/lsi53c895a.c | 42 +++++++++++++++++++++++-------------------
> 1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
> index 6d280f8b77..fb9c6db4b2 100644
> --- a/hw/scsi/lsi53c895a.c
> +++ b/hw/scsi/lsi53c895a.c
> @@ -194,6 +194,13 @@ typedef struct lsi_request {
> QTAILQ_ENTRY(lsi_request) next;
> } lsi_request;
>
> +enum {
> + LSI_NOWAIT,
You forgot the comment for NOWAIT.
> + LSI_WAIT_RESELECT, /* Wait Reselect instruction has been issued */
> + LSI_DMA_SCRIPTS, /* processing DMA from lsi_execute_script */
> + LSI_DMA_IN_PROGRESS, /* DMA operation is in progress */
> +};
> +
> typedef struct {
> /*< private >*/
> PCIDevice parent_obj;
> @@ -212,10 +219,6 @@ typedef struct {
> int msg_action;
> int msg_len;
> uint8_t msg[LSI_MAX_MSGIN_LEN];
> - /* 0 if SCRIPTS are running or stopped.
> - * 1 if a Wait Reselect instruction has been issued.
> - * 2 if processing DMA from lsi_execute_script.
> - * 3 if a DMA operation is in progress. */
> int waiting;
When a field is not used by migration, you can declare it as enum:
enum {
LSI_NOWAIT = 0, /* SCRIPTS are running or stopped */
LSI_WAIT_RESELECT = 1, /* Wait Reselect instruction has been
issued */
LSI_DMA_SCRIPTS = 2, /* processing DMA from lsi_execute_script */
LSI_DMA_IN_PROGRESS = 3, /* DMA operation is in progress */
} waiting;
This gives hints to the compiler about values to check.
> SCSIBus bus;
> int current_lun;
> @@ -322,7 +325,7 @@ static void lsi_soft_reset(LSIState *s)
>
> s->msg_action = 0;
> s->msg_len = 0;
> - s->waiting = 0;
> + s->waiting = LSI_NOWAIT;
> s->dsa = 0;
> s->dnad = 0;
> s->dbc = 0;
> @@ -564,10 +567,10 @@ static void lsi_bad_phase(LSIState *s, int out, int
> new_phase)
> static void lsi_resume_script(LSIState *s)
> {
> if (s->waiting != 2) {
> - s->waiting = 0;
> + s->waiting = LSI_NOWAIT;
> lsi_execute_script(s);
> } else {
> - s->waiting = 0;
> + s->waiting = LSI_NOWAIT;
> }
> }
>
> @@ -744,7 +747,7 @@ static int lsi_queue_req(LSIState *s, SCSIRequest *req,
> uint32_t len)
> Since no interrupt stacking is implemented in the emulation, it
> is also required that there are no pending interrupts waiting
> for service from the device driver. */
> - if (s->waiting == 1 ||
> + if (s->waiting == LSI_WAIT_RESELECT ||
> (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON) &&
> !(s->istat0 & (LSI_ISTAT0_SIP | LSI_ISTAT0_DIP)))) {
> /* Reselect device. */
> @@ -789,7 +792,7 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
> len)
> int out;
>
> assert(req->hba_private);
> - if (s->waiting == 1 || req->hba_private != s->current ||
> + if (s->waiting == LSI_WAIT_RESELECT || req->hba_private != s->current ||
> (lsi_irq_on_rsl(s) && !(s->scntl1 & LSI_SCNTL1_CON))) {
> if (lsi_queue_req(s, req, len)) {
> return;
> @@ -803,7 +806,7 @@ static void lsi_transfer_data(SCSIRequest *req, uint32_t
> len)
> s->current->dma_len = len;
> s->command_complete = 1;
> if (s->waiting) {
> - if (s->waiting == 1 || s->dbc == 0) {
> + if (s->waiting == LSI_WAIT_RESELECT || s->dbc == 0) {
> lsi_resume_script(s);
> } else {
> lsi_do_dma(s, out);
> @@ -1093,7 +1096,7 @@ static void lsi_wait_reselect(LSIState *s)
> lsi_reselect(s, p);
> }
> if (s->current == NULL) {
> - s->waiting = 1;
> + s->waiting = LSI_WAIT_RESELECT;
> }
> }
>
> @@ -1202,16 +1205,16 @@ again:
> s->dnad64 = addr_high;
> switch (s->sstat1 & 0x7) {
> case PHASE_DO:
> - s->waiting = 2;
> + s->waiting = LSI_DMA_SCRIPTS;
> lsi_do_dma(s, 1);
> if (s->waiting)
> - s->waiting = 3;
> + s->waiting = LSI_DMA_IN_PROGRESS;
> break;
> case PHASE_DI:
> - s->waiting = 2;
> + s->waiting = LSI_DMA_SCRIPTS;
> lsi_do_dma(s, 0);
> if (s->waiting)
> - s->waiting = 3;
> + s->waiting = LSI_DMA_IN_PROGRESS;
> break;
> case PHASE_CMD:
> lsi_do_command(s);
> @@ -1278,6 +1281,7 @@ again:
> }
> s->sbcl |= LSI_SBCL_BSY;
> lsi_set_phase(s, PHASE_MO);
> + s->waiting = LSI_NOWAIT;
> break;
> case 1: /* Disconnect */
> trace_lsi_execute_script_io_disconnect();
> @@ -1542,7 +1546,7 @@ again:
> }
> }
> }
> - if (insn_processed > 10000 && !s->waiting) {
> + if (insn_processed > 10000 && s->waiting == LSI_NOWAIT) {
> /* Some windows drivers make the device spin waiting for a memory
> location to change. If we have been executed a lot of code then
> assume this is the case and force an unexpected device disconnect.
> @@ -1554,7 +1558,7 @@ again:
> }
> lsi_script_scsi_interrupt(s, LSI_SIST0_UDC, 0);
> lsi_disconnect(s);
> - } else if (s->istat1 & LSI_ISTAT1_SRUN && !s->waiting) {
> + } else if (s->istat1 & LSI_ISTAT1_SRUN && s->waiting == LSI_NOWAIT) {
> if (s->dcntl & LSI_DCNTL_SSM) {
> lsi_script_dma_interrupt(s, LSI_DSTAT_SSI);
> } else {
> @@ -1885,9 +1889,9 @@ static void lsi_reg_writeb(LSIState *s, int offset,
> uint8_t val)
> s->istat0 &= ~LSI_ISTAT0_INTF;
> lsi_update_irq(s);
> }
> - if (s->waiting == 1 && val & LSI_ISTAT0_SIGP) {
> + if (s->waiting == LSI_WAIT_RESELECT && val & LSI_ISTAT0_SIGP) {
> trace_lsi_awoken();
> - s->waiting = 0;
> + s->waiting = LSI_NOWAIT;
> s->dsp = s->dnad;
> lsi_execute_script(s);
> }
>
- [Qemu-devel] [PATCH 1/5] lsi: use ldn_le_p()/stn_le_p(), Sven Schnelle, 2019/03/04
- [Qemu-devel] [PATCH 3/5] lsi: use enum type for s->msg_action, Sven Schnelle, 2019/03/04
- [Qemu-devel] [PATCH 5/5] lsi: return dfifo value, Sven Schnelle, 2019/03/04
- [Qemu-devel] [PATCH 4/5] lsi: use SCSI phase names instead of numbers in trace, Sven Schnelle, 2019/03/04
- Re: [Qemu-devel] [PATCH 1/5] lsi: use ldn_le_p()/stn_le_p(), Eric Blake, 2019/03/04