Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-27 Thread Artyom Tarasenko
On Sat, Apr 20, 2013 at 12:39 PM, Blue Swirl blauwir...@gmail.com wrote:
 On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko atar4q...@gmail.com wrote:
 On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl blauwir...@gmail.com wrote:
 On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko atar4q...@gmail.com 
 wrote:
 On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org 
 wrote:
 As m48t59 devices can only be created with m48t59_init() or 
 m48t59_init_isa(),
 we know exactly which nvram types are required. Register only those three
 types.
 Remove .model and .size properties as they can be infered from nvram name.
 Remove .io_base ISA address port as m48t59_init_isa() is always called 
 with ioport 0x74.

 While this it indeed how it's currently called, this is wrong for the
 sun4u emulation.
 The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
 with a mem_base, not io_base.

 Why? I don't see much difference between EBUS and ISA and with the
 memory API, the difference between PIO and MMIO is almost nonexistent
 anyway.

 Can you elaborate? Do you mean we just need to change the io_base?

 Why wouldn't that work?

Because the PIO variant registers just 4 ports, whereas MMIO registers
the whole device (0x2000 bytes).
The sun4u machines use a slightly different modification of the ISA Mostek chip.
It has MMIO, 1968 as a base year and no IRQ line. Since it matches our m48t08,
I'll send a patch fixing it.

 But it should be possible to change the base to match real HW, whatever it 
 is:
 http://git.kernel.org/cgit/linux/kernel/git/davem/prtconfs.git/tree/ultra5#n273

 Yes, I know where it is supposed to be, I'm just asking how to achieve
 this best with our current tooling.

 So NACK for the original patch.

 Do you think it should be implemented as another device type?


 Signed-off-by: Hervé Poussineau hpous...@reactos.org
 ---
  hw/timer/m48t59.c |  187 
 -
  1 file changed, 126 insertions(+), 61 deletions(-)

 diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
 index 41022f2..29ec462 100644
 --- a/hw/timer/m48t59.c
 +++ b/hw/timer/m48t59.c
 @@ -43,6 +43,13 @@
   * PPC platform there is also a nvram lock function.
   */

 +typedef struct M48txxInfo {
 +const char *isa_name;
 +const char *sysbus_name;
 +uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 +uint32_t size;
 +} M48txxInfo;
 +
  /*
   * Chipset docs:
   * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
 @@ -54,7 +61,6 @@ struct M48t59State {
  /* Hardware parameters */
  qemu_irq IRQ;
  MemoryRegion iomem;
 -uint32_t io_base;
  uint32_t size;
  /* RTC management */
  time_t   time_offset;
 @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
  MemoryRegion io;
  } M48t59ISAState;

 +typedef struct M48txxISADeviceClass {
 +ISADeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxISADeviceClass;
 +
  typedef struct M48t59SysBusState {
  SysBusDevice busdev;
  M48t59State state;
  MemoryRegion io;
  } M48t59SysBusState;

 +typedef struct M48txxSysBusDeviceClass {
 +SysBusDeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxSysBusDeviceClass;
 +
 +static M48txxInfo m48txx_info[] = {
 +{
 +.sysbus_name = m48t02,
 +.model = 2,
 +.size = 0x800,
 +},{
 +.sysbus_name = m48t08,
 +.model = 8,
 +.size = 0x2000,
 +},{
 +.isa_name = m48t59_isa,
 +.model = 59,
 +.size = 0x2000,
 +}
 +};
 +
 +
  /* Fake timer functions */

  /* Alarm management */
 @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr 
 mem_base,
  SysBusDevice *s;
  M48t59SysBusState *d;
  M48t59State *state;
 +int i;

 -dev = qdev_create(NULL, m48t59);
 -qdev_prop_set_uint32(dev, model, model);
 -qdev_prop_set_uint32(dev, size, size);
 -qdev_prop_set_uint32(dev, io_base, io_base);
 -qdev_init_nofail(dev);
 -s = SYS_BUS_DEVICE(dev);
 -d = FROM_SYSBUS(M48t59SysBusState, s);
 -state = d-state;
 -sysbus_connect_irq(s, 0, IRQ);
 -memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 -if (io_base != 0) {
 -memory_region_add_subregion(get_system_io(), io_base, d-io);
 -}
 -if (mem_base != 0) {
 -sysbus_mmio_map(s, 0, mem_base);
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].sysbus_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}
 +
 +dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
 +qdev_init_nofail(dev);
 +s = SYS_BUS_DEVICE(dev);
 +d = FROM_SYSBUS(M48t59SysBusState, s);
 +state = d-state;
 +sysbus_connect_irq(s, 0, IRQ);
 +memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 
 4);
 +if (io_base != 0) {
 +

Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-20 Thread Blue Swirl
On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko atar4q...@gmail.com wrote:
 On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org 
 wrote:
 As m48t59 devices can only be created with m48t59_init() or 
 m48t59_init_isa(),
 we know exactly which nvram types are required. Register only those three
 types.
 Remove .model and .size properties as they can be infered from nvram name.
 Remove .io_base ISA address port as m48t59_init_isa() is always called with 
 ioport 0x74.

 While this it indeed how it's currently called, this is wrong for the
 sun4u emulation.
 The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
 with a mem_base, not io_base.

Why? I don't see much difference between EBUS and ISA and with the
memory API, the difference between PIO and MMIO is almost nonexistent
anyway.

But it should be possible to change the base to match real HW, whatever it is:
http://git.kernel.org/cgit/linux/kernel/git/davem/prtconfs.git/tree/ultra5#n273

So NACK for the original patch.

 Do you think it should be implemented as another device type?


 Signed-off-by: Hervé Poussineau hpous...@reactos.org
 ---
  hw/timer/m48t59.c |  187 
 -
  1 file changed, 126 insertions(+), 61 deletions(-)

 diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
 index 41022f2..29ec462 100644
 --- a/hw/timer/m48t59.c
 +++ b/hw/timer/m48t59.c
 @@ -43,6 +43,13 @@
   * PPC platform there is also a nvram lock function.
   */

 +typedef struct M48txxInfo {
 +const char *isa_name;
 +const char *sysbus_name;
 +uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 +uint32_t size;
 +} M48txxInfo;
 +
  /*
   * Chipset docs:
   * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
 @@ -54,7 +61,6 @@ struct M48t59State {
  /* Hardware parameters */
  qemu_irq IRQ;
  MemoryRegion iomem;
 -uint32_t io_base;
  uint32_t size;
  /* RTC management */
  time_t   time_offset;
 @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
  MemoryRegion io;
  } M48t59ISAState;

 +typedef struct M48txxISADeviceClass {
 +ISADeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxISADeviceClass;
 +
  typedef struct M48t59SysBusState {
  SysBusDevice busdev;
  M48t59State state;
  MemoryRegion io;
  } M48t59SysBusState;

 +typedef struct M48txxSysBusDeviceClass {
 +SysBusDeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxSysBusDeviceClass;
 +
 +static M48txxInfo m48txx_info[] = {
 +{
 +.sysbus_name = m48t02,
 +.model = 2,
 +.size = 0x800,
 +},{
 +.sysbus_name = m48t08,
 +.model = 8,
 +.size = 0x2000,
 +},{
 +.isa_name = m48t59_isa,
 +.model = 59,
 +.size = 0x2000,
 +}
 +};
 +
 +
  /* Fake timer functions */

  /* Alarm management */
 @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
  SysBusDevice *s;
  M48t59SysBusState *d;
  M48t59State *state;
 +int i;

 -dev = qdev_create(NULL, m48t59);
 -qdev_prop_set_uint32(dev, model, model);
 -qdev_prop_set_uint32(dev, size, size);
 -qdev_prop_set_uint32(dev, io_base, io_base);
 -qdev_init_nofail(dev);
 -s = SYS_BUS_DEVICE(dev);
 -d = FROM_SYSBUS(M48t59SysBusState, s);
 -state = d-state;
 -sysbus_connect_irq(s, 0, IRQ);
 -memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 -if (io_base != 0) {
 -memory_region_add_subregion(get_system_io(), io_base, d-io);
 -}
 -if (mem_base != 0) {
 -sysbus_mmio_map(s, 0, mem_base);
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].sysbus_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}
 +
 +dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
 +qdev_init_nofail(dev);
 +s = SYS_BUS_DEVICE(dev);
 +d = FROM_SYSBUS(M48t59SysBusState, s);
 +state = d-state;
 +sysbus_connect_irq(s, 0, IRQ);
 +memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 +if (io_base != 0) {
 +memory_region_add_subregion(get_system_io(), io_base, d-io);
 +}
 +if (mem_base != 0) {
 +sysbus_mmio_map(s, 0, mem_base);
 +}
 +
 +return state;
  }

 -return state;
 +assert(false);
 +return NULL;
  }

  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 @@ -667,16 +709,27 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t 
 io_base, uint16_t size,
  M48t59ISAState *d;
  ISADevice *dev;
  M48t59State *s;
 +int i;
 +
 +assert(io_base == 0x74);
 +
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].isa_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +  

Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-20 Thread Artyom Tarasenko
On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl blauwir...@gmail.com wrote:
 On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko atar4q...@gmail.com wrote:
 On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org 
 wrote:
 As m48t59 devices can only be created with m48t59_init() or 
 m48t59_init_isa(),
 we know exactly which nvram types are required. Register only those three
 types.
 Remove .model and .size properties as they can be infered from nvram name.
 Remove .io_base ISA address port as m48t59_init_isa() is always called with 
 ioport 0x74.

 While this it indeed how it's currently called, this is wrong for the
 sun4u emulation.
 The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
 with a mem_base, not io_base.

 Why? I don't see much difference between EBUS and ISA and with the
 memory API, the difference between PIO and MMIO is almost nonexistent
 anyway.

