From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 86028C433FE for ; Fri, 7 Oct 2022 12:22:12 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id EDB5584EA4; Fri, 7 Oct 2022 14:21:24 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=thorsis.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=thorsis.com header.i=@thorsis.com header.b="eCzjw459"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 046FF84E86; Fri, 7 Oct 2022 14:21:23 +0200 (CEST) Received: from mail.thorsis.com (mail.thorsis.com [92.198.35.195]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5FA4584E85 for ; Fri, 7 Oct 2022 14:21:19 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=quarantine dis=none) header.from=thorsis.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ada@thorsis.com From: Alexander Dahl DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=thorsis.com; s=default; t=1665145278; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=7ENHcQACyfByUyCe05/gH7Qe5s/hByn8jefiB37myFA=; b=eCzjw459j3kpJX7OtCJrztrESBSN/OEjp7JLpRwts0PJa9NdQllPM/5Gg2WXiqbDrniF7e LOF4/CCealCX2dltttmQaFXvkfgqMJb5nKagF6uVpz8tg6hchmnECNT8N5hCJvznoLPoRF VQjj7u4n2eONXTacOPtmaUuHNN2pq35TA6m05q4LpzCRb2r99c3vYwNjmwTqa3POM24PUQ 4ul0Dh8JbBtUS8W7KXVwYjDjE/P6JelJcN8kfCsGoNM4PHF8uy+DYBeHMRIei6kMNJTjtY t5kyC9X/kRHxYJ8ZpCNiuuYllbNfO9x66v6hzR4zRaK5QNXmAAbWh2tI0TOJGg== To: Michal Simek Cc: u-boot@lists.denx.de Subject: Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG Date: Fri, 07 Oct 2022 14:21:18 +0200 Message-ID: <5733058.DAevxlbgZK@ada> In-Reply-To: References: <20221005114421.24340-1-ada@thorsis.com> <2373162.YMkKv9AQid@ada> Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="UTF-8" X-Clacks-Overhead: GNU Terry Pratchett X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean Hello Michal, Am Freitag, 7. Oktober 2022, 08:34:15 CEST schrieb Michal Simek: > Hi, >=20 > On 10/6/22 16:56, Alexander Dahl wrote: > > Hello Michal, > >=20 > > Am Donnerstag, 6. Oktober 2022, 16:44:29 CEST schrieb Michal Simek: > >> Hi, > >>=20 > >> On 10/5/22 13:44, Alexander Dahl wrote: > >>> Hei hei, > >>>=20 > >>> 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. > >>>=20 > >>> Notes: Adding those Kconfig symbols in patch 3 is just to be able to > >>> build those two old drivers. > >>>=20 > >>> All drivers touched were build tested with sandbox_defconfig and GCC 8 > >>> on Debian GNU/Linux 10 (buster). > >>>=20 > >>> 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. > >>>=20 > >>> Changelog: > >>>=20 > >>> v2 -> v3: > >>> - Patch introducing FPGA uclass was completely reworked, sent > >>>=20 > >>> independently from this series, and applied already, thus removed > >>>=20 > >>> - Because requiring that new FPGA uclass changes, rebased on Michal's > >>>=20 > >>> microblaze branch '20221005' > >>>=20 > >>> - Removed '"%s =E2=80=A6", __func__' and '"%d =E2=80=A6", __line__' f= rom log messages, > >>>=20 > >>> because log framework can add those (enabled by CONFIG_LOGF_FUNC = and > >>> CONFIG_LOGF_LINE) > >>>=20 > >>> v1 -> v2: > >>> - Rebased on master > >>> - Added patch to introduce new FPGA uclass in front of the other patc= hes > >>> - Use that new uclass as log category > >>> - Slightly reworded cover letter > >>>=20 > >>> Greets > >>> Alex > >>>=20 > >>> Cc: Michal Simek > >>>=20 > >>> 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 > >>> =20 > >>> 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(-) > >>=20 > >> I pushed it to CI loop and got failure. > >>=20 > >> https://source.denx.de/u-boot/custodians/u-boot-microblaze/-/jobs/5089= 06 > >>=20 > >> Building current source for 136 boards (64 threads, 1 job per thread) > >>=20 > >> m68k: + astro_mcf5373l > >>=20 > >> +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=3Dformat-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_debu= g' > >> + | ^~~~~~~~~ > >> +cc1: all warnings being treated as errors > >> +make[3]: *** [scripts/Makefile.build:258: drivers/fpga/spartan3.o] Er= ror > >> 1 > >> +make[2]: *** [scripts/Makefile.build:398: drivers/fpga] Error 2 > >> +make[1]: *** [Makefile:1883: drivers] Error 2 > >>=20 > >> Please fix it up. > >=20 > > 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 li= ke > > in 'drivers/>=20 > > fpga/virtex2.c' already, where you have the following comment: > > /* Gotta split this one up (so the stack won't blow??) */ > >=20 > > Not sure however if debug printing all function pointers in those funct= ion > > tables has any value at all? Maybe that can just be dropped? >=20 > 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. Turned out there was a different cause for those warnings. =20 See v4 of the series which I just sent. Have a nice weekend Alex