Re: [PATCH for-5.2 04/19] aspeed/scu: Fix valid access size on AST2400

From: "Cédric Le Goater" <clg@kaod.org>
To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>,
	"Peter Maydell" <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>,
	erik-smit <erik.lucas.smit@gmail.com>,
	qemu-arm@nongnu.org, qemu-devel@nongnu.org,
	Joel Stanley <joel@jms.id.au>
Subject: Re: [PATCH for-5.2 04/19] aspeed/scu: Fix valid access size on AST2400
Date: Thu, 6 Aug 2020 15:49:42 +0200	[thread overview]
Message-ID: <bd58f180-1c18-c998-5fa7-da41177deb02@kaod.org> (raw)
In-Reply-To: <4a41f234-ab81-d078-0fcd-c1b20fceb1e9@amsat.org>

On 8/6/20 3:32 PM, Philippe Mathieu-Daudé wrote:
> On 8/6/20 3:20 PM, Cédric Le Goater wrote:
>> The read access size of the SCU registers can be 1/2/4 bytes and write
>> is 4 bytes. Set the min access size to 1 byte to cover both read and
>> write operations on the AST2400 but keep the min access size of the
>> other SoCs to 4 bytes as this is an unusual access size.
> 
> From your description it seems you need to implement .valid.accepts().

Ah yes. 

Can this come as a follow up ? because this patch is enabling
support for the Supermicro X11 BMC machine.

Thanks,

C. 

> 
>>
>> This fixes support for some old firmware doing 2 bytes reads on the
>> AST2400 SoC.
>>
>> Reported-by: erik-smit <erik.lucas.smit@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  hw/misc/aspeed_scu.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/misc/aspeed_scu.c b/hw/misc/aspeed_scu.c
>> index ec4fef900e27..764222404bef 100644
>> --- a/hw/misc/aspeed_scu.c
>> +++ b/hw/misc/aspeed_scu.c
>> @@ -328,9 +328,10 @@ static const MemoryRegionOps aspeed_ast2400_scu_ops = {
>>      .read = aspeed_scu_read,
>>      .write = aspeed_ast2400_scu_write,
>>      .endianness = DEVICE_LITTLE_ENDIAN,
>> -    .valid.min_access_size = 4,
>> -    .valid.max_access_size = 4,
>> -    .valid.unaligned = false,
>> +    .valid = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 4,
>> +    },
>>  };
>>  
>>  static const MemoryRegionOps aspeed_ast2500_scu_ops = {
>>
> 