On 9/16/2010 1:49 PM, Kevin Hilman wrote: > Michael Williamson <[email protected]> writes: > >> For the MityDSP-L138/MityARM-1808 SOMS, read the factory assigned >> MAC address from the onboard I2C EPROM and assign it to the emac >> device during platform initialization. >> >> Signed-off-by: Michael Williamson <[email protected]> > > Hi Michael, > > A couple minor comments below... > > Also, the right address for the linux-arm-kernel mailing list is > [email protected]. Please CC that one. Thanks. >
These should only go to [email protected]? Or both linux-arm-kernel as well as davini-linux-open-source? Thanks. >> --- >> arch/arm/mach-davinci/board-mityomapl138.c | 55 >> ++++++++++++++++++++++++++++ >> 1 files changed, 55 insertions(+), 0 deletions(-) >> >> diff --git a/arch/arm/mach-davinci/board-mityomapl138.c >> b/arch/arm/mach-davinci/board-mityomapl138.c >> index e872fcc..e1cffb8 100644 >> --- a/arch/arm/mach-davinci/board-mityomapl138.c >> +++ b/arch/arm/mach-davinci/board-mityomapl138.c >> @@ -15,6 +15,7 @@ >> #include <linux/mtd/partitions.h> >> #include <linux/regulator/machine.h> >> #include <linux/i2c.h> >> +#include <linux/i2c/at24.h> >> >> #include <asm/mach-types.h> >> #include <asm/mach/arch.h> >> @@ -24,6 +25,59 @@ >> #include <mach/nand.h> >> #include <mach/mux.h> >> >> +#define FACTORY_CONFIG_MAGIC 0x012C0138 >> +#define FACTORY_CONFIG_VERSION 0x00010001 >> + >> +/* Data Held in On-Board I2C device */ >> +struct factory_config { >> + uint32_t magic; >> + uint32_t version; >> + uint8_t mac[6]; >> + uint32_t fpga_type; >> + uint32_t spare; >> + uint32_t serialnumber; > > u32 and u8 are more customary here. OK. I've always wondered why there were those types in addition to the ones in stdint.h. I will switch. Just never understood why there are separate types. maybe before stdint came into existence? > >> + char partnum[32]; >> +}; >> + >> +static struct factory_config factory_config; >> + >> +static void read_factory_config(struct memory_accessor *a, void *context) >> +{ >> + int ret; >> + struct davinci_soc_info *soc_info = &davinci_soc_info; >> + >> + ret = a->read(a, (char *)&factory_config, 0, sizeof(factory_config)); >> + if (ret != sizeof(struct factory_config)) { >> + pr_warning("Read Factory Config Failed: %d\n", ret); >> + return; >> + } >> + >> + if (factory_config.magic != FACTORY_CONFIG_MAGIC) { >> + pr_warning("Factory Config Magic Wrong (%X)\n", >> + factory_config.magic); >> + return; >> + } >> + >> + if (factory_config.version != FACTORY_CONFIG_VERSION) { >> + pr_warning("Factory Config Version Wrong (%X)\n", >> + factory_config.version); >> + return; >> + } >> + >> + pr_info("Found MAC = %pM\n", factory_config.mac); >> + pr_info("Part Number = %s\n", factory_config.partnum); > > Might be nice to prefix these prings with 'MityOMAPL138' or something > similar. Sure. > >> + memcpy(&soc_info->emac_pdata->mac_addr[0], >> + &factory_config.mac[0], 6); > > Rather than '6', use ETH_ALEN (from #include <linux/etherdevice.h>) > > Also, for some extra sanity, you might want to test the MAC using > is_valid_ether_addr() (from the same header) and warn if it's not a > valid MAC. > No problem. I will resubmit and make sure the other list is on there. Thanks for the comments. -Mike _______________________________________________ Davinci-linux-open-source mailing list [email protected] http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
