[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/2] m48t59: use mmio for the m48t08 model of th
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 1/2] m48t59: use mmio for the m48t08 model of the m48t59_isa card |
Date: |
Sat, 27 Apr 2013 17:16:27 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130329 Thunderbird/17.0.5 |
Am 27.04.2013 09:12, schrieb Artyom Tarasenko:
> PrEP and SPARC machines use slightly different variations of
PReP :)
> a Mostek NVRAM chip. Since the SPARC variant is much closer
> to a m48t08 type, the model can be used to differentiate between
> the PIO and MMIO accesses.
>
> Signed-off-by: Artyom Tarasenko <address@hidden>
> ---
> hw/timer/m48t59.c | 38 +++++++++++++++++++++++++++++++++++---
> 1 files changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
> index 5019e06..00ad417 100644
> --- a/hw/timer/m48t59.c
> +++ b/hw/timer/m48t59.c
> @@ -632,6 +632,33 @@ static const MemoryRegionOps m48t59_io_ops = {
> .endianness = DEVICE_LITTLE_ENDIAN,
> };
>
> +static uint64_t nvram_read(void *opaque, hwaddr addr, unsigned size)
> +{
> + M48t59State *NVRAM = opaque;
Probably this is just copy&paste from old code, but please use
lower_case_names for variables in new code.
> + uint32_t retval;
> +
> + retval = m48t59_read(NVRAM, addr);
> + return retval;
> +}
> +
> +static void nvram_write(void *opaque, hwaddr addr, uint64_t value,
> + unsigned size)
Indentation one-off.
> +{
> + M48t59State *NVRAM = opaque;
> +
> + m48t59_write(NVRAM, addr, value & 0xff);
> +}
> +
> +static const MemoryRegionOps m48t59_mmio_ops = {
> + .read = nvram_read,
> + .write = nvram_write,
> + .impl = {
> + .min_access_size = 1,
> + .max_access_size = 1,
> + },
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
> +
> /* Initialisation routine */
> M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
> uint32_t io_base, uint16_t size, int model)
> @@ -676,7 +703,11 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t
> io_base, uint16_t size,
> d = DO_UPCAST(M48t59ISAState, busdev, dev);
> s = &d->state;
>
> - memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4);
> + if (model == 59) {
> + memory_region_init_io(&d->io, &m48t59_io_ops, s, "m48t59", 4);
> + } else {
> + memory_region_init_io(&d->io, &m48t59_mmio_ops, s, "m48t59", size);
If you distinguish here, it may be a good idea to reflect that in the
region's name.
> + }
> if (io_base != 0) {
> isa_register_ioport(dev, &d->io, io_base);
> }
> @@ -700,8 +731,9 @@ static int m48t59_init_isa1(ISADevice *dev)
> {
> M48t59ISAState *d = DO_UPCAST(M48t59ISAState, busdev, dev);
> M48t59State *s = &d->state;
> -
> - isa_init_irq(dev, &s->IRQ, 8);
> + if (s->model == 59) {
> + isa_init_irq(dev, &s->IRQ, 8);
> + }
> m48t59_init_common(s);
>
> return 0;
Regarding your question of whether to move this: With my ISA realize
series this function becomes a ..._realize function. isa_init_irq()
relies on the device being on an ISA bus to retrieve the qemu_irq, so
this cannot be done in instance_init, thus in DeviceClass::init/realize.
The existing legacy m48t59_init_isa() function should probably rather
shrink in size and one day possibly be inlined rather than growing with
stuff that was encapsulated in initfn or realizefn.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg