On 26/09/17 04:47, David Gibson wrote: > On Sun, Sep 24, 2017 at 03:47:45PM +0100, Mark Cave-Ayland wrote: >> Using this we can change the MACIO_IDE instance to register the channel >> itself via a type method instead of requiring a separate >> DBDMA_register_channel() function. >> >> As a consequence of this it is now possible to remove the old external >> macio_ide_register_dma() function. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayl...@ilande.co.uk> > > Ok, two concerns about this. > > First, you've added the function pointer to the instance, not to the > class, which is not how a QOM method would normally be done.
Yeah I did think about whether I needed to create a full class but was torn since as you say there is only one implementation. > More generally, though, why is a method preferable to a plain > function? AFAICT it's not plausible that there will ever be more than > one implementation of the method. > > Same comments apply to patch 7/7. For me it's really for encapsulation. It seems a little odd requiring a global function to configure a QOM object to which I already have a reference. If I were to redo the last 2 patches using a proper class, would you accept them? ATB, Mark.
