| Message ID | 1411530303-23893-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Changes Requested |
| Headers | show |
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? Regards, Jeroen
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.
On Wednesday, September 24, 2014 at 09:44:40 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. OK, waiting for V2 Best regards, Marek Vasut
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?
On Thursday, September 25, 2014 at 04:25:37 AM, Stephen Warren wrote: [...] > >> 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 Unless you place it there because you know what you're doing ... > , there is no > persistent environment storage on this board There is SD/MMC slot, that's enough to implement persistent storage. > 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? I'd guard it to be future proof. Best regards, Marek Vasut
diff --git a/arch/arm/include/asm/arch-bcm2835/mbox.h b/arch/arm/include/asm/arch-bcm2835/mbox.h index dded857..61f427d 100644 --- a/arch/arm/include/asm/arch-bcm2835/mbox.h +++ b/arch/arm/include/asm/arch-bcm2835/mbox.h @@ -119,6 +119,20 @@ struct bcm2835_mbox_tag_hdr { * }; */ +#define BCM2835_MBOX_TAG_GET_MAC_ADDRESS 0x00010003 + +struct bcm2835_mbox_tag_get_mac_address { + struct bcm2835_mbox_tag_hdr tag_hdr; + union { + struct { + } req; + struct { + u8 mac[6]; + u8 pad[2]; + } resp; + } body; +}; + #define BCM2835_MBOX_TAG_GET_ARM_MEMORY 0x00010005 struct bcm2835_mbox_tag_get_arm_mem { diff --git a/board/raspberrypi/rpi_b/rpi_b.c b/board/raspberrypi/rpi_b/rpi_b.c index 220bb90..c80a934 100644 --- a/board/raspberrypi/rpi_b/rpi_b.c +++ b/board/raspberrypi/rpi_b/rpi_b.c @@ -31,6 +31,12 @@ struct msg_get_arm_mem { u32 end_tag; }; +struct msg_get_mac_address { + struct bcm2835_mbox_hdr hdr; + struct bcm2835_mbox_tag_get_mac_address get_mac_address; + u32 end_tag; +}; + struct msg_set_power_state { struct bcm2835_mbox_hdr hdr; struct bcm2835_mbox_tag_set_power_state set_power_state; @@ -62,6 +68,26 @@ int dram_init(void) return 0; } +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; +} + static int power_on_module(u32 module) { ALLOC_ALIGN_BUFFER(struct msg_set_power_state, msg_pwr, 1, 16); diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h index 9a53232..3c3ba9a 100644 --- a/include/configs/rpi_b.h +++ b/include/configs/rpi_b.h @@ -182,6 +182,7 @@ #define CONFIG_USB_STORAGE #define CONFIG_USB_HOST_ETHER #define CONFIG_USB_ETHER_SMSC95XX +#define CONFIG_MISC_INIT_R #endif #endif
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(+)