Can you elaborate? Do you mean we just need to change the io_base?

 But it should be possible to change the base to match real HW, whatever it is:
 http://git.kernel.org/cgit/linux/kernel/git/davem/prtconfs.git/tree/ultra5#n273

Yes, I know where it is supposed to be, I'm just asking how to achieve
this best with our current tooling.

 So NACK for the original patch.

 Do you think it should be implemented as another device type?


 Signed-off-by: Hervé Poussineau hpous...@reactos.org
 ---
  hw/timer/m48t59.c |  187 
 -
  1 file changed, 126 insertions(+), 61 deletions(-)

 diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
 index 41022f2..29ec462 100644
 --- a/hw/timer/m48t59.c
 +++ b/hw/timer/m48t59.c
 @@ -43,6 +43,13 @@
   * PPC platform there is also a nvram lock function.
   */

 +typedef struct M48txxInfo {
 +const char *isa_name;
 +const char *sysbus_name;
 +uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 +uint32_t size;
 +} M48txxInfo;
 +
  /*
   * Chipset docs:
   * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
 @@ -54,7 +61,6 @@ struct M48t59State {
  /* Hardware parameters */
  qemu_irq IRQ;
  MemoryRegion iomem;
 -uint32_t io_base;
  uint32_t size;
  /* RTC management */
  time_t   time_offset;
 @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
  MemoryRegion io;
  } M48t59ISAState;

 +typedef struct M48txxISADeviceClass {
 +ISADeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxISADeviceClass;
 +
  typedef struct M48t59SysBusState {
  SysBusDevice busdev;
  M48t59State state;
  MemoryRegion io;
  } M48t59SysBusState;

 +typedef struct M48txxSysBusDeviceClass {
 +SysBusDeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxSysBusDeviceClass;
 +
 +static M48txxInfo m48txx_info[] = {
 +{
 +.sysbus_name = m48t02,
 +.model = 2,
 +.size = 0x800,
 +},{
 +.sysbus_name = m48t08,
 +.model = 8,
 +.size = 0x2000,
 +},{
 +.isa_name = m48t59_isa,
 +.model = 59,
 +.size = 0x2000,
 +}
 +};
 +
 +
  /* Fake timer functions */

  /* Alarm management */
 @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr 
 mem_base,
  SysBusDevice *s;
  M48t59SysBusState *d;
  M48t59State *state;
 +int i;

 -dev = qdev_create(NULL, m48t59);
 -qdev_prop_set_uint32(dev, model, model);
 -qdev_prop_set_uint32(dev, size, size);
 -qdev_prop_set_uint32(dev, io_base, io_base);
 -qdev_init_nofail(dev);
 -s = SYS_BUS_DEVICE(dev);
 -d = FROM_SYSBUS(M48t59SysBusState, s);
 -state = d-state;
 -sysbus_connect_irq(s, 0, IRQ);
 -memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 -if (io_base != 0) {
 -memory_region_add_subregion(get_system_io(), io_base, d-io);
 -}
 -if (mem_base != 0) {
 -sysbus_mmio_map(s, 0, mem_base);
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].sysbus_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}
 +
 +dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
 +qdev_init_nofail(dev);
 +s = SYS_BUS_DEVICE(dev);
 +d = FROM_SYSBUS(M48t59SysBusState, s);
 +state = d-state;
 +sysbus_connect_irq(s, 0, IRQ);
 +memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 +if (io_base != 0) {
 +memory_region_add_subregion(get_system_io(), io_base, d-io);
 +}
 +if (mem_base != 0) {
 +sysbus_mmio_map(s, 0, mem_base);
 +}
 +
 +return state;
  }

 -return state;
 +assert(false);
 +return NULL;
  }

  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 @@ -667,16 +709,27 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t 
 io_base, uint16_t size,
  M48t59ISAState *d;
  ISADevice *dev;
  

Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-20 Thread Blue Swirl
On Sat, Apr 20, 2013 at 9:56 AM, Artyom Tarasenko atar4q...@gmail.com wrote:
 On Sat, Apr 20, 2013 at 11:34 AM, Blue Swirl blauwir...@gmail.com wrote:
 On Sun, Apr 14, 2013 at 9:41 PM, Artyom Tarasenko atar4q...@gmail.com 
 wrote:
 On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org 
 wrote:
 As m48t59 devices can only be created with m48t59_init() or 
 m48t59_init_isa(),
 we know exactly which nvram types are required. Register only those three
 types.
 Remove .model and .size properties as they can be infered from nvram name.
 Remove .io_base ISA address port as m48t59_init_isa() is always called 
 with ioport 0x74.

 While this it indeed how it's currently called, this is wrong for the
 sun4u emulation.
 The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
 with a mem_base, not io_base.

 Why? I don't see much difference between EBUS and ISA and with the
 memory API, the difference between PIO and MMIO is almost nonexistent
 anyway.

 Can you elaborate? Do you mean we just need to change the io_base?

