Re: [Qemu-devel] [PATCH v4 2/2] Add GE IP-Octal 232 IndustryPack emulation

[Alberto Garcia <agarcia@igalia.com>]

On Tue, Jan 08, 2013 at 07:08:57PM +0100, Andreas Färber wrote:

> On my suggestion you changed the type name from "ipoctal" to
> "ipoctal232" - the file name should probably then be updated, too.
> For the function prefixes inside the file it shouldn't matter as
> long as they're consistent.

The reason why I decided to stay with 'ipoctal' everywhere except in
the visible fields (TypeInfo.name, and VMStateDescription.name) is
because the other devices of the IP-Octal family are almost identical
from the emulation point of view, so if a new one ever gets emulated
most code will be shared.

I'm not sure if it makes sense to rename anything else in this
situation...

> > +struct IPOctalState {
> > +    IPackDevice dev;
> 
> By convention:
> 
> IPackDevice parent_obj;

I only see that in a few places, is that a recent thing? Most devices
that I see use 'dev' or similar to refer to their parent objects.

> But if, as brought up at LinuxCon, you may want to add qtest cases
> it may be helpful to move some of the constants above to their
> own header file ipoctal232_regs.h as done for RTC or pending for
> TMP105. That could be split out in a later step though, once PCI
> libqos is available.

Yeah, I'm not sure if I'll have much time to work on this in the short
term...

> P.S. No need to CC avi@redhat.com any longer, you may have noticed.
> ;)

Yes, I did :)

Berto