Re: [Qemu-devel] [Qemu-arm] [PATCH 1/2] hw/adc: Add basic Aspeed ADC model

[Andrew Jeffery <andrew@aj.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1628 bytes --]

On Wed, 2017-05-24 at 02:15 -0300, Philippe Mathieu-Daudé wrote:
> Hi Andrew,
> 
> On 05/22/2017 02:14 AM, Andrew Jeffery wrote:
> > On Mon, 2017-05-22 at 03:15 +0000, Ryan Chen wrote:
> > > In ASPEED SoC chip, all register access have following rule.
> > > Most of controller write access is only support 32bit access.
> > > Read is support 8bits/16bits/32bits.
> 
> This makes sens thinking about how a DMA controller can take advantage 
> of the ADC.
> 
> > 
> > Thanks for clearing that up Ryan.
> > 
> > Phil: I'll rework the model so the reads are 16-bits.
> 
> This shouldn't be necessary, QEMU is supposed to supports different 
> access size for different implemented size, so you can declare your 
> implementation as 32-bit and valid accesses from 8 to 32:
> 
>   static const MemoryRegionOps aspeed_adc_ops = {
>       .read = aspeed_adc_read,
>       .write = aspeed_adc_write,
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .valid.min_access_size = 1,
>       .valid.max_access_size = 4,
>       .valid.unaligned = false,
> +    .impl.min_access_size = 4,
> +    .impl.max_access_size = 4,
>   };
> 
> This way an I/O access from the CPU or a DMA could use 8/16-bit while 
> you keep a 32-bit implementation. The adjustment is done by 
> access_with_adjusted_size() from memory.c.
> 
> Afaik there is, however, no distinction between read/write different 
> access size in current QEMU MemoryRegionOps model.

Yep, I realised all of the above when I went to implement it. Thanks
for pointing it out though!

Andrew

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]