On 2/19/19 2:41 PM, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <[email protected]> writes:
>
>> On 2/18/19 1:56 PM, Markus Armbruster wrote:
>>> flash.h's incomplete struct pflash_t is completed both in
>>> pflash_cfi01.c and in pflash_cfi02.c. The complete types are
>>> incompatible. This can hide type errors, such as passing a pflash_t
>>> created with pflash_cfi02_register() to pflash_cfi01_get_memory().
>>>
>>> Furthermore, POSIX reserves typedef names ending with _t.
Worth adding in CODING_STYLE 'Naming' section :)
>>>
>>> Rename the two structs to PFlashCFI01 and PFlashCFI02.
>>
>> Why not ParallelFlashCFIxx?
>
> Feels a bit long, and we abbreviate to pflash pretty consistently. That
> said, I'm not particularly enamored with my choice of name :)
>
>> Ideally ParallelFlashCFI would be an InterfaceInfo...
>
> You mean TYPE_CFI_PFLASH0{1,2} should be children of an abstract parent?
I'd use "TYPE_PFLASH_CFI0[12]".
---
The "Common Flash memory Interface" as stated is definitively an
interface :)
QEMU models the 2 most famous industry implementations:
- vendor 0x01: Intel (and Sharp)
- vendor 0x02: AMD (and Fujistu)
---
My first refactor attempt was to have both implementations inheritate
from an abstract parent, but since the migration structures are
different, it looked easier to me to extract an InterfaceInfo.
>From here my idea was to add an NorFlash abstract parent where to share
the block devices and some of the reset:
+-------------------+
| |
| NOR Flash |
| |
+-------------------+
| Parent |
| |
| |
+------------v---+ +---v------------+
| | | |
| PFlash01 | | PFlash02 |
| | | |
+----------------+ +----------------+
Child | |
| |
| Implements|
+---v-----------v---+
| |
| CFI Flash |
| |
+-------------------+
But since there is no consumer of the CFI InterfaceInfo, we can simply
go the way you suggested:
+-------------------+
| |
| CFI NOR Flash |
| |
+-------------------+
| Parent |
| |
+------------v---+ +---v------------+
| | | |
| PFlash01 | | PFlash02 |
| | | |
+----------------+ +----------------+
Child Child
