On Thu, Feb 7, 2013 at 10:13 PM, Andreas Färber <afaer...@suse.de> wrote: > Am 07.02.2013 13:04, schrieb Peter Maydell: >> On 7 February 2013 11:50, Andreas Färber <afaer...@suse.de> wrote: >>> Am 07.02.2013 07:59, schrieb Kuo-Jung Su: >>>>>> +/****************************************************************************** >>>>>> + * FTSPI020 registers >>>>>> + >>>>>> *****************************************************************************/ >>>>>> +#define REG_CMD0 0x00 /* Flash address */ >>>>>> +#define REG_CMD1 0x04 >>>>>> +#define REG_CMD2 0x08 >>>>>> +#define REG_CMD3 0x0c >>>>>> +#define REG_CTRL 0x10 /* Control Register */ >>>>>> +#define REG_ACT 0x14 /* AC Timing Register */ >>>>>> +#define REG_STR 0x18 /* Status Register */ >>>>>> +#define REG_ICR 0x20 /* Interrupt Control Register */ >>>>>> +#define REG_ISR 0x24 /* Interrupt Status Register */ >>>>>> +#define REG_RDST 0x28 /* Read Status Register */ >>>>>> +#define REG_REV 0x50 /* Revision Register */ >>>>>> +#define REG_FEA 0x54 /* Feature Register */ >>>>>> +#define REG_DATA 0x100 /* Data Register */ >>>>>> + >>>>> >>>>> These can just live in the C file - they are private to the >>>>> implementation. >>> >>> No. Having them in a header means they can be reused by qtests. >> >> Do we really want to change the public/private boundaries of >> our source code just for the benefit of the test framework? Ugh. > > The truth is (and that applies to the hw/ refactoring series as well) > that we don't have a clear distinction between "public" and "private" > headers for devices. >
I guess a logical conclusion here is that defines that relate to the programmers model (eg #define REG_ISR 0x0C/4) and parameterization (e.g. #define NUM_CHIP_SELECTS 4) need to be public while everything else (implementation stuff) is private. In theory however, the only client of a programmers model is unit testing. So perhaps there should be some sort of poisoning system like with the cpu headers? Your only allowed to include a header for a concrete class (such as this) if you own it, or if your a test. >>> To embed the device into a SoC object, the state struct and TYPE_ >>> constant should be in the header. That applies to each device part of >>> the SoC rather than on the board. >> I really think this is a step backwards. Every little lightweight device model needs its own header just for #define TYPE_FOO "foo" Regards, Peter >> I would like it if we could define what we consider to be best >> practice for public/private header files for QOM devices >> (in particular whether we need a public header for every device >> with the TYPE_ constants &c). > > gtk-doc for instance needs to scan one directory, so having all headers > in include/ would be beneficial long-term - today we do not get > generated documentation for qdev-core.h. > > The separation between PCI internal helpers and > structs/typedefs-to-be-used is made by including specific headers > depending on what you need. > > In the refcatoring thread I suggested that if we want to have all SPI > devices in hw/spi/ then each device that needs a public function > declaration or TYPE_ or struct should export its own header. If we group > them into a common directory such as hw/arm/ that could be shared across > SoC devices. Therefore I was suggesting not to split up too much but > seem to be overruled. ;) There's downsides to everything. > > Andreas > > -- > SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany > GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg >