QE firmware/microcode handling

Hi everyone,

following up Timur's patch on QE microcode:

http://marc.info/?l=u-boot&m=132197537730440&w=2

I was wondering, would it make any sense to *embed* QE's firmware within u-boot image itself?
After all, it should be some ~64KB worth of data, right?
Not quite sure whether this has ever been done for similar purposes, but to me it looks way more convenient to have a single image file to be flashed, rather than reserving room (e.g. within the same NOR flash where u-boot lies) for it.
Easier upgrading of this microcode from within the application, is perhaps the only reason I could think of for *NOT* doing so. How to actually implement it, well that's another issue, and on that I'm open to suggestions, too.

Looking forward to hearing your thoughts.

Thanks in advance,
Gerlando

Dear Gerlando Falauto,

It's a matter of taste/requirements/system design.

A similar situation is with the persistent copy of the environment - it might be "embedded" within the U-Boot image (actually even with U-Boot code before and after it), it may be prepended or appended, or it may be completely separate. All options make sense for certain configurations, and none for others.

Having a single file may be an advantage, or a disadvantage. You cannot tell in advance without knowing the exact specifications of the hardware and the requirements for the software design.

So the most flexible way is actually to make no assumptions about the location, and handle it in a board-specific way if needed (say, through a board specific linker script, or some scripts).

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"One planet is all you get."

There are three problems:

1) A lot of U-Boot images are already very close to the 512KB limit. We often have to disable features in U-Boot to get it to build properly. An additional 64KB would be prohibitive.

2) The firmware license is not exactly GPL-compatible. The firmware files themselves are freely distributable, but the source code is closed. I'm not sure how that would work in a GPL U-Boot.

3) For Fman, at least, we often update the firmware after the board ships, which means you would need to update U-Boot to get a new firmware. So no one might actually use this feature because they're afraid they won't be able to update the firmware.

If you can do it, go ahead. I would approve a patch that adds the ability to inject a firmware binary into u-boot.bin, provided it's controlled via a build or CONFIG_SYS_xxx option.

Timur Tabi
Linux kernel developer at Freescale

The 512K limit is arbitrary and can be changed. It exists just to provide a stable start address on something that grows from the end of flash.

CONFIG_xxx. CONFIG_SYS_xxx options are supposed to describe hardware, and CONFIG_xxx options are supposed to be software config.

-Scott

True, but we haven't actually done changed it. Instead of allowing for a larger u-boot.bin, we have ALWAYS disabled features or rewritten code to make it smaller. If making U-boot larger were so simple, it would have been done by now, I think.

Timur Tabi
Linux kernel developer at Freescale

Hmm, I think it is basically that simple:

$ git diff

index bbfeda5..2fd7aba 100644
@@ -52,7 +52,7 @@
 #define CONFIG_MP                      /* support multiple processors */
 
 #ifndef CONFIG_SYS_TEXT_BASE
-#define CONFIG_SYS_TEXT_BASE   0xeff80000
+#define CONFIG_SYS_TEXT_BASE   0xeff00000
 #endif
 
 #ifndef CONFIG_RESET_VECTOR_ADDRESS

$ make P4080DS -j48
[ snip ]
$ ls -alh u-boot.bin
-rwxrwxr-x 1 mattsm mattsm 1.0M Sep 20 20:54 u-boot.bin

-M

From: Chris Packham <[email protected]>

Instead of storing the mircocode in a separate flash block simply embed the image as C code array of bytes.

---
So here's a patch that embeds the Fman firmware in a board specific way for one of our out of tree boards (based on the P2041RDB) if anyone feels like picking it up and generalising it. One thing that I haven't done yet is added code to figure out the offset from the bootloader base and pass that in the device tree for linux.

 board/alliedtelesis/SBx81CFC960/Makefile           |   12 +-
 .../SBx81CFC960/fsl_fman_ucode_P2041_101_8.bin     |  Bin 0 -> 7240 bytes
 .../SBx81CFC960/fsl_fman_ucode_P2041_101_8.c       |  607 ++++++++++++++++++++
 drivers/net/fm/fm.c                                |    2 +
 include/configs/SBx81CFC960.h                      |   27 +-
 5 files changed, 625 insertions(+), 23 deletions(-)
 create mode 100644 board/alliedtelesis/SBx81CFC960/fsl_fman_ucode_P2041_101_8.bin
 create mode 100644 board/alliedtelesis/SBx81CFC960/fsl_fman_ucode_P2041_101_8.c

