On 2011-12-10 17:26, Blue Swirl wrote: > On Sat, Dec 10, 2011 at 16:03, Jan Kiszka <jan.kiszka@xxxxxx> wrote: >> On 2011-12-10 16:54, Blue Swirl wrote: >>> On Sat, Dec 10, 2011 at 15:51, Jan Kiszka <jan.kiszka@xxxxxx> wrote: >>>> On 2011-12-10 16:49, Blue Swirl wrote: >>>>>> >>>>>> +ISADevice *pit_init(int base, qemu_irq irq) >>>>> >>>>> Please retain this function in pc.h, or even better, introduce i8254.h. >>>> >>>> No concerns about i8254.h, but this function does not qualify for static >>>> inline. >>> >>> The function is static inline in a header file not for performance >>> reasons, but to keep the instantiation separate from device internals. >> >> Not performance, footprint and header dependencies. You need to pull in >> all the stuff the inline function needs for everyone including the >> header that contains this function. That's messy. > > There's only ISA and qdev stuff, that's not messy since both are > needed in any case. > >> Even if the instantiation helper should not poke into the device model >> internals (and I don't want this to change as well), it belongs to the >> module that implements the device. We do the same with other fabric >> functions. > > In this case, the callers have the same needs and there are several of > them. In general this need not be true at all, if for example some > part of instantiation would have to be skipped, the functions may need > to be manually inlined to the board level anyway. The instantiation > definitely does not belong to the implementer but to the creator. > Ideally file implementing the device contains only static functions > and instantiation is either in a header file or at the board. This is > true for example for several Sparc32 devices. The helper is wrapping the property base API into a proper function call - nothing that is board-specific. Jan
Attachment:
signature.asc
Description: OpenPGP digital signature