Hi,
On 10/6/22 16:56, Alexander Dahl wrote:
Hello Michal,
Am Donnerstag, 6. Oktober 2022, 16:44:29 CEST schrieb Michal Simek:
Hi,
On 10/5/22 13:44, Alexander Dahl wrote:
Hei hei,
while working on FPGA support for a new device I discovered debug
logging in some FPGA drivers is still done as in the old days. Bring
that to what I thougt would be the currently preferred approach.
Notes: Adding those Kconfig symbols in patch 3 is just to be able to
build those two old drivers.
All drivers touched were build tested with sandbox_defconfig and GCC 8
on Debian GNU/Linux 10 (buster).
Lines with other possibly questionable output were not touched, only
what seemed to be designated debug output, and only for FPGA drivers
having that ancient FPGA_DEBUG / PRINTF macros, so there's room for
future improvements.
Changelog:
v2 -> v3:
- Patch introducing FPGA uclass was completely reworked, sent
independently from this series, and applied already, thus removed
- Because requiring that new FPGA uclass changes, rebased on Michal's
microblaze branch '20221005'
- Removed '"%s …", __func__' and '"%d …", __line__' from log messages,
because log framework can add those (enabled by CONFIG_LOGF_FUNC and
CONFIG_LOGF_LINE)
v1 -> v2:
- Rebased on master
- Added patch to introduce new FPGA uclass in front of the other patches
- Use that new uclass as log category
- Slightly reworded cover letter
Greets
Alex
Cc: Michal Simek <michal.si...@amd.com>
Alexander Dahl (7):
fpga: altera: Use logging feature instead of FPGA_DEBUG
fpga: cyclon2: Use logging feature instead of FPGA_DEBUG
fpga: Add missing Kconfig symbols for old FPGA drivers
fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG
fpga: spartan2: Use logging feature instead of FPGA_DEBUG
fpga: spartan3: Use logging feature instead of FPGA_DEBUG
fpga: virtex2: Use logging feature instead of FPGA_DEBUG
drivers/fpga/ACEX1K.c | 37 +++++++++----------
drivers/fpga/Kconfig | 12 +++++++
drivers/fpga/altera.c | 11 +++---
drivers/fpga/cyclon2.c | 38 +++++++++-----------
drivers/fpga/spartan2.c | 80 +++++++++++++++++++----------------------
drivers/fpga/spartan3.c | 80 +++++++++++++++++++----------------------
drivers/fpga/virtex2.c | 69 ++++++++++++++++-------------------
7 files changed, 152 insertions(+), 175 deletions(-)
I pushed it to CI loop and got failure.
https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/508906
Building current source for 136 boards (64 threads, 1 job per thread)
m68k: + astro_mcf5373l
+In file included from include/linux/printk.h:4,
+ from include/common.h:20,
+ from drivers/fpga/spartan3.c:14:
+drivers/fpga/spartan3.c: In function 'spartan3_sp_load':
+drivers/fpga/spartan3.c:112:27: error: too many arguments for format
[-Werror=format-extra-args]
+ 112 | log_debug("Function Table:\n"
+ | ^~~~~~~~~~~~~~~~~~~
+include/log.h:220:24: note: in definition of macro 'log'
+ 220 | printf(_fmt, ##_args); \
+ | ^~~~
+drivers/fpga/spartan3.c:112:17: note: in expansion of macro 'log_debug'
+ | ^~~~~~~~~
+cc1: all warnings being treated as errors
+make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Error 1
+make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2
+make[1]: *** [Makefile:1883: drivers] Error 2
Please fix it up.
Not sure if those warnings were present before on the old PRINTF calls, but we
got them now. However the underlying problem was there before: putting to
much things in one printf/log line. I can go split it up like in 'drivers/
fpga/virtex2.c' already, where you have the following comment:
/* Gotta split this one up (so the stack won't blow??) */
Not sure however if debug printing all function pointers in those function
tables has any value at all? Maybe that can just be dropped?
No idea if this is useful or not. But it is there and I would split it as it is
done in virtex2. Please do it in separate patch with mentioning virtex2 to have
the same change.
Thanks,
Michal
