wifi: rt2x00: check for of_get_mac_address error

[PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Rosen Penev 1 month, 3 weeks ago

is_valid_ether_addr is already a check of of_get_mac_address, in which
case it returns an error if false. Just set a random MAC on all errors
except for EPROBE_DEFER.

Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
index 2d778ea5f1b5..51b836343f07 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
@@ -991,7 +991,7 @@ int rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr)
 	if (ret == -EPROBE_DEFER)
 		return ret;
 
-	if (!is_valid_ether_addr(eeprom_mac_addr)) {
+	if (ret) {
 		eth_random_addr(eeprom_mac_addr);
 		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", eeprom_mac_addr);
 	}
-- 
2.54.0

Re: [PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Stanislaw Gruszka 1 month, 2 weeks ago

On Sun, Apr 26, 2026 at 10:16:52PM -0700, Rosen Penev wrote:
> is_valid_ether_addr is already a check of of_get_mac_address, in which
> case it returns an error if false. Just set a random MAC on all errors
> except for EPROBE_DEFER.
> 
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

> ---
>  drivers/net/wireless/ralink/rt2x00/rt2x00dev.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> index 2d778ea5f1b5..51b836343f07 100644
> --- a/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00dev.c
> @@ -991,7 +991,7 @@ int rt2x00lib_set_mac_address(struct rt2x00_dev *rt2x00dev, u8 *eeprom_mac_addr)
>  	if (ret == -EPROBE_DEFER)
>  		return ret;
>  
> -	if (!is_valid_ether_addr(eeprom_mac_addr)) {
> +	if (ret) {
>  		eth_random_addr(eeprom_mac_addr);
>  		rt2x00_eeprom_dbg(rt2x00dev, "MAC: %pM\n", eeprom_mac_addr);
>  	}
> -- 
> 2.54.0
>

Re: [PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Johannes Berg 1 month, 2 weeks ago

On Mon, 2026-04-27 at 09:32 +0200, Stanislaw Gruszka wrote:
> On Sun, Apr 26, 2026 at 10:16:52PM -0700, Rosen Penev wrote:
> > is_valid_ether_addr is already a check of of_get_mac_address, in which
> > case it returns an error if false. Just set a random MAC on all errors
> > except for EPROBE_DEFER.
> > 
> > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>

Are you sure? I just randomly checked one caller to see what the
*eeprom_mac_addr would contain, and I see

        mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
        rt2x00lib_set_mac_address(rt2x00dev, mac);

so that case assumes it can get it from EEPROM and override with OF, but
if OF fails then it would still use the EEPROM address as long as it's
valid ...

johannes

Re: [PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Stanislaw Gruszka 1 month, 2 weeks ago

On Mon, Apr 27, 2026 at 12:45:14PM +0200, Johannes Berg wrote:
> On Mon, 2026-04-27 at 09:32 +0200, Stanislaw Gruszka wrote:
> > On Sun, Apr 26, 2026 at 10:16:52PM -0700, Rosen Penev wrote:
> > > is_valid_ether_addr is already a check of of_get_mac_address, in which
> > > case it returns an error if false. Just set a random MAC on all errors
> > > except for EPROBE_DEFER.
> > > 
> > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> 
> Are you sure? I just randomly checked one caller to see what the
> *eeprom_mac_addr would contain, and I see
> 
>         mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
>         rt2x00lib_set_mac_address(rt2x00dev, mac);
> 
> so that case assumes it can get it from EEPROM and override with OF, but
> if OF fails then it would still use the EEPROM address as long as it's
> valid ...

You have right, please drop the patch. 

BTW, the code should be changed to move getting the address from OF
out of _set routine. 

Regards
Stanislaw

Re: [PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Rosen Penev 1 month, 2 weeks ago

On Mon, Apr 27, 2026 at 3:54 AM Stanislaw Gruszka <stf_xl@wp.pl> wrote:
>
> On Mon, Apr 27, 2026 at 12:45:14PM +0200, Johannes Berg wrote:
> > On Mon, 2026-04-27 at 09:32 +0200, Stanislaw Gruszka wrote:
> > > On Sun, Apr 26, 2026 at 10:16:52PM -0700, Rosen Penev wrote:
> > > > is_valid_ether_addr is already a check of of_get_mac_address, in which
> > > > case it returns an error if false. Just set a random MAC on all errors
> > > > except for EPROBE_DEFER.
> > > >
> > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> >
> > Are you sure? I just randomly checked one caller to see what the
> > *eeprom_mac_addr would contain, and I see
> >
> >         mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
> >         rt2x00lib_set_mac_address(rt2x00dev, mac);
> >
> > so that case assumes it can get it from EEPROM and override with OF, but
> > if OF fails then it would still use the EEPROM address as long as it's
> > valid ...
>
> You have right, please drop the patch.
>
> BTW, the code should be changed to move getting the address from OF
> out of _set routine.
Where should it go?
>
> Regards
> Stanislaw

Re: [PATCH wireless-next] wifi: rt2x00: check for of_get_mac_address error

Posted by Stanislaw Gruszka 1 month, 2 weeks ago

On Mon, Apr 27, 2026 at 12:29:08PM -0700, Rosen Penev wrote:
> On Mon, Apr 27, 2026 at 3:54 AM Stanislaw Gruszka <stf_xl@wp.pl> wrote:
> >
> > On Mon, Apr 27, 2026 at 12:45:14PM +0200, Johannes Berg wrote:
> > > On Mon, 2026-04-27 at 09:32 +0200, Stanislaw Gruszka wrote:
> > > > On Sun, Apr 26, 2026 at 10:16:52PM -0700, Rosen Penev wrote:
> > > > > is_valid_ether_addr is already a check of of_get_mac_address, in which
> > > > > case it returns an error if false. Just set a random MAC on all errors
> > > > > except for EPROBE_DEFER.
> > > > >
> > > > > Signed-off-by: Rosen Penev <rosenp@gmail.com>
> > > > Acked-by: Stanislaw Gruszka <stf_xl@wp.pl>
> > >
> > > Are you sure? I just randomly checked one caller to see what the
> > > *eeprom_mac_addr would contain, and I see
> > >
> > >         mac = rt2x00_eeprom_addr(rt2x00dev, EEPROM_MAC_ADDR_0);
> > >         rt2x00lib_set_mac_address(rt2x00dev, mac);
> > >
> > > so that case assumes it can get it from EEPROM and override with OF, but
> > > if OF fails then it would still use the EEPROM address as long as it's
> > > valid ...
> >
> > You have right, please drop the patch.
> >
> > BTW, the code should be changed to move getting the address from OF
> > out of _set routine.
> Where should it go?

Never mind, I got confused by "set" word in the rt2x00lib_set_mac_address().

Regards
Stanislaw