Skip to contentSkip to conversation

am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

PATCH In review Peter Korsgaard 3 participants 7 messages Jan 16, 2013 – Jan 18, 2013 3 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

When booting with CONFIG_ARM_APPENDED_DTB (either because of using an old U-Boot, not wanting the hassle of 2 files or when using Falcon fast boot mode in U-Boot), nothing updates the ethernet hwaddr specified for the CPSW slaves, causing the driver to use a random hwaddr, which is some times troublesome.

The am33xx has unique ethernet hwaddrs programmed in the efuse, so it makes more sense to default to these rather than random ones. Add a fixup step which adds mac-address dt properties using the efuse addresses if the DTB didn't contain valid ones.

---
 arch/arm/mach-omap2/Makefile      |    3 ++
 arch/arm/mach-omap2/am33xx-cpsw.c |   94 +++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/control.h     |    4 ++
 3 files changed, 101 insertions(+)
 create mode 100644 arch/arm/mach-omap2/am33xx-cpsw.c
arch/arm/mach-omap2/Makefile +3 -0
index 65fb6fb..54fb2ee 100644
@@ -298,4 +298,7 @@ endif
 emac-$(CONFIG_TI_DAVINCI_EMAC)		:= am35xx-emac.o
 obj-y					+= $(emac-m) $(emac-y)
 
+cpsw-$(CONFIG_TI_CPSW)			:= am33xx-cpsw.o
+obj-y					+= $(cpsw-m) $(cpsw-y)
+
 obj-y					+= common-board-devices.o twl-common.o dss-common.o
arch/arm/mach-omap2/am33xx-cpsw.c new file +94 -0

94 lines not on this page · raw message

arch/arm/mach-omap2/control.h +4 -0
index e6c3281..266d512 100644
@@ -352,6 +352,10 @@
 /* AM33XX CONTROL_STATUS register */
 #define AM33XX_CONTROL_STATUS		0x040
 #define AM33XX_CONTROL_SEC_CLK_CTRL	0x1bc
+#define AM33XX_CONTROL_MAC_ID0_LOW	0x630
+#define AM33XX_CONTROL_MAC_ID0_HIGH	0x634
+#define AM33XX_CONTROL_MAC_ID1_LOW	0x638
+#define AM33XX_CONTROL_MAC_ID1_HIGH	0x63c
 
 /* AM33XX CONTROL_STATUS bitfields (partial) */
 #define AM33XX_CONTROL_STATUS_SYSBOOT1_SHIFT		22
Signature · 1 line
1.7.10.4
#1 lore raw

Hi Peter,

I made changes to cpsw driver two weeks ago, which add support for reading MAC address from CPU and also I posted it to review.
You can find patch here : https://patchwork.kernel.org/patch/1966481/
I will create updated patch next week, depended on reactions. Can you try that patch? I'll be glad to hear your
opinion.

Michal

#2 lore raw

PATCH 1/1 am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Michal" == Michal Bachraty <[email protected]> writes:

Hi Michael,

Michal> I made changes to cpsw driver two weeks ago, which add support Michal> for reading MAC address from CPU and also I posted it to Michal> review. You can find patch here :
Michal> https://patchwork.kernel.org/patch/1966481/ I will create Michal> updated patch next week, depended on reactions. Can you try Michal> that patch? I'll be glad to hear your opinion.

Thanks, I missed that patch. Functionality wise the patches do the same, but I must say I find my patch cleaner, as it doesn't involve any new dt bindings or access to the am33xx specific control module registers from the cpsw driver.

The way I've handled it is similar to how it is done on atleast one other arm subarchicture, see arch/arm/mach-mxs/mach-mxs.c::update_fec_mac_prop()

Signature · 1 line
Bye, Peter Korsgaard
maintainer
#3 lore raw

PATCH 1/1 am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

Quoted 11 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Michal" == Michal Bachraty <[email protected]> writes:

Hi Michael,

Michal> I made changes to cpsw driver two weeks ago, which add support Michal> for reading MAC address from CPU and also I posted it to Michal> review. You can find patch here :
Michal> https://patchwork.kernel.org/patch/1966481/ I will create Michal> updated patch next week, depended on reactions. Can you try Michal> that patch? I'll be glad to hear your opinion.

