From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Wed, 24 Sep 2014 20:25:37 -0600
Subject: [U-Boot] ARM: rpi_b: query internal MAC address from firmware
In-Reply-To: <54231F28.4020402@wwwdotorg.org>
References: <1411530303-23893-1-git-send-email-swarren@wwwdotorg.org>	<54231B15.3020103@myspectrum.nl>
	<54231F28.4020402@wwwdotorg.org>
Message-ID: <54237D21.5060907@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 09/24/2014 01:44 PM, Stephen Warren wrote:
> On 09/24/2014 01:27 PM, Jeroen Hofstee wrote:
>> Hello Stephan,
>>
>> On 24-09-14 05:45, Stephen Warren wrote:
>>> The built-in SMSC 95xx chip doesn't know its own MAC address. Instead,
>>> we must query it from the VC firmware; it's probably encoded in fuses
>>> on the BCM2835.
>>>
>>> Signed-off-by: Stephen Warren <swarren@wwwdotorg.org>
>>> ---
>>> This depends on "usb: dwc2: Add driver for Synopsis DWC2 USB IP block
>>> from Marek Vasut".
>>>
>>>   arch/arm/include/asm/arch-bcm2835/mbox.h | 14 ++++++++++++++
>>>   board/raspberrypi/rpi_b/rpi_b.c          | 26
>>> ++++++++++++++++++++++++++
>>>   include/configs/rpi_b.h                  |  1 +
>>>   3 files changed, 41 insertions(+)
>>>
>> [snip]
>>> +int misc_init_r(void)
>>> +{
>>> +    ALLOC_ALIGN_BUFFER(struct msg_get_mac_address, msg, 1, 16);
>>> +    int ret;
>>> +
>>> +    BCM2835_MBOX_INIT_HDR(msg);
>>> +    BCM2835_MBOX_INIT_TAG(&msg->get_mac_address, GET_MAC_ADDRESS);
>>> +
>>> +    ret = bcm2835_mbox_call_prop(BCM2835_MBOX_PROP_CHAN, &msg->hdr);
>>> +    if (ret) {
>>> +        printf("bcm2835: Could not query MAC address\n");
>>> +        /* Ignore error; not critical */
>>> +        return 0;
>>> +    }
>>> +
>>> +    eth_setenv_enetaddr("usbethaddr",
>>> msg->get_mac_address.body.resp.mac);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>
>> Normally this should be within a check for if (!getenv("usbethaddr")).
>> Any reason to deviate from that?
> 
> That makes sense. I simply wasn't aware of the convention. I'll amend
> the patch.

Oh, actually...

When this code runs, there's no way that usbethaddr can already be set.
There's no usbethaddr value set in the default environment, there is no
persistent environment storage on this board so the default environment
is always used, and misc_init_r() happens well before any kind script
that the user might modify (such as uEnv.txt or boot.scr) can run. If
the user wants to override usbethaddr, they can just set a new value in
uEnv.txt.

Does it still make sense to add this guard anyway, or does this justify
the existing version of the patch?