Re: [RFC PATCH 06/10] hw/avr: Add ATmega microcontrollers

[dovgaluk <dovgaluk@ispras.ru>]

Aleksandar Markovic писал 2019-11-28 12:28:
> On Thursday, November 28, 2019, Philippe Mathieu-Daudé
> <f4bug@amsat.org> wrote:
> 
>> Add famous ATmega MCUs:
>> 
>> - middle range: ATmega168 and ATmega328
>> - high range: ATmega1280 and ATmega2560
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
> 
> Philippe, hi.
> 
> Thank you for the impetus you give us all.
> 
> However, is this the right direction?
> 
> Let's analyse some bits and pieces.
> 
> Starting from the commit message, the word "famous" is used, but I
> really don't see enumerated CPUs/MCUs are any special in Atmel lineup.
> Other than we often used the doc describing them (cited several times
> in our discussions) as our reference, but that doesn't make them
> "famous". Ofcourse, there other docs for other Atmel CPUs/MCUs, of at
> lest equivalent significance. For example, "tiny" ones are at least as
> famous as "mega" ones.
> 
> Then, you introduce the term MCU, without proper discussion with
> others on terminology. In parlance of QEMU, ATmega168 at al. are CPUs
> (we all know and assume that that are some peripherals in it). I am
> not against using the term MCU, but let's first sync up on that.
> 
> The added terminology trouble is that MCUs, as you defined them, have
> in array atmega_mcu[] a field called "cpu_type" - why is that field
> not called "mcu_type"? *Very* confusing for any future reader. And
> then, similar terminology conundrum continues with macro
> AVR_CPU_TYPE_NAME().

MCU is a system-on-chip which includes CPU core and peripheral devices.
Separating this is better that including everything into the machine.

E.g., different MCUs may have different IO addresses for USART.

> All of the above is far less important than this question: What are we
> achieving with proposed CPU/MCU definitions? I certainly see the value
> of fitting the complex variety of AVR CPUs/MCUs into QEMU object
> model. No question about that. However, is this the right moment to do
> it? There are still some unresolved architectural problems with
> peripheral definitions, as I noted in yhe comment to Michael's last
> cover letter. This patch does not solve them. It just assumes
> everything is ready with peripherals, let's build CPUs/MCUs. The
> machines based on proposed CPUs/MCUs are not more real that machine
> based on Michael's "sample" machine. At least Michal says "this is not
> a real machine". If we used proposed CPUs/MCUs from this patch, the
> resulting machine is as "paper" machine as yhe "sample" machine. We
> would just live in a la-la lend of thinking: "wow, we model real
> hardware now".
> 
> I would rather accept into QEMU a series admitting we are still far
> from modelling real machine or CPU/MCU, than a series that gives an
> illusion that we are modelling real machine or CPU/MCU.
> 
> As far as utility of this patch for Michael's series, it looks to me
> they add more headake than help (not saying that the help is not
> present) to Michael. He would have anotter abstraction layer to think
> of, at the moment he desperately needs (in my opinion) to focus on
> providing the as solid as possible, and as complete as possinle
> foundations. This patch looks like building castles in the air. Again,
> I am not claiming that the patch is not helpful at all.
> 
> In summary, I think that this patch is premature.
> 


Pavel Dovgalyuk