Re: [Qemu-arm] [PATCH 4/7] m25p80: add a m25p80_set_rom_storage() routine

["mar.krzeminski" <mar.krzeminski@gmail.com>]

W dniu 04.07.2016 o 20:12, Cédric Le Goater pisze:
> On 07/04/2016 07:57 PM, mar.krzeminski wrote:
>>
>> W dniu 04.07.2016 o 14:18, Cédric Le Goater pisze:
>>> Some SPI controllers, such as the Aspeed AST2400, have a mode in which
>>> accesses to the flash content are no different than doing MMIOs. The
>>> controller generates all the necessary commands to load (or store)
>>> data in memory.
>>>
>>> To emulate such a behavior, we need to have access to the underlying
>>> storage of the flash object. The purpose of the routine is to set the
>>> storage of a preinitialized SPI flash object, which can then be used
>>> by the object itself but also by a SPI controller to handled the
>>> MMIOs.
>> Hi,
>>
>> I am not sure if this approach is correct. I can not find any datasheet
>> to this SPI controller, but as you mentioned in first paragraph, controller
>> generates all commands (probably ones are somehow configurable).
> yes. see this patch :
>
> 	http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg08229.html
>
> 	/* CEx Control Register */
> 	#define R_CTRL0           (0x10 / 4)
> 	#define   CTRL_CMD_SHIFT           16
> 	#define   CTRL_CMD_MASK            0xff
>
>
>> In this series you hack this behaviour and you do direct access to file.
> It is true it is not very respectful of the m25p80 interface.
>
>> IMHO you should emulate sending such commands in SPI controller
>> model.
> I will give it a try. I don't think the alternative is a complex
> change anyhow.
Register logic + few commands, then write is almost copy-paste.
Shouldn’t be complicated, but you have real HW modelling.

Thanks,
Marcin
> Thanks,
>
> C.
>
>> Thanks,
>> Marcin
>>
>>> This is sufficient to support read only accesses. For writes, we would
>>> certainly need to externalize flash_write8() routine.
>>>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>    hw/block/m25p80.c        | 14 +++++++++++++-
>>>    include/hw/block/flash.h |  2 ++
>>>    2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
>>> index aa964280beab..fec0fd4c2228 100644
>>> --- a/hw/block/m25p80.c
>>> +++ b/hw/block/m25p80.c
>>> @@ -29,6 +29,7 @@
>>>    #include "qemu/bitops.h"
>>>    #include "qemu/log.h"
>>>    #include "qapi/error.h"
>>> +#include "hw/block/flash.h"
>>>      #ifndef M25P80_ERR_DEBUG
>>>    #define M25P80_ERR_DEBUG 0
>>> @@ -1152,7 +1153,11 @@ static void m25p80_realize(SSISlave *ss, Error **errp)
>>>          if (s->blk) {
>>>            DB_PRINT_L(0, "Binding to IF_MTD drive\n");
>>> -        s->storage = blk_blockalign(s->blk, s->size);
>>> +
>>> +        /* using an external storage. see m25p80_create_rom() */
>>> +        if (!s->storage) {
>>> +            s->storage = blk_blockalign(s->blk, s->size);
>>> +        }
>>>              if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
>>>                error_setg(errp, "failed to read the initial flash content");
>>> @@ -1259,3 +1264,10 @@ static void m25p80_register_types(void)
>>>    }
>>>      type_init(m25p80_register_types)
>>> +
>>> +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom)
>>> +{
>>> +    Flash *s = M25P80(dev);
>>> +
>>> +    s->storage = memory_region_get_ram_ptr(rom);
>>> +}
>>> diff --git a/include/hw/block/flash.h b/include/hw/block/flash.h
>>> index 50ccbbcf1352..9d780f4ae0c9 100644
>>> --- a/include/hw/block/flash.h
>>> +++ b/include/hw/block/flash.h
>>> @@ -61,4 +61,6 @@ uint8_t ecc_digest(ECCState *s, uint8_t sample);
>>>    void ecc_reset(ECCState *s);
>>>    extern VMStateDescription vmstate_ecc_state;
>>>    +void m25p80_set_rom_storage(DeviceState *dev, MemoryRegion *rom);
>>> +
>>>    #endif
>