Re: [Qemu-devel] [PATCH 09/14] hw/lan9118.c: Basic byte/word/long access support.

[Evgeny Voevodin <e.voevodin@samsung.com>]

On 12/07/2011 02:09 PM, Peter Maydell wrote:
> On 7 December 2011 09:47, Evgeny Voevodin<e.voevodin@samsung.com>  wrote:
>> We included this chip into s5pc210 platform because SMDK board holds
>> lan9215 chip. Difference is that 9215 access is 16-bit wide and some
>> registers differ. By addition basic 16-bit access to 9118 emulation we
>> achieved ethernet controller support by Linux lernel on SMDK boards.
>
> If it differs then shouldn't we add a new qdev device for 9215 ?
> (sharing most of the implementation code, obviously)
>

This patch could be interpreted as lan9118 emulation expansion since 
this chip supports 16-bit access too. These changes don't cover all the 
difference between 9118 and 9215, but it's enough to provide network 
support to Samsung boards. When 9215 support will be added we can easily 
switch to this chip.

>>   static const MemoryRegionOps lan9118_mem_ops = {
>> -    .read = lan9118_readl,
>> -    .write = lan9118_writel,
>> +    .old_mmio = {
>> +        .read = { lan9118_readb, lan9118_readw, lan9118_readl, },
>> +        .write = { lan9118_writeb, lan9118_writew, lan9118_writel, },
>> +    },
>>      .endianness = DEVICE_NATIVE_ENDIAN,
>>   };
>
> This is going backwards -- the .old_mmio hooks are for backwards
> compatibility when converting old devices to MemoryRegions -- they
> shouldn't be added in new code.
>
> You need to make the lan9118_read/write functions look at their
> 'size' argument instead.
>
> -- PMM
>
>