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=-16.2 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,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 CD5A8C433F5 for ; Mon, 13 Sep 2021 22:03:25 +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 06D0B61056 for ; Mon, 13 Sep 2021 22:03:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 06D0B61056 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.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 E36E2836B2; Tue, 14 Sep 2021 00:03:21 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.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=gmail.com header.i=@gmail.com header.b="WP9/9UD9"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CD52B836B8; Tue, 14 Sep 2021 00:03:19 +0200 (CEST) Received: from mail-oo1-xc32.google.com (mail-oo1-xc32.google.com [IPv6:2607:f8b0:4864:20::c32]) (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 2E0A98368F for ; Tue, 14 Sep 2021 00:03:16 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mr.nuke.me@gmail.com Received: by mail-oo1-xc32.google.com with SMTP id y3-20020a4ab403000000b00290e2a52c71so3903403oon.2 for ; Mon, 13 Sep 2021 15:03:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=KD7Cq0BgidKycO5ahoW4Ax5poxLaHxzl5Jxz1ubcMvg=; b=WP9/9UD9f1PJkf071SE2Gi3e5faUQzS1nbb1rD0q8xa8SaCvL3k67QWNdzAgvEO+13 ajZWc9ucUqtHXkVL4cLB77O5ClRTqj3YnXc/C9nqJKsB/79pXDPZoITF3esWnCAWAQ7I pOaoZwB7tILWIZ1DTAydc7nPuJVeBgCzXHcbB7CHXva32wOED5eA6/vM2EP4RCTfm8uC 9ihaBi4yCszNcDh0DsKn7aYOkuIsiDQmUS4CCPgNpYn56G8NwReKS+OUKYaXwilyvocs B78ZHoKnn64KfnMVjVRw8itOhY3vXuNNXAFoTkxbhnvcHTHkF9lCRHK3SxTCry7GYXNR 63fA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=KD7Cq0BgidKycO5ahoW4Ax5poxLaHxzl5Jxz1ubcMvg=; b=zW45BvQXORGnt5MCftiTIvN4giagB8vXRUcYNqP42enZHnEGy5RXYMbQOpmqPUhMjT AXW86eSAZVb4CSvf1UuCady/LWHcG9d96Zyqy/nyy0WKYBb4REaCCh/K1ZzOPIswBUOI eRAumB07mdtqaHGzSXOPshiiW1nP/885gI2f/VgAevc/N3lmwBC+tJdAaUVVGAtjXn1b qKCwa33Twz55gnZNu0w6HvpEEHnT2OCoDg/uP2166XAC5nMwg6kf78mmL2Pc0NIQEtKi 0hl8n1RKNeWpATMPwbvUOLU1y6zLw7gClkScd+SCsuT6dRjfgTy6+qrdope54IhA2fyH AZkA== X-Gm-Message-State: AOAM531tubIyaGQBRG8fbuXfi/LouTBeVa8xZWkOKU39PHrjdTVDs0fU EV5LCMH0fYEyDETnvZZBkDQ= X-Google-Smtp-Source: ABdhPJzmYCUZeE/XbetoG1WXuIv6/7iTBjvUsE7Xw1Kj4nqQkmha8jeA1Up7vDqXInAp+zGteGtitw== X-Received: by 2002:a4a:a3c5:: with SMTP id t5mr11118706ool.36.1631570594676; Mon, 13 Sep 2021 15:03:14 -0700 (PDT) Received: from nuclearis3.gtech (c-98-195-139-126.hsd1.tx.comcast.net. [98.195.139.126]) by smtp.gmail.com with ESMTPSA id w15sm2003172oiw.19.2021.09.13.15.03.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Sep 2021 15:03:14 -0700 (PDT) Subject: Re: [PATCH 5/5] serial: Rework CONFIG_SYS_BAUDRATE_TABLE To: u-boot@lists.denx.de, Tom Rini References: <20210913212455.29165-1-trini@konsulko.com> <20210913212455.29165-5-trini@konsulko.com> From: "Alex G." Message-ID: <596e1bf3-0c51-c528-77e0-2be943c18e28@gmail.com> Date: Mon, 13 Sep 2021 17:03:13 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210913212455.29165-5-trini@konsulko.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 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. > > Signed-off-by: Tom Rini > --- > 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]; > 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. 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'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. Suggestion I: Can we have a MIN/MAX value for baudrates, and have the code work from there ? 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 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. There's a lot of unrealized potential here. Alex