Why wouldn't that work?

 But it should be possible to change the base to match real HW, whatever it 
 is:
 http://git.kernel.org/cgit/linux/kernel/git/davem/prtconfs.git/tree/ultra5#n273

 Yes, I know where it is supposed to be, I'm just asking how to achieve
 this best with our current tooling.

 So NACK for the original patch.

 Do you think it should be implemented as another device type?


 Signed-off-by: Hervé Poussineau hpous...@reactos.org
 ---
  hw/timer/m48t59.c |  187 
 -
  1 file changed, 126 insertions(+), 61 deletions(-)

 diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
 index 41022f2..29ec462 100644
 --- a/hw/timer/m48t59.c
 +++ b/hw/timer/m48t59.c
 @@ -43,6 +43,13 @@
   * PPC platform there is also a nvram lock function.
   */

 +typedef struct M48txxInfo {
 +const char *isa_name;
 +const char *sysbus_name;
 +uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 +uint32_t size;
 +} M48txxInfo;
 +
  /*
   * Chipset docs:
   * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
 @@ -54,7 +61,6 @@ struct M48t59State {
  /* Hardware parameters */
  qemu_irq IRQ;
  MemoryRegion iomem;
 -uint32_t io_base;
  uint32_t size;
  /* RTC management */
  time_t   time_offset;
 @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
  MemoryRegion io;
  } M48t59ISAState;

 +typedef struct M48txxISADeviceClass {
 +ISADeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxISADeviceClass;
 +
  typedef struct M48t59SysBusState {
  SysBusDevice busdev;
  M48t59State state;
  MemoryRegion io;
  } M48t59SysBusState;

 +typedef struct M48txxSysBusDeviceClass {
 +SysBusDeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxSysBusDeviceClass;
 +
 +static M48txxInfo m48txx_info[] = {
 +{
 +.sysbus_name = m48t02,
 +.model = 2,
 +.size = 0x800,
 +},{
 +.sysbus_name = m48t08,
 +.model = 8,
 +.size = 0x2000,
 +},{
 +.isa_name = m48t59_isa,
 +.model = 59,
 +.size = 0x2000,
 +}
 +};
 +
 +
  /* Fake timer functions */

  /* Alarm management */
 @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr 
 mem_base,
  SysBusDevice *s;
  M48t59SysBusState *d;
  M48t59State *state;
 +int i;

 -dev = qdev_create(NULL, m48t59);
 -qdev_prop_set_uint32(dev, model, model);
 -qdev_prop_set_uint32(dev, size, size);
 -qdev_prop_set_uint32(dev, io_base, io_base);
 -qdev_init_nofail(dev);
 -s = SYS_BUS_DEVICE(dev);
 -d = FROM_SYSBUS(M48t59SysBusState, s);
 -state = d-state;
 -sysbus_connect_irq(s, 0, IRQ);
 -memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 -if (io_base != 0) {
 -memory_region_add_subregion(get_system_io(), io_base, d-io);
 -}
 -if (mem_base != 0) {
 -sysbus_mmio_map(s, 0, mem_base);
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].sysbus_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}
 +
 +dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
 +qdev_init_nofail(dev);
 +s = SYS_BUS_DEVICE(dev);
 +d = FROM_SYSBUS(M48t59SysBusState, s);
 +state = d-state;
 +sysbus_connect_irq(s, 0, IRQ);
 +memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 +if (io_base != 0) {
 +memory_region_add_subregion(get_system_io(), io_base, d-io);
 +}
 +if (mem_base != 0) {
 +sysbus_mmio_map(s, 0, mem_base);
 +}
 +
 +return state;
  }

 -return state;
 +assert(false);
 +return NULL;
  }

  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 @@ -667,16 +709,27 @@ M48t59State 

Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-14 Thread Artyom Tarasenko
On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org wrote:
 As m48t59 devices can only be created with m48t59_init() or m48t59_init_isa(),
 we know exactly which nvram types are required. Register only those three
 types.
 Remove .model and .size properties as they can be infered from nvram name.
 Remove .io_base ISA address port as m48t59_init_isa() is always called with 
 ioport 0x74.

