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.simek@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?
Greets Alex