On 7 December 2011 10:58, Evgeny Voevodin <e.voevo...@samsung.com> wrote: > On 12/07/2011 02:09 PM, Peter Maydell wrote: >> >> On 7 December 2011 09:47, Evgeny Voevodin<e.voevo...@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.
If you're adding a legitimate missing feature (16 bit access support) to lan9118 then your commit message should say so, not talk about 9215. The right place to say "this should be a 9215 but the 9118 is close enough" is in the patch adding the network chip to the board model, not in the commit adding a feature to the network chip model. We should probably make 16 vs 32 bit be a qdev property of lan9118 (corresponding to the way the hardware is configured by an input pin on startup) and reflect this properly in the HW_CFG register. Also, this todo comment is too obscure: + /* TODO: Implement fair word operation, because this implementation + * assumes that any register is accessed as two 16-bit operations. */ I have no idea what it means. -- PMM
