[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 15/20] acpi: switch smbus to memory api
From: |
Andreas Färber |
Subject: |
Re: [Qemu-devel] [PATCH 15/20] acpi: switch smbus to memory api |
Date: |
Mon, 26 Nov 2012 16:18:05 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:16.0) Gecko/20121025 Thunderbird/16.0.2 |
Am 23.11.2012 16:48, schrieb Gerd Hoffmann:
> diff --git a/hw/pm_smbus.c b/hw/pm_smbus.c
> index 5d6046d..ea1380c 100644
> --- a/hw/pm_smbus.c
> +++ b/hw/pm_smbus.c
[...]
> @@ -170,7 +170,16 @@ uint32_t smb_ioport_readb(void *opaque, uint32_t addr)
> return val;
> }
>
> +static const MemoryRegionOps pm_smbus_ops = {
> + .read = smb_ioport_readb,
> + .write = smb_ioport_writeb,
> + .valid.min_access_size = 1,
> + .valid.max_access_size = 1,
> + .endianness = DEVICE_LITTLE_ENDIAN,
> +};
I notice that in comparison to Julien's patch, you are setting .valid
here where he used .impl.
Also a generic C question: When using C99-style struct initializers as
for the MemoryRegionOps, I understand that the fields not explicitly
assigned are zero-initialized. Does that also apply to .foo.bar = baz
notation or would it be advisable to use nested .foo = { .bar = baz }?
> +
> void pm_smbus_init(DeviceState *parent, PMSMBus *smb)
> {
> smb->smbus = i2c_init_bus(parent, "i2c");
> + memory_region_init_io(&smb->io, &pm_smbus_ops, smb, "pm-smbus", 64);
> }
> diff --git a/hw/pm_smbus.h b/hw/pm_smbus.h
> index 4750a40..e3069bf 100644
> --- a/hw/pm_smbus.h
> +++ b/hw/pm_smbus.h
> @@ -3,6 +3,7 @@
>
> typedef struct PMSMBus {
> i2c_bus *smbus;
> + MemoryRegion io;
>
> uint8_t smb_stat;
> uint8_t smb_ctl;
With a view to further QOM'ifying these devices, please keep the parent
field separated, i.e.
i2c_bus *smbus;
+ MemoryRegion io;
+
uint8_t ...
(Once macros and types are correctly used - as you do for ICH9 below -
smbus would also be renamed to parent_obj for clarity, cf. QOM PHB/IDE
series, and optionally excluded from gtk-doc documentation.)
Regards,
Andreas
> diff --git a/hw/smbus_ich9.c b/hw/smbus_ich9.c
> index 6940583..54e7e12 100644
> --- a/hw/smbus_ich9.c
> +++ b/hw/smbus_ich9.c
[...]
> @@ -54,42 +53,23 @@ static const VMStateDescription vmstate_ich9_smbus = {
> }
> };
>
> -static void ich9_smb_ioport_writeb(void *opaque, hwaddr addr,
> - uint64_t val, unsigned size)
> +static void ich9_smbus_write_config(PCIDevice *d, uint32_t address,
> + uint32_t val, int len)
> {
> - ICH9SMBState *s = opaque;
> - uint8_t hostc = s->dev.config[ICH9_SMB_HOSTC];
> + ICH9SMBState *s = ICH9_SMB_DEVICE(d);
[snip]
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
- [Qemu-devel] [PATCH 16/20] acpi: fix piix4 smbus mapping, (continued)
- [Qemu-devel] [PATCH 16/20] acpi: fix piix4 smbus mapping, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 06/20] apci: switch cnt to memory api, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 18/20] acpi: remove acpi_gpe_blk, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 20/20] q35: update lpc pci config space according to configured devices, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 11/20] acpi: cleanup vt82c686 memory region, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 09/20] apci: switch evt to memory api [ich9], Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 14/20] acpi: cleanup ich9 memory region, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 10/20] acpi: cleanup piix4 memory region, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 04/20] apci: switch timer to memory api, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 15/20] acpi: switch smbus to memory api, Gerd Hoffmann, 2012/11/23
- Re: [Qemu-devel] [PATCH 15/20] acpi: switch smbus to memory api,
Andreas Färber <=
- [Qemu-devel] [PATCH 19/20] apci: switch piix4 pci hotplug to memory api, Gerd Hoffmann, 2012/11/23
- [Qemu-devel] [PATCH 13/20] apci: switch ich9 smi to memory api, Gerd Hoffmann, 2012/11/23