| Message ID | e24e156d-ff96-4901-997a-e31178b08bee@VA3EHSMHS021.ehs.local |
|---|---|
| State | New |
| Headers | show |
Am 07.02.2013 06:51, schrieb Peter Crosthwaite: > Currently, M25P80 uses an object property to differentiate between flash parts. > Changed this over to use QOM sub-classes - the actual names of the different parts > are used to create a set of dynamic classes which passes the part info as class > data. The object no longer needs to search the known_devices table for itself, > instead it just gets its info from its own class. > > Kept the intermediate class definition private to m25p80.c for the moment, as > the expectation is parts will only be added as new entries in the table. We can > factor out the TYPE_M25P80 abstraction into a header on a demand basis. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Reviewed-by: Andreas Färber <afaerber@suse.de> Looks fine to me except that I would suggest to #define a TYPE_N25Q128 constant. No typos though, so that can be done later. Cheers, Andreas
On Thu, Feb 7, 2013 at 8:40 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 07.02.2013 06:51, schrieb Peter Crosthwaite: >> Currently, M25P80 uses an object property to differentiate between flash parts. >> Changed this over to use QOM sub-classes - the actual names of the different parts >> are used to create a set of dynamic classes which passes the part info as class >> data. The object no longer needs to search the known_devices table for itself, >> instead it just gets its info from its own class. >> >> Kept the intermediate class definition private to m25p80.c for the moment, as >> the expectation is parts will only be added as new entries in the table. We can >> factor out the TYPE_M25P80 abstraction into a header on a demand basis. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > Reviewed-by: Andreas Färber <afaerber@suse.de> > Thanks, > Looks fine to me except that I would suggest to #define a TYPE_N25Q128 > constant. No typos though, so that can be done later. > I think the TYPE_N25Q128 define is related to the header discussion anyway and we should resolve this following the outcome of that. Regards, Peter > Cheers, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
Hi Peter, Edgar, Can we get a merge of this either individually or as part of the next arm-devs? If not I will create a [PULL] but I'm guessing singles like this are better to go via a patch queue. Regards, Peter On Thu, Feb 7, 2013 at 8:40 PM, Andreas Färber <afaerber@suse.de> wrote: > Am 07.02.2013 06:51, schrieb Peter Crosthwaite: >> Currently, M25P80 uses an object property to differentiate between flash parts. >> Changed this over to use QOM sub-classes - the actual names of the different parts >> are used to create a set of dynamic classes which passes the part info as class >> data. The object no longer needs to search the known_devices table for itself, >> instead it just gets its info from its own class. >> >> Kept the intermediate class definition private to m25p80.c for the moment, as >> the expectation is parts will only be added as new entries in the table. We can >> factor out the TYPE_M25P80 abstraction into a header on a demand basis. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > > Reviewed-by: Andreas Färber <afaerber@suse.de> > > Looks fine to me except that I would suggest to #define a TYPE_N25Q128 > constant. No typos though, so that can be done later. > > Cheers, > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >
Ping On Tue, Feb 19, 2013 at 5:50 PM, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > Hi Peter, Edgar, > > Can we get a merge of this either individually or as part of the next > arm-devs? If not I will create a [PULL] but I'm guessing singles like > this are better to go via a patch queue. > > Regards, > Peter > > On Thu, Feb 7, 2013 at 8:40 PM, Andreas Färber <afaerber@suse.de> wrote: >> Am 07.02.2013 06:51, schrieb Peter Crosthwaite: >>> Currently, M25P80 uses an object property to differentiate between flash parts. >>> Changed this over to use QOM sub-classes - the actual names of the different parts >>> are used to create a set of dynamic classes which passes the part info as class >>> data. The object no longer needs to search the known_devices table for itself, >>> instead it just gets its info from its own class. >>> >>> Kept the intermediate class definition private to m25p80.c for the moment, as >>> the expectation is parts will only be added as new entries in the table. We can >>> factor out the TYPE_M25P80 abstraction into a header on a demand basis. >>> >>> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> >> Reviewed-by: Andreas Färber <afaerber@suse.de> >> >> Looks fine to me except that I would suggest to #define a TYPE_N25Q128 >> constant. No typos though, so that can be done later. >> >> Cheers, >> Andreas >> >> -- >> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany >> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >>
On 7 February 2013 05:51, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > Currently, M25P80 uses an object property to differentiate between flash parts. > Changed this over to use QOM sub-classes - the actual names of the different parts > are used to create a set of dynamic classes which passes the part info as class > data. The object no longer needs to search the known_devices table for itself, > instead it just gets its info from its own class. > > Kept the intermediate class definition private to m25p80.c for the moment, as > the expectation is parts will only be added as new entries in the table. We can > factor out the TYPE_M25P80 abstraction into a header on a demand basis. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> Thanks; added to arm-devs.next. -- PMM
diff --git a/hw/m25p80.c b/hw/m25p80.c index 788c196..fbb9f19 100644 --- a/hw/m25p80.c +++ b/hw/m25p80.c @@ -178,8 +178,6 @@ static const FlashPartInfo known_devices[] = { /* Numonyx -- n25q128 */ { INFO("n25q128", 0x20ba18, 0, 64 << 10, 256, 0) }, - - { }, }; typedef enum { @@ -235,11 +233,23 @@ typedef struct Flash { int64_t dirty_page; - char *part_name; const FlashPartInfo *pi; } Flash; +typedef struct M25P80Class { + SSISlaveClass parent_class; + FlashPartInfo *pi; +} M25P80Class; + +#define TYPE_M25P80 "m25p80-generic" +#define M25P80(obj) \ + OBJECT_CHECK(Flash, (obj), TYPE_M25P80) +#define M25P80_CLASS(klass) \ + OBJECT_CLASS_CHECK(M25P80Class, (klass), TYPE_M25P80) +#define M25P80_GET_CLASS(obj) \ + OBJECT_GET_CLASS(M25P80Class, (obj), TYPE_M25P80) + static void bdrv_sync_complete(void *opaque, int ret) { /* do nothing. Masters do not directly interact with the backing store, @@ -556,23 +566,9 @@ static int m25p80_init(SSISlave *ss) { DriveInfo *dinfo; Flash *s = FROM_SSI_SLAVE(Flash, ss); - const FlashPartInfo *i; + M25P80Class *mc = M25P80_GET_CLASS(s); - if (!s->part_name) { /* default to actual m25p80 if no partname given */ - s->part_name = (char *)"m25p80"; - } - - i = known_devices; - for (i = known_devices;; i++) { - assert(i); - if (!i->part_name) { - fprintf(stderr, "Unknown SPI flash part: \"%s\"\n", s->part_name); - return 1; - } else if (!strcmp(i->part_name, s->part_name)) { - s->pi = i; - break; - } - } + s->pi = mc->pi; s->size = s->pi->sector_size * s->pi->n_sectors; s->dirty_page = -1; @@ -620,34 +616,42 @@ static const VMStateDescription vmstate_m25p80 = { } }; -static Property m25p80_properties[] = { - DEFINE_PROP_STRING("partname", Flash, part_name), - DEFINE_PROP_END_OF_LIST(), -}; - static void m25p80_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); SSISlaveClass *k = SSI_SLAVE_CLASS(klass); + M25P80Class *mc = M25P80_CLASS(klass); k->init = m25p80_init; k->transfer = m25p80_transfer8; k->set_cs = m25p80_cs; k->cs_polarity = SSI_CS_LOW; - dc->props = m25p80_properties; dc->vmsd = &vmstate_m25p80; + mc->pi = data; } static const TypeInfo m25p80_info = { - .name = "m25p80", + .name = TYPE_M25P80, .parent = TYPE_SSI_SLAVE, .instance_size = sizeof(Flash), - .class_init = m25p80_class_init, + .class_size = sizeof(M25P80Class), + .abstract = true, }; static void m25p80_register_types(void) { + int i; + type_register_static(&m25p80_info); + for (i = 0; i < ARRAY_SIZE(known_devices); ++i) { + TypeInfo ti = { + .name = known_devices[i].part_name, + .parent = TYPE_M25P80, + .class_init = m25p80_class_init, + .class_data = (void *)&known_devices[i], + }; + type_register(&ti); + } } type_init(m25p80_register_types) diff --git a/hw/petalogix_ml605_mmu.c b/hw/petalogix_ml605_mmu.c index bdfc6ce..4741278 100644 --- a/hw/petalogix_ml605_mmu.c +++ b/hw/petalogix_ml605_mmu.c @@ -156,8 +156,7 @@ petalogix_ml605_init(QEMUMachineInitArgs *args) for (i = 0; i < NUM_SPI_FLASHES; i++) { qemu_irq cs_line; - dev = ssi_create_slave_no_init(spi, "m25p80"); - qdev_prop_set_string(dev, "partname", "n25q128"); + dev = ssi_create_slave_no_init(spi, "n25q128"); qdev_init_nofail(dev); cs_line = qdev_get_gpio_in(dev, 0); sysbus_connect_irq(busdev, i+1, cs_line); diff --git a/hw/xilinx_zynq.c b/hw/xilinx_zynq.c index 0ac33b5..3a33c97 100644 --- a/hw/xilinx_zynq.c +++ b/hw/xilinx_zynq.c @@ -82,8 +82,7 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, spi = (SSIBus *)qdev_get_child_bus(dev, bus_name); for (j = 0; j < num_ss; ++j) { - flash_dev = ssi_create_slave_no_init(spi, "m25p80"); - qdev_prop_set_string(flash_dev, "partname", "n25q128"); + flash_dev = ssi_create_slave_no_init(spi, "n25q128"); qdev_init_nofail(flash_dev); cs_line = qdev_get_gpio_in(flash_dev, 0);
Currently, M25P80 uses an object property to differentiate between flash parts. Changed this over to use QOM sub-classes - the actual names of the different parts are used to create a set of dynamic classes which passes the part info as class data. The object no longer needs to search the known_devices table for itself, instead it just gets its info from its own class. Kept the intermediate class definition private to m25p80.c for the moment, as the expectation is parts will only be added as new entries in the table. We can factor out the TYPE_M25P80 abstraction into a header on a demand basis. Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> --- hw/m25p80.c | 58 ++++++++++++++++++++++++--------------------- hw/petalogix_ml605_mmu.c | 3 +- hw/xilinx_zynq.c | 3 +- 3 files changed, 33 insertions(+), 31 deletions(-)