Re: [PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality

[Alistair Francis <alistair23@gmail.com>]

On Tue, Aug 30, 2022 at 2:37 PM Philippe Mathieu-Daudé via
<qemu-devel@nongnu.org> wrote:
>
> On 23/8/22 08:12, Wilfred Mallawa wrote:
> > From: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> >
> > This patch adds the `rw1c` functionality to the respective
> > registers. The status fields are cleared when the respective
> > field is set.
> >
> > Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   hw/ssi/ibex_spi_host.c         | 34 ++++++++++++++++++++++++++++++++--
> >   include/hw/ssi/ibex_spi_host.h |  4 ++--
> >   2 files changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index d52b193a1a..40d401ad47 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -352,7 +352,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
> >
> >       switch (addr) {
> >       /* Skipping any R/O registers */
> > -    case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
> > +    case IBEX_SPI_HOST_INTR_STATE:
> > +        /* rw1c status register */
> > +        if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
> > +            data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
> > +        }
> > +        if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
> > +            data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
> > +        }
> > +        s->regs[addr] = data;
> > +        break;
> > +    case IBEX_SPI_HOST_INTR_ENABLE:
> >           s->regs[addr] = val32;
> >           break;
> >       case IBEX_SPI_HOST_INTR_TEST:
> > @@ -495,7 +505,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
> >        *  When an error occurs, the corresponding bit must be cleared
> >        *  here before issuing any further commands
> >        */
> > -        s->regs[addr] = val32;
> > +        status = s->regs[addr];
> > +        /* rw1c status register */
> > +        if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
> > +        }
> > +        if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
> > +        }
> > +        if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
> > +        }
> > +        if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
> > +        }
> > +        if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
> > +        }
> > +        if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
> > +            status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
> > +        }
>
> Alistair, does this call to add some FIELD_1CLEAR() API?

Yeah, that's probably something useful to add! Good idea

Alistair

>