Re: [PATCH] net: avoid assigning ethaddr to wrong devices

[Sascha Hauer <sha@pengutronix.de>]

On Tue, Jun 26, 2018 at 10:09:36AM +0300, Nikita Yushchenko wrote:
> 
> 
> 26.06.2018 10:05, Nikita Yushchenko wrote:
> >>>> It's PowerPC hardware which on barebox is not probed from devicetree, so
> >>>> indeed there is no device node.
> >>>
> >>> But if no device tree, then
> >>>   alias = of_find_node_by_alias(of_get_root_node(), eth);
> >>> should return NULL, and eth_is_stranger() should return false, thus
> >>> making my patch no-op?
> >>
> >> Ok, you are right. I misread the code. You call of_find_node_by_alias()
> >> on the internal device tree, not the one the Kernel is started with, so
> >> indeed eth_is_stranger() should return false.
> >>
> >> Nevertheless I do not like this patch very much as it adds more code to
> >> a place that is already hard to understand in all of its consequences.
> >>
> >> I would like to explore the route that we assign these dynamic devices
> >> an id that is not present in any alias node. That could be done by
> >> searching for the highest alias number and give the dynamic devices one
> >> number higher. Would that be doable?
> > 
> > Dynamic device number is assigned via
> > - setting id to DEVICE_ID_DYNAMIC, either by driver or by code at top of
> > eth_register(),
> > - replacing that with lowest currently-unused number at top of
> > register_device()
> > 
> > Probably we can add one more magic value that driver could set into
> > edev->dev.id before calling eth_register(), that will be replaced with
> > lowest currently-unused number that does not have aliases. However, this
> > will change eth numbering in existing setups and thus can break them.
> > 
> > Possible option could be a flag in edev that forbids setting/exporting
> > ethaddr for this device. Doing so for usbnet seems safe. This will fix
> > my case.
> 
> Thinking more on this, maybe cleaner is NOT to match by id of in-barebox
> ethdevice when setting/exporting ethaddr by default, but explicitly
> allow such matching for platforms that need it.

Yes, that would be a good solution. Maybe struct eth_device could get a
new char *of_alias field which the network driver populates from the
platform_data.

Since Renauds address bounces I'm not sure how much interest he still
has in the barebox port of the boards using this feature. I think the
devices are all registered with fsl_eth_init(). I would think that he
does the board fixup himself if he still has interest, but we should
prepare the network device generic part.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox