Re: [PATCH net-next] cdc_ether: Improve ZTE MF823/831/910 handling

From: Oliver Neukum
Date: Mon Aug 08 2016 - 10:02:00 EST


On Mon, 2016-08-08 at 14:44 +0200, BjÃrn Mork wrote:
> Oliver Neukum <oneukum@xxxxxxxx> writes:

> > I don't see how it would be specific for a subsystem. If the patch
> > is correct, it belongs into the networking core.
>
> The bug is in the firmware implementation of the "read unique vendor
> assigned mac address" functions, and should therefore be fixed there.

Well, if you define the semantics of the operation in that manner, it
certainly does. That however is not given. You could also define it
as returning the MAC the hardware listens to. The difference was just
unclear when the API was defined.

> It cannot be put into the networking core because "read unique vendor
> assigned mac address" is a hardware dependent function. It's
> implemented in each ethernet driver based of whatever interface the
> firmware provides to that register.

Again, that depends on that particular semantics.

> IMHO, usbnet_get_ethernet_addr() would be the most appropriate place for
> cdc_ether and other CDC drivers. And generic_rndis_bind() is the correct
> place for rndis_host.
>
> Putting this in usbnet_get_ethernet_addr() will also fix the XMM7160
> based devices having an FF:FF:FF:FF:FF:FF mac address (sic). I'm pretty
> sure there are other examples too. There is no end to the creative
> crazyness of firmware engineers...

It is clear that that would work. No question about that.

But why fix similar issues at two different places? And what about
PCI or other cards that show the same problem?

Regards
Oliver