index b606873..f1e1d7c 100644
@@ -26,6 +26,16 @@ include $(TOPDIR)/config.mk
 
 LIB	= $(obj)lib$(BOARD).o
 
+ifeq ($(CONFIG_FMAN_ENET),y)
+FMAN_UCODE = fsl_fman_ucode_P2041_101_8.bin
+FMAN_UCODE_C = $(patsubst %.bin,%.c,$(FMAN_UCODE))
+FMAN_UCODE_OBJ = $(patsubst %.bin,%.o,$(FMAN_UCODE))
+
+# xxd not in chroot.
+#$(FMAN_UCODE_C): $(FMAN_UCODE)
+#	xxd -i $< $@
+endif
+
 COBJS-y				+= $(BOARD).o
 COBJS-y				+= ddr.o
 COBJS-y				+= ddr_param.o

@@ -33,7 +43,7 @@ COBJS-y				+= eth.o
 COBJS-y				+= law.o
 COBJS-$(CONFIG_PCI)		+= pci.o
 COBJS-y				+= tlb.o
-COBJS-$(CONFIG_FMAN_ENET)	+= fman.o
+COBJS-$(CONFIG_FMAN_ENET)	+= fman.o $(FMAN_UCODE_OBJ)
 
 SRCS	:= $(SOBJS:.o=.S) $(COBJS-y:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS-y))

new file mode 100644
index 0000000000000000000000000000000000000000..db4477f6f5276431664c752f60ad37a504082795
GIT binary patch