While this it indeed how it's currently called, this is wrong for the
sun4u emulation.
The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
with a mem_base, not io_base.
Do you think it should be implemented as another device type?


 Signed-off-by: Hervé Poussineau hpous...@reactos.org
 ---
  hw/timer/m48t59.c |  187 
 -
  1 file changed, 126 insertions(+), 61 deletions(-)

 diff --git a/hw/timer/m48t59.c b/hw/timer/m48t59.c
 index 41022f2..29ec462 100644
 --- a/hw/timer/m48t59.c
 +++ b/hw/timer/m48t59.c
 @@ -43,6 +43,13 @@
   * PPC platform there is also a nvram lock function.
   */

 +typedef struct M48txxInfo {
 +const char *isa_name;
 +const char *sysbus_name;
 +uint32_t model; /* 2 = m48t02, 8 = m48t08, 59 = m48t59 */
 +uint32_t size;
 +} M48txxInfo;
 +
  /*
   * Chipset docs:
   * http://www.st.com/stonline/products/literature/ds/2410/m48t02.pdf
 @@ -54,7 +61,6 @@ struct M48t59State {
  /* Hardware parameters */
  qemu_irq IRQ;
  MemoryRegion iomem;
 -uint32_t io_base;
  uint32_t size;
  /* RTC management */
  time_t   time_offset;
 @@ -78,12 +84,39 @@ typedef struct M48t59ISAState {
  MemoryRegion io;
  } M48t59ISAState;

 +typedef struct M48txxISADeviceClass {
 +ISADeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxISADeviceClass;
 +
  typedef struct M48t59SysBusState {
  SysBusDevice busdev;
  M48t59State state;
  MemoryRegion io;
  } M48t59SysBusState;

 +typedef struct M48txxSysBusDeviceClass {
 +SysBusDeviceClass parent_class;
 +M48txxInfo info;
 +} M48txxSysBusDeviceClass;
 +
 +static M48txxInfo m48txx_info[] = {
 +{
 +.sysbus_name = m48t02,
 +.model = 2,
 +.size = 0x800,
 +},{
 +.sysbus_name = m48t08,
 +.model = 8,
 +.size = 0x2000,
 +},{
 +.isa_name = m48t59_isa,
 +.model = 59,
 +.size = 0x2000,
 +}
 +};
 +
 +
  /* Fake timer functions */

  /* Alarm management */
 @@ -640,25 +673,34 @@ M48t59State *m48t59_init(qemu_irq IRQ, hwaddr mem_base,
  SysBusDevice *s;
  M48t59SysBusState *d;
  M48t59State *state;
 +int i;

 -dev = qdev_create(NULL, m48t59);
 -qdev_prop_set_uint32(dev, model, model);
 -qdev_prop_set_uint32(dev, size, size);
 -qdev_prop_set_uint32(dev, io_base, io_base);
 -qdev_init_nofail(dev);
 -s = SYS_BUS_DEVICE(dev);
 -d = FROM_SYSBUS(M48t59SysBusState, s);
 -state = d-state;
 -sysbus_connect_irq(s, 0, IRQ);
 -memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 -if (io_base != 0) {
 -memory_region_add_subregion(get_system_io(), io_base, d-io);
 -}
 -if (mem_base != 0) {
 -sysbus_mmio_map(s, 0, mem_base);
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].sysbus_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}
 +
 +dev = qdev_create(NULL, m48txx_info[i].sysbus_name);
 +qdev_init_nofail(dev);
 +s = SYS_BUS_DEVICE(dev);
 +d = FROM_SYSBUS(M48t59SysBusState, s);
 +state = d-state;
 +sysbus_connect_irq(s, 0, IRQ);
 +memory_region_init_io(d-io, m48t59_io_ops, state, m48t59, 4);
 +if (io_base != 0) {
 +memory_region_add_subregion(get_system_io(), io_base, d-io);
 +}
 +if (mem_base != 0) {
 +sysbus_mmio_map(s, 0, mem_base);
 +}
 +
 +return state;
  }

 -return state;
 +assert(false);
 +return NULL;
  }

  M48t59State *m48t59_init_isa(ISABus *bus, uint32_t io_base, uint16_t size,
 @@ -667,16 +709,27 @@ M48t59State *m48t59_init_isa(ISABus *bus, uint32_t 
 io_base, uint16_t size,
  M48t59ISAState *d;
  ISADevice *dev;
  M48t59State *s;
 +int i;
 +
 +assert(io_base == 0x74);
 +
 +for (i = 0; i  ARRAY_SIZE(m48txx_info); i++) {
 +if (!m48txx_info[i].isa_name ||
 +m48txx_info[i].size != size ||
 +m48txx_info[i].model != model) {
 +continue;
 +}

 -dev = isa_create(bus, m48t59_isa);
 -qdev_prop_set_uint32(dev-qdev, model, model);
 -qdev_prop_set_uint32(dev-qdev, size, size);
 -qdev_prop_set_uint32(dev-qdev, io_base, io_base);
 -qdev_init_nofail(dev-qdev);
 -d = DO_UPCAST(M48t59ISAState, busdev, dev);
 -s = d-state;
 +dev = isa_create(bus, m48txx_info[i].isa_name);
 +qdev_init_nofail(dev-qdev);
 +  

Re: [Qemu-devel] [RFC v2 3/7] m48t59: register a QOM type for each nvram type we support

2013-04-14 Thread Hervé Poussineau

Artyom Tarasenko a écrit :

On Sun, Apr 14, 2013 at 10:05 AM, Hervé Poussineau hpous...@reactos.org wrote:

As m48t59 devices can only be created with m48t59_init() or m48t59_init_isa(),
we know exactly which nvram types are required. Register only those three
types.
Remove .model and .size properties as they can be infered from nvram name.
Remove .io_base ISA address port as m48t59_init_isa() is always called with 
ioport 0x74.


While this it indeed how it's currently called, this is wrong for the
sun4u emulation.
The isa (ebus) variant of the sun4u m48t59_init_isa() should be called
with a mem_base, not io_base.
Do you think it should be implemented as another device type?



I don't know EBUS, but I think it should be implemented either as a 
completly new bus (1), or as a child of the ISA bus type (2).

For 1), you'll need to add a another device type to be plugged on EBUS.
For 2), I let experts answer :)

In all cases, maybe the m48t59_init() wrapper is what you need? You can 
already give him a membase.
Otherwise, you can maybe use sysbus_create_simple(m48t59), get the 
resulting MemoryRegion from the device, and add it in whatever MemoryRegion.


Hervé