Re: [PULL v2 03/45] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
On Wed, 2023-01-04 at 22:30 +1000, Alistair Francis wrote:
> On Thu, Dec 22, 2022 at 8:40 AM Alistair Francis
> wrote:
> >
> > From: Wilfred Mallawa
> >
> > use the `FIELD32_1CLEAR` macro to implement register
> > `rw1c` functionality to `ibex_spi`.
> >
> > This change was tested by running the `SPI_HOST` from TockOS.
> >
> > Signed-off-by: Wilfred Mallawa
> > Reviewed-by: Alistair Francis
> > Message-Id:
> > <20221017054950.317584-3-wilfred.mall...@opensource.wdc.com>
> > Signed-off-by: Alistair Francis
> > ---
> > hw/ssi/ibex_spi_host.c | 21 +
> > 1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index 57df462e3c..0a456cd1ed 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque,
> > hwaddr addr,
> > {
> > IbexSPIHostState *s = opaque;
> > uint32_t val32 = val64;
> > - uint32_t shift_mask = 0xff, status = 0, data = 0;
> > + uint32_t shift_mask = 0xff, status = 0;
> > uint8_t txqd_len;
> >
> > trace_ibex_spi_host_write(addr, size, val64);
> > @@ -355,12 +355,11 @@ static void ibex_spi_host_write(void *opaque,
> > hwaddr addr,
> > case IBEX_SPI_HOST_INTR_STATE:
> > /* rw1c status register */
> > if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
> > - data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > INTR_STATE, ERROR);
>
> It seems that this change doesn't build on Windows
> (https://cirrus-ci.com/task/697832247296?logs=main#L2163)
>
> Maybe ERROR is reserved? Either way I'll have to drop this commit.
> Maybe just drop this change and keep the rest?
>
> Alistair
Odd! yea I'll do that and send a new patch.
Wilfred
>
> > }
> > if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
> > - data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > INTR_STATE, SPI_EVENT);
> > }
> > - s->regs[addr] = data;
> > break;
> > case IBEX_SPI_HOST_INTR_ENABLE:
> > s->regs[addr] = val32;
> > @@ -505,27 +504,25 @@ 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
> > */
> > - status = s->regs[addr];
> > /* rw1c status register */
> > if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CMDBUSY);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, OVERFLOW);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, UNDERFLOW);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CMDINVAL);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CSIDINVAL);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, ACCESSINVAL);
> > }
> > - s->regs[addr] = status;
> > break;
> > case IBEX_SPI_HOST_EVENT_ENABLE:
> > /* Controls which classes of SPI events raise an interrupt. */
> > --
> > 2.38.1
> >
Re: [PULL v2 03/45] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
On Thu, Dec 22, 2022 at 8:40 AM Alistair Francis
wrote:
>
> From: Wilfred Mallawa
>
> use the `FIELD32_1CLEAR` macro to implement register
> `rw1c` functionality to `ibex_spi`.
>
> This change was tested by running the `SPI_HOST` from TockOS.
>
> Signed-off-by: Wilfred Mallawa
> Reviewed-by: Alistair Francis
> Message-Id: <20221017054950.317584-3-wilfred.mall...@opensource.wdc.com>
> Signed-off-by: Alistair Francis
> ---
> hw/ssi/ibex_spi_host.c | 21 +
> 1 file changed, 9 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> index 57df462e3c..0a456cd1ed 100644
> --- a/hw/ssi/ibex_spi_host.c
> +++ b/hw/ssi/ibex_spi_host.c
> @@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
> {
> IbexSPIHostState *s = opaque;
> uint32_t val32 = val64;
> -uint32_t shift_mask = 0xff, status = 0, data = 0;
> +uint32_t shift_mask = 0xff, status = 0;
> uint8_t txqd_len;
>
> trace_ibex_spi_host_write(addr, size, val64);
> @@ -355,12 +355,11 @@ static void ibex_spi_host_write(void *opaque, hwaddr
> addr,
> case IBEX_SPI_HOST_INTR_STATE:
> /* rw1c status register */
> if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
> -data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE, ERROR);
It seems that this change doesn't build on Windows
(https://cirrus-ci.com/task/697832247296?logs=main#L2163)
Maybe ERROR is reserved? Either way I'll have to drop this commit.
Maybe just drop this change and keep the rest?
Alistair
> }
> if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
> -data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE,
> SPI_EVENT);
> }
> -s->regs[addr] = data;
> break;
> case IBEX_SPI_HOST_INTR_ENABLE:
> s->regs[addr] = val32;
> @@ -505,27 +504,25 @@ 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
> */
> -status = s->regs[addr];
> /* rw1c status register */
> if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
> -status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> CMDBUSY);
> }
> if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
> -status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> OVERFLOW);
> }
> if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
> -status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> UNDERFLOW);
> }
> if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
> -status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> CMDINVAL);
> }
> if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
> -status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> CSIDINVAL);
> }
> if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
> -status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
> +s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
> ACCESSINVAL);
> }
> -s->regs[addr] = status;
> break;
> case IBEX_SPI_HOST_EVENT_ENABLE:
> /* Controls which classes of SPI events raise an interrupt. */
> --
> 2.38.1
>
Re: [PULL v2 03/45] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
Hi Alistair, On 21/12/22 23:39, Alistair Francis wrote: From: Wilfred Mallawa use the `FIELD32_1CLEAR` macro to implement register `rw1c` functionality to `ibex_spi`. This change was tested by running the `SPI_HOST` from TockOS. Signed-off-by: Wilfred Mallawa Reviewed-by: Alistair Francis Message-Id: <20221017054950.317584-3-wilfred.mall...@opensource.wdc.com> Signed-off-by: Alistair Francis --- hw/ssi/ibex_spi_host.c | 21 + 1 file changed, 9 insertions(+), 12 deletions(-) If a v3 is required, please s/implement/Use/ in subject.
[PULL v2 03/45] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
From: Wilfred Mallawa
use the `FIELD32_1CLEAR` macro to implement register
`rw1c` functionality to `ibex_spi`.
This change was tested by running the `SPI_HOST` from TockOS.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Message-Id: <20221017054950.317584-3-wilfred.mall...@opensource.wdc.com>
Signed-off-by: Alistair Francis
---
hw/ssi/ibex_spi_host.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 57df462e3c..0a456cd1ed 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
{
IbexSPIHostState *s = opaque;
uint32_t val32 = val64;
-uint32_t shift_mask = 0xff, status = 0, data = 0;
+uint32_t shift_mask = 0xff, status = 0;
uint8_t txqd_len;
trace_ibex_spi_host_write(addr, size, val64);
@@ -355,12 +355,11 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_INTR_STATE:
/* rw1c status register */
if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
-data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE, ERROR);
}
if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
-data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE,
SPI_EVENT);
}
-s->regs[addr] = data;
break;
case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
@@ -505,27 +504,25 @@ 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
*/
-status = s->regs[addr];
/* rw1c status register */
if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
-status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CMDBUSY);
}
if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
-status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
OVERFLOW);
}
if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
-status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
UNDERFLOW);
}
if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CMDINVAL);
}
if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CSIDINVAL);
}
if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
ACCESSINVAL);
}
-s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
--
2.38.1
