On Mon, Jan 15, 2018 at 11:47 AM, Peter Maydell
<peter.mayd...@linaro.org> wrote:
> On 15 January 2018 at 14:04, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>> On 01/15/2018 10:51 AM, Peter Maydell wrote:
>>> On 13 January 2018 at 05:07, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>>>> Since v6:
>>>> - addressed Peter reviews
>>>> - do not use an unique Property[] for both sysbus/pci
>>>> Peter didn't recommend me to use the qdev_property_add_static() API
>>>> since
>>>> it is only used by the ARM cpus and may be due for removal, however I
>>>> found
>>>> it cleaner.
>>>
>>> Your cover letter says this but patches 3 and 14 in the v7 you sent
>>> to the list use qdev_property_add_static(). Did you send the wrong
>>> version of the code?
>>
>> The cover is not clear, I'll try to reword it:
>>
>> '''
>> Peter recommended me to NON use the qdev_property_add_static() API since
>> it is only used by the ARM cpus and may be due for removal.
>>
>> However I found using qdev_property_add_static() cleaner, and sent this
>> series with using the not recommended qdev_property_add_static().
>> '''
>
>> Is it possible to share properties without using qdev_property_add_static()?
>
> Not very neatly. There's the DEFINE_BLOCK_PROPERTIES approach that
> include/hw/block/block.h has.
>
> In this case there aren't very many properties involved so I would
> just leave them as two separate Property arrays.
>
> (The Arm cpu models are odd because they dynamically decide which
> properties they have based on the CPU model. That isn't the case here:
> these devices always have the same set of properties.)
Yes, both sysbus/pci devices share:
static Property sdhci_common_properties[] = {
DEFINE_PROP_UINT8("sd-spec-version", SDHCIState, spec_version, 2),
/* Timeout clock frequency 1-63, 0 - not defined */
DEFINE_PROP_UINT8("timeout-freq", SDHCIState, cap.timeout_clk_freq, 0),
/* Timeout clock unit 0 - kHz, 1 - MHz */
DEFINE_PROP_BOOL("freq-in-mhz", SDHCIState, cap.timeout_clk_in_mhz, true),
/* Maximum base clock frequency for SD clock in MHz (range 10-63 MHz, 0) */
DEFINE_PROP_UINT8("max-frequency", SDHCIState, cap.base_clk_freq_mhz, 0),
/* Maximum host controller R/W buffers size
* Possible values: 512, 1024, 2048 bytes */
DEFINE_PROP_UINT16("max-block-length", SDHCIState, cap.max_blk_len, 512),
/* DMA */
DEFINE_PROP_BOOL("sdma", SDHCIState, cap.sdma, true),
DEFINE_PROP_BOOL("adma1", SDHCIState, cap.adma1, false),
DEFINE_PROP_BOOL("adma2", SDHCIState, cap.adma2, true),
/* Suspend/resume support */
DEFINE_PROP_BOOL("suspend", SDHCIState, cap.suspend, false),
/* High speed support */
DEFINE_PROP_BOOL("high-speed", SDHCIState, cap.high_speed, true),
/* Voltage support 3.3/3.0/1.8V */
DEFINE_PROP_BOOL("3v3", SDHCIState, cap.v33, true),
DEFINE_PROP_BOOL("3v0", SDHCIState, cap.v30, false),
DEFINE_PROP_BOOL("1v8", SDHCIState, cap.v18, false),
DEFINE_PROP_UINT8("uhs", SDHCIState, cap.uhs_mode, UHS_NOT_SUPPORTED),
DEFINE_PROP_BOOL("64bit", SDHCIState, cap.bus64, false),
DEFINE_PROP_UINT8("slot-type", SDHCIState, cap.slot_type, 0),
DEFINE_PROP_UINT8("bus-speed", SDHCIState, cap.sdr, 0),
DEFINE_PROP_UINT8("driver-strength", SDHCIState, cap.strength, 0),
DEFINE_PROP_UINT64("maxcurr", SDHCIState, maxcurr, 0),
DEFINE_PROP_END_OF_LIST(),
};
The only 2 properties specific to sysbus are:
static Property sdhci_sysbus_properties[] = {
DEFINE_PROP_BOOL("pending-insert-quirk", SDHCIState, pending_insert_quirk,
false),
DEFINE_PROP_LINK("dma", SDHCIState, dma_mr,
TYPE_MEMORY_REGION, MemoryRegion *),
}
I guess I assumed the Property to be const (being compilation-time
allocated, ended with DEFINE_PROP_END_OF_LIST),
so I couldn't add more properties to it, but it seems each device has
his properties allocated at runtime, so I can use the same
sdhci_common_properties[] array and only use
qdev_property_add_static() for the 2 sysbus specific properties.
Does this sound alright to you?
Thanks,
Phil.
