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 2D8CAECAAD1 for ; Wed, 31 Aug 2022 13:55:27 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1D977849D9; Wed, 31 Aug 2022 15:55:25 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=konsulko.com header.i=@konsulko.com header.b="KmrbtMix"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CD2EE849DB; Wed, 31 Aug 2022 15:55:23 +0200 (CEST) Received: from mail-qv1-xf2c.google.com (mail-qv1-xf2c.google.com [IPv6:2607:f8b0:4864:20::f2c]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 94F1E849CC for ; Wed, 31 Aug 2022 15:55:20 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qv1-xf2c.google.com with SMTP id kh8so11103429qvb.1 for ; Wed, 31 Aug 2022 06:55:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc; bh=lQvpZFth5PzxKKToMUugY9vTGRmOMnhEVravtTVQRYA=; b=KmrbtMixa+6webh+od97XGARXYVTNI0nAs13/9MBdoY9YPHJ5kdl7kwAYZ+C+goJ+O T//k0/cuVhrM0WhiF/wHO1lnzhi5n0kq3dxy6ibnELNj/Icy0I1ZDJqM1XIIp5nW4s8J uVPujfhosTa3zbDiDj4+73TJY9I9kjHPng/2Q= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc; bh=lQvpZFth5PzxKKToMUugY9vTGRmOMnhEVravtTVQRYA=; b=AXMhW1Zlhe/7qZT+uB/pIU2a1r1KuZD9QdNzdc6iXwU+KJ96M5512eY7BzB4tfd/Yq o5oyijfkFxElr++K+aSSUX0dkZjAQ2PwDbWQcZa9KoMt04EMqTRvAfI188g16TUWo0pZ a/Q+jbMGKgxn/7IQ+WHYUUmrKe0Ve1P//Nuq5tECkCX3FMbzfdcfjj3CKu3hAPEIoSHM AYZJkSAMwlxT9styD5UN+vwqV9Yi2SEO+1CI8eU4bUEEWcQm9Ewny60cixuJG57DHJZS jTHz0lWjxjsmR3JYOmPQdaYUWzqF0BhsUVoTaLYyZVnnFdbrRVz3QWX7DurWIy39WGUc mN+Q== X-Gm-Message-State: ACgBeo2DkA+IlNraXm8SNZ/4hZox+VSPF4uRxOglfAgJxDLzCgBg+Cj5 iem37Zc0ZUSb2jQKYTm5vqcvgg== X-Google-Smtp-Source: AA6agR7QLCWMD8tsn78yHKMeIWnRrIJ94Plpu4cnXVotMIWcRe3PJJS9EOBvDFUQwq+APpSXRwqt/w== X-Received: by 2002:a05:6214:27ed:b0:499:2399:89d6 with SMTP id jt13-20020a05621427ed00b00499239989d6mr924951qvb.5.1661954119287; Wed, 31 Aug 2022 06:55:19 -0700 (PDT) Received: from bill-the-cat (cpe-65-184-195-139.ec.res.rr.com. [65.184.195.139]) by smtp.gmail.com with ESMTPSA id n19-20020ac85b53000000b003447c4f5aa5sm9010065qtw.24.2022.08.31.06.55.18 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 31 Aug 2022 06:55:18 -0700 (PDT) Date: Wed, 31 Aug 2022 09:55:16 -0400 From: Tom Rini To: Simon Glass Cc: Pali =?iso-8859-1?Q?Roh=E1r?= , U-Boot Mailing List Subject: Re: [PATCH v2 2/6] console: Implement flush() function Message-ID: <20220831135516.GD7942@bill-the-cat> References: <20220811144953.vjcobevwf5o2syg4@pali> <20220812090130.2wcjq2u3qndma3ig@pali> <20220812151704.wa3n2jaugsd5h3c7@pali> <20220825142027.76yo7eyg6md6u7r4@pali> <20220831091302.g65ajndqlioj35vu@pali> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="9+yvdhHLKXN64C4s" Content-Disposition: inline In-Reply-To: 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 --9+yvdhHLKXN64C4s Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Aug 31, 2022 at 07:46:58AM -0600, Simon Glass wrote: > Hi Pali, >=20 > On Wed, 31 Aug 2022 at 03:13, Pali Roh=E1r wrote: > > > > On Thursday 25 August 2022 09:08:18 Simon Glass wrote: > > > Hi Pali, > > > > > > On Thu, 25 Aug 2022 at 08:20, Pali Roh=E1r wrote: > > > > > > > > On Friday 12 August 2022 20:21:00 Simon Glass wrote: > > > > > Hi Pali, > > > > > > > > > > On Fri, 12 Aug 2022 at 09:17, Pali Roh=E1r wrot= e: > > > > > > > > > > > > On Friday 12 August 2022 09:11:10 Simon Glass wrote: > > > > > > > Hi Pali, > > > > > > > > > > > > > > On Fri, 12 Aug 2022 at 03:01, Pali Roh=E1r = wrote: > > > > > > > > > > > > > > > > On Thursday 11 August 2022 18:08:46 Simon Glass wrote: > > > > > > > > > Hi Pali, > > > > > > > > > > > > > > > > > > On Thu, 11 Aug 2022 at 08:50, Pali Roh=E1r wrote: > > > > > > > > > > > > > > > > > > > > On Thursday 11 August 2022 08:47:50 Simon Glass wrote: > > > > > > > > > > > > diff --git a/include/stdio_dev.h b/include/stdio_de= v.h > > > > > > > > > > > > index 270fa2729fb2..06278366ae88 100644 > > > > > > > > > > > > --- a/include/stdio_dev.h > > > > > > > > > > > > +++ b/include/stdio_dev.h > > > > > > > > > > > > @@ -37,6 +37,10 @@ struct stdio_dev { > > > > > > > > > > > > void (*putc)(struct stdio_dev *dev, const c= har c); > > > > > > > > > > > > /* To put a string (accelerator) */ > > > > > > > > > > > > void (*puts)(struct stdio_dev *dev, const c= har *s); > > > > > > > > > > > > +#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT > > > > > > > > > > > > > > > > > > > > > > I'd argue it isn't worth the #ifdef and we might as w= ell have this > > > > > > > > > > > member here always. Then you can drop some #ifdefs fr= om your code. > > > > > > > > > > > > > > > > > > > > But then it will increase binary code size. > > > > > > > > > > > > > > > > > > Using the member will increase code size. But I think the= only place > > > > > > > > > you need an #ifdef for that is when you include it in the= driver > > > > > > > > > struct. You can use __maybe_unused in the other place. > > > > > > > > > > > > > > > > > > Having the member will only increase memory usage, not co= de size. > > > > > > > > > > > > > > > > But static memory structures are part of the u-boot.bin bin= ary and also > > > > > > > > their usage increase code size (required copy, etc...), so = at the end it > > > > > > > > increase code size. > > > > > > > > > > > > > > From what I understand stdio_dev is allocated at runtime in B= SS: > > > > > > > > > > > > > > struct stdio_dev *stdio_devices[] =3D { NULL, NULL, NULL }; > > > > > > > > > > > > > > (the NULL stuff should not be there, but does nothing, I beli= eve) > > > > > > > > > > > > > > So long as no code accesses your new member, then it should o= nly > > > > > > > affect the BSS size. > > > > > > > > > > > > Code of course has to access new member. How otherwise would be= able to > > > > > > call that new callbacks? E.g. all new code in patch 2/6 or 3/6. > > > > > > > > > > Yes, see below. Also, do check whether it actually adds code or n= ot. > > > > > > > > > > > > > > > > > > If you are very worried about it, you could use the technique= in > > > > > > > asm-generic/global_data.h to avoid #ifdefs in the C code: > > > > > > > > > > > > > > #ifdef CONFIG_GENERATE_ACPI_TABLE > > > > > > > #define gd_acpi_start() gd->acpi_start > > > > > > > #define gd_set_acpi_start(addr) gd->acpi_start =3D addr > > > > > > > #else > > > > > > > #define gd_acpi_start() 0UL > > > > > > > #define gd_set_acpi_start(addr) > > > > > > > #endif > > > > > > > > > > > > > > Regards, > > > > > > > Simon > > > > > > > > > > Regards, > > > > > Simon > > > > > > > > But in this case, it is needed to introduce a new special macro and= hide > > > > assigning code into it. In my opinion such macro with side effect a= nd > > > > worse than writing it explicitly - open coded to show what the code= is > > > > doing. > > > > > > You can use an inline function if you prefer. > > > > But it has still same issue, hiding conditional logic indirectly to > > additional file. Some members would be assigned directly via =3D operat= or > > and some via macro/inlinefunction is inconsistent. >=20 > I've said everything I can say about this. If Tom has some thoughts > I'll leave it to him. >=20 > > > > > My main objection is to adding #ifdefs to C files. They are OK (for > > > now) in driver struct declarations, where some members are optional. > > > But I think it is worth going to some lengths to avoid them elsewhere. > > > > > > +Tom Rini for thoughts on this > > > > > > Regards, > > > SImon > > > > Any comments on this? Given the plethora of tools to demonstrate that approach X results in size change Y, I'm not sure why we're pondering if something will or won't result in increases? I would like to see this series result in the smallest increase in binary size on platforms that don't enable the feature. An #ifdef isn't more or less readable than using some macro instead, and decorators like __maybe_unused have their own impact on code readability. --=20 Tom --9+yvdhHLKXN64C4s Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmMPaEEACgkQFHw5/5Y0 tyyH1Av/ZcVV7eWG94Jt4tpgDWkkZeqLPdIhdte1sLtuH8ibfMSespXC+jmgQiv4 9Pm+MNuYNL8H0PcGZpHjDCFJBcvvmJialzKRr9HIY8zCx5ufv5eC7xcvxsuEL6zv 1tHlGddZ8swGhgOxlkFlE/Xg3lrnxNrC5hh1J5M4vdx2P/9J66KL6wPVOK0ilXEB K9VUNJjZeX0n7L87RkwBn8YQLyqmsTA9YA6rM14JsvttYC7X8x+/K5ZwwzICCjJi /Ykul6Ky/CqP1iLVGloKKn8rVXS3fC1fFSsjjgJBgxw9fuhwusVM3RxPO//hJL+C 8AOwRi4VJVPyKd9vOi+CytcIO8/yvT1paWCOiJNbMnjdJ6iZ4mzBpVZaQ98cnoSq iW86cOoWglC11BfNh9pyBda+L0oKw6j8gbGsfccJQumz4QGSdZ2o1SpXrmZdsKyk oBCkz4jtY5ewl3xobx4SKihwKnjt/Ri++jAbP7ykJsFdcJEPh+L00/JXyayvIZIF I8Wf/9Aj =P/He -----END PGP SIGNATURE----- --9+yvdhHLKXN64C4s--