Messages in this thread | | | Date | Fri, 16 Apr 2021 09:29:59 +0200 | From | Michael Walle <> | Subject | Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices |
| |
Am 2021-04-16 05:24, schrieb Benjamin Herrenschmidt: > On Mon, 2021-04-12 at 19:47 +0200, Michael Walle wrote: >> >> /** >> * of_get_phy_mode - Get phy mode for given device_node >> @@ -59,15 +60,39 @@ static int of_get_mac_addr(struct device_node *np, >> const char *name, u8 *addr) >> static int of_get_mac_addr_nvmem(struct device_node *np, u8 *addr) >> { >> struct platform_device *pdev = of_find_device_by_node(np); >> + struct nvmem_cell *cell; >> + const void *mac; >> + size_t len; >> int ret; >> >> - if (!pdev) >> - return -ENODEV; >> + /* Try lookup by device first, there might be a >> nvmem_cell_lookup >> + * associated with a given device. >> + */ >> + if (pdev) { >> + ret = nvmem_get_mac_address(&pdev->dev, addr); >> + put_device(&pdev->dev); >> + return ret; >> + } >> + > > This smells like the wrong band aid :) > > Any struct device can contain an OF node pointer these days.
But not all nodes might have an associated device, see DSA for example. And as the name suggests of_get_mac_address() operates on a node. So if a driver calls of_get_mac_address() it should work on the node. What is wrong IMHO, is that the ethernet drivers where the corresponding board has a nvmem_cell_lookup registered is calling of_get_mac_address(node). It should rather call eth_get_mac_address(dev) in the first place.
One would need to figure out if there is an actual device (with an assiciated of_node), then call eth_get_mac_address(dev) and if there isn't a device call of_get_mac_address(node).
But I don't know if that is easy to figure out. Well, one could start with just the device where nvmem_cell_lookup is used. Then we could drop the workaround above.
> This seems all backwards. I think we are dealing with bad evolution. > > We need to do a lookup for the device because we get passed an of_node. > We should just get passed a device here... or rather stop calling > of_get_mac_addr() from all those drivers and instead call > eth_platform_get_mac_address() which in turns calls of_get_mac_addr(). > > Then the nvmem stuff gets put in eth_platform_get_mac_address(). > > of_get_mac_addr() becomes a low-level thingy that most drivers don't > care about.
The NVMEM thing is just another (optional) way how the MAC address is fetched from the device tree. Thus, if the drivers have the of_get_mac_address() call they should automatically get the NVMEM method, too.
-michael
| |