On Thu, Sep 28, 2017 at 07:40:18AM +0100, Mark Cave-Ayland wrote: > 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 <[email protected]> > > > > 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.
Instead you're using the method which is defined in a global type
definition. I don't think it really makes any different to
encapsulation.
> If I were to redo the last 2 patches using a proper class, would you
> accept them?
>
>
> ATB,
>
> Mark.
>
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
signature.asc
Description: PGP signature
