On 05/09/2018 12:42 PM, Alistair Francis wrote:
> On Tue, May 8, 2018 at 11:01 PM, Philippe Mathieu-Daudé <f4...@amsat.org>
> wrote:
>> Add more descriptive comments to keep a clear separation
>> between static property vs runtime changeable.
>>
>> Suggested-by: Peter Maydell <peter.mayd...@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org>
>> ---
>> hw/sd/sd.c | 14 +++++++++-----
>> 1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 235e0518d6..5fb4787671 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -90,12 +90,15 @@ struct SDState {
>> uint32_t card_status;
>> uint8_t sd_status[64];
>>
>> - /* Configurable properties */
>> + /* Static properties */
>> +
>> BlockBackend *blk;
>> bool spi;
>>
>> - uint32_t mode; /* current card mode, one of SDCardModes */
>> - int32_t state; /* current card state, one of SDCardStates */
>> + /* Runtime changeables */
>> +
>> + uint32_t mode; /** current card mode, one of #SDCardModes */
>> + int32_t state; /** current card state, one of #SDCardStates */
>> uint32_t vhs;
>> bool wp_switch;
>> unsigned long *wp_groups;
>> @@ -109,8 +112,9 @@ struct SDState {
>> uint32_t pwd_len;
>> uint8_t function_group[6];
>> uint8_t current_cmd;
>> - /* True if we will handle the next command as an ACMD. Note that this
>> does
>> - * *not* track the APP_CMD status bit!
>> + /**
>> + * #True if we will handle the next command as an ACMD.
>
> Why do we need a # here?
I used the CPUState as a documentation example, but this might not be
the correct use...
/**
* CPUState:
* @running: #true if CPU is currently running (lockless).
...
$ git grep -Ei '#(true|false)'
include/exec/memory.h:164: * If present, and returns #false, the
transaction is not accepted
include/qom/cpu.h:280: * @running: #true if CPU is currently running
(lockless).
include/qom/cpu.h:281: * @has_waiter: #true if a CPU is currently
waiting for the cpu_exec_end;
MemoryRegionOps does not use the double '/**' style:
bool unaligned;
/*
* If present, and returns #false, the transaction is not accepted
* by the device (and results in machine dependent behaviour such
* as a machine check exception).
*/
Peter if you take this, feel free to drop this '#'.
>
> Otherwise:
>
> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Thanks!
>
> Alistair
>
>
>> + * Note that this does *not* track the APP_CMD status bit!
>> */
>> bool expecting_acmd;
>> uint32_t blk_written;
>> --
>> 2.17.0
>>
>>
