Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/19/2012 01:12:17 AM, Stefan Roese wrote: On 09/18/2012 11:51 PM, Scott Wood wrote: >>> (and then see if we can change since >>> as Scott notes, this needs to work for 4kb boards and that is >> tight). >> >> What exactly are the 4k boards? > > Anything that uses fsl_elbc_nand is 4K. I think most (all?) of the ppc > 4xx boards are 4K. Yes. All ppc4xx boards with "old" nand_spl support are 4KiB. And they don't use serial at all right now. The FSL 4K boards use serial (but not printf) with CONFIG_NS16550_MIN_FUNCTIONS. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/18/2012 11:51 PM, Scott Wood wrote: >>> (and then see if we can change since >>> as Scott notes, this needs to work for 4kb boards and that is >> tight). >> >> What exactly are the 4k boards? > > Anything that uses fsl_elbc_nand is 4K. I think most (all?) of the ppc > 4xx boards are 4K. Yes. All ppc4xx boards with "old" nand_spl support are 4KiB. And they don't use serial at all right now. Thanks, Stefan ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 12:25, Marek Vasut wrote: > > Dear Tom Rini, > > > >> On 09/18/12 12:19, Marek Vasut wrote: > >>> Dear Tom Rini, > >>> > On 09/18/12 11:33, Marek Vasut wrote: > > Dear Scott Wood, > > [snip] > > >> I think I got some wires crossed and was thinking about > >> printf/puts. We want those to be optimized away at > >> compile time (not pointed to a stub at link time) on an > >> SPL that has no output support, but once that's done the > >> low level serial functions shouldn't be referenced > >> anymore, right? > > > > But if you point them to stubs, that's OK. The compiler > > will GC these useless stubs anyway. But wait, we're getting > > to LTO here, right? > > > > So the safest bet really is macro in serial.h ? > > Due to the gcc bug I've mentioned before, yes. Dummy > functions will, I bet, keep the string constants around. do > {} while(0) will drop them out entirely. > >>> > >>> Damn, not much gain on m28evk (with C functionss/with macros), > >>> using gcc 4.7.1: > >>> > >>> Configuring for m28evk board... textdata bss dec > >>> hex filename 4189947780 288632 715406 aea8e ./u-boot > >>> 11773 788 12 12573311d ./spl/u-boot-spl > >>> > >>> Configuring for m28evk board... textdata bss dec > >>> hex filename 4189987780 288628 715406 aea8e ./u-boot > >>> 11765 788 12 125653115 ./spl/u-boot-spl > >> > >> Right, didn't have many strings. But do you see what I mean now > >> about not needing this patch as it stands currently? > > > > No, I don't. If I remove this patch, the build breaks either > > because serial_* is defined twice or not defined at all. > > m28evk currently, needlessly, defines serial_puts/putc. I locally > patched master to drop them from arch/arm/cpu/arm926ejs/mxs/spl_boot.c > and the references in common/libcommon.o are correctly > garbage-collected. They are in fact unused functions today as they're > garbage collected without patching, see spl/u-boot-spl.map after > building. So again I say, if common/serial.o is NOT being discard in > your series on m28evk there is a bug in your series to fix or a change > to better understand and document (and then see if we can change since > as Scott notes, this needs to work for 4kb boards and that is tight). Ok, so we cleared this up on jabber, latest ToT works correctly, discard this patch. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/18/2012 04:19:07 PM, Marek Vasut wrote: Dear Tom Rini, > (and then see if we can change since > as Scott notes, this needs to work for 4kb boards and that is tight). What exactly are the 4k boards? Anything that uses fsl_elbc_nand is 4K. I think most (all?) of the ppc 4xx boards are 4K. fsl_ifc_nand is 8K, though the linker script needs some rework to fully take advantage of that. At least some of the i.MX boards have constraints but I don't know exactly what they are. Maybe some others. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 12:25, Marek Vasut wrote: > > Dear Tom Rini, > > > >> On 09/18/12 12:19, Marek Vasut wrote: > >>> Dear Tom Rini, > >>> > On 09/18/12 11:33, Marek Vasut wrote: > > Dear Scott Wood, > > [snip] > > >> I think I got some wires crossed and was thinking about > >> printf/puts. We want those to be optimized away at > >> compile time (not pointed to a stub at link time) on an > >> SPL that has no output support, but once that's done the > >> low level serial functions shouldn't be referenced > >> anymore, right? > > > > But if you point them to stubs, that's OK. The compiler > > will GC these useless stubs anyway. But wait, we're getting > > to LTO here, right? > > > > So the safest bet really is macro in serial.h ? > > Due to the gcc bug I've mentioned before, yes. Dummy > functions will, I bet, keep the string constants around. do > {} while(0) will drop them out entirely. > >>> > >>> Damn, not much gain on m28evk (with C functionss/with macros), > >>> using gcc 4.7.1: > >>> > >>> Configuring for m28evk board... textdata bss dec > >>> hex filename 4189947780 288632 715406 aea8e ./u-boot > >>> 11773 788 12 12573311d ./spl/u-boot-spl > >>> > >>> Configuring for m28evk board... textdata bss dec > >>> hex filename 4189987780 288628 715406 aea8e ./u-boot > >>> 11765 788 12 125653115 ./spl/u-boot-spl > >> > >> Right, didn't have many strings. But do you see what I mean now > >> about not needing this patch as it stands currently? > > > > No, I don't. If I remove this patch, the build breaks either > > because serial_* is defined twice or not defined at all. > > m28evk currently, needlessly, defines serial_puts/putc. I locally > patched master to drop them from arch/arm/cpu/arm926ejs/mxs/spl_boot.c > and the references in common/libcommon.o are correctly > garbage-collected. They are in fact unused functions today as they're > garbage collected without patching, see spl/u-boot-spl.map after > building. I'd love to, this is what I get with my patchset when I remove the #ifdef from serial.c: $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabi- ./MAKEALL m28evk Configuring for m28evk board... make[1]: *** [/home/marex/U-Boot/u-boot-marex/spl/u-boot-spl] Error 1 make: *** [spl/u-boot-spl.bin] Error 2 arm-linux-gnueabi-size: './u-boot': No such file common/libcommon.o: In function `get_current': /home/marex/U-Boot/u-boot-marex/common/serial.c:229: undefined reference to `default_serial_console' make[1]: *** [/home/marex/U-Boot/u-boot-marex/spl/u-boot-spl] Error 1 make: *** [spl/u-boot-spl.bin] Error 2 make: *** Waiting for unfinished jobs So someone still has to implement default_serial_console() call. Is that fine ? > So again I say, if common/serial.o is NOT being discard in > your series on m28evk there is a bug in your series to fix or a change > to better understand and document Ok, I spent hours documenting this series. What else is there to document? Besides, my team is starting to collect dead weight and we're running behind the schedule a lot ... I'm fucked, I'm desperate and I really don't know what to do anymore. Pardon if I'm a bit unpleasant to deal with recently. > (and then see if we can change since > as Scott notes, this needs to work for 4kb boards and that is tight). What exactly are the 4k boards? One more question -- if I have two __weak functions and one non-weak, the non- weak wins and noone complains, right? Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 12:25, Marek Vasut wrote: > Dear Tom Rini, > >> On 09/18/12 12:19, Marek Vasut wrote: >>> Dear Tom Rini, >>> On 09/18/12 11:33, Marek Vasut wrote: > Dear Scott Wood, [snip] >> I think I got some wires crossed and was thinking about >> printf/puts. We want those to be optimized away at >> compile time (not pointed to a stub at link time) on an >> SPL that has no output support, but once that's done the >> low level serial functions shouldn't be referenced >> anymore, right? > > But if you point them to stubs, that's OK. The compiler > will GC these useless stubs anyway. But wait, we're getting > to LTO here, right? > > So the safest bet really is macro in serial.h ? Due to the gcc bug I've mentioned before, yes. Dummy functions will, I bet, keep the string constants around. do {} while(0) will drop them out entirely. >>> >>> Damn, not much gain on m28evk (with C functionss/with macros), >>> using gcc 4.7.1: >>> >>> Configuring for m28evk board... textdata bss dec >>> hex filename 4189947780 288632 715406 aea8e ./u-boot >>> 11773 788 12 12573311d ./spl/u-boot-spl >>> >>> Configuring for m28evk board... textdata bss dec >>> hex filename 4189987780 288628 715406 aea8e ./u-boot >>> 11765 788 12 125653115 ./spl/u-boot-spl >> >> Right, didn't have many strings. But do you see what I mean now >> about not needing this patch as it stands currently? > > No, I don't. If I remove this patch, the build breaks either > because serial_* is defined twice or not defined at all. m28evk currently, needlessly, defines serial_puts/putc. I locally patched master to drop them from arch/arm/cpu/arm926ejs/mxs/spl_boot.c and the references in common/libcommon.o are correctly garbage-collected. They are in fact unused functions today as they're garbage collected without patching, see spl/u-boot-spl.map after building. So again I say, if common/serial.o is NOT being discard in your series on m28evk there is a bug in your series to fix or a change to better understand and document (and then see if we can change since as Scott notes, this needs to work for 4kb boards and that is tight). - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWN59AAoJENk4IS6UOR1WtG0P/iYMxTtr0X/F6RRhZ9sLlSNt pj6o+MTd9eO4e64+0A/jyXDuFd9NBvYdsjBKz8iyQFOrUYAyCdKdl+MZ+E3sD57O gHKVXcfnMS2jnXTvKMabn4Ddjl8FVjLz+YGXFcwqBpcN5KjuyRKJnl4jntrj8QgK 1d9aqYwnQcMbP36ApPS1WRGotAlydhmL9Bw++ebk+j28iBs1KZWiFK1RUCYc1b5T bverqK4EQTMxOh8KJNvGs5J61bO1BUA3fWHv2kzKo4XEr+XjRkoFVXDb1Tjf1Xlw ZDaaky+zyq7D4zwZUFJxseDN7dBTuAYNoYj5UhYvkbTO04s60vKzhyuLMDWxyHrx ABrmisfWB44K6sjQKZUTBvO6gajA8fe9kTg/uaMaG+9h9xyM0oNBfDmDd7/CK6PE Mi7q9TJ7cOh1DgHrzSrLaO8n5cam4B3XecLH5Rj1uOA5HHCnKExIxWTdeEg4w63b VQVdgQ7g/2x8TINdjz9oo1B79n+yHlmIHzc64ZNnBAJhVDKM9h9h6FarAvfwLM1B Ns/vUCrM17vsaduNcvD0ZGDZSk4MOc8dfPLqfLe9rzFH4VAEmTV8qEaFjnbeORiD jknV1mLAYD1A/eu1AHdwE58OgPgCTKyYxLA4bE/Yldxy1L+E5DUx+nmOnrCNWPPm dqdGjBg7P4HHZo2mw6oZ =QI/g -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 12:19, Marek Vasut wrote: > > Dear Tom Rini, > > > >> On 09/18/12 11:33, Marek Vasut wrote: > >>> Dear Scott Wood, > >> > >> [snip] > >> > I think I got some wires crossed and was thinking about > printf/puts. We want those to be optimized away at compile > time (not pointed to a stub at link time) on an SPL that has > no output support, but once that's done the low level serial > functions shouldn't be referenced anymore, right? > >>> > >>> But if you point them to stubs, that's OK. The compiler will GC > >>> these useless stubs anyway. But wait, we're getting to LTO > >>> here, right? > >>> > >>> So the safest bet really is macro in serial.h ? > >> > >> Due to the gcc bug I've mentioned before, yes. Dummy functions > >> will, I bet, keep the string constants around. do {} while(0) > >> will drop them out entirely. > > > > Damn, not much gain on m28evk (with C functionss/with macros), > > using gcc 4.7.1: > > > > Configuring for m28evk board... textdata bss dec hex > > filename 4189947780 288632 715406 aea8e ./u-boot 11773 788 > > 12 12573311d ./spl/u-boot-spl > > > > Configuring for m28evk board... textdata bss dec hex > > filename 4189987780 288628 715406 aea8e ./u-boot 11765 788 > > 12 125653115 ./spl/u-boot-spl > > Right, didn't have many strings. But do you see what I mean now about > not needing this patch as it stands currently? No, I don't. If I remove this patch, the build breaks either because serial_* is defined twice or not defined at all. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Scott Wood, > On 09/18/2012 01:33:11 PM, Marek Vasut wrote: > > Dear Scott Wood, > > > > > On 09/18/2012 01:03:17 PM, Marek Vasut wrote: > > > > I'd say the GCC must optimize it out anyway. > > > > > > I think I got some wires crossed and was thinking about printf/puts. > > > We want those to be optimized away at compile time (not pointed to a > > > stub at link time) on an SPL that has no output support, but once > > > that's done the low level serial functions shouldn't be referenced > > > anymore, right? > > > > But if you point them to stubs, that's OK. The compiler will GC these > > useless > > stubs anyway. But wait, we're getting to LTO here, right? > > > > So the safest bet really is macro in serial.h ? > > For printf/puts, we want something header-based. For the serial > functions it depends on whether we have call sites that do not get GCed. I'm not removing printf() puts() etc. .. only the serial_ goo ... and see my other email, not much gain :( > -Scott Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 12:19, Marek Vasut wrote: > Dear Tom Rini, > >> On 09/18/12 11:33, Marek Vasut wrote: >>> Dear Scott Wood, >> >> [snip] >> I think I got some wires crossed and was thinking about printf/puts. We want those to be optimized away at compile time (not pointed to a stub at link time) on an SPL that has no output support, but once that's done the low level serial functions shouldn't be referenced anymore, right? >>> >>> But if you point them to stubs, that's OK. The compiler will GC >>> these useless stubs anyway. But wait, we're getting to LTO >>> here, right? >>> >>> So the safest bet really is macro in serial.h ? >> >> Due to the gcc bug I've mentioned before, yes. Dummy functions >> will, I bet, keep the string constants around. do {} while(0) >> will drop them out entirely. > > Damn, not much gain on m28evk (with C functionss/with macros), > using gcc 4.7.1: > > Configuring for m28evk board... textdata bss dec hex > filename 4189947780 288632 715406 aea8e ./u-boot 11773 788 > 12 12573311d ./spl/u-boot-spl > > Configuring for m28evk board... textdata bss dec hex > filename 4189987780 288628 715406 aea8e ./u-boot 11765 788 > 12 125653115 ./spl/u-boot-spl Right, didn't have many strings. But do you see what I mean now about not needing this patch as it stands currently? - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWMojAAoJENk4IS6UOR1Ws/EP/jx1vvg3N2gIlSVRfl6em3ul VwBi/tLW+mAlF3V/+Ge3h//U9gAef6uDbRlLUngxAvVuHQZZb7gqtf6T9Zw7DDu/ BFLSocaLi99rnEdwEZe4lApJnBP3pZEcLnHiKVvFN+lGSA7G6vEzJemawnxhFdKh B9MgtxgKEe3EUxdKj8rXaXvPUIO+NpQ/BcI2FLQrJfr8nH0mK6m1yNFEe3VYc64y 3dUTxr1ILS6O2uLvf1ErUdSi7YZOnkAwpyw+mTLF6weCJNisrDCrChjZibeBEtVN ZdH5ZkKckXegy3N6HM/tDuLGaO5spvxM797gS1tzqesPrMWy+ng9npFwqk6zxM8Y rtG6G0ddtAk0u6UCEDvoQiYPciNY4F+YhuhesVXZVUe7l09XbZdiDLHXlqR34hVo 9H1qPCfi7DmvRR/mArG4URc9TkbsjsQkZp1s1/3jDFlM6xenM+2SdTy3ncrsMwmx Ri92BjdOE+VQSdgqexV660yjNB3qYn2AC7/dtgNhaZA7/+p7XSip3NnjTmTr7buL xoo8sse/sr8viGDCyyWf8Bv/sOvc5pqR0SYu3187BkgkMlAv3Se/lwNT/r+lPqFd K0w69mMqN+WNEQYHkisE2bKGsuKCWOLt/KTFvDNXQJZsxa4uE2lbNSTXzStsGcAw rh+d7dV2ylpGxRccUYDb =dyAB -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/18/2012 01:33:11 PM, Marek Vasut wrote: Dear Scott Wood, > On 09/18/2012 01:03:17 PM, Marek Vasut wrote: > > I'd say the GCC must optimize it out anyway. > > I think I got some wires crossed and was thinking about printf/puts. > We want those to be optimized away at compile time (not pointed to a > stub at link time) on an SPL that has no output support, but once > that's done the low level serial functions shouldn't be referenced > anymore, right? But if you point them to stubs, that's OK. The compiler will GC these useless stubs anyway. But wait, we're getting to LTO here, right? So the safest bet really is macro in serial.h ? For printf/puts, we want something header-based. For the serial functions it depends on whether we have call sites that do not get GCed. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 11:33, Marek Vasut wrote: > > Dear Scott Wood, > > [snip] > > >> I think I got some wires crossed and was thinking about > >> printf/puts. We want those to be optimized away at compile time > >> (not pointed to a stub at link time) on an SPL that has no output > >> support, but once that's done the low level serial functions > >> shouldn't be referenced anymore, right? > > > > But if you point them to stubs, that's OK. The compiler will GC > > these useless stubs anyway. But wait, we're getting to LTO here, > > right? > > > > So the safest bet really is macro in serial.h ? > > Due to the gcc bug I've mentioned before, yes. Dummy functions will, > I bet, keep the string constants around. do {} while(0) will drop > them out entirely. Damn, not much gain on m28evk (with C functionss/with macros), using gcc 4.7.1: Configuring for m28evk board... textdata bss dec hex filename 4189947780 288632 715406 aea8e ./u-boot 11773 788 12 12573311d ./spl/u-boot-spl Configuring for m28evk board... textdata bss dec hex filename 4189987780 288628 715406 aea8e ./u-boot 11765 788 12 125653115 ./spl/u-boot-spl Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 11:33, Marek Vasut wrote: > > Dear Scott Wood, > > [snip] > > >> I think I got some wires crossed and was thinking about > >> printf/puts. We want those to be optimized away at compile time > >> (not pointed to a stub at link time) on an SPL that has no output > >> support, but once that's done the low level serial functions > >> shouldn't be referenced anymore, right? > > > > But if you point them to stubs, that's OK. The compiler will GC > > these useless stubs anyway. But wait, we're getting to LTO here, > > right? > > > > So the safest bet really is macro in serial.h ? > > Due to the gcc bug I've mentioned before, yes. Dummy functions will, > I bet, keep the string constants around. do {} while(0) will drop > them out entirely. Yea ... the GCC bug, what a crap :-( Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 11:33, Marek Vasut wrote: > Dear Scott Wood, [snip] >> I think I got some wires crossed and was thinking about >> printf/puts. We want those to be optimized away at compile time >> (not pointed to a stub at link time) on an SPL that has no output >> support, but once that's done the low level serial functions >> shouldn't be referenced anymore, right? > > But if you point them to stubs, that's OK. The compiler will GC > these useless stubs anyway. But wait, we're getting to LTO here, > right? > > So the safest bet really is macro in serial.h ? Due to the gcc bug I've mentioned before, yes. Dummy functions will, I bet, keep the string constants around. do {} while(0) will drop them out entirely. - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWL/mAAoJENk4IS6UOR1WwXQQAJXFmoGOjTxtuq1PMDYIEUSg mGwZUgjTDy3wrzVl8xkzuSeRYtqL/vFbThHDVoAiWXcQ2/4Mrcunl3v3UW+QV2ye KwESGqd05CIUEDxFqihOIKCR2KZHpUkt45Uf6SPOXfB4A0O7N9CuvyxPl2ZFHGxx ePFwopmX9gL7xO3u1cjAtxtmiCS22ulztW3ROU3+NTsVKA3k4AXW617tjpsPmQzJ L9N2LX49Z+UGDxh7YW/M4wcD50GlZFyIUY1COyhxeAQXmCXRMeDJdqxU1f3+SG1G fnWsBPoVdIJEv8XBr+ABNd4DYDZCWsTA7uklvkt2NDh64Lp+Nge5dD92fZJfrKoc NUWLhXN1U9ko9xbflxlBK94zkmtJfLfvtboK58frjv/H7MlSIuUzbgH4ixq/5ZOM g5pKFQ2YynrZ0yrjqH8I/v50GsziT+LpAiQnE62Yt2EQMkNCIC1zDz9ikg3MhL63 sxiZPi0mpcbvao6f6l0JIllMkvEWBnM4fGQCWMGGOkjbCqvkSnBNt4BhgAK2ZXuC kcLkdeOhszdWZxhfK+V0d5U+XQdIJoHdYyVC+6hAEd5iO4++gXgx+8feQV+ZQvSS 8iCdnobNp6XfM6agNOpkJto0+ROqEIyDSUzBAOb3+474fSWBjhhY7ievGxZiKikx mhsHRYG6ziEdOt4bkQ5H =m9GH -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Scott Wood, > On 09/18/2012 01:03:17 PM, Marek Vasut wrote: > > Dear Scott Wood, > > > > > On 09/18/2012 12:13:57 PM, Marek Vasut wrote: > > > > Dear Tom Rini, > > > > > > > > > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > > > > > Implement empty serial_* functions for SPL without serial > > > > > > support enabled. This is imperative to haave once serial > > > > > > multi is enabled unconditionally. > > > > > > > > > > > > Signed-off-by: Marek Vasut > > > > > > Cc: Marek Vasut > > > > > > Cc: Tom Rini > > > > > > --- > > > > > > > > > > > > common/serial.c | 12 > > > > > > 1 file changed, 12 insertions(+) > > > > > > > > > > > > diff --git a/common/serial.c b/common/serial.c > > > > > > index 631af65..cef4ba8 100644 > > > > > > --- a/common/serial.c > > > > > > +++ b/common/serial.c > > > > > > @@ -27,6 +27,16 @@ > > > > > > > > > > > > #include > > > > > > #include > > > > > > > > > > > > +/* Implement prototypes for SPL without serial support */ > > > > > > +#if defined(CONFIG_SPL_BUILD) && > > > > > > > > !defined(CONFIG_SPL_SERIAL_SUPPORT) > > > > > > > > > > +int serial_init(void) { return 0; } > > > > > > +void serial_setbrg(void) {} > > > > > > +int serial_getc(void) { return 0; } > > > > > > +int serial_tstc(void) { return 0; } > > > > > > +void serial_putc(const char c) {} > > > > > > +void serial_puts(const char *s) {} > > > > > > > > > > This isn't quite right. We need to allow for: > > > > > - No output SPL, strings collected (so #defined to do{} while > > > > (0)) > > > > > > Which is not type-checked and will drag in bugs. > > > > > > Not all that likely, since most code will either be built with > > > > printf > > > > > enabled some of the time, or not contain printf (i.e. it's not quite > > > the same situation as debug prints which may be rarely enabled). > > > > > > An inline function would be fine, but has to be done at the same > > > > place > > > > > that normal printf is declared -- whereas a macro could be done > > > > after > > > > > the fact. Unless you use a macro to redirect it to an inline > > > > function > > > > > of a different name... > > > > > > I do not understand why you want to put these stubs in a C file > > > > instead > > > > > of a header file, though -- you're preventing code from being > > > > optimized > > > > > away, which is important in some SPLs. > > > > I'd say the GCC must optimize it out anyway. > > I think I got some wires crossed and was thinking about printf/puts. > We want those to be optimized away at compile time (not pointed to a > stub at link time) on an SPL that has no output support, but once > that's done the low level serial functions shouldn't be referenced > anymore, right? But if you point them to stubs, that's OK. The compiler will GC these useless stubs anyway. But wait, we're getting to LTO here, right? So the safest bet really is macro in serial.h ? > -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 11:24, Marek Vasut wrote: > Dear Tom Rini, > >> On 09/18/12 11:01, Marek Vasut wrote: >>> Dear Tom Rini, >>> On 09/18/12 10:13, Marek Vasut wrote: > Dear Tom Rini, > >> On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut >> wrote: >>> Implement empty serial_* functions for SPL without >>> serial support enabled. This is imperative to haave >>> once serial multi is enabled unconditionally. >>> >>> Signed-off-by: Marek Vasut Cc: Marek >>> Vasut Cc: Tom Rini >>> --- >>> >>> common/serial.c | 12 1 file changed, 12 >>> insertions(+) >>> >>> diff --git a/common/serial.c b/common/serial.c index >>> 631af65..cef4ba8 100644 --- a/common/serial.c +++ >>> b/common/serial.c @@ -27,6 +27,16 @@ >>> >>> #include #include >>> >>> +/* Implement prototypes for SPL without serial support >>> */ +#if defined(CONFIG_SPL_BUILD) && >>> !defined(CONFIG_SPL_SERIAL_SUPPORT) +int >>> serial_init(void) { return 0; } +void >>> serial_setbrg(void) {} +int serial_getc(void) { return >>> 0; } +int serial_tstc(void) { return 0; } +void >>> serial_putc(const char c) {} +void serial_puts(const >>> char *s) {} >> >> This isn't quite right. We need to allow for: - No >> output SPL, strings collected (so #defined to do{} while >> (0)) > > Which is not type-checked and will drag in bugs. Scott has addressed this. >> - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + >> CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL >> (CONFIG_SPL_SERIAL_SUPPORT + #define puts >> serial_puts/putc). >> >> I'm not asking you to do that, but you will have to adapt >> to it once Jose is done with it. What that means, off >> the top of my head, is we can just drop this patch as in >> the first and last case serial.o will be >> garbage-collected (or not built) and in the middle case, >> this will be fully used. > > I can't drop this patch as it will break all of SPL when > CONFIG_SERIAL_MULTI is unconditionally enabled. Why is it breaking _all_ of SPL? Have you run-tested this anywhere, especially with SPL? In most cases it should be used and real functions provided or garbage collected away. >>> >>> Yes, try compiling m28evk without this patch for example, it's >>> going to break it. Because CONFIG_SPL_SERIAL_SUPPORT is >>> disabled, yet it uses code that contains references to puts() >>> etc. >> >> Progress! Now, why isn't this file being garbage collected? > > What file? common/serial.o since as you point out, m28evk doesn't define any way to do output. >> m28evk is fitting into the first category I said (no output) but >> now it's not discarding things that it should be discarding. > > What is not discarding things and what things should be discarded? > I believe if you're missing symbols, the compiler will error-out. > Always. Nope. This is fine: gc_this_function(void) { never_define_this_at_link(); return; } And nothing calling gc_this_function means that it's fine that never_define_this_at_link isn't seen by the linker. - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWL7QAAoJENk4IS6UOR1W+KgP/jjDmO3c+WfYqEjuEjLMjSAW qTLZOLdsmsU7HoFa+/fWNgmvvmcJlTgqzo0z5izku2d0xx8TO3R6rpZa9weHXhr1 yuKs+CzP/6A1Kntd8VC0SRUU+Rb4onPPoY0kw0QDL01zug5DBEu+saI08CJRrtki DLzayRPNoTcppffp1r2nstyAJJWvuYcFO4A3wzR3h5U1lQNHK7Yt8KRtmCFQW1d1 Y98ikHi75PDcSZDjj60OHVhNtaDDcLUu2NWAXrf4gI13WLPxcNXHRTq1uufY38Pv fNd5wqrC7qDq7I6uomwuy+b6aDYYPqsrh9T/h/tjWO235mA+7Dnkl2qvHrYOOcV6 1zBef8M+vuawVWYZnsJO4k1Cg2Ci9Gl4sPqJVYaSnhhXjQawZbztQpT1P4tN1DEG 8r7mpt6bWGG1nnEgiNWvFZvv798sj2Lh/T0yxAsnX9CgnlxZ7lh+uqirMmUJeUKB aWiuDJIMqQORXcJIO1tDwtL+2EA5CxofLa11Y0tpT0r2G0cOsQQQfJTQ6K9p4KhB gkOhRmlPQs12WV+9r6LWuUqDRuIbMjOUHfNOf9eZfKTvptMMRhwT1zCVBdMVwbwO 3e/WpNTDjRLpqj08bs6OHOVO7GvXXtZJGHJJGlJ3a49pHMnqNUjBGSajDYJyHL0O /75PPDTIXSrUJw1anFC8 =yREs -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/18/2012 01:03:17 PM, Marek Vasut wrote: Dear Scott Wood, > On 09/18/2012 12:13:57 PM, Marek Vasut wrote: > > Dear Tom Rini, > > > > > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > > > Implement empty serial_* functions for SPL without serial > > > > support enabled. This is imperative to haave once serial > > > > multi is enabled unconditionally. > > > > > > > > Signed-off-by: Marek Vasut > > > > Cc: Marek Vasut > > > > Cc: Tom Rini > > > > --- > > > > > > > > common/serial.c | 12 > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/common/serial.c b/common/serial.c > > > > index 631af65..cef4ba8 100644 > > > > --- a/common/serial.c > > > > +++ b/common/serial.c > > > > @@ -27,6 +27,16 @@ > > > > > > > > #include > > > > #include > > > > > > > > +/* Implement prototypes for SPL without serial support */ > > > > +#if defined(CONFIG_SPL_BUILD) && > > > > !defined(CONFIG_SPL_SERIAL_SUPPORT) > > > > > > +int serial_init(void) { return 0; } > > > > +void serial_setbrg(void) {} > > > > +int serial_getc(void) { return 0; } > > > > +int serial_tstc(void) { return 0; } > > > > +void serial_putc(const char c) {} > > > > +void serial_puts(const char *s) {} > > > > > > This isn't quite right. We need to allow for: > > > - No output SPL, strings collected (so #defined to do{} while (0)) > > > > Which is not type-checked and will drag in bugs. > > Not all that likely, since most code will either be built with printf > enabled some of the time, or not contain printf (i.e. it's not quite > the same situation as debug prints which may be rarely enabled). > > An inline function would be fine, but has to be done at the same place > that normal printf is declared -- whereas a macro could be done after > the fact. Unless you use a macro to redirect it to an inline function > of a different name... > > I do not understand why you want to put these stubs in a C file instead > of a header file, though -- you're preventing code from being optimized > away, which is important in some SPLs. I'd say the GCC must optimize it out anyway. I think I got some wires crossed and was thinking about printf/puts. We want those to be optimized away at compile time (not pointed to a stub at link time) on an SPL that has no output support, but once that's done the low level serial functions shouldn't be referenced anymore, right? -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 11:01, Marek Vasut wrote: > > Dear Tom Rini, > > > >> On 09/18/12 10:13, Marek Vasut wrote: > >>> Dear Tom Rini, > >>> > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > Implement empty serial_* functions for SPL without serial > > support enabled. This is imperative to haave once serial > > multi is enabled unconditionally. > > > > Signed-off-by: Marek Vasut Cc: Marek Vasut > > Cc: Tom Rini --- > > > > common/serial.c | 12 1 file changed, 12 > > insertions(+) > > > > diff --git a/common/serial.c b/common/serial.c index > > 631af65..cef4ba8 100644 --- a/common/serial.c +++ > > b/common/serial.c @@ -27,6 +27,16 @@ > > > > #include #include > > > > +/* Implement prototypes for SPL without serial support */ > > +#if defined(CONFIG_SPL_BUILD) && > > !defined(CONFIG_SPL_SERIAL_SUPPORT) +int serial_init(void) > > { return 0; } +void serial_setbrg(void) {} +int > > serial_getc(void) { return 0; } +int serial_tstc(void) { > > return 0; } +void serial_putc(const char c) {} +void > > serial_puts(const char *s) {} > > This isn't quite right. We need to allow for: - No output > SPL, strings collected (so #defined to do{} while (0)) > >>> > >>> Which is not type-checked and will drag in bugs. > >> > >> Scott has addressed this. > >> > - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + > CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL > (CONFIG_SPL_SERIAL_SUPPORT + #define puts serial_puts/putc). > > I'm not asking you to do that, but you will have to adapt to > it once Jose is done with it. What that means, off the top > of my head, is we can just drop this patch as in the first > and last case serial.o will be garbage-collected (or not > built) and in the middle case, this will be fully used. > >>> > >>> I can't drop this patch as it will break all of SPL when > >>> CONFIG_SERIAL_MULTI is unconditionally enabled. > >> > >> Why is it breaking _all_ of SPL? Have you run-tested this > >> anywhere, especially with SPL? In most cases it should be used > >> and real functions provided or garbage collected away. > > > > Yes, try compiling m28evk without this patch for example, it's > > going to break it. Because CONFIG_SPL_SERIAL_SUPPORT is disabled, > > yet it uses code that contains references to puts() etc. > > Progress! Now, why isn't this file being garbage collected? What file? > m28evk > is fitting into the first category I said (no output) but now it's not > discarding things that it should be discarding. What is not discarding things and what things should be discarded? I believe if you're missing symbols, the compiler will error-out. Always. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 11:01, Marek Vasut wrote: > Dear Tom Rini, > >> On 09/18/12 10:13, Marek Vasut wrote: >>> Dear Tom Rini, >>> On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > Implement empty serial_* functions for SPL without serial > support enabled. This is imperative to haave once serial > multi is enabled unconditionally. > > Signed-off-by: Marek Vasut Cc: Marek Vasut > Cc: Tom Rini --- > > common/serial.c | 12 1 file changed, 12 > insertions(+) > > diff --git a/common/serial.c b/common/serial.c index > 631af65..cef4ba8 100644 --- a/common/serial.c +++ > b/common/serial.c @@ -27,6 +27,16 @@ > > #include #include > > +/* Implement prototypes for SPL without serial support */ > +#if defined(CONFIG_SPL_BUILD) && > !defined(CONFIG_SPL_SERIAL_SUPPORT) +int serial_init(void) > { return 0; } +void serial_setbrg(void) {} +int > serial_getc(void) { return 0; } +int serial_tstc(void) { > return 0; } +void serial_putc(const char c) {} +void > serial_puts(const char *s) {} This isn't quite right. We need to allow for: - No output SPL, strings collected (so #defined to do{} while (0)) >>> >>> Which is not type-checked and will drag in bugs. >> >> Scott has addressed this. >> - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL (CONFIG_SPL_SERIAL_SUPPORT + #define puts serial_puts/putc). I'm not asking you to do that, but you will have to adapt to it once Jose is done with it. What that means, off the top of my head, is we can just drop this patch as in the first and last case serial.o will be garbage-collected (or not built) and in the middle case, this will be fully used. >>> >>> I can't drop this patch as it will break all of SPL when >>> CONFIG_SERIAL_MULTI is unconditionally enabled. >> >> Why is it breaking _all_ of SPL? Have you run-tested this >> anywhere, especially with SPL? In most cases it should be used >> and real functions provided or garbage collected away. > > Yes, try compiling m28evk without this patch for example, it's > going to break it. Because CONFIG_SPL_SERIAL_SUPPORT is disabled, > yet it uses code that contains references to puts() etc. Progress! Now, why isn't this file being garbage collected? m28evk is fitting into the first category I said (no output) but now it's not discarding things that it should be discarding. - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWLifAAoJENk4IS6UOR1WyiIP/1h3r1OBQ8sxHCq6nuWT7cvQ 6taDBU6780uOY+YsbNgjlImp7lSM30HYxXou2j2kykPcRjUPMzLoYDRZio+8d8RW ETQcMld8I/OMz52HT6lnjIaqVBOpK42vlRW86LNcxIFOasYlK4qxO3kjKmshu5aC ct7b5xcHFaqfxLp2EjkUOgmoyPXhNTdsnDVdOTaXG7qGKAffDFCeUHTsBB3kc/S6 w5HwDNTBWZYVMWuTXKXLXh+3h4x+VL1LCxCsnu8R88cEkj7b9DkqGUsUrCDPFqc/ YAqiUqacTa0V9h9XeE/OdZUo7uS04FibPHzvho91LcnBdjOJ7jPLY7k/IJ7guhqp aRC9UrB/AAPkpLExo32Ksx+7wAJThsfWY6DL5oI76E4FYZP2WaqygBM/WDCbcOK7 6HIGItjGwFpXBCDbawKob395Kt5gK2J43qXR2E7CR4p3ic8liMqsWu5J+TCUVF6b jxjLZ22Bw5zolUkhUE5u+M+O/rxCjYG0HNTssC1ymYR/jU36p1m6oGqxVN8Voi0R 1ARQB2yY3uuQOqR1URZMzuA94d1Qffnhg3LwSm3cJRH825WNDkxEXq/hvK/4hLbH DXb79+zRqv7f80jPUEk60sKFI3YzJMvRBaaxjXqOkMFywaNMQjbsnXCmvztoqRqG A1gmJGTWixOQbrN56DjF =wRYN -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Scott Wood, > On 09/18/2012 12:13:57 PM, Marek Vasut wrote: > > Dear Tom Rini, > > > > > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > > > Implement empty serial_* functions for SPL without serial > > > > support enabled. This is imperative to haave once serial > > > > multi is enabled unconditionally. > > > > > > > > Signed-off-by: Marek Vasut > > > > Cc: Marek Vasut > > > > Cc: Tom Rini > > > > --- > > > > > > > > common/serial.c | 12 > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/common/serial.c b/common/serial.c > > > > index 631af65..cef4ba8 100644 > > > > --- a/common/serial.c > > > > +++ b/common/serial.c > > > > @@ -27,6 +27,16 @@ > > > > > > > > #include > > > > #include > > > > > > > > +/* Implement prototypes for SPL without serial support */ > > > > +#if defined(CONFIG_SPL_BUILD) && > > > > !defined(CONFIG_SPL_SERIAL_SUPPORT) > > > > > > +int serial_init(void) { return 0; } > > > > +void serial_setbrg(void) {} > > > > +int serial_getc(void) { return 0; } > > > > +int serial_tstc(void) { return 0; } > > > > +void serial_putc(const char c) {} > > > > +void serial_puts(const char *s) {} > > > > > > This isn't quite right. We need to allow for: > > > - No output SPL, strings collected (so #defined to do{} while (0)) > > > > Which is not type-checked and will drag in bugs. > > Not all that likely, since most code will either be built with printf > enabled some of the time, or not contain printf (i.e. it's not quite > the same situation as debug prints which may be rarely enabled). > > An inline function would be fine, but has to be done at the same place > that normal printf is declared -- whereas a macro could be done after > the fact. Unless you use a macro to redirect it to an inline function > of a different name... > > I do not understand why you want to put these stubs in a C file instead > of a header file, though -- you're preventing code from being optimized > away, which is important in some SPLs. I'd say the GCC must optimize it out anyway. And if not, what solution do you suggest, move these into include/serial.h and compile serial.c in conditionally? > -Scott Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On 09/18/12 10:13, Marek Vasut wrote: > > Dear Tom Rini, > > > >> On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > >>> Implement empty serial_* functions for SPL without serial > >>> support enabled. This is imperative to haave once serial multi > >>> is enabled unconditionally. > >>> > >>> Signed-off-by: Marek Vasut Cc: Marek Vasut > >>> Cc: Tom Rini --- > >>> > >>> common/serial.c | 12 1 file changed, 12 > >>> insertions(+) > >>> > >>> diff --git a/common/serial.c b/common/serial.c index > >>> 631af65..cef4ba8 100644 --- a/common/serial.c +++ > >>> b/common/serial.c @@ -27,6 +27,16 @@ > >>> > >>> #include #include > >>> > >>> +/* Implement prototypes for SPL without serial support */ > >>> +#if defined(CONFIG_SPL_BUILD) && > >>> !defined(CONFIG_SPL_SERIAL_SUPPORT) +int serial_init(void) { > >>> return 0; } +void serial_setbrg(void) {} +int > >>> serial_getc(void) { return 0; } +int serial_tstc(void) { return > >>> 0; } +void serial_putc(const char c) {} +void serial_puts(const > >>> char *s) {} > >> > >> This isn't quite right. We need to allow for: - No output SPL, > >> strings collected (so #defined to do{} while (0)) > > > > Which is not type-checked and will drag in bugs. > > Scott has addressed this. > > >> - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + > >> CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL > >> (CONFIG_SPL_SERIAL_SUPPORT + #define puts serial_puts/putc). > >> > >> I'm not asking you to do that, but you will have to adapt to it > >> once Jose is done with it. What that means, off the top of my > >> head, is we can just drop this patch as in the first and last > >> case serial.o will be garbage-collected (or not built) and in > >> the middle case, this will be fully used. > > > > I can't drop this patch as it will break all of SPL when > > CONFIG_SERIAL_MULTI is unconditionally enabled. > > Why is it breaking _all_ of SPL? Have you run-tested this anywhere, > especially with SPL? In most cases it should be used and real > functions provided or garbage collected away. Yes, try compiling m28evk without this patch for example, it's going to break it. Because CONFIG_SPL_SERIAL_SUPPORT is disabled, yet it uses code that contains references to puts() etc. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 09/18/12 10:13, Marek Vasut wrote: > Dear Tom Rini, > >> On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: >>> Implement empty serial_* functions for SPL without serial >>> support enabled. This is imperative to haave once serial multi >>> is enabled unconditionally. >>> >>> Signed-off-by: Marek Vasut Cc: Marek Vasut >>> Cc: Tom Rini --- >>> >>> common/serial.c | 12 1 file changed, 12 >>> insertions(+) >>> >>> diff --git a/common/serial.c b/common/serial.c index >>> 631af65..cef4ba8 100644 --- a/common/serial.c +++ >>> b/common/serial.c @@ -27,6 +27,16 @@ >>> >>> #include #include >>> >>> +/* Implement prototypes for SPL without serial support */ >>> +#if defined(CONFIG_SPL_BUILD) && >>> !defined(CONFIG_SPL_SERIAL_SUPPORT) +int serial_init(void) { >>> return 0; } +void serial_setbrg(void) {} +int >>> serial_getc(void) { return 0; } +int serial_tstc(void) { return >>> 0; } +void serial_putc(const char c) {} +void serial_puts(const >>> char *s) {} >> >> This isn't quite right. We need to allow for: - No output SPL, >> strings collected (so #defined to do{} while (0)) > > Which is not type-checked and will drag in bugs. Scott has addressed this. >> - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + >> CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL >> (CONFIG_SPL_SERIAL_SUPPORT + #define puts serial_puts/putc). >> >> I'm not asking you to do that, but you will have to adapt to it >> once Jose is done with it. What that means, off the top of my >> head, is we can just drop this patch as in the first and last >> case serial.o will be garbage-collected (or not built) and in >> the middle case, this will be fully used. > > I can't drop this patch as it will break all of SPL when > CONFIG_SERIAL_MULTI is unconditionally enabled. Why is it breaking _all_ of SPL? Have you run-tested this anywhere, especially with SPL? In most cases it should be used and real functions provided or garbage collected away. - -- Tom -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://www.enigmail.net/ iQIcBAEBAgAGBQJQWLUQAAoJENk4IS6UOR1WxoAP/3Lto025hWPgi7obJR1nrl63 r84gfCPkVsjqWHmYJl+vFOlyTuEiXaW9K5PWNxRA7xXbm5GbBe90fZBdYVxCSh+f 7KuSJ5jBEItmma9eeva7Af8FRtoC487yM9MpAOUKQ2pKsOiPR7lbiGQamHvWUssA 1BQfALQSlWgdlsk93EHXwxRoQD97ojAfCWPybObpd0C3oYUHVPhOYHS9NtLiRr8e L58e7XhPksxNEyx29qrbmSwFGE8a4zSeu9SHjCfVk9Z2j2cD0zXpgqbwTe8/U259 31KUoRoLpqbSfOl4jcmZ54lyWZNbh1p45cyZAtOy4JJE99YkE951p342wC7sseQM AtGqWQidKvHszpiSFkhu2pbTHQbTZfnYA4cvKL4x5S6zXKLEk/Ybfn9RTXQpdnaQ 6yUeYLjHl3bXeVb/JhtD5oouzyCfQmDyVnS4CUTAD4rVBS1CNpjpLa/mDoAcZNJr zFLtkBW/72toOV7Xy2YsLtiWvw3bL6gKeKueegX5vHRBIx9csHWB3SZc7qMCZyIW xk3CIhJEru/RsWMAqynCX5lfRSuw2+Zflnq5YJnqeVRY2mdasXewLsvhW2iRRn7n C/Wf43wU782RK/8KbBYlokiDjV5egipxccW3lOIAr1HtvSQNfQgUDrum4opx+Jrw nbA1JTydUvu0YSzId43A =Lc/6 -END PGP SIGNATURE- ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On 09/18/2012 12:13:57 PM, Marek Vasut wrote: Dear Tom Rini, > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > Implement empty serial_* functions for SPL without serial > > support enabled. This is imperative to haave once serial > > multi is enabled unconditionally. > > > > Signed-off-by: Marek Vasut > > Cc: Marek Vasut > > Cc: Tom Rini > > --- > > > > common/serial.c | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/common/serial.c b/common/serial.c > > index 631af65..cef4ba8 100644 > > --- a/common/serial.c > > +++ b/common/serial.c > > @@ -27,6 +27,16 @@ > > > > #include > > #include > > > > +/* Implement prototypes for SPL without serial support */ > > +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SERIAL_SUPPORT) > > +int serial_init(void) { return 0; } > > +void serial_setbrg(void) {} > > +int serial_getc(void) { return 0; } > > +int serial_tstc(void) { return 0; } > > +void serial_putc(const char c) {} > > +void serial_puts(const char *s) {} > > This isn't quite right. We need to allow for: > - No output SPL, strings collected (so #defined to do{} while (0)) Which is not type-checked and will drag in bugs. Not all that likely, since most code will either be built with printf enabled some of the time, or not contain printf (i.e. it's not quite the same situation as debug prints which may be rarely enabled). An inline function would be fine, but has to be done at the same place that normal printf is declared -- whereas a macro could be done after the fact. Unless you use a macro to redirect it to an inline function of a different name... I do not understand why you want to put these stubs in a C file instead of a header file, though -- you're preventing code from being optimized away, which is important in some SPLs. -Scott ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Dear Tom Rini, > On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > > Implement empty serial_* functions for SPL without serial > > support enabled. This is imperative to haave once serial > > multi is enabled unconditionally. > > > > Signed-off-by: Marek Vasut > > Cc: Marek Vasut > > Cc: Tom Rini > > --- > > > > common/serial.c | 12 > > 1 file changed, 12 insertions(+) > > > > diff --git a/common/serial.c b/common/serial.c > > index 631af65..cef4ba8 100644 > > --- a/common/serial.c > > +++ b/common/serial.c > > @@ -27,6 +27,16 @@ > > > > #include > > #include > > > > +/* Implement prototypes for SPL without serial support */ > > +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SERIAL_SUPPORT) > > +int serial_init(void) { return 0; } > > +void serial_setbrg(void) {} > > +int serial_getc(void) { return 0; } > > +int serial_tstc(void) { return 0; } > > +void serial_putc(const char c) {} > > +void serial_puts(const char *s) {} > > This isn't quite right. We need to allow for: > - No output SPL, strings collected (so #defined to do{} while (0)) Which is not type-checked and will drag in bugs. > - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + > CONFIG_SPL_LIBCOMMON_SUPPORT) > - puts only SPL (CONFIG_SPL_SERIAL_SUPPORT + #define puts > serial_puts/putc). > > I'm not asking you to do that, but you will have to adapt to it once > Jose is done with it. What that means, off the top of my head, is we > can just drop this patch as in the first and last case serial.o will be > garbage-collected (or not built) and in the middle case, this will be > fully used. I can't drop this patch as it will break all of SPL when CONFIG_SERIAL_MULTI is unconditionally enabled. Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
On Mon, Sep 17, 2012 at 01:21:27AM +0200, Marek Vasut wrote: > Implement empty serial_* functions for SPL without serial > support enabled. This is imperative to haave once serial > multi is enabled unconditionally. > > Signed-off-by: Marek Vasut > Cc: Marek Vasut > Cc: Tom Rini > --- > common/serial.c | 12 > 1 file changed, 12 insertions(+) > > diff --git a/common/serial.c b/common/serial.c > index 631af65..cef4ba8 100644 > --- a/common/serial.c > +++ b/common/serial.c > @@ -27,6 +27,16 @@ > #include > #include > > +/* Implement prototypes for SPL without serial support */ > +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SERIAL_SUPPORT) > +int serial_init(void) { return 0; } > +void serial_setbrg(void) {} > +int serial_getc(void) { return 0; } > +int serial_tstc(void) { return 0; } > +void serial_putc(const char c) {} > +void serial_puts(const char *s) {} This isn't quite right. We need to allow for: - No output SPL, strings collected (so #defined to do{} while (0)) - puts + printf SPL (CONFIG_SPL_SERIAL_SUPPORT + CONFIG_SPL_LIBCOMMON_SUPPORT) - puts only SPL (CONFIG_SPL_SERIAL_SUPPORT + #define puts serial_puts/putc). I'm not asking you to do that, but you will have to adapt to it once Jose is done with it. What that means, off the top of my head, is we can just drop this patch as in the first and last case serial.o will be garbage-collected (or not built) and in the middle case, this will be fully used. -- Tom signature.asc Description: Digital signature ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH 62/71] serial: spl: Implement empty functions for SPL
Implement empty serial_* functions for SPL without serial support enabled. This is imperative to haave once serial multi is enabled unconditionally. Signed-off-by: Marek Vasut Cc: Marek Vasut Cc: Tom Rini --- common/serial.c | 12 1 file changed, 12 insertions(+) diff --git a/common/serial.c b/common/serial.c index 631af65..cef4ba8 100644 --- a/common/serial.c +++ b/common/serial.c @@ -27,6 +27,16 @@ #include #include +/* Implement prototypes for SPL without serial support */ +#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_SPL_SERIAL_SUPPORT) +int serial_init(void) { return 0; } +void serial_setbrg(void) {} +int serial_getc(void) { return 0; } +int serial_tstc(void) { return 0; } +void serial_putc(const char c) {} +void serial_puts(const char *s) {} +#else + DECLARE_GLOBAL_DATA_PTR; static struct serial_device *serial_devices; @@ -344,3 +354,5 @@ int uart_post_test(int flags) return ret; } #endif + +#endif -- 1.7.10.4 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot