Re: hw/misc/aspeed_scu: 5d971f9e breaks Supermicro AST2400

[Philippe Mathieu-Daudé]

Hi Joel,

On 7/1/20 4:32 AM, Joel Stanley wrote:
> On Wed, 1 Jul 2020 at 01:23, Ryan Chen <ryan_chen@aspeedtech.com> wrote:
> 
>> On Tue, 30 Jun 2020 at 15:32, Erik Smit <erik.lucas.smit@gmail.com> wrote:
>>>
>>> Hi Philippe,
>>>
>>> On Tue, 30 Jun 2020 at 17:29, Philippe Mathieu-Daudé <f4bug@amsat.org> 
>>> wrote:
>>>>
>>>> Hi Erik,
>>>>
>>>> On 6/30/20 5:11 PM, Erik Smit wrote:
>>>>> Hello,
>>>>>
>>>>> 5d971f9e memory: Revert "memory: accept mismatching sizes in
>>>>> memory_region_access_valid" breaks Supermicro AST2400 u-boot.
>>>>> Supermicro AST2500 u-boot is fine.
>>>>>
>>>>> The u-boot tries to make a 2-byte read from address 0x84, but
>>>>> aspeed_ast2400_scu_ops has min_access = 4. When I change
>>>>> min_access to
>>>>> 2 or revert above commit, u-boot boots again.
>>>>>
>>>>> Is changing min_access to 2 the right way to fix this?
>>
>> Ryan, do all addresses on the AST2400's APB respond to single byte accesses?
> 
>>         Most register is not support byte access, it need to refer the 
>> datasheet descript each area. [chapter : ARM Address Space Mapping]
> 
> Thanks Ryan.
> 
> Summarising that table:
> 
>  - most addresses are 4 byte write accesses.
>  - all addresses support 1/2/4 byte reads.
> 
> The regions that accept 1/2/4 byte access are the SDRAM, SRAM,
> ethernet MACs, video engine, the flash controller regions (2000_0000
> -> 2FFF_FFFF and 3000_0000 -> 3FFF_FFFF), and the AHB to LPC bridges
> (6000_0000 -> 6FFF_FFFF and 7000_0000 -> 7FFF_FFFF).

Useful info. We have:

static const hwaddr aspeed_soc_ast2400_memmap[] = {

    /* AHB ??? */

    [ASPEED_IOMEM]  = 0x1E600000,
    [ASPEED_FMC]    = 0x1E620000,
    [ASPEED_SPI1]   = 0x1E630000,
    [ASPEED_ETH1]   = 0x1E660000,
    [ASPEED_ETH2]   = 0x1E680000,
    [ASPEED_EHCI1]  = 0x1E6A1000,
    [ASPEED_VIC]    = 0x1E6C0000,
    [ASPEED_SDMC]   = 0x1E6E0000,
    [ASPEED_SCU]    = 0x1E6E2000,
    [ASPEED_XDMA]   = 0x1E6E7000,
    [ASPEED_ADC]    = 0x1E6E9000,
    [ASPEED_VIDEO]  = 0x1E700000,
    [ASPEED_SRAM]   = 0x1E720000,
    [ASPEED_SDHCI]  = 0x1E740000,

This block ^ contains "SRAM, ethernet MACs, video engine
that accept 1/2/4 byte access". Can we assume all peripherals
in [0x1E60.0000-0x1E7F.FFFF] accept 1/2/4 byte access?

    /* APB ??? */

    [ASPEED_GPIO]   = 0x1E780000,
    [ASPEED_RTC]    = 0x1E781000,
    [ASPEED_TIMER1] = 0x1E782000,
    [ASPEED_UART1]  = 0x1E783000,
    [ASPEED_UART5]  = 0x1E784000,
    [ASPEED_WDT]    = 0x1E785000,
    [ASPEED_PWM]    = 0x1E786000,
    [ASPEED_VUART]  = 0x1E787000,
    [ASPEED_LPC]    = 0x1E789000,
    [ASPEED_IBT]    = 0x1E789140,
    [ASPEED_I2C]    = 0x1E78A000,

    [ASPEED_SDRAM]  = 0x40000000,
};

The best description I could find is
http://i.cloud.opensystemsmedia.com/i__srcfa3ad69f86b70417ba72d4fcbbef08cd_paraf0d99c20bd457d46a92c72841873c47.jpeg
But the buses are not mentioned.

> 
> We don't model the AHB to LPC bridges in qemu, nor the legacy flash
> controller region at 1000_0000 -> 15FF_FFFF.
> 
>>>>
>>>> If you have access to the datasheet and can verify, then yes.
>>>> Else I suppose Cédric, Andrew or Joel can check for you.
>>>
>>> I do not have a datasheet. Aspeed seems quite picky about sharing this
>>> and I'm just a random researcher.
>>
>> Erik, can you point us to the image you're using?
>>
>> What is the command line are you testing with?
>>
>> Happy to fix the model to support random firmwares, as long as the fix is 
>> correct for other (open source) firmwares.
>>
>>>
>>> Best regards,
>>>
>>> Erik Smit
>