Re: [Qemu-devel] [PATCH v2 1/4] Add i.MX FEC Ethernet driver

[Andreas Färber]

Am 05.05.2013 15:14, schrieb Jean-Christophe DUBOIS:
> On 05/05/2013 05:11 AM, Peter Crosthwaite wrote:
>> Hi JC,
>>
>> On Sun, May 5, 2013 at 12:09 AM, Jean-Christophe DUBOIS
>> <address@hidden>  wrote:
>>> This is based on the mcf_fec.c FEC implementation for ColdFire.

Note that ColdFire is one of the least maintained targets in QEMU...

>>> +static int imx_fec_can_receive(NetClientState *nc)
>>> +{
>>> +    imx_fec_state *s = qemu_get_nic_opaque(nc);
>>> +
>> Andreas, do we care about QOM casts coming from random void* opaques?

Generally no. If we ever switch to C++ or some other OO language we'll
have to touch casts anyway.

Peter, please note that I usually don't have time to read through all
patches - noticed this inline question incidentally only.

> OK, I am going to do a QOM cast anyway.

You should check how frequently this function is called - the current
OBJECT_CHECK() implementation does at least one string comparison, so
I'd suggest to avoid it here. That is, if the type passed in as opaque
matches the type assigned here (thinking of interfaces).

>>> +    return s->rx_enabled;
>>> +}
[snip]

Not knowing this piece of hardware, might it make sense to call it
"imx-fec" when there's another FEC for ColdFire? Or are they the same
thing and should be unified?

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg