Re: [PATCH v2] net: cpsw: fix obtaining mac address for am3517

From: Jeroen Hofstee
Date: Fri Jun 23 2023 - 17:13:36 EST



On 23-06-2023 22:14, Simon Horman wrote:
On Fri, Jun 23, 2023 at 03:58:03PM +0100, Måns Rullgård wrote:
Tony Lindgren <tony@xxxxxxxxxxx> writes:

* Jeroen Hofstee <jhofstee@xxxxxxxxxxxxxxxxx> [161028 11:19]:
Hello Tony,

On 28-10-16 17:52, Tony Lindgren wrote:
* Jeroen Hofstee <jhofstee@xxxxxxxxxxxxxxxxx> [161028 08:33]:
Commit b6745f6e4e63 ("drivers: net: cpsw: davinci_emac: move reading mac
id to common file") did not only move the code for an am3517, it also
added the slave parameter, resulting in an invalid (all zero) mac address
being returned for an am3517, since it only has a single emac and the slave
parameter is pointing to the second. So simply always read the first and
valid mac-address for a ti,am3517-emac.
And others davinci_emac.c users can have more than one. So is the
reason the slave parameter points to the second instance because
of the location in the hardware?
Sort of, the slave parameter gets determined by the fact if there is one
or two register range(s) associated with the davinci_emac. In davinci_emac.c

res_ctrl = platform_get_resource(pdev, IORESOURCE_MEM, 1);
...
rc = davinci_emac_try_get_mac(pdev, res_ctrl ? 0 : 1,
priv->mac_addr);

So it there are two ranges, the slave param becomes 0. It there is only one,
it
will be 1. Since the am3517 only has a single regs entry it ends up with
slave 1,
while there is only a single davinci_emac.
OK thanks for clarifying it:

Acked-by: Tony Lindgren <tony@xxxxxxxxxxx>
Is there some reason this patch was never picked up, or was it simply
forgotten?
I feel like I am missing something here.

That is a weird response, you feel like something is missing, while
it is a recognized bug, acknowledged by the maintainer at that time
and apparently still not fixed and Måns simply asked it there is a
reason for that.

This is not a really helpful response,

Jeroen