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 X-Spam-Level: X-Spam-Status: No, score=-17.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0C8ADC433EF for ; Mon, 13 Sep 2021 22:11:54 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 4B18260F92 for ; Mon, 13 Sep 2021 22:11:53 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4B18260F92 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 078688368F; Tue, 14 Sep 2021 00:11:50 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (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="pxTZ58Ka"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0D47F8378B; Tue, 14 Sep 2021 00:11:47 +0200 (CEST) Received: from mail-qk1-x72a.google.com (mail-qk1-x72a.google.com [IPv6:2607:f8b0:4864:20::72a]) (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 A1210833E4 for ; Tue, 14 Sep 2021 00:11:42 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=konsulko.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=trini@konsulko.com Received: by mail-qk1-x72a.google.com with SMTP id ay33so12414460qkb.10 for ; Mon, 13 Sep 2021 15:11:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=dv2RHK3COvgFHK1bjU43zlLb9SUteu3nrcfZTY7t81A=; b=pxTZ58KacZ0WzTw11GCkonoxGvxnR6ci2uHsaArmpt+oQbFOz7Sd1VF21CS93vDc8U zL7NGHpuu5H+6C0gX0y0okGZ05ttznVf8o8K/e1uDA/lWt8Kmg2CH1yk0ne2rHpukKx5 GyVEs2F3Puz20QKq8mQ26hVwX6/KkSwjtIJ4k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=dv2RHK3COvgFHK1bjU43zlLb9SUteu3nrcfZTY7t81A=; b=25rH3a5ed+q+oB3ZkLJGMWC10+lmUpLGFhQi9LUgt809FxB3a5efHtBqiSZB9yE4vL JODnIwoCYSLYGK2cezSEv7UTyoeZu8IWo+qj1UGFJEQkkuCyVHQ1pADPyhevvXQXHn4H 9j3S/aJqCsBnSTiZJr5Mwg8J6caMILFWXqvPDtO69KXNullJA8+oAYzSO1rZ3J7myrS6 vSU/WtFHmZr1MzzRdYgTIMpTb4FyBxZNVuISaCR0iULCkFMMujlO8s0oQaBBLbVifGwg oOLBKv404Bf6xvgrtnLU5gWUWydeAM3VOjv0kSsOH7A4gPJY4aQOa/Fus3dnBWIPsEab fE4w== X-Gm-Message-State: AOAM533ReI/TpVhqGGKErPS5eeCY8xa+oQ5efi8FJ+nJToBt4gOu2MYt yNaSZoQswRe3AnIk6pCFFYA9ew== X-Google-Smtp-Source: ABdhPJyIiQwSM+FmDC9KFbtGqCQuO3YPO+zv7Zuak4DDPoUTnki3WMM2e/XK8Afelqf2AwehqNyoaQ== X-Received: by 2002:ae9:c119:: with SMTP id z25mr1842935qki.201.1631571101379; Mon, 13 Sep 2021 15:11:41 -0700 (PDT) Received: from bill-the-cat (2603-6081-7b01-cbda-0166-c2e8-bb10-efce.res6.spectrum.com. [2603:6081:7b01:cbda:166:c2e8:bb10:efce]) by smtp.gmail.com with ESMTPSA id m8sm6104605qkk.130.2021.09.13.15.11.40 (version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256); Mon, 13 Sep 2021 15:11:40 -0700 (PDT) Date: Mon, 13 Sep 2021 18:11:38 -0400 From: Tom Rini To: "Alex G." Cc: u-boot@lists.denx.de Subject: Re: [PATCH 5/5] serial: Rework CONFIG_SYS_BAUDRATE_TABLE Message-ID: <20210913221138.GT12964@bill-the-cat> References: <20210913212455.29165-1-trini@konsulko.com> <20210913212455.29165-5-trini@konsulko.com> <596e1bf3-0c51-c528-77e0-2be943c18e28@gmail.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="+9D0x2PrroKZTtS9" Content-Disposition: inline In-Reply-To: <596e1bf3-0c51-c528-77e0-2be943c18e28@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.9.4 (2018-02-28) X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 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.2 at phobos.denx.de X-Virus-Status: Clean --+9D0x2PrroKZTtS9 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 13, 2021 at 05:03:13PM -0500, Alex G. wrote: >=20 >=20 > On 9/13/21 4:24 PM, Tom Rini wrote: > > In order to move CONFIG_SYS_BAUDRATE_TABLE to Kconfig, we need to rework > > the logic a bit. Rename the users of CONFIG_SYS_BAUDRATE_TABLE to > > SYS_BAUDRATE_TABLE. Introduce a series of CONFIG_BAUDRATE_TABLE_... > > that include some number of baud rates. These match all existing users. > > The help for each entry specifies what the exact table is, for a given > > option. Define what SYS_BAUDRATE_TABLE will be in include/serial.h now. > >=20 > > Signed-off-by: Tom Rini > > --- >=20 > > diff --git a/include/serial.h b/include/serial.h > > index 6d1e62c6770c..150644c4c3d4 100644 > > --- a/include/serial.h > > +++ b/include/serial.h > > @@ -3,6 +3,42 @@ > > #include > > +#if defined(CONFIG_BAUDRATE_TABLE_300_TO_38400_115200) > > +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \ > > + 38400, 115200 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_115200) > > +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \ > > + 38400, 57600, 115200 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_230400) > > +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 2400, 4800, 9600, 19200, \ > > + 38400, 57600, 115200, 230400 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_300_TO_6000000) > > +#define SYS_BAUDRATE_TABLE { 300, 600, 1200, 1800, 2400, 4800, 9600, \ > > + 19200, 38400, 57600, 115200, 230400, \ > > + 460800, 500000, 576000, 921600, 1000000, \ > > + 1152000, 1500000, 2000000, 2500000, \ > > + 3000000, 3500000, 4000000, 4500000, \ > > + 5000000, 5500000, 6000000 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_4800_TO_115200) > > +#define SYS_BAUDRATE_TABLE { 4800, 9600, 19200, 38400, 57600, 115200 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_115200) > > +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_230400) > > +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400= } > > +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_460800) > > +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400= , 460800 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_921600) > > +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400= , \ > > + 460800, 921600 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_9600_TO_230400_500000_1500000) > > +#define SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400= , \ > > + 500000, 1500000 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_38400_115200_ONLY) > > +#define SYS_BAUDRATE_TABLE { 38400, 115200 } > > +#elif defined(CONFIG_BAUDRATE_TABLE_115200_ONLY) > > +#define SYS_BAUDRATE_TABLE { 115200 } > > +#endif > > + > > struct serial_device { > > /* enough bytes to match alignment of following func pointer */ > > char name[16]; >=20 >=20 > This opens the gates to #ifdefing the heck out of serial.h. What happens = to > my board that goes from 300 to 2000000? > * We need a new Kconfig and new ifdef > What happens to my other board that goes from 300 to 2500000? > * We need a new Kconfig and new ifdef > The pattern doesn't look promising. This reminds me I was tempted to do a cover letter, but didn't. What happens is I tell you no. Most boards are using the standard table of common rates from 9600 to 115200. A nice follow-up would be to change every board with a special case that's not above 115200 to just use the normal table. Everyone else? There's the maximal table. That's it. That's even come in fairly recently, for mvebu platforms. > I actually think this change can make the situation worse. We trade having > an antiquated and inconvenient SYS_BAUDRATE_TABLE for one Kconfig per each > possible baudrate combination. How does this make sense? I'm going, as much as possible, for migrating the current situation. There's many places things could be cleaner, but "we'll clean this up and then migrate ..." is why we're so very very far behind where I hoped to be. > I've seen situations were SPL boots with 2Mbaud and executes succesfully, > u-boot starts up with 2Mbaud just fine. few lines later, u-boot, > downswitches to 115200 because CONFIG_SYS_BAUDRATE_TABLE says so. Well, the table and CONFIG_BAUDRATE. > Suggestion I: Can we have a MIN/MAX value for baudrates, and have the code > work from there ? >=20 > Suggestion II: Define the Kconfig SYS_BAUDRATE_TABLE table to a C array, > like 'default "{ 300, 420, 690}" ' and forego the #ifdefs in serial.h >=20 > Suggestion III: Get rid of the logic that says "baudrate must be one of > these predefined values" and let the serial driver return -ENOBUENO or > -EINVAL if the hardware really can't do that baudrate. Most UARTs nowadays > can do a wide range of values, and the baudrate table doesn't model that > very well. Combine this with a CONFIG_MAX_BAUDRATE so that boards with > shitty RS232 converters can set a safe upper limit -- and make sure > CONFIG_BAUDRATE also enforces this. >=20 > There's a lot of unrealized potential here. I'd certainly like to see something done as a follow-up that makes it easier to support platforms that can do something faster. --=20 Tom --+9D0x2PrroKZTtS9 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQGzBAABCgAdFiEEGjx/cOCPqxcHgJu/FHw5/5Y0tywFAmE/zJcACgkQFHw5/5Y0 tyxtBAwAkVN4aAOuvInckS6hUzmA4r9SN5BAtDKv3pCOt7T9hTSJ16ypmCYaZPn/ J6pdfcUqEJ4EXO7bomez7v6u8M3IAfZBmDLuKOnvqzgRJoTt1G0N4f+r1LCc8VzX B/Q89UYJMZCspt1rUn7Qoun7yn8mVX749/CGa23y/qngSB2f5bp+kpvSznwxW31w +ZoEXsfgBmSOr9mdm32tTMBNRhMlkniZ8/GHFUAg3ZNZGmS7rIWMtN0UnDpCAE4V FHPCQR4xbgHNo7zSoxg+2EQedffHvKHQO0/r+Pa9HD4PujUbc22IbZU9bk6x3fvB EptyXysSYA5Fu8+LRmo3vpHJvZ12Oy7q0/yTYtvahC1RB5KNLUXA7v4nQvIKok/a ZNRfgBusRiMjBucBUNcJfB7/6nGMt1HNvOwiSOpeRQnESVwkeJ//A1+2WAS3xtjg hRqD/oGj84s36X8572l89Sw73WTeYsVUiXa9AvjdobJPHih1jz0AJkTYKwX0QoJ0 4XIlIUjL =IxLB -----END PGP SIGNATURE----- --+9D0x2PrroKZTtS9--