* [PATCH v2 0/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Marc-André Lureau, Bin Meng, Alistair Francis,
Paolo Bonzini, Philippe Mathieu-Daudé
Hi,
This series QOM'ify the PolarFire UART (the project
would get ride of non-QOM devices).
Since v1:
- Simplify MCHP_PFSOC_MMUART_REG_SIZE
- Use MemoryRegion container
- Mention qdev_set_legacy_instance_id() is not needed (Bin)
- Properly map the 16550 (Bin)
- Add DeviceReset() method (Peter)
- Add migration state (Peter)
Tested with U-Boot v2021.07 following documentation in
docs/system/riscv/microchip-icicle-kit.rst.
Regards,
Phil.
Philippe Mathieu-Daudé (3):
hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
include/hw/char/mchp_pfsoc_mmuart.h | 17 ++--
hw/char/mchp_pfsoc_mmuart.c | 126 ++++++++++++++++++++++------
2 files changed, 114 insertions(+), 29 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Bin Meng, Philippe Mathieu-Daudé,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
size occupied by all the registers. However all registers are
32-bit wide, and the MemoryRegionOps handlers are restricted to
32-bit:
static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
.read = mchp_pfsoc_mmuart_read,
.write = mchp_pfsoc_mmuart_write,
.impl = {
.min_access_size = 4,
.max_access_size = 4,
},
Avoid being triskaidekaphobic, simplify by using the number of
registers.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index f61990215f0..9c012e6c977 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -30,7 +30,7 @@
#include "hw/char/serial.h"
-#define MCHP_PFSOC_MMUART_REG_SIZE 52
+#define MCHP_PFSOC_MMUART_REG_COUNT 13
typedef struct MchpPfSoCMMUartState {
MemoryRegion iomem;
@@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
SerialMM *serial;
- uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
+ uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
} MchpPfSoCMMUartState;
/**
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index 2facf85c2d8..584e7fec17c 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
{
MchpPfSoCMMUartState *s = opaque;
- if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+ addr >>= 2;
+ if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
- __func__, addr);
+ __func__, addr << 2);
return 0;
}
- return s->reg[addr / sizeof(uint32_t)];
+ return s->reg[addr];
}
static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
@@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
MchpPfSoCMMUartState *s = opaque;
uint32_t val32 = (uint32_t)value;
- if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+ addr >>= 2;
+ if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
- " v=0x%x\n", __func__, addr, val32);
+ " v=0x%x\n", __func__, addr << 2, val32);
return;
}
- s->reg[addr / sizeof(uint32_t)] = val32;
+ s->reg[addr] = val32;
}
static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Marc-André Lureau, Bin Meng, Alistair Francis,
Paolo Bonzini, Philippe Mathieu-Daudé
The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
size occupied by all the registers. However all registers are
32-bit wide, and the MemoryRegionOps handlers are restricted to
32-bit:
static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
.read = mchp_pfsoc_mmuart_read,
.write = mchp_pfsoc_mmuart_write,
.impl = {
.min_access_size = 4,
.max_access_size = 4,
},
Avoid being triskaidekaphobic, simplify by using the number of
registers.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index f61990215f0..9c012e6c977 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -30,7 +30,7 @@
#include "hw/char/serial.h"
-#define MCHP_PFSOC_MMUART_REG_SIZE 52
+#define MCHP_PFSOC_MMUART_REG_COUNT 13
typedef struct MchpPfSoCMMUartState {
MemoryRegion iomem;
@@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
SerialMM *serial;
- uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
+ uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
} MchpPfSoCMMUartState;
/**
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index 2facf85c2d8..584e7fec17c 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
{
MchpPfSoCMMUartState *s = opaque;
- if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+ addr >>= 2;
+ if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
- __func__, addr);
+ __func__, addr << 2);
return 0;
}
- return s->reg[addr / sizeof(uint32_t)];
+ return s->reg[addr];
}
static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
@@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
MchpPfSoCMMUartState *s = opaque;
uint32_t val32 = (uint32_t)value;
- if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+ addr >>= 2;
+ if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
- " v=0x%x\n", __func__, addr, val32);
+ " v=0x%x\n", __func__, addr << 2, val32);
return;
}
- s->reg[addr / sizeof(uint32_t)] = val32;
+ s->reg[addr] = val32;
}
static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-26 8:31 ` Bin Meng
-1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Paolo Bonzini, Marc-André Lureau, Alistair Francis
On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
typo? See https://www.dictionary.com/browse/triskaidekaphobia
Learned a new word today but I have to say this word is too hard for a
non-native speaker :)
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-26 8:31 ` Bin Meng
0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
typo? See https://www.dictionary.com/browse/triskaidekaphobia
Learned a new word today but I have to say this word is too hard for a
non-native speaker :)
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-26 8:31 ` Bin Meng
@ 2021-09-26 8:38 ` Bin Meng
-1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Paolo Bonzini, Marc-André Lureau, Alistair Francis
On Sun, Sep 26, 2021 at 4:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> > size occupied by all the registers. However all registers are
> > 32-bit wide, and the MemoryRegionOps handlers are restricted to
> > 32-bit:
> >
> > static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> > .read = mchp_pfsoc_mmuart_read,
> > .write = mchp_pfsoc_mmuart_write,
> > .impl = {
> > .min_access_size = 4,
> > .max_access_size = 4,
> > },
> >
> > Avoid being triskaidekaphobic, simplify by using the number of
>
> typo? See https://www.dictionary.com/browse/triskaidekaphobia
>
> Learned a new word today but I have to say this word is too hard for a
> non-native speaker :)
>
Never mind, triskaidekaphobia is a noun, and triskaidekaphobic is the
adjective which is grammarly correct :)
> > registers.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> > hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> > 2 files changed, 10 insertions(+), 8 deletions(-)
> >
>
> Reviewed-by: Bin Meng <bin.meng@windriver.com>
> Tested-by: Bin Meng <bin.meng@windriver.com>
Regards,
Bin
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-26 8:38 ` Bin Meng
0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
On Sun, Sep 26, 2021 at 4:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> > size occupied by all the registers. However all registers are
> > 32-bit wide, and the MemoryRegionOps handlers are restricted to
> > 32-bit:
> >
> > static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> > .read = mchp_pfsoc_mmuart_read,
> > .write = mchp_pfsoc_mmuart_write,
> > .impl = {
> > .min_access_size = 4,
> > .max_access_size = 4,
> > },
> >
> > Avoid being triskaidekaphobic, simplify by using the number of
>
> typo? See https://www.dictionary.com/browse/triskaidekaphobia
>
> Learned a new word today but I have to say this word is too hard for a
> non-native speaker :)
>
Never mind, triskaidekaphobia is a noun, and triskaidekaphobic is the
adjective which is grammarly correct :)
> > registers.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> > hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> > 2 files changed, 10 insertions(+), 8 deletions(-)
> >
>
> Reviewed-by: Bin Meng <bin.meng@windriver.com>
> Tested-by: Bin Meng <bin.meng@windriver.com>
Regards,
Bin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-26 8:31 ` Bin Meng
@ 2021-09-28 22:19 ` Alistair Francis
-1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:19 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Bin Meng, Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, Marc-André Lureau,
Paolo Bonzini
On Sun, Sep 26, 2021 at 6:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> > size occupied by all the registers. However all registers are
> > 32-bit wide, and the MemoryRegionOps handlers are restricted to
> > 32-bit:
> >
> > static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> > .read = mchp_pfsoc_mmuart_read,
> > .write = mchp_pfsoc_mmuart_write,
> > .impl = {
> > .min_access_size = 4,
> > .max_access_size = 4,
> > },
> >
> > Avoid being triskaidekaphobic, simplify by using the number of
>
> typo? See https://www.dictionary.com/browse/triskaidekaphobia
>
> Learned a new word today but I have to say this word is too hard for a
> non-native speaker :)
Ha! Even as a native English speaker I had to look it up :)
Alistair
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-28 22:19 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:19 UTC (permalink / raw)
To: Bin Meng
Cc: Philippe Mathieu-Daudé,
qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
Paolo Bonzini, Marc-André Lureau
On Sun, Sep 26, 2021 at 6:31 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Sat, Sep 25, 2021 at 9:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> > size occupied by all the registers. However all registers are
> > 32-bit wide, and the MemoryRegionOps handlers are restricted to
> > 32-bit:
> >
> > static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> > .read = mchp_pfsoc_mmuart_read,
> > .write = mchp_pfsoc_mmuart_write,
> > .impl = {
> > .min_access_size = 4,
> > .max_access_size = 4,
> > },
> >
> > Avoid being triskaidekaphobic, simplify by using the number of
>
> typo? See https://www.dictionary.com/browse/triskaidekaphobia
>
> Learned a new word today but I have to say this word is too hard for a
> non-native speaker :)
Ha! Even as a native English speaker I had to look it up :)
Alistair
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-28 22:16 ` Alistair Francis
-1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Marc-André Lureau, Bin Meng, open list:RISC-V,
qemu-devel@nongnu.org Developers, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index f61990215f0..9c012e6c977 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -30,7 +30,7 @@
>
> #include "hw/char/serial.h"
>
> -#define MCHP_PFSOC_MMUART_REG_SIZE 52
> +#define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> MemoryRegion iomem;
> @@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
>
> SerialMM *serial;
>
> - uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
> + uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
>
> /**
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 2facf85c2d8..584e7fec17c 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
> - __func__, addr);
> + __func__, addr << 2);
> return 0;
> }
>
> - return s->reg[addr / sizeof(uint32_t)];
> + return s->reg[addr];
> }
>
> static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> @@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> MchpPfSoCMMUartState *s = opaque;
> uint32_t val32 = (uint32_t)value;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
> - " v=0x%x\n", __func__, addr, val32);
> + " v=0x%x\n", __func__, addr << 2, val32);
> return;
> }
>
> - s->reg[addr / sizeof(uint32_t)] = val32;
> + s->reg[addr] = val32;
> }
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-28 22:16 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Marc-André Lureau, Bin Meng, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index f61990215f0..9c012e6c977 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -30,7 +30,7 @@
>
> #include "hw/char/serial.h"
>
> -#define MCHP_PFSOC_MMUART_REG_SIZE 52
> +#define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> MemoryRegion iomem;
> @@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
>
> SerialMM *serial;
>
> - uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
> + uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
>
> /**
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 2facf85c2d8..584e7fec17c 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
> - __func__, addr);
> + __func__, addr << 2);
> return 0;
> }
>
> - return s->reg[addr / sizeof(uint32_t)];
> + return s->reg[addr];
> }
>
> static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> @@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> MchpPfSoCMMUartState *s = opaque;
> uint32_t val32 = (uint32_t)value;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
> - " v=0x%x\n", __func__, addr, val32);
> + " v=0x%x\n", __func__, addr << 2, val32);
> return;
> }
>
> - s->reg[addr / sizeof(uint32_t)] = val32;
> + s->reg[addr] = val32;
> }
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-28 22:42 ` Alistair Francis
-1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Marc-André Lureau, Bin Meng, open list:RISC-V,
qemu-devel@nongnu.org Developers, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index f61990215f0..9c012e6c977 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -30,7 +30,7 @@
>
> #include "hw/char/serial.h"
>
> -#define MCHP_PFSOC_MMUART_REG_SIZE 52
> +#define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> MemoryRegion iomem;
> @@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
>
> SerialMM *serial;
>
> - uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
> + uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
>
> /**
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 2facf85c2d8..584e7fec17c 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
> - __func__, addr);
> + __func__, addr << 2);
> return 0;
> }
>
> - return s->reg[addr / sizeof(uint32_t)];
> + return s->reg[addr];
> }
>
> static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> @@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> MchpPfSoCMMUartState *s = opaque;
> uint32_t val32 = (uint32_t)value;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
> - " v=0x%x\n", __func__, addr, val32);
> + " v=0x%x\n", __func__, addr << 2, val32);
> return;
> }
>
> - s->reg[addr / sizeof(uint32_t)] = val32;
> + s->reg[addr] = val32;
> }
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 1/3] hw/char/mchp_pfsoc_mmuart: Simplify MCHP_PFSOC_MMUART_REG definition
@ 2021-09-28 22:42 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Marc-André Lureau, Bin Meng, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The current MCHP_PFSOC_MMUART_REG_SIZE definition represent the
> size occupied by all the registers. However all registers are
> 32-bit wide, and the MemoryRegionOps handlers are restricted to
> 32-bit:
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> .read = mchp_pfsoc_mmuart_read,
> .write = mchp_pfsoc_mmuart_write,
> .impl = {
> .min_access_size = 4,
> .max_access_size = 4,
> },
>
> Avoid being triskaidekaphobic, simplify by using the number of
> registers.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 4 ++--
> hw/char/mchp_pfsoc_mmuart.c | 14 ++++++++------
> 2 files changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index f61990215f0..9c012e6c977 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -30,7 +30,7 @@
>
> #include "hw/char/serial.h"
>
> -#define MCHP_PFSOC_MMUART_REG_SIZE 52
> +#define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> MemoryRegion iomem;
> @@ -39,7 +39,7 @@ typedef struct MchpPfSoCMMUartState {
>
> SerialMM *serial;
>
> - uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
> + uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
>
> /**
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 2facf85c2d8..584e7fec17c 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -29,13 +29,14 @@ static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
> - __func__, addr);
> + __func__, addr << 2);
> return 0;
> }
>
> - return s->reg[addr / sizeof(uint32_t)];
> + return s->reg[addr];
> }
>
> static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> @@ -44,13 +45,14 @@ static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
> MchpPfSoCMMUartState *s = opaque;
> uint32_t val32 = (uint32_t)value;
>
> - if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
> + addr >>= 2;
> + if (addr >= MCHP_PFSOC_MMUART_REG_COUNT) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: bad write: addr=0x%" HWADDR_PRIx
> - " v=0x%x\n", __func__, addr, val32);
> + " v=0x%x\n", __func__, addr << 2, val32);
> return;
> }
>
> - s->reg[addr / sizeof(uint32_t)] = val32;
> + s->reg[addr] = val32;
> }
>
> static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Bin Meng, Philippe Mathieu-Daudé,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
Our device have 2 different I/O regions:
- a 16550 UART mapped for 32-bit accesses
- 13 extra registers
Instead of mapping each region on the main bus, introduce
a container, map the 2 devices regions on the container,
and map the container on the main bus.
Before:
(qemu) info mtree
...
0000000020100000-000000002010001f (prio 0, i/o): serial
0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020102000-000000002010201f (prio 0, i/o): serial
0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020104000-000000002010401f (prio 0, i/o): serial
0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020106000-000000002010601f (prio 0, i/o): serial
0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
After:
(qemu) info mtree
...
0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020100000-000000002010001f (prio 0, i/o): serial
0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020102000-000000002010201f (prio 0, i/o): serial
0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020104000-000000002010401f (prio 0, i/o): serial
0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020106000-000000002010601f (prio 0, i/o): serial
0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/char/mchp_pfsoc_mmuart.h | 1 +
hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index 9c012e6c977..864ac1a36b5 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -33,6 +33,7 @@
#define MCHP_PFSOC_MMUART_REG_COUNT 13
typedef struct MchpPfSoCMMUartState {
+ MemoryRegion container;
MemoryRegion iomem;
hwaddr base;
qemu_irq irq;
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index 584e7fec17c..ea586559761 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -25,6 +25,8 @@
#include "chardev/char.h"
#include "hw/char/mchp_pfsoc_mmuart.h"
+#define REGS_OFFSET 0x20
+
static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
{
MchpPfSoCMMUartState *s = opaque;
@@ -72,16 +74,19 @@ MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
s = g_new0(MchpPfSoCMMUartState, 1);
+ memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
+
memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
- "mchp.pfsoc.mmuart", 0x1000);
+ "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
+ memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
s->base = base;
s->irq = irq;
- s->serial = serial_mm_init(sysmem, base, 2, irq, 399193, chr,
+ s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
DEVICE_LITTLE_ENDIAN);
- memory_region_add_subregion(sysmem, base + 0x20, &s->iomem);
+ memory_region_add_subregion(sysmem, base, &s->container);
return s;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Marc-André Lureau, Bin Meng, Alistair Francis,
Paolo Bonzini, Philippe Mathieu-Daudé
Our device have 2 different I/O regions:
- a 16550 UART mapped for 32-bit accesses
- 13 extra registers
Instead of mapping each region on the main bus, introduce
a container, map the 2 devices regions on the container,
and map the container on the main bus.
Before:
(qemu) info mtree
...
0000000020100000-000000002010001f (prio 0, i/o): serial
0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020102000-000000002010201f (prio 0, i/o): serial
0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020104000-000000002010401f (prio 0, i/o): serial
0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
0000000020106000-000000002010601f (prio 0, i/o): serial
0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
After:
(qemu) info mtree
...
0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020100000-000000002010001f (prio 0, i/o): serial
0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020102000-000000002010201f (prio 0, i/o): serial
0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020104000-000000002010401f (prio 0, i/o): serial
0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
0000000020106000-000000002010601f (prio 0, i/o): serial
0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/char/mchp_pfsoc_mmuart.h | 1 +
hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
2 files changed, 9 insertions(+), 3 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index 9c012e6c977..864ac1a36b5 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -33,6 +33,7 @@
#define MCHP_PFSOC_MMUART_REG_COUNT 13
typedef struct MchpPfSoCMMUartState {
+ MemoryRegion container;
MemoryRegion iomem;
hwaddr base;
qemu_irq irq;
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index 584e7fec17c..ea586559761 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -25,6 +25,8 @@
#include "chardev/char.h"
#include "hw/char/mchp_pfsoc_mmuart.h"
+#define REGS_OFFSET 0x20
+
static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
{
MchpPfSoCMMUartState *s = opaque;
@@ -72,16 +74,19 @@ MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
s = g_new0(MchpPfSoCMMUartState, 1);
+ memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
+
memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
- "mchp.pfsoc.mmuart", 0x1000);
+ "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
+ memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
s->base = base;
s->irq = irq;
- s->serial = serial_mm_init(sysmem, base, 2, irq, 399193, chr,
+ s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
DEVICE_LITTLE_ENDIAN);
- memory_region_add_subregion(sysmem, base + 0x20, &s->iomem);
+ memory_region_add_subregion(sysmem, base, &s->container);
return s;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-26 8:31 ` Bin Meng
-1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: open list:RISC-V, Bin Meng, qemu-devel@nongnu.org Developers,
Paolo Bonzini, Marc-André Lureau, Alistair Francis
On Sat, Sep 25, 2021 at 9:37 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Our device have 2 different I/O regions:
> - a 16550 UART mapped for 32-bit accesses
> - 13 extra registers
>
> Instead of mapping each region on the main bus, introduce
> a container, map the 2 devices regions on the container,
> and map the container on the main bus.
>
> Before:
>
> (qemu) info mtree
> ...
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
>
> After:
>
> (qemu) info mtree
> ...
> 0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 1 +
> hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
@ 2021-09-26 8:31 ` Bin Meng
0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V, Bin Meng,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
On Sat, Sep 25, 2021 at 9:37 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Our device have 2 different I/O regions:
> - a 16550 UART mapped for 32-bit accesses
> - 13 extra registers
>
> Instead of mapping each region on the main bus, introduce
> a container, map the 2 devices regions on the container,
> and map the container on the main bus.
>
> Before:
>
> (qemu) info mtree
> ...
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
>
> After:
>
> (qemu) info mtree
> ...
> 0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 1 +
> hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-28 22:12 ` Alistair Francis
-1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Marc-André Lureau, Bin Meng, open list:RISC-V,
qemu-devel@nongnu.org Developers, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Our device have 2 different I/O regions:
> - a 16550 UART mapped for 32-bit accesses
> - 13 extra registers
>
> Instead of mapping each region on the main bus, introduce
> a container, map the 2 devices regions on the container,
> and map the container on the main bus.
>
> Before:
>
> (qemu) info mtree
> ...
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
>
> After:
>
> (qemu) info mtree
> ...
> 0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 1 +
> hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index 9c012e6c977..864ac1a36b5 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -33,6 +33,7 @@
> #define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> + MemoryRegion container;
> MemoryRegion iomem;
> hwaddr base;
> qemu_irq irq;
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 584e7fec17c..ea586559761 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -25,6 +25,8 @@
> #include "chardev/char.h"
> #include "hw/char/mchp_pfsoc_mmuart.h"
>
> +#define REGS_OFFSET 0x20
> +
> static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
> @@ -72,16 +74,19 @@ MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
>
> s = g_new0(MchpPfSoCMMUartState, 1);
>
> + memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
> +
> memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
> - "mchp.pfsoc.mmuart", 0x1000);
> + "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
> + memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
>
> s->base = base;
> s->irq = irq;
>
> - s->serial = serial_mm_init(sysmem, base, 2, irq, 399193, chr,
> + s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
> DEVICE_LITTLE_ENDIAN);
>
> - memory_region_add_subregion(sysmem, base + 0x20, &s->iomem);
> + memory_region_add_subregion(sysmem, base, &s->container);
>
> return s;
> }
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 2/3] hw/char/mchp_pfsoc_mmuart: Use a MemoryRegion container
@ 2021-09-28 22:12 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:12 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Marc-André Lureau, Bin Meng, Paolo Bonzini
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Our device have 2 different I/O regions:
> - a 16550 UART mapped for 32-bit accesses
> - 13 extra registers
>
> Instead of mapping each region on the main bus, introduce
> a container, map the 2 devices regions on the container,
> and map the container on the main bus.
>
> Before:
>
> (qemu) info mtree
> ...
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-000000002010101f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-000000002010301f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-000000002010501f (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-000000002010701f (prio 0, i/o): mchp.pfsoc.mmuart
>
> After:
>
> (qemu) info mtree
> ...
> 0000000020100000-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020100000-000000002010001f (prio 0, i/o): serial
> 0000000020100020-0000000020100fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020102000-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020102000-000000002010201f (prio 0, i/o): serial
> 0000000020102020-0000000020102fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020104000-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020104000-000000002010401f (prio 0, i/o): serial
> 0000000020104020-0000000020104fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
> 0000000020106000-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart
> 0000000020106000-000000002010601f (prio 0, i/o): serial
> 0000000020106020-0000000020106fff (prio 0, i/o): mchp.pfsoc.mmuart.regs
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 1 +
> hw/char/mchp_pfsoc_mmuart.c | 11 ++++++++---
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index 9c012e6c977..864ac1a36b5 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -33,6 +33,7 @@
> #define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> typedef struct MchpPfSoCMMUartState {
> + MemoryRegion container;
> MemoryRegion iomem;
> hwaddr base;
> qemu_irq irq;
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index 584e7fec17c..ea586559761 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -25,6 +25,8 @@
> #include "chardev/char.h"
> #include "hw/char/mchp_pfsoc_mmuart.h"
>
> +#define REGS_OFFSET 0x20
> +
> static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned size)
> {
> MchpPfSoCMMUartState *s = opaque;
> @@ -72,16 +74,19 @@ MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
>
> s = g_new0(MchpPfSoCMMUartState, 1);
>
> + memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
> +
> memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
> - "mchp.pfsoc.mmuart", 0x1000);
> + "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
> + memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
>
> s->base = base;
> s->irq = irq;
>
> - s->serial = serial_mm_init(sysmem, base, 2, irq, 399193, chr,
> + s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
> DEVICE_LITTLE_ENDIAN);
>
> - memory_region_add_subregion(sysmem, base + 0x20, &s->iomem);
> + memory_region_add_subregion(sysmem, base, &s->container);
>
> return s;
> }
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
-1 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, qemu-riscv, Bin Meng, Philippe Mathieu-Daudé,
Alistair Francis, Paolo Bonzini, Marc-André Lureau
- Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
- Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
- Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
- Add DeviceReset() method
- Add vmstate structure for migration
- Register device in 'input' category
- Keep mchp_pfsoc_mmuart_create() behavior
Note, serial_mm_init() calls qdev_set_legacy_instance_id().
This call is only needed for backwards-compatibility of incoming
migration data with old versions of QEMU which implemented migration
of devices with hand-rolled code. Since this device didn't previously
handle migration at all, then it doesn't need to set the legacy
instance ID.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Peter Maydell <peter.maydell@linaro.org>
I haven't kept Alistair R-b tag from v1.
---
include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
2 files changed, 97 insertions(+), 20 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index 864ac1a36b5..b0e14ca3554 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -28,17 +28,23 @@
#ifndef HW_MCHP_PFSOC_MMUART_H
#define HW_MCHP_PFSOC_MMUART_H
+#include "hw/sysbus.h"
#include "hw/char/serial.h"
#define MCHP_PFSOC_MMUART_REG_COUNT 13
+#define TYPE_MCHP_PFSOC_UART "mchp.pfsoc.uart"
+OBJECT_DECLARE_SIMPLE_TYPE(MchpPfSoCMMUartState, MCHP_PFSOC_UART)
+
typedef struct MchpPfSoCMMUartState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+
+ /*< public >*/
MemoryRegion container;
MemoryRegion iomem;
- hwaddr base;
- qemu_irq irq;
- SerialMM *serial;
+ SerialMM serial_mm;
uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
} MchpPfSoCMMUartState;
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index ea586559761..22f3e78eb9e 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -22,8 +22,10 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
-#include "chardev/char.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
#include "hw/char/mchp_pfsoc_mmuart.h"
+#include "hw/qdev-properties.h"
#define REGS_OFFSET 0x20
@@ -67,26 +69,95 @@ static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
},
};
-MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
- hwaddr base, qemu_irq irq, Chardev *chr)
+static void mchp_pfsoc_mmuart_reset(DeviceState *dev)
{
- MchpPfSoCMMUartState *s;
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
- s = g_new0(MchpPfSoCMMUartState, 1);
+ memset(s->reg, 0, sizeof(s->reg));
+ device_cold_reset(DEVICE(&s->serial_mm));
+}
- memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
+static void mchp_pfsoc_mmuart_init(Object *obj)
+{
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(obj);
- memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
+ object_initialize_child(obj, "serial-mm", &s->serial_mm, TYPE_SERIAL_MM);
+ object_property_add_alias(obj, "chardev", OBJECT(&s->serial_mm), "chardev");
+}
+
+static void mchp_pfsoc_mmuart_realize(DeviceState *dev, Error **errp)
+{
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
+
+ qdev_prop_set_uint8(DEVICE(&s->serial_mm), "regshift", 2);
+ qdev_prop_set_uint32(DEVICE(&s->serial_mm), "baudbase", 399193);
+ qdev_prop_set_uint8(DEVICE(&s->serial_mm), "endianness",
+ DEVICE_LITTLE_ENDIAN);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->serial_mm), errp)) {
+ return;
+ }
+
+ sysbus_pass_irq(SYS_BUS_DEVICE(dev), SYS_BUS_DEVICE(&s->serial_mm));
+
+ memory_region_init(&s->container, OBJECT(s), "mchp.pfsoc.mmuart", 0x1000);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
+
+ memory_region_add_subregion(&s->container, 0,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->serial_mm), 0));
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &mchp_pfsoc_mmuart_ops, s,
"mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
-
- s->base = base;
- s->irq = irq;
-
- s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
- DEVICE_LITTLE_ENDIAN);
-
- memory_region_add_subregion(sysmem, base, &s->container);
-
- return s;
+}
+
+static const VMStateDescription mchp_pfsoc_mmuart_vmstate = {
+ .name = "mchp.pfsoc.uart",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(reg, MchpPfSoCMMUartState,
+ MCHP_PFSOC_MMUART_REG_COUNT),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void mchp_pfsoc_mmuart_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = mchp_pfsoc_mmuart_realize;
+ dc->reset = mchp_pfsoc_mmuart_reset;
+ dc->vmsd = &mchp_pfsoc_mmuart_vmstate;
+ set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo mchp_pfsoc_mmuart_info = {
+ .name = TYPE_MCHP_PFSOC_UART,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(MchpPfSoCMMUartState),
+ .instance_init = mchp_pfsoc_mmuart_init,
+ .class_init = mchp_pfsoc_mmuart_class_init,
+};
+
+static void mchp_pfsoc_mmuart_register_types(void)
+{
+ type_register_static(&mchp_pfsoc_mmuart_info);
+}
+
+type_init(mchp_pfsoc_mmuart_register_types)
+
+MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
+ hwaddr base,
+ qemu_irq irq, Chardev *chr)
+{
+ DeviceState *dev = qdev_new(TYPE_MCHP_PFSOC_UART);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+ qdev_prop_set_chr(dev, "chardev", chr);
+ sysbus_realize(sbd, &error_fatal);
+
+ memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(sbd, 0));
+ sysbus_connect_irq(sbd, 0, irq);
+
+ return MCHP_PFSOC_UART(dev);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
@ 2021-09-25 13:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 26+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-09-25 13:34 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-riscv, Marc-André Lureau, Bin Meng, Alistair Francis,
Paolo Bonzini, Philippe Mathieu-Daudé,
Peter Maydell
- Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
- Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
- Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
- Add DeviceReset() method
- Add vmstate structure for migration
- Register device in 'input' category
- Keep mchp_pfsoc_mmuart_create() behavior
Note, serial_mm_init() calls qdev_set_legacy_instance_id().
This call is only needed for backwards-compatibility of incoming
migration data with old versions of QEMU which implemented migration
of devices with hand-rolled code. Since this device didn't previously
handle migration at all, then it doesn't need to set the legacy
instance ID.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: Peter Maydell <peter.maydell@linaro.org>
I haven't kept Alistair R-b tag from v1.
---
include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
2 files changed, 97 insertions(+), 20 deletions(-)
diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
index 864ac1a36b5..b0e14ca3554 100644
--- a/include/hw/char/mchp_pfsoc_mmuart.h
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -28,17 +28,23 @@
#ifndef HW_MCHP_PFSOC_MMUART_H
#define HW_MCHP_PFSOC_MMUART_H
+#include "hw/sysbus.h"
#include "hw/char/serial.h"
#define MCHP_PFSOC_MMUART_REG_COUNT 13
+#define TYPE_MCHP_PFSOC_UART "mchp.pfsoc.uart"
+OBJECT_DECLARE_SIMPLE_TYPE(MchpPfSoCMMUartState, MCHP_PFSOC_UART)
+
typedef struct MchpPfSoCMMUartState {
+ /*< private >*/
+ SysBusDevice parent_obj;
+
+ /*< public >*/
MemoryRegion container;
MemoryRegion iomem;
- hwaddr base;
- qemu_irq irq;
- SerialMM *serial;
+ SerialMM serial_mm;
uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
} MchpPfSoCMMUartState;
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
index ea586559761..22f3e78eb9e 100644
--- a/hw/char/mchp_pfsoc_mmuart.c
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -22,8 +22,10 @@
#include "qemu/osdep.h"
#include "qemu/log.h"
-#include "chardev/char.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
#include "hw/char/mchp_pfsoc_mmuart.h"
+#include "hw/qdev-properties.h"
#define REGS_OFFSET 0x20
@@ -67,26 +69,95 @@ static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
},
};
-MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
- hwaddr base, qemu_irq irq, Chardev *chr)
+static void mchp_pfsoc_mmuart_reset(DeviceState *dev)
{
- MchpPfSoCMMUartState *s;
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
- s = g_new0(MchpPfSoCMMUartState, 1);
+ memset(s->reg, 0, sizeof(s->reg));
+ device_cold_reset(DEVICE(&s->serial_mm));
+}
- memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
+static void mchp_pfsoc_mmuart_init(Object *obj)
+{
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(obj);
- memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
+ object_initialize_child(obj, "serial-mm", &s->serial_mm, TYPE_SERIAL_MM);
+ object_property_add_alias(obj, "chardev", OBJECT(&s->serial_mm), "chardev");
+}
+
+static void mchp_pfsoc_mmuart_realize(DeviceState *dev, Error **errp)
+{
+ MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
+
+ qdev_prop_set_uint8(DEVICE(&s->serial_mm), "regshift", 2);
+ qdev_prop_set_uint32(DEVICE(&s->serial_mm), "baudbase", 399193);
+ qdev_prop_set_uint8(DEVICE(&s->serial_mm), "endianness",
+ DEVICE_LITTLE_ENDIAN);
+ if (!sysbus_realize(SYS_BUS_DEVICE(&s->serial_mm), errp)) {
+ return;
+ }
+
+ sysbus_pass_irq(SYS_BUS_DEVICE(dev), SYS_BUS_DEVICE(&s->serial_mm));
+
+ memory_region_init(&s->container, OBJECT(s), "mchp.pfsoc.mmuart", 0x1000);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
+
+ memory_region_add_subregion(&s->container, 0,
+ sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->serial_mm), 0));
+
+ memory_region_init_io(&s->iomem, OBJECT(s), &mchp_pfsoc_mmuart_ops, s,
"mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
-
- s->base = base;
- s->irq = irq;
-
- s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
- DEVICE_LITTLE_ENDIAN);
-
- memory_region_add_subregion(sysmem, base, &s->container);
-
- return s;
+}
+
+static const VMStateDescription mchp_pfsoc_mmuart_vmstate = {
+ .name = "mchp.pfsoc.uart",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32_ARRAY(reg, MchpPfSoCMMUartState,
+ MCHP_PFSOC_MMUART_REG_COUNT),
+ VMSTATE_END_OF_LIST()
+ }
+};
+
+static void mchp_pfsoc_mmuart_class_init(ObjectClass *oc, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(oc);
+
+ dc->realize = mchp_pfsoc_mmuart_realize;
+ dc->reset = mchp_pfsoc_mmuart_reset;
+ dc->vmsd = &mchp_pfsoc_mmuart_vmstate;
+ set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
+}
+
+static const TypeInfo mchp_pfsoc_mmuart_info = {
+ .name = TYPE_MCHP_PFSOC_UART,
+ .parent = TYPE_SYS_BUS_DEVICE,
+ .instance_size = sizeof(MchpPfSoCMMUartState),
+ .instance_init = mchp_pfsoc_mmuart_init,
+ .class_init = mchp_pfsoc_mmuart_class_init,
+};
+
+static void mchp_pfsoc_mmuart_register_types(void)
+{
+ type_register_static(&mchp_pfsoc_mmuart_info);
+}
+
+type_init(mchp_pfsoc_mmuart_register_types)
+
+MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
+ hwaddr base,
+ qemu_irq irq, Chardev *chr)
+{
+ DeviceState *dev = qdev_new(TYPE_MCHP_PFSOC_UART);
+ SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
+
+ qdev_prop_set_chr(dev, "chardev", chr);
+ sysbus_realize(sbd, &error_fatal);
+
+ memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(sbd, 0));
+ sysbus_connect_irq(sbd, 0, irq);
+
+ return MCHP_PFSOC_UART(dev);
}
--
2.31.1
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-26 8:31 ` Bin Meng
-1 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, open list:RISC-V, Bin Meng,
qemu-devel@nongnu.org Developers, Marc-André Lureau,
Alistair Francis, Paolo Bonzini
On Sat, Sep 25, 2021 at 9:37 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> - Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
> - Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
> - Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
> - Add DeviceReset() method
> - Add vmstate structure for migration
> - Register device in 'input' category
> - Keep mchp_pfsoc_mmuart_create() behavior
>
> Note, serial_mm_init() calls qdev_set_legacy_instance_id().
> This call is only needed for backwards-compatibility of incoming
> migration data with old versions of QEMU which implemented migration
> of devices with hand-rolled code. Since this device didn't previously
> handle migration at all, then it doesn't need to set the legacy
> instance ID.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Peter Maydell <peter.maydell@linaro.org>
>
> I haven't kept Alistair R-b tag from v1.
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
> hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
@ 2021-09-26 8:31 ` Bin Meng
0 siblings, 0 replies; 26+ messages in thread
From: Bin Meng @ 2021-09-26 8:31 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, Peter Maydell,
open list:RISC-V, Bin Meng, Alistair Francis, Paolo Bonzini,
Marc-André Lureau
On Sat, Sep 25, 2021 at 9:37 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> - Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
> - Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
> - Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
> - Add DeviceReset() method
> - Add vmstate structure for migration
> - Register device in 'input' category
> - Keep mchp_pfsoc_mmuart_create() behavior
>
> Note, serial_mm_init() calls qdev_set_legacy_instance_id().
> This call is only needed for backwards-compatibility of incoming
> migration data with old versions of QEMU which implemented migration
> of devices with hand-rolled code. Since this device didn't previously
> handle migration at all, then it doesn't need to set the legacy
> instance ID.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: Peter Maydell <peter.maydell@linaro.org>
>
> I haven't kept Alistair R-b tag from v1.
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
> hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
Tested-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
2021-09-25 13:34 ` Philippe Mathieu-Daudé
@ 2021-09-28 22:15 ` Alistair Francis
-1 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Peter Maydell, open list:RISC-V, Bin Meng,
qemu-devel@nongnu.org Developers, Paolo Bonzini,
Marc-André Lureau
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> - Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
> - Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
> - Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
> - Add DeviceReset() method
> - Add vmstate structure for migration
> - Register device in 'input' category
> - Keep mchp_pfsoc_mmuart_create() behavior
>
> Note, serial_mm_init() calls qdev_set_legacy_instance_id().
> This call is only needed for backwards-compatibility of incoming
> migration data with old versions of QEMU which implemented migration
> of devices with hand-rolled code. Since this device didn't previously
> handle migration at all, then it doesn't need to set the legacy
> instance ID.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Cc: Peter Maydell <peter.maydell@linaro.org>
>
> I haven't kept Alistair R-b tag from v1.
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
> hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index 864ac1a36b5..b0e14ca3554 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -28,17 +28,23 @@
> #ifndef HW_MCHP_PFSOC_MMUART_H
> #define HW_MCHP_PFSOC_MMUART_H
>
> +#include "hw/sysbus.h"
> #include "hw/char/serial.h"
>
> #define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> +#define TYPE_MCHP_PFSOC_UART "mchp.pfsoc.uart"
> +OBJECT_DECLARE_SIMPLE_TYPE(MchpPfSoCMMUartState, MCHP_PFSOC_UART)
> +
> typedef struct MchpPfSoCMMUartState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> MemoryRegion container;
> MemoryRegion iomem;
> - hwaddr base;
> - qemu_irq irq;
>
> - SerialMM *serial;
> + SerialMM serial_mm;
>
> uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index ea586559761..22f3e78eb9e 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -22,8 +22,10 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> -#include "chardev/char.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> #include "hw/char/mchp_pfsoc_mmuart.h"
> +#include "hw/qdev-properties.h"
>
> #define REGS_OFFSET 0x20
>
> @@ -67,26 +69,95 @@ static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> },
> };
>
> -MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
> - hwaddr base, qemu_irq irq, Chardev *chr)
> +static void mchp_pfsoc_mmuart_reset(DeviceState *dev)
> {
> - MchpPfSoCMMUartState *s;
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
>
> - s = g_new0(MchpPfSoCMMUartState, 1);
> + memset(s->reg, 0, sizeof(s->reg));
> + device_cold_reset(DEVICE(&s->serial_mm));
> +}
>
> - memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
> +static void mchp_pfsoc_mmuart_init(Object *obj)
> +{
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(obj);
>
> - memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
> + object_initialize_child(obj, "serial-mm", &s->serial_mm, TYPE_SERIAL_MM);
> + object_property_add_alias(obj, "chardev", OBJECT(&s->serial_mm), "chardev");
> +}
> +
> +static void mchp_pfsoc_mmuart_realize(DeviceState *dev, Error **errp)
> +{
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
> +
> + qdev_prop_set_uint8(DEVICE(&s->serial_mm), "regshift", 2);
> + qdev_prop_set_uint32(DEVICE(&s->serial_mm), "baudbase", 399193);
> + qdev_prop_set_uint8(DEVICE(&s->serial_mm), "endianness",
> + DEVICE_LITTLE_ENDIAN);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->serial_mm), errp)) {
> + return;
> + }
> +
> + sysbus_pass_irq(SYS_BUS_DEVICE(dev), SYS_BUS_DEVICE(&s->serial_mm));
> +
> + memory_region_init(&s->container, OBJECT(s), "mchp.pfsoc.mmuart", 0x1000);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> +
> + memory_region_add_subregion(&s->container, 0,
> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->serial_mm), 0));
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &mchp_pfsoc_mmuart_ops, s,
> "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
> memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
> -
> - s->base = base;
> - s->irq = irq;
> -
> - s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
> - DEVICE_LITTLE_ENDIAN);
> -
> - memory_region_add_subregion(sysmem, base, &s->container);
> -
> - return s;
> +}
> +
> +static const VMStateDescription mchp_pfsoc_mmuart_vmstate = {
> + .name = "mchp.pfsoc.uart",
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(reg, MchpPfSoCMMUartState,
> + MCHP_PFSOC_MMUART_REG_COUNT),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void mchp_pfsoc_mmuart_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->realize = mchp_pfsoc_mmuart_realize;
> + dc->reset = mchp_pfsoc_mmuart_reset;
> + dc->vmsd = &mchp_pfsoc_mmuart_vmstate;
> + set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +}
> +
> +static const TypeInfo mchp_pfsoc_mmuart_info = {
> + .name = TYPE_MCHP_PFSOC_UART,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(MchpPfSoCMMUartState),
> + .instance_init = mchp_pfsoc_mmuart_init,
> + .class_init = mchp_pfsoc_mmuart_class_init,
> +};
> +
> +static void mchp_pfsoc_mmuart_register_types(void)
> +{
> + type_register_static(&mchp_pfsoc_mmuart_info);
> +}
> +
> +type_init(mchp_pfsoc_mmuart_register_types)
> +
> +MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
> + hwaddr base,
> + qemu_irq irq, Chardev *chr)
> +{
> + DeviceState *dev = qdev_new(TYPE_MCHP_PFSOC_UART);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> + qdev_prop_set_chr(dev, "chardev", chr);
> + sysbus_realize(sbd, &error_fatal);
> +
> + memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(sbd, 0));
> + sysbus_connect_irq(sbd, 0, irq);
> +
> + return MCHP_PFSOC_UART(dev);
> }
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH v2 3/3] hw/char/mchp_pfsoc_mmuart: QOM'ify PolarFire MMUART
@ 2021-09-28 22:15 ` Alistair Francis
0 siblings, 0 replies; 26+ messages in thread
From: Alistair Francis @ 2021-09-28 22:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Marc-André Lureau, Bin Meng, Paolo Bonzini, Peter Maydell
On Sat, Sep 25, 2021 at 11:34 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> - Embed SerialMM in MchpPfSoCMMUartState and QOM-initialize it
> - Alias SERIAL_MM 'chardev' property on MCHP_PFSOC_UART
> - Forward SerialMM sysbus IRQ in mchp_pfsoc_mmuart_realize()
> - Add DeviceReset() method
> - Add vmstate structure for migration
> - Register device in 'input' category
> - Keep mchp_pfsoc_mmuart_create() behavior
>
> Note, serial_mm_init() calls qdev_set_legacy_instance_id().
> This call is only needed for backwards-compatibility of incoming
> migration data with old versions of QEMU which implemented migration
> of devices with hand-rolled code. Since this device didn't previously
> handle migration at all, then it doesn't need to set the legacy
> instance ID.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> Cc: Peter Maydell <peter.maydell@linaro.org>
>
> I haven't kept Alistair R-b tag from v1.
> ---
> include/hw/char/mchp_pfsoc_mmuart.h | 12 +++-
> hw/char/mchp_pfsoc_mmuart.c | 105 +++++++++++++++++++++++-----
> 2 files changed, 97 insertions(+), 20 deletions(-)
>
> diff --git a/include/hw/char/mchp_pfsoc_mmuart.h b/include/hw/char/mchp_pfsoc_mmuart.h
> index 864ac1a36b5..b0e14ca3554 100644
> --- a/include/hw/char/mchp_pfsoc_mmuart.h
> +++ b/include/hw/char/mchp_pfsoc_mmuart.h
> @@ -28,17 +28,23 @@
> #ifndef HW_MCHP_PFSOC_MMUART_H
> #define HW_MCHP_PFSOC_MMUART_H
>
> +#include "hw/sysbus.h"
> #include "hw/char/serial.h"
>
> #define MCHP_PFSOC_MMUART_REG_COUNT 13
>
> +#define TYPE_MCHP_PFSOC_UART "mchp.pfsoc.uart"
> +OBJECT_DECLARE_SIMPLE_TYPE(MchpPfSoCMMUartState, MCHP_PFSOC_UART)
> +
> typedef struct MchpPfSoCMMUartState {
> + /*< private >*/
> + SysBusDevice parent_obj;
> +
> + /*< public >*/
> MemoryRegion container;
> MemoryRegion iomem;
> - hwaddr base;
> - qemu_irq irq;
>
> - SerialMM *serial;
> + SerialMM serial_mm;
>
> uint32_t reg[MCHP_PFSOC_MMUART_REG_COUNT];
> } MchpPfSoCMMUartState;
> diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
> index ea586559761..22f3e78eb9e 100644
> --- a/hw/char/mchp_pfsoc_mmuart.c
> +++ b/hw/char/mchp_pfsoc_mmuart.c
> @@ -22,8 +22,10 @@
>
> #include "qemu/osdep.h"
> #include "qemu/log.h"
> -#include "chardev/char.h"
> +#include "qapi/error.h"
> +#include "migration/vmstate.h"
> #include "hw/char/mchp_pfsoc_mmuart.h"
> +#include "hw/qdev-properties.h"
>
> #define REGS_OFFSET 0x20
>
> @@ -67,26 +69,95 @@ static const MemoryRegionOps mchp_pfsoc_mmuart_ops = {
> },
> };
>
> -MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
> - hwaddr base, qemu_irq irq, Chardev *chr)
> +static void mchp_pfsoc_mmuart_reset(DeviceState *dev)
> {
> - MchpPfSoCMMUartState *s;
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
>
> - s = g_new0(MchpPfSoCMMUartState, 1);
> + memset(s->reg, 0, sizeof(s->reg));
> + device_cold_reset(DEVICE(&s->serial_mm));
> +}
>
> - memory_region_init(&s->container, NULL, "mchp.pfsoc.mmuart", 0x1000);
> +static void mchp_pfsoc_mmuart_init(Object *obj)
> +{
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(obj);
>
> - memory_region_init_io(&s->iomem, NULL, &mchp_pfsoc_mmuart_ops, s,
> + object_initialize_child(obj, "serial-mm", &s->serial_mm, TYPE_SERIAL_MM);
> + object_property_add_alias(obj, "chardev", OBJECT(&s->serial_mm), "chardev");
> +}
> +
> +static void mchp_pfsoc_mmuart_realize(DeviceState *dev, Error **errp)
> +{
> + MchpPfSoCMMUartState *s = MCHP_PFSOC_UART(dev);
> +
> + qdev_prop_set_uint8(DEVICE(&s->serial_mm), "regshift", 2);
> + qdev_prop_set_uint32(DEVICE(&s->serial_mm), "baudbase", 399193);
> + qdev_prop_set_uint8(DEVICE(&s->serial_mm), "endianness",
> + DEVICE_LITTLE_ENDIAN);
> + if (!sysbus_realize(SYS_BUS_DEVICE(&s->serial_mm), errp)) {
> + return;
> + }
> +
> + sysbus_pass_irq(SYS_BUS_DEVICE(dev), SYS_BUS_DEVICE(&s->serial_mm));
> +
> + memory_region_init(&s->container, OBJECT(s), "mchp.pfsoc.mmuart", 0x1000);
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->container);
> +
> + memory_region_add_subregion(&s->container, 0,
> + sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->serial_mm), 0));
> +
> + memory_region_init_io(&s->iomem, OBJECT(s), &mchp_pfsoc_mmuart_ops, s,
> "mchp.pfsoc.mmuart.regs", 0x1000 - REGS_OFFSET);
> memory_region_add_subregion(&s->container, REGS_OFFSET, &s->iomem);
> -
> - s->base = base;
> - s->irq = irq;
> -
> - s->serial = serial_mm_init(&s->container, 0, 2, irq, 399193, chr,
> - DEVICE_LITTLE_ENDIAN);
> -
> - memory_region_add_subregion(sysmem, base, &s->container);
> -
> - return s;
> +}
> +
> +static const VMStateDescription mchp_pfsoc_mmuart_vmstate = {
> + .name = "mchp.pfsoc.uart",
> + .version_id = 0,
> + .minimum_version_id = 0,
> + .fields = (VMStateField[]) {
> + VMSTATE_UINT32_ARRAY(reg, MchpPfSoCMMUartState,
> + MCHP_PFSOC_MMUART_REG_COUNT),
> + VMSTATE_END_OF_LIST()
> + }
> +};
> +
> +static void mchp_pfsoc_mmuart_class_init(ObjectClass *oc, void *data)
> +{
> + DeviceClass *dc = DEVICE_CLASS(oc);
> +
> + dc->realize = mchp_pfsoc_mmuart_realize;
> + dc->reset = mchp_pfsoc_mmuart_reset;
> + dc->vmsd = &mchp_pfsoc_mmuart_vmstate;
> + set_bit(DEVICE_CATEGORY_INPUT, dc->categories);
> +}
> +
> +static const TypeInfo mchp_pfsoc_mmuart_info = {
> + .name = TYPE_MCHP_PFSOC_UART,
> + .parent = TYPE_SYS_BUS_DEVICE,
> + .instance_size = sizeof(MchpPfSoCMMUartState),
> + .instance_init = mchp_pfsoc_mmuart_init,
> + .class_init = mchp_pfsoc_mmuart_class_init,
> +};
> +
> +static void mchp_pfsoc_mmuart_register_types(void)
> +{
> + type_register_static(&mchp_pfsoc_mmuart_info);
> +}
> +
> +type_init(mchp_pfsoc_mmuart_register_types)
> +
> +MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
> + hwaddr base,
> + qemu_irq irq, Chardev *chr)
> +{
> + DeviceState *dev = qdev_new(TYPE_MCHP_PFSOC_UART);
> + SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
> +
> + qdev_prop_set_chr(dev, "chardev", chr);
> + sysbus_realize(sbd, &error_fatal);
> +
> + memory_region_add_subregion(sysmem, base, sysbus_mmio_get_region(sbd, 0));
> + sysbus_connect_irq(sbd, 0, irq);
> +
> + return MCHP_PFSOC_UART(dev);
> }
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 26+ messages in thread