Re: [Qemu-arm] [PATCH 1/7] tests: add a m25p80 test

[Cédric Le Goater]

On 07/04/2016 02:51 PM, Peter Maydell wrote:
> On 4 July 2016 at 13:39, Cédric Le Goater <address@hidden> wrote:
>> On 07/04/2016 02:24 PM, Peter Maydell wrote:
>>> On 4 July 2016 at 13:18, Cédric Le Goater <address@hidden> wrote:
>>>> This test uses the palmetto platform and the AST2400 SPI controller to
>>>> test the m25p80 flash module device model. The flash model is defined
>>>> by the platform (n25q256a) and it would be nice to find way to control
>>>> it, using a property probably.
>>>
>>>
>>>> +static inline void flash_writel(uint32_t data)
>>>> +{
>>>> +    data = cpu_to_be32(data);
>>>> +    memwrite(AST2400_FLASH_BASE, &data, 4);
>>>> +}
>>>> +
>>>> +static inline uint32_t flash_readl(void)
>>>> +{
>>>> +    uint32_t data;
>>>> +
>>>> +    memread(AST2400_FLASH_BASE, &data, 4);
>>>> +    return be32_to_cpu(data);
>>>> +}
>>>
>>> These are weird. As per my question in the other thread, if you
>>> were writing this as a piece of test code that ran natively
>>> in the guest, how would you write it?
>>
>> Here is the uboot flash code :
>>
>>         
>> https://github.com/openbmc/u-boot/blob/v2016.05-aspeed-openbmc/arch/arm/mach-aspeed/flash.c#L420
>>
>> So these are doing byte per byte, but there is no reason not to do
>> long.
> 
> If you want "write a long", that is what writel does,
> and your test code should be able to use it in exactly
> the same way native guest code would write a register
> value with a str instruction.
> 
> It's very easy to work around endianness issues by
> randomly changing things until the tests pass, but it's
> much better to actually identify what you need to do
> and where the code is diverging from that.

I agree. Let's forget that patch until I have clarified
how it should be done.

Thanks,

C.