Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ae73d34782af7863e4b18495c0588a8cc2162612.camel@perches.com>
Subject: Re: [PATCH 07/14] staging: r8188eu: use memcpy for fallback mac
 address
From:   Joe Perches <joe@perches.com>
To:     Martin Kaiser <martin@kaiser.cx>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     Larry Finger <Larry.Finger@lwfinger.net>,
        Phillip Potter <phil@philpotter.co.uk>,
        Michael Straube <straube.linux@gmail.com>,
        Pavel Skripkin <paskripkin@gmail.com>,
        linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org
Date:   Tue, 12 Jul 2022 09:28:59 -0700
In-Reply-To: <20220709171000.180481-8-martin@kaiser.cx>
References: <20220709171000.180481-1-martin@kaiser.cx>
         <20220709171000.180481-8-martin@kaiser.cx>
Content-Type: text/plain; charset="ISO-8859-1"
Content-Transfer-Encoding: 8BIT
User-Agent: Evolution 3.44.1-0ubuntu1 
MIME-Version: 1.0
Precedence: bulk

On Sat, 2022-07-09 at 19:09 +0200, Martin Kaiser wrote:
> Use memcpy to store the fallback mac address in eeprom->mac_addr. Do not
> copy byte by byte.
[]
> diff --git a/drivers/staging/r8188eu/hal/usb_halinit.c b/drivers/staging/r8188eu/hal/usb_halinit.c
[]
> @@ -912,13 +912,11 @@ unsigned int rtl8188eu_inirp_init(struct adapter *Adapter)
>  
>  static void Hal_EfuseParseMACAddr_8188EU(struct adapter *adapt, u8 *hwinfo, bool AutoLoadFail)
>  {
> -	u16 i;
>  	u8 sMacAddr[6] = {0x00, 0xE0, 0x4C, 0x81, 0x88, 0x02};

static const sMacAddr[ETH_ALEN] = {...};

But maybe this should use eth_random_addr instead as there might
be more than one of these in a network.

>  	struct eeprom_priv *eeprom = &adapt->eeprompriv;
>  
>  	if (AutoLoadFail) {
> -		for (i = 0; i < 6; i++)
> -			eeprom->mac_addr[i] = sMacAddr[i];
> +		memcpy(eeprom->mac_addr, sMacAddr, ETH_ALEN);
>  	} else {
>  		/* Read Permanent MAC address */
>  		memcpy(eeprom->mac_addr, &hwinfo[EEPROM_MAC_ADDR_88EU], ETH_ALEN);

So perhaps something like:

{
	static const u8 sMacAddr[ETH_ALEN] = {
		0x00, 0xE0, 0x4C, 0x81, 0x88, 0x02
	};

	/* Not ether_addr_copy - EEPROM_MAC_ADDR_88EU isn't __aligned(2) */

	memcpy(adapt->eeprompriv.mac_addr,
	       AutoLoadFail ? sMacAddr : &hwinfo[EEPROM_MAC_ADDR_88EU],
	       ETH_ALEN);
}

or maybe:

{
	if (AutoLoadFail)
		eth_random_addr(adapt->eeprompriv.mac_addr);
	else
		memcpy(adapt->eeprompriv.mac_addr,
		       &hwinfo[EEPROM_MAC_ADDR_88EU], ETH_ALEN);