Quoted 8 lines

Thanks, I missed that patch. Functionality wise the patches do the same, but I must say I find my patch cleaner, as it doesn't involve any new dt bindings or access to the am33xx specific control module registers from the cpsw driver.

The way I've handled it is similar to how it is done on atleast one other arm subarchicture, see arch/arm/mach-mxs/mach-mxs.c::update_fec_mac_prop()

Cool this seems like the cleanest way to deal with it so far. Can you please resend with also linux-net and DT list Cc:d too?

Also the arch_initcall should be omap_arch_initcall in linux next for consistency if we ever have SoC specific sections for those.

Regards,

Tony

#4 lore raw

PATCH 1/1 am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Tony" == Tony Lindgren <[email protected]> writes:

Hi,

Quoted 3 lines
Quoted 3 lines

The way I've handled it is similar to how it is done on atleast one other arm subarchicture, see arch/arm/mach-mxs/mach-mxs.c::update_fec_mac_prop()

Tony> Cool this seems like the cleanest way to deal with it so far. Can Tony> you please resend with also linux-net and DT list Cc:d too?

Tony> Also the arch_initcall should be omap_arch_initcall in linux next Tony> for consistency if we ever have SoC specific sections for those.

Sure, resent with the initcall change and netdev/devicetree-discuss added.

Signature · 1 line
Bye, Peter Korsgaard
#5 lore raw

PATCH 1/1 am33xx: cpsw: default to ethernet hwaddr from efuse if not defined in dt

Quoted 11 lines · Peter Korsgaard
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Tony" == Tony Lindgren <[email protected]> writes:

Hi,

Quoted 3 lines
Quoted 3 lines

The way I've handled it is similar to how it is done on atleast one other arm subarchicture, see arch/arm/mach-mxs/mach-mxs.c::update_fec_mac_prop()

Tony> Cool this seems like the cleanest way to deal with it so far. Can Tony> you please resend with also linux-net and DT list Cc:d too?

Quoted 8 lines

Tony> Also the arch_initcall should be omap_arch_initcall in linux next Tony> for consistency if we ever have SoC specific sections for those.

Sure, resent with the initcall change and netdev/devicetree-discuss added.

--
Bye, Peter Korsgaard

Thanks,
I added DT option mac-address-source because there is still possiblility to have mac address set by the u-boot in the future. Beaglebone have special eeprom on board, were you can have stored user MAC and u-boot can read eeprom
data. Maybe some users will want this feature. Also in cpsw dt binding documentation, there is mention /* Filled in by U-Boot */. For that purpose,
code I proposed can easily deal with any driver exteds.

Reading CPU mac address registers with (my approach) or without (your approach) DT should be more discussed. Also your way seems to be fine.

Regards
Michal
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20130118/6d8983db/attachment-0001.html>

A message that is not in the archive (sent off-list or not yet received): CAAfrEwYGMwVfxD2z6xKfzyn-252B2UT0AuxD36rZsct11Pi52w@mail.gmail.com

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): CAAfrEwaoFxRON0dJceMXeGEG-=tgUmqThEcwid2wiiLWTJYE-A@mail.gmail.com

#9 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Michal" == Michal Bachraty <[email protected]> writes:

Hi,

Michal> I'm ok with your changes. Have you tried to set MAC from Michal> u-boot? I haven't seen any code for that purpose in cpsw. I Michal> tried put MAC with kernel command parameter (in u-boot), but Michal> nothing happen.

I didn't actually try, but mainline U-Boot has support for updating the mac-address properties if you:

- build U-Boot with dt support (CONFIG_OF_LIBFDT)
- use a seperate .dtb file
- add aliases called ethernet{0,1} to the 2 cpsw slaves in the dts (this should arguable be done in am33xx.dtsi) - E.G.:

aliases {
..
ethernet0 = &cpsw_emac0;
ethernet1 = &cpsw_emac1;
};

Then U-boot will set the mac-address properties to the content of the ethaddr / eth1addr environment variables. See fdt_fixup_ethernet() for details.

Signature · 1 line
Bye, Peter Korsgaard