On 12 March 2018 at 12:36, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
> On 03/09/2018 06:06 PM, Peter Maydell wrote:
>> On 9 March 2018 at 15:36, Philippe Mathieu-Daudé <f4...@amsat.org> wrote:
>>> [based on a patch from Alistair Francis <alistair.fran...@xilinx.com>
>>>  from qemu/xilinx tag xilinx-v2015.2]
>>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>>> ---
>>>  hw/sd/sd.c         | 148 
>>> +++++++++++++++++++++++++++++++++++++++++++++--------
>>>  hw/sd/trace-events |   1 +
>>>  2 files changed, 127 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>>> index 235e0518d6..b907d62aef 100644
>>> --- a/hw/sd/sd.c
>>> +++ b/hw/sd/sd.c
>>> @@ -124,6 +124,7 @@ struct SDState {
>>>      bool enable;
>>>      uint8_t dat_lines;
>>>      bool cmd_line;
>>> +    bool uhs_enabled;
>>
>> Oh, and what's the difference between s->uhs_enabled = false
>> (this patch) and s->uhs_mode = UHS_NOT_SUPPORTED (next patch) ?
>>
>> Do we need both? If so, a comment noting the difference would help
>> people to know which one various bits of code should be checking.
>
> Ok.
>
> The 'uhs_mode' is a read-only property before realize().
> Now if the sdcard supports any UHS, the host may negotiate to switch to
> UHS mode.
> To enter this mode with voltage level adjusted, the card needs to
> 'soft-reset' itself in the new mode. We use 'uhs_enabled' to track this
> runtime state. Maybe 'uhs_active' is more explicit?
> I intended to keep the properties vs runtime fields separated in the
> SDState.

Yes, static property vs runtime changeable should indeed be kept
separate -- we should just make sure it's clear which is which,
with a suitable comment and/or field name.

thanks
-- PMM

Reply via email to