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.6 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_IN_DEF_DKIM_WL autolearn=unavailable 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 C4C36C433E1 for ; Tue, 14 Jul 2020 20:45:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 9EA442064B for ; Tue, 14 Jul 2020 20:45:16 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="dsDrvsXK" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727058AbgGNUpP (ORCPT ); Tue, 14 Jul 2020 16:45:15 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56998 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726634AbgGNUpO (ORCPT ); Tue, 14 Jul 2020 16:45:14 -0400 Received: from mail-ej1-x642.google.com (mail-ej1-x642.google.com [IPv6:2a00:1450:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 77DE8C08C5C1 for ; Tue, 14 Jul 2020 13:45:14 -0700 (PDT) Received: by mail-ej1-x642.google.com with SMTP id f12so24378453eja.9 for ; Tue, 14 Jul 2020 13:45:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Kqh1NBVBlHrZFE286BUwAHdoh8ntAaaIXSLlXQ7Bm6s=; b=dsDrvsXKwnz4Ec5AoGC7UzNnGr6HGFuQx0fMIrzZdpvzT8V+sMBjSseqibtKkUleNp YAQBi76uicXluskJd+hTeTsyWAg05jNYM0frcHY4PBAtEKlADxwqvaJdPcxxHuR6fSg4 Z8Av9Xw3kWBU7AcvAiZLJvP0ANAQYQY/1HRVk5m0U6KRDc+OtwfbXN0N1c0AAmeVdh96 7p0jLgi5ACnL+H0e8teJtGjWlj+U1ay7gVzmHsVYk1zN8dRbuaztcrZ66pzJ2Ok7zto2 EGMDow93S9QTHHgzoSZB16/uOQk9fIg12WPTEmg38d7cg2Gx2p4CpzISSSFIUXkbDAsr HC/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Kqh1NBVBlHrZFE286BUwAHdoh8ntAaaIXSLlXQ7Bm6s=; b=s564itoBBH8w0+aAOM4quROCFpqzTYYoa45Yz+NuqwGNEgshNgt26VmKifHw9SyPgU wTUvJTtbKh2zEYVYaFKM6umRMYvWovFEz0xFj32LajsU8TIjuJHynjWgHCSsSqyVCDWU GRmWLDctX3LsLaYmmJHm9Hm0RxrEtwTzEX5tEl8zMH8KV6R1Si3EDYsUcHnNff+FtkOG hMWb8za8eO2HlSWyHHnugzoT3+JLn5JkO8hQMXeb4hCThMk56pQCQvfu+dkRWHqiWFRR 6kQ/x7CuaZuDstrhtOyp6YVWuz0OINY6jdhSqKN/eiUfo7v6ummd+lvl6oFlghhwcSqI 4BQQ== X-Gm-Message-State: AOAM5315bd9mT1zcVDttlEYyHKfzGqC4rD06GT6cb2w3lvdqs4DG3QZq pkUr7TalBz8TkILQQuhFS3pZQt1KCTYZiZ82cgNh+A== X-Google-Smtp-Source: ABdhPJysUSBHVOa53sWJQjIcFkn4l3OgSNQEcR9y15L+mAV25fgE46PISgPYdobloPCBMeTthBtC00kZdSRclXpMQu0= X-Received: by 2002:a17:906:fa9b:: with SMTP id lt27mr6000034ejb.513.1594759512692; Tue, 14 Jul 2020 13:45:12 -0700 (PDT) MIME-Version: 1.0 References: <20200714124113.20918-1-Sergey.Semin@baikalelectronics.ru> In-Reply-To: <20200714124113.20918-1-Sergey.Semin@baikalelectronics.ru> From: Daniel Winkler Date: Tue, 14 Jul 2020 13:45:01 -0700 Message-ID: Subject: Re: [PATCH] serial: 8250_mtk: Fix high-speed baud rates clamping To: Serge Semin , Claire Chang , Nicolas Boichat Cc: Greg Kroah-Hartman , Jiri Slaby , Matthias Brugger , Serge Semin , Alexey Malahov , Aaron Sierra , Andy Shevchenko , Lukas Wunner , Vignesh Raghavendra , linux-serial@vger.kernel.org, linux-mediatek@lists.infradead.org, BlueZ , chromeos-bluetooth-upstreaming , abhishekpandit@chromium.org, stable@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Thank you Sergey for looking into this. Adding folks working on this platform to perform validation of the proposed patch. Best, Daniel On Tue, Jul 14, 2020 at 5:41 AM Serge Semin wrote: > > Commit 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 > port") fixed limits of a baud rate setting for a generic 8250 port. > In other words since that commit the baud rate has been permitted to be > within [uartclk / 16 / UART_DIV_MAX; uartclk / 16], which is absolutely > normal for a standard 8250 UART port. But there are custom 8250 ports, > which provide extended baud rate limits. In particular the Mediatek 8250 > port can work with baud rates up to "uartclk" speed. > > Normally that and any other peculiarity is supposed to be handled in a > custom set_termios() callback implemented in the vendor-specific > 8250-port glue-driver. Currently that is how it's done for the most of > the vendor-specific 8250 ports, but for some reason for Mediatek a > solution has been spread out to both the glue-driver and to the generic > 8250-port code. Due to that a bug has been introduced, which permitted the > extended baud rate limit for all even for standard 8250-ports. The bug > has been fixed by the commit 7b668c064ec3 ("serial: 8250: Fix max baud > limit in generic 8250 port") by narrowing the baud rates limit back down to > the normal bounds. Unfortunately by doing so we also broke the > Mediatek-specific extended bauds feature. > > A fix of the problem described above is twofold. First since we can't get > back the extended baud rate limits feature to the generic set_termios() > function and that method supports only a standard baud rates range, the > requested baud rate must be locally stored before calling it and then > restored back to the new termios structure after the generic set_termios() > finished its magic business. By doing so we still use the > serial8250_do_set_termios() method to set the LCR/MCR/FCR/etc. registers, > while the extended baud rate setting procedure will be performed later in > the custom Mediatek-specific set_termios() callback. Second since a true > baud rate is now fully calculated in the custom set_termios() method we > need to locally update the port timeout by calling the > uart_update_timeout() function. After the fixes described above are > implemented in the 8250_mtk.c driver, the Mediatek 8250-port should > get back to normally working with extended baud rates. > > Link: https://lore.kernel.org/linux-serial/20200701211337.3027448-1-danielwinkler@google.com > > Fixes: 7b668c064ec3 ("serial: 8250: Fix max baud limit in generic 8250 port") > Reported-by: Daniel Winkler > Signed-off-by: Serge Semin > > --- > > Folks, sorry for a delay with the problem fix. A solution is turned out to > be a bit more complicated than I originally thought in my comment to the > Daniel revert-patch. > > Please also note, that I don't have a Mediatek hardware to test the > solution suggested in the patch. The code is written as on so called > the tip of the pen after digging into the 8250_mtk.c and 8250_port.c > drivers code. So please Daniel or someone with Mediatek 8250-port > available on a board test this patch first and report about the results in > reply to this emailing thread. After that, if your conclusion is positive > and there is no objection against the solution design the patch can be > merged in. > > Cc: Alexey Malahov > Cc: Daniel Winkler > Cc: Aaron Sierra > Cc: Andy Shevchenko > Cc: Lukas Wunner > Cc: Vignesh Raghavendra > Cc: linux-serial@vger.kernel.org > Cc: linux-mediatek@lists.infradead.org > Cc: BlueZ > Cc: chromeos-bluetooth-upstreaming > Cc: abhishekpandit@chromium.org > Cc: stable@vger.kernel.org > --- > drivers/tty/serial/8250/8250_mtk.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/drivers/tty/serial/8250/8250_mtk.c b/drivers/tty/serial/8250/8250_mtk.c > index f839380c2f4c..98b8a3e30733 100644 > --- a/drivers/tty/serial/8250/8250_mtk.c > +++ b/drivers/tty/serial/8250/8250_mtk.c > @@ -306,8 +306,21 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios, > } > #endif > > + /* > + * Store the requested baud rate before calling the generic 8250 > + * set_termios method. Standard 8250 port expects bauds to be > + * no higher than (uartclk / 16) so the baud will be clamped if it > + * gets out of that bound. Mediatek 8250 port supports speed > + * higher than that, therefore we'll get original baud rate back > + * after calling the generic set_termios method and recalculate > + * the speed later in this method. > + */ > + baud = tty_termios_baud_rate(termios); > + > serial8250_do_set_termios(port, termios, old); > > + tty_termios_encode_baud_rate(termios, baud, baud); > + > /* > * Mediatek UARTs use an extra highspeed register (MTK_UART_HIGHS) > * > @@ -339,6 +352,11 @@ mtk8250_set_termios(struct uart_port *port, struct ktermios *termios, > */ > spin_lock_irqsave(&port->lock, flags); > > + /* > + * Update the per-port timeout. > + */ > + uart_update_timeout(port, termios->c_cflag, baud); > + > /* set DLAB we have cval saved in up->lcr from the call to the core */ > serial_port_out(port, UART_LCR, up->lcr | UART_LCR_DLAB); > serial_dl_write(up, quot); > -- > 2.26.2 >