zcmcIpe{5UVbw2Nho}WnCdZK8Bj^gpjatysJY{Y>u8AN at R2(|76{VX2$H)-IvKt#l zXFDt=W|M0=$+ENv1iEd&q$CAwUO<=Y*8Bml{=D>6-V}J&T|7Q<X}m5+!YM4r-UA! zszls(E=ie`m99%R0ay3E``vr)E}1Z_mSwX&|?p7m3AB$c;?W+p}qUeKil_A<iMdn zHSg=VZ?kMhxyGz&zSvR>F{@$&;0)84qE<CYOv(xk>4<SYtP?mkKaIPdB}`a#<eSW zD(urs@9z|8740J(eQOba=zZn>>!n_Kr_{@zlzL6fdsepTSrtt~)6pDgF0 at r>2AUhq zgVv1ZMY|L2F0|jVY4DgWqbX=An)XjVQlBN8+I=?h9&Z=5lPMx8N^>-Vk!&JycHGqw z9sJHG6s at 19^YX)oBL_aALCnE8{VF9Zkv({4oaSk>k7TBkPmpwYN|H8rmEJM;+t*SQ z<GGuC#W|j%QrfF|(j8iJ+OMgkYhI%4SmW6dKWX2;L5;&+k~B4;zg;ZOYu*HncrnLF z8^gOuo^M=Fo4QQFGt=U~W^dZq<i9mG@0lV^pQ4C8X6^``r_E1FvC_=a|1z#s;n5m= z^%SY)_1jD957*+?NmWbq+5ca{k8awrFggoZ=PZLV*fJzUA1@>{V_qmzZS2eU&2GR zxZvMlL>@I>jSsp>XgKIpDJ+wZMZ&FFV^b<?^p(RRnc!>ibMYb-%;A%T#7I1uf)Ii zHoT&r(*DL$JU>Pa))PUmt#0jLK0>8=8qb?=o}x0Y*W$Dw at H?PieM%Pmhgd$cYv|u0 z>?8bVyP!+fGPrt6e)^APaJ>qjT8cwIVtq+Jlv#>{zbgAtZm1AesumeyvBZ%SO%}M zAH|pAwf3X8mg2ScqrblmudyG!u at qn4e)I#LHxK=Pj{W|+gv0-9&L_Q8OJCsC_Khur zm+X69*gsm0fA~a^<~=iXe%DNuZ_%O3oDzP0DgJaVeq$;AWCg#pH<-*kfqmR!%ZI2 zqvpJN{eetqCi- at fdd}~9xML}f@xTRrW~j2B=P%(cRKd7soM|ta(^Z+n=Y!NKJ%53W zkJgivj_y+4E-t*zu~@7x;VYKlKdj)BS1;T1>J#NXc310Y1FtXRB-1XM47B^fznR%Z z{>uK9_2+lpT041FwQ&vdCsl#Om2Q>om7pj<gw5DiIW?sDFFX_0H8|@#^D!Q4gML zbIiiGzJFsOI_zE8FDrURR>_r5kyHpOQZ{;DfB9OHboCu!4}C)Uew=O|KUVBx}ZH zLA?hx^f*kcwO%9*66U0R>%tJxnR4U>Gj&#QS%p&w+zPx!!j1E}l26iI4cGPL{}l z97&j?x+Np+22IYHVh@=d*#@NLb4O6X7o&tH=1ju=#P(4ea_VVBV7bLQ5~<(1q^>K zNd5raMTYn)(AfHqm-B)rGEK2eHHU+^)yKnvK at 2d9edM1AEy=hM2C6lxbfh0f?Szy zie5_uC=zxeuX>5~Dr<BZbktm1haLBk0{xK{+(K@R-k0ksogH7-nb{VwSuV9jCrvXb z<}fs^Yk5GhQFJ0?LVr7h7YpWazn8ZxnK$^~^h7xF#}J_ARlSJF<6#JWsC|3jWz z2s)(-1{pQR3!z(M{EnL9alzk;3V(j)ua5aEJhVl%g7-6zm)$GMe7^fX@#!q_=>VS_ z?_~QJx!^aVxlm}1Wjk_hE$!S;k}uN at -kHyBC4C;5SuHTD$0>o|m2QdBg-|V)YwNcw zHgo$Z&2ux^eE3|d*_qN%h^QWGpoB%N56JRnybqu!$&hcdNeZNr0Vy<r<@d3=TRrc z{)ByEq^o%IU&aEBOb7Lp0Uu644JzCCr%#bSoudBR?HnuH*${TtylGOUv|;U5=tsrk z^Qx??JSJ26it0)msJZB;Q>0tF51z<_x|A7quzSY$9SKPkz3C*!zS67AAiSIoZ z1O5TVb8g>$S#1fEET=R1q(r$^Bl?z)R)g2;*i&~)z@@ZHmiN=8<FVju(xFeLSU3Ag zmeQF*ca`lo!vDadwcW{l9=H0M;0wi3;S2J9+3ImiZuAF>pV3at<vj~xES<q~u7ES( zMlY{R=U|UDp#MJ8ccYhE)9+)<@n*638q@QDzpA~yS>Ed>j9EJl8Y~wYvCQD*1k228 zSZ>s-=UDd4L5B_OyP1h!$1#>*T<Cgb+EC>*^ywsPheF^fqpsSoTDzYo&t2*3KPoQd ztiHc+fW~F35Ar_B at -EC+yPGIy+Wz4#s^f9cUSaKU^10V!DRvoiKIU;Z9d@{5QTSB{ zO()4UL^fhtqH{K(D15$grX{N%bgD at 0v*k*#IBG=r%9nRvaX-hRGo5&RaW4S!tpNZ zL=GUsN2k$CYD at XdKHZITJ{K>aaYwG*;TLDTI}k6dYYM>5T0TjEgI%zP*3Yr-UPGcY zgPIakm<~k>v!|m2F65;=nnb|n+8hjMr+osHNcnQqBELqP%apba*UXGX&q%Rwyw>N zwH_^OZ<8_)-xFn;g5EmA0X;S3*6HzK#O4}5(%X4oooP~;pG>zAW!}Vk;MOAw at tI<M zztu&mrklJ*$xq;LIm}l)lKBeNnWqr*kU!zqk8HFjoWD*?yx1L+>kat4mvRSo~UG6 z$JTrOg-2SX#aN<+Df9H>LU+wr at 74_&v>k at eJXP3MJ5S^vpCM-+^uU at JjMLLPhGi< zjyU9VT$~8}T{an5JDH+*KB2rcq|DV>fqFeV8tz0)_eb`W<WN;z=e1V%{ck{?rfcV7 zbTJWlisQ{>l5DY7E3X|duN_d>E+=I&b*tNHl;VxZJ&N0By4;fKQjhyZE>un91o)}< z;C)CE{<fCyh>;KPkv4SE1FXA2NtZk^ztAh6gNq*orn4UeUYi_k^mFXu^T2%NQ;>ss z=~<&^$HQkcK_ed7+eAaME#*hy)1&0drjnhLW9hj1BE7-+Ew^DV4!vj|#39df^v3L1 z6X!@*WKRo;F}XvG=M%CrIR?FiTWD~gMci2{cVFgJl}VQs19{zW<pxX}GAXJ;bXFk+ zdB?%$ALr$3ZZ&<fd=0pBagpXq;~Er at ihOQ{B3J at kfz_GG-9egq=5ef_(#6YjwU{v z9lcYEK*uPz=qypa6%uFh2b|ZNeF3sUL7n?eU7=DwCh(<zudtVW0Q(ki@P1Y|&T>e^ zM8mDr at TD>izQmne at U;p#BB8^?!!xcWiYHzbmcsatQ53w@>-(9r%Q37g-{o*M(fP< zD7H*d*?P1I@wkl;*L1F3pqH;2zVxZVZyiv059r^p0^ik#2>C1lpL&e^wR at 14_Ym}% zR$q&t2Rd!61KEtq$ZxaQi*5$w7~<ipf}en57~qS?e-=Y5L=3h1g!~V2jD$V5Fxs0 zVojejyvTpI!u}p(-pEX!V!u^are6{AFb^CTAs6_7y@=y$!{^IT{N?g8EPPxg!!Q3A zGBlTExMefW>+m62PoHG})U-7CbCRYSl-buEW&g^I&G{!@xQ{qCeCG=Dr(wSfA&17 z9-r=7^dH60tx%hhTfav551&c5Pe18a^^<QF7ml+WRX$zgJIGfC=WAqv)2JoG$o&nO z9d6;l{wBpS7+D{R7F?W*Yo4lpuXT0^|uGHXL8Jld!F;UxK1o=*s=mT*Hk#ZwY5dM zsM0 at 7ro3?vWE1kCfcg1MSjy}pJ2Pg-kKFhw?OdnRwFLd~rY;g^0sH7bQK%)KlIfCS zoI{=C;y5w5r%BXD!z$yU_4$KLl9Rly%0NoLADd9lZx4t_tQ%n-qpFl1!ayf9-Bn1 z{_j0zd~(!?^ZT6q71M)th at UFfoQSWdR;Gy@ZC9%$leyt3-!3ClGvw?Ycu0B)QPsI z&TBwds@2;j71p;IKR#Bpp_fO$s1C5tM%Dvpy!>-j>Yg4a7seX$@8Y}JllOFjCtogT z+80o>W*=M=&AwaWqtM%C;JaF<s(6h|PpdyN&_>on_e;6E=b|YA9KAI*|P;jBXlZL zBj~UAqC7Hxq84R+jW*J3uc2gT60f0#IKcWESx;Zfry4(H9db+;I+GD|ddM?7W|$uE z@@4jiJ&7>un9h7iWtu}NixLw)#DFrh%NQ!Y&eOl+|UM417^}vCnB?6Ua0><_!qB z2Aa5Fz;{dHdK|X7?X+3?Iqi6T@%ly*fwkYtRof4GkdP_DC-A#-e#X-{z^HmILl{h zQD>lCrL(B9sNJiS%hVPJ|yH7KWt<!?gtzFmb%o-y8OK_q07gFF3&-iz3Yn8=gR&) z&N?*DBA)vAzIy?8-E6Pq&yE|S#z9?gK%Zw3HVjcKgpo_OZjo~9=Md-M-v}#^;p>s zy(}Yg3jM(slu<6rc*mEJ(fj;Bg@BUuR2abXXWqk#d(hJoD)BK914-95Pqk04QIZY zbBe5UzPpKM!~w=h<+UCOnO1zqFMeEEPs)_SccYKms1En;7V8No{V+%p@}#Jb&^vi9 zdKdT5{YtJ;@V<)Qk!${jd&cqnU*+d|+WELe%A1OwfwAL&Ov^zkt@}4crmss^El^g zlzdWq6Fhr=l_ajQoROy#d{aRV!1J{v?!2XURba<}CFKO>x8sh~UzvY1h~FsSKe)H( z7;g~wZTzk=n~*(xce<85&iWO!$4QH9>as*WLatH4k5imYXVi6CEu*e)k&gyp{Rb zOYALbAkZ8py1OzT*bCsHp at Nlb?X?(xd71G$Fus?^{}Zwbxo+oIMa^W0(6UNLd_W& zx}}c@<Og2?{#_;#{$%m@pZ2cO??9-JUgFvu?I_>JvEIpb3GX%d=l{yJHQ((ho5UGJ z9pLfdZjQTjq173{8JW)~B-Dk5?(rF}@qRKYdcrydv5=z3w<8LEC&C>SwJ^>$>hcu% yMxT6PIW{V?F)ToMN8gyX%X%tDttrT>y95LB%U=@D;x^7%qJ`+mU<m_5TKkiTnKk

literal 0
HcmV?d00001

index 0b8c33f..6eb6c2a 100644
@@ -408,6 +408,8 @@ int fm_init_common(int index, struct ccsr_fman *reg)
 		/* flush cache after read */
 		flush_cache((ulong)addr, cnt * 512);
 	}
+#elif defined (CONFIG_SYS_QE_FMAN_FW_IN_IMAGE)
+	void *addr = CONFIG_SYS_QE_FMAN_FW_IMAGE;
 #endif
 
 	/* Upload the Fman microcode if it's present */

index 7e1728a..fff5e5f 100644
@@ -374,28 +374,11 @@ unsigned long get_board_sys_clk(unsigned long dummy);
 
 #define CONFIG_SYS_DPAA_FMAN
 #define CONFIG_SYS_DPAA_PME
-/* Default address of microcode for the Linux Fman driver */
-#if defined(CONFIG_SPIFLASH)
-/*
- * env is stored at 0x100000, sector size is 0x10000, ucode is stored after
- * env, so we got 0x110000.
- */
-#define CONFIG_SYS_QE_FW_IN_SPIFLASH
-#define CONFIG_SYS_QE_FMAN_FW_ADDR	0x110000
-#elif defined(CONFIG_SDCARD)
-/*
- * PBL SD boot image should stored@0x1000(8 blocks), the size of the image is
- * about 545KB (1089 blocks), Env is stored after the image, and the env size is
- * 0x2000 (16 blocks), 8 + 1089 + 16 = 1113, enlarge it to 1130.
- */
-#define CONFIG_SYS_QE_FMAN_FW_IN_MMC
-#define CONFIG_SYS_QE_FMAN_FW_ADDR	(512 * 1130)
-#elif defined(CONFIG_NAND)
-#define CONFIG_SYS_QE_FMAN_FW_IN_NAND
-#define CONFIG_SYS_QE_FMAN_FW_ADDR	(6 * CONFIG_SYS_NAND_BLOCK_SIZE)
-#else
-#define CONFIG_SYS_QE_FMAN_FW_IN_NOR
-#define CONFIG_SYS_QE_FMAN_FW_ADDR	0xEF000000
+/* Fman microcode */
+#define CONFIG_SYS_QE_FMAN_FW_IN_IMAGE
+#ifndef __ASSEMBLY__
+extern unsigned char fsl_fman_ucode_P2041_101_8_bin[];
+#define CONFIG_SYS_QE_FMAN_FW_IMAGE		&fsl_fman_ucode_P2041_101_8_bin[0]
 #endif
 #define CONFIG_SYS_QE_FMAN_FW_LENGTH	0x10000
 #define CONFIG_SYS_FDT_PAD		(0x3000 + CONFIG_SYS_QE_FMAN_FW_LENGTH)

1.7.10.2.dirty

Including the firmware itself in the patch is a bad idea. I do not want any Freescale firmware to be included in the U-Boot source tree itself.

Timur Tabi
Linux kernel developer at Freescale

Dear judge.packham at gmail.com,

What exactly is the licensing of this code?

Would it not have to be GPL compatible to "link" it in such a way, which in turn would require the source code and the tools to build it ?

Best regards,

Wolfgang Denk

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
As far as the laws of mathematics refer  to  reality,  they  are  not
certain;  and  as  far  as  they  are  certain,  they do not refer to
reality.                                           -- Albert Einstein

It's not GPL-compatible, so linking the code with U-boot and shipping the binary as one blob is probably a GPL violation. That's why I created the "QE firmware binary format" and wrote all that code to treat the firmware as a separate binary blob.

Timur Tabi
Linux kernel developer at Freescale

Good point. I got the firmware out of the SDK that came with our P2041RDB. I believe the intent was that the Fman microcode is written to a reserved flash block and downloaded to the frame manager by the OS when needed. As a Freescale customer I would actually prefer to have the source released under the GPL and build everything into the u-boot image.

Well I took one law paper at university and quickly decided to major in computer science instead so I am entirely unqualified to comment :). That being said it probably depends on the definition of "link". No code is actually being called directly from u-boot, it's just a convenient transportation mechanism. Still there are statements in the GPL about what constitutes distributing the code, someone more qualified can tackle that.

I can understand why for u-boot this might be a gray area. Writing firmware blobs to a flash block somewhere is a solution that avoids any problems and makes my patch unsuitable/unnecessary.

For the record, Freescale considers embedding Fman or QE microcode into a "GPL binary" (e.g. a compiled U-boot or Linux image) to be a GPL violation. I'm sure there might be some linker or compiler tricks that some people would say are not GPL violations, but I'm not going to debate any of that. I'm just stating what our official position is.

Having said that, I do realize that Freescale still ships some QE firmware binaries as C header/source files that are supposed to be #included into customer code. Unfortunately, not all Freescale developers have gotten the message.

Timur Tabi
Linux kernel developer at Freescale