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 ADF87C433FE for ; Thu, 6 Oct 2022 14:56:14 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 788B284D94; Thu, 6 Oct 2022 16:56:12 +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="lmWY12Bb"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 1C18984BE3; Thu, 6 Oct 2022 16:56:10 +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 ED95684D94 for ; Thu, 6 Oct 2022 16:56:06 +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=1665068165; 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=SKvapaIN6CkQs7CO6SdRnzUanufH+QFQRMpIa7e56Ro=; b=lmWY12BbPsTnfpkSX/76/lu2/rRZLgQjy/fQzvA6hSNLCJHUiu2QUOghezcI3W0mK/XHDe plzFQC6y5N+MetxnhzikdPkg9amhQloioT4EmO0xQhPuqWb0xZkxeTg+idGRw7MIHD4DLb Qgc6Xk8ptSB1LXIJeFr51M4Q7ZHq5T+896fYsJ/JsE1PCUBD+NWXfEqO+wKq6mVZlsaZw7 jfE/k9Zv1BQmirm6p4Wzl16r6h5h8Ur+8LNL0qZDeGrBEEj86DAcgk7nFuALpVvqtO/s5A cl0hsOsX/95NdVvjmjM1WNSQHiw0gxRWxrC3T97RJb/NcxE/mbjGghzht3I1HA== To: Michal Simek Cc: u-boot@lists.denx.de Subject: Re: [PATCH v3 0/8] Use logging feature instead of FPGA_DEBUG Date: Thu, 06 Oct 2022 16:56:05 +0200 Message-ID: <2373162.YMkKv9AQid@ada> In-Reply-To: <5f7a9b48-8cb0-795f-464d-d91f87f69905@amd.com> References: <20221005114421.24340-1-ada@thorsis.com> <5f7a9b48-8cb0-795f-464d-d91f87f69905@amd.com> 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 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__' fro= m 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 patches > > - 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/508906 >=20 > 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=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_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 >=20 > Please fix it up. Not sure if those warnings were present before on the old PRINTF calls, but= we=20 got them now. However the underlying problem was there before: putting to= =20 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= =20 tables has any value at all? Maybe that can just be dropped? Greets Alex