On Sat, Dec 10, 2011 at 16:29, Jan Kiszka <jan.kiszka@xxxxxx> wrote: > 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. Not in this case, but in general boards could need to pass different sets of properties or avoid passing something at all. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
