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 83594C433EF for ; Sat, 19 Mar 2022 14:32:44 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 16C3683689; Sat, 19 Mar 2022 15:32:42 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=akkea.ca 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=akkea.ca header.i=@akkea.ca header.b="WJQ+BfJw"; dkim=pass (1024-bit key) header.d=akkea.ca header.i=@akkea.ca header.b="KjRD+b47"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 8F94583921; Sat, 19 Mar 2022 15:32:39 +0100 (CET) Received: from node.akkea.ca (li1434-30.members.linode.com [45.33.107.30]) (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 9B2EA81DFA for ; Sat, 19 Mar 2022 15:32:34 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=akkea.ca Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=angus@akkea.ca Received: from localhost (localhost [127.0.0.1]) by node.akkea.ca (Postfix) with ESMTP id 141984E200C; Sat, 19 Mar 2022 14:32:33 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akkea.ca; s=mail; t=1647700353; bh=UCftCqB6JW36Hvzhl8WXoYscg0Prtwu896xUG1aPGu4=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=WJQ+BfJwGUA3Iv7Nmf3e2qhN3khT/2x6raPjEsz0JoDnkkHhjUADwesykEKE8AMd/ SshfLqxFBUvkCdzx5BnZ3FMh5FxDNCD4FAo7yHIW0oOg/xQRbB2cAiXJomrMomnhBL IliTjX69u5XId3zCZJwK34w6n0NPmgbUliop2Te0= Received: from node.akkea.ca ([127.0.0.1]) by localhost (mail.akkea.ca [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lclzdHUMyf65; Sat, 19 Mar 2022 14:32:32 +0000 (UTC) Received: from www.akkea.ca (localhost [127.0.0.1]) by node.akkea.ca (Postfix) with ESMTP id 004724E2006; Sat, 19 Mar 2022 14:32:32 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akkea.ca; s=mail; t=1647700352; bh=UCftCqB6JW36Hvzhl8WXoYscg0Prtwu896xUG1aPGu4=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=KjRD+b47zRXxU7ZFJLfWD5R089F9hRT54C1VI4/fwOLVQX/URHJxNa4uVuoTu6//y YL5Eb9SsQW80R4or+ZTx10Z7ABqCktAlkn2T9IUU03AROkn9fBcW3GzSu1mD2o0+Ny N1LSZ4+U2eBBOUhlCi//gu7Hac1MoN6fdnCa3FOE= MIME-Version: 1.0 Date: Sat, 19 Mar 2022 07:32:31 -0700 From: Angus Ainslie To: Heiko Thiery Cc: u-boot@lists.denx.de, Marek Vasut , Michale Walle , Angus Ainslie , lukma@denx.de, seanga2@gmail.com, sbabic@denx.de, festevam@gmail.com, uboot-imx@nxp.com, peng.fan@nxp.com Subject: Re: [RFC] serial: mxc: get the clock frequency from the used clock for the device In-Reply-To: References: <20220317124127.1783768-1-heiko.thiery@gmail.com> <775d1b35685c64474efa90ae281726d5@akkea.ca> Message-ID: <187a931fce064feacb9e72b603d37140@akkea.ca> X-Sender: angus@akkea.ca User-Agent: Roundcube Webmail/1.3.17 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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.5 at phobos.denx.de X-Virus-Status: Clean On 2022-03-18 12:06, Heiko Thiery wrote: > Hi Angus, > > Am Do., 17. März 2022 um 14:19 Uhr schrieb Angus Ainslie > : >> >> Hi Heiko, >> >> On 2022-03-17 05:41, Heiko Thiery wrote: >> > With the clock driver enabled for the imx8mq, it was noticed that the >> > frequency used to calculate the baud rate is always taken from the root >> > clock of UART1. This can cause problems if UART1 is not used as console >> > and the settings are different from UART1. The result is that the >> > console >> > output is garbage. To do this correctly the UART frequency is taken >> > from >> > the used device. For the implementations that don't have the igp clock >> > frequency written or can't return it the old way is tried. >> > >> > Signed-off-by: Heiko Thiery >> > --- >> > drivers/serial/serial_mxc.c | 15 +++++++++++++-- >> > 1 file changed, 13 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c >> > index e4970a169b..6fdb2b2397 100644 >> > --- a/drivers/serial/serial_mxc.c >> > +++ b/drivers/serial/serial_mxc.c >> > @@ -3,6 +3,7 @@ >> > * (c) 2007 Sascha Hauer >> > */ >> > >> > +#include >> > #include >> > #include >> > #include >> > @@ -266,9 +267,19 @@ __weak struct serial_device >> > *default_serial_console(void) >> > int mxc_serial_setbrg(struct udevice *dev, int baudrate) >> > { >> > struct mxc_serial_plat *plat = dev_get_plat(dev); >> > - u32 clk = imx_get_uartclk(); >> > + u32 rate = 0; >> > + >> > + if (IS_ENABLED(CONFIG_CLK)) { >> > + struct clk clk; >> > + if(!clk_get_by_name(dev, "ipg", &clk)) >> > + rate = clk_get_rate(&clk); >> >> Is the "ipg" clock the correct name for all of the imx DM boards ? > > I checked the dtsi files for all the boards that use the compatible > strings from the serial_mxc and see that there are always 2 clocks: > 'ipg' and 'per'. > > The imx8 dtsi files describe ipg and per always the same clock. > The imx7 dtsi files descibe ipg and per always the same clock. > The imx6 dtsi files describe ipg and per are 2 different clocks > The imx51 dtsi files describeipg and per are 2 different clocks > > So I'm not sure if the ipg clock is the right one for the boards that > has different clock for ipg and per. So I only looked at imx6qdl.dtsi where the clocks are different clocks = <&clks IMX6QDL_CLK_UART_IPG>, <&clks IMX6QDL_CLK_UART_SERIAL>; clock-names = "ipg", "per"; And from that file it looks like the per clock would be the correct one. Should the clock be looked up by id instead of by name and then have a different code path for each imx board type ? > >> > + } >> > + >> > + /* as fallback we try to get the clk rate that way */ >> > + if (rate == 0) >> > + rate = imx_get_uartclk(); >> >> Would it be better to re-write imx_get_uartclk so that both the >> getting >> and setting of clocks was correct ? > > I do not understand what you mean with that. > There are other places in the code that imx_get_uartclk gets called. If an index was added to imx_get_uartclk(int index) then you wouldn't need the code above in the mxc_serial_setbrg function. That would also make all of the places where imx_get_uartclk gets called return the correct value. It might make sense to create a new function imx7_get_uartclk that gets called on newer SOCs so that the imx6 and earlier code doesn't need to get changed. >> With DM clocks enabled I don't even think it makes sense to call those >> older functions. > > You mean when DM clocks are available the "new" method should be used > and no fallback to the old mechanism? > For older boards it makes sense to fallback to the single clock. On newer boards if it returned an error instead it would have been easier for you to figure out where the serial console failed. Angus >> >> Angus >> > >> > - _mxc_serial_setbrg(plat->reg, clk, baudrate, plat->use_dte); >> > + _mxc_serial_setbrg(plat->reg, rate, baudrate, plat->use_dte); >> > >> > return 0; >> > }