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