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=-15.7 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 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 F0869C4338F for ; Mon, 2 Aug 2021 23:06:22 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D418060D07 for ; Mon, 2 Aug 2021 23:06:22 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232979AbhHBXGb (ORCPT ); Mon, 2 Aug 2021 19:06:31 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36304 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232130AbhHBXG3 (ORCPT ); Mon, 2 Aug 2021 19:06:29 -0400 Received: from mail-vs1-xe2a.google.com (mail-vs1-xe2a.google.com [IPv6:2607:f8b0:4864:20::e2a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7BE3BC061764 for ; Mon, 2 Aug 2021 16:06:18 -0700 (PDT) Received: by mail-vs1-xe2a.google.com with SMTP id bg4so10433295vsb.6 for ; Mon, 02 Aug 2021 16:06:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xpyVU3ei/AP0KmybjlqX88a6HrifRHguk/VSuzK6yLo=; b=ohEu439CYwzdTzfT11BrJ44b15ggEvl6dUJjbnhwnZVt5QmMPMHbfZX+Y4vMm5gv2x MBIpC1bQ+TY5lKTRmdNBt+pW38dNcGDFpfFiy/EnwvOvW+uomleQyojIFn6NIDyDghgR XwH7Ud7MvQ2uk7HCNkeYZb8G4Zpa4COEHhPw4zGqht5omFkvUnxcLaOqJ0KA2Ohi/Tgz A1k5o3iiTu7IF+YkXGofinmq0KwAhxekEOP9ivHeh0DJZIBoe/2/dUc/xbHK7oVNhdno YMIy56Zp5BRQFzJAJhqmPmnbJ3o+TeAJntwXv7E8QGwHBbCjaDq2BY2doN6FCTdEWg/2 92GQ== 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=xpyVU3ei/AP0KmybjlqX88a6HrifRHguk/VSuzK6yLo=; b=UmDPkqgkd0Y1CItJaL/JeMxpVNVvM5aYM42JnYCCgTuy1Neu0ilgJwvqK4U5kW83dH yJA2CLagRtAKSscu4Gn1Jo9dEHHWwKN52ZHxhL6pucIj9juPnN3rWY60wD/5bRA8Wtz/ q5tHqb7cocrIE5XXDUhAi4qShEiHS4FHq8gttdQC5ANE1rzfof4j3nqcVJdaycs/wuNV s6SL3A/JBTSuSpTqbBZ2RjVmP8XduoJMIr4v7qexigUHqCcxAygfr7qkU+0fGOl4Yb6H rLl5Rh2mMzNL6ofnQ8dRvC1k0PcAtm1mvxh5nDglh5r4OAu8V6lSwKfh/5EbVx7aP80h a/1w== X-Gm-Message-State: AOAM5338cuSRsjkQh0RQDyXwgVQTAGaFBOpzR1oi4mWq9dl49vhUaeBJ H7kiZp4LYhnkMCkqFxy6E5nL9x20vS0K+Ja6yc/iPQ== X-Google-Smtp-Source: ABdhPJy4VIZWnscV1Z5U3ENe5sVJSmubutu5e/54rLKoUQ9TLsjSTGstTYauXexlME0HYczpKQNgpC7vEN1H5rzIAmc= X-Received: by 2002:a05:6102:21b:: with SMTP id z27mr12281035vsp.27.1627945576875; Mon, 02 Aug 2021 16:06:16 -0700 (PDT) MIME-Version: 1.0 References: <20210730144922.29111-1-semen.protsenko@linaro.org> <20210730144922.29111-5-semen.protsenko@linaro.org> In-Reply-To: From: Sam Protsenko Date: Tue, 3 Aug 2021 02:06:05 +0300 Message-ID: Subject: Re: [PATCH 04/12] tty: serial: samsung: Init USI to keep clocks running To: Krzysztof Kozlowski Cc: Sylwester Nawrocki , Chanwoo Choi , Linus Walleij , Tomasz Figa , Rob Herring , Stephen Boyd , Michael Turquette , Jiri Slaby , Greg Kroah-Hartman , Charles Keepax , Ryu Euiyoul , Tom Gall , Sumit Semwal , John Stultz , Amit Pundir , devicetree , linux-arm Mailing List , linux-clk , "open list:GPIO SUBSYSTEM" , Linux Kernel Mailing List , Linux Samsung SOC , "open list:SERIAL DRIVERS" Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 30 Jul 2021 at 19:32, Krzysztof Kozlowski wrote: > > On 30/07/2021 16:49, Sam Protsenko wrote: > > UART block is a part of USI (Universal Serial Interface) IP-core in > > Samsung SoCs since Exynos9810 (e.g. in Exynos850). USI allows one to > > enable one of three types of serial interface: UART, SPI or I2C. That's > > possible because USI shares almost all internal circuits within each > > protocol. USI also provides some additional registers so it's possible > > to configure it. > > > > One USI register called USI_OPTION has reset value of 0x0. Because of > > this the clock gating behavior is controlled by hardware (HWACG = > > Hardware Auto Clock Gating), which simply means the serial won't work > > after reset as is. In order to make it work, USI_OPTION[2:1] bits must > > be set to 0b01, so that HWACG is controlled manually (by software). > > Bits meaning: > > - CLKREQ_ON = 1: clock is continuously provided to IP > > - CLKSTOP_ON = 0: drive IP_CLKREQ to High (needs to be set along with > > CLKREQ_ON = 1) > > > > USI is not present on older chips, like s3c2410, s3c2412, s3c2440, > > s3c6400, s5pv210, exynos5433, exynos4210. So the new boolean field > > '.has_usi' was added to struct s3c24xx_uart_info. USI registers will be > > only actually accessed when '.has_usi' field is set to "1". > > > > This feature is needed for further serial enablement on Exynos850, but > > some other new Exynos chips (like Exynos9810) may benefit from this > > feature as well. > > > > Signed-off-by: Sam Protsenko > > --- > > drivers/tty/serial/samsung_tty.c | 33 +++++++++++++++++++++++++++++++- > > include/linux/serial_s3c.h | 9 +++++++++ > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > > index 9fbc61151c2e..0f3cbd0b37e3 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -65,6 +65,7 @@ enum s3c24xx_port_type { > > struct s3c24xx_uart_info { > > char *name; > > enum s3c24xx_port_type type; > > + unsigned int has_usi; > > unsigned int port_type; > > unsigned int fifosize; > > unsigned long rx_fifomask; > > @@ -1352,6 +1353,29 @@ static int apple_s5l_serial_startup(struct uart_port *port) > > return ret; > > } > > > > +static void exynos_usi_init(struct uart_port *port) > > +{ > > + struct s3c24xx_uart_port *ourport = to_ourport(port); > > + struct s3c24xx_uart_info *info = ourport->info; > > + > > + if (!info->has_usi) > > + return; > > + > > + /* > > + * USI_RESET is an active High signal. Reset value of USI_RESET is 0x1 > > + * to drive stable value to PAD. Due to this feature, the USI_RESET must > > + * be cleared (set as 0x0) before starting a transaction. > > "before starting a transaction" suggests it is related with transaction > or something before starting it. Don't you need it simply after reset or > resume? > Not sure what you are suggesting. USI_RESET is set to "1" at start up (means USI block hangs in reset state), so we have to make it "0" (that's what this code does); only then UART becomes functional and UART transactions can be performed. And exynos_usi_init() is called exactly where you hinted: at init and on resume. Anyway, the whole comment is confusing, I'll simplify and rework it in v2. Please let me know if I'm missing the point though. > > + */ > > + wr_regl(port, USI_CON, USI_RESET); > > You are clearing entire register, not only USI_RESET bitfield. Is it > really what you want? > Yeah, USI_CON[31:1] bits are reserved, and the reset value of this register is 0x00000001. But anyway, I'm going to rework that code like this, for clarity and consistence: 8<--------------------------------------------------------------------->8 /* Clear the software reset of USI block (it's set at startup) */ val = rd_regl(port, USI_CON); val &= ~(USI_RESET_MASK) wr_regl(port, USI_CON, val); udelay(1); /* Continuously provide the clock to USI IP w/o gating (for Rx mode) */ val = rd_regl(port, USI_OPTION); val &= ~USI_HWACG_MASK; val |= USI_HWACG_CLKREQ_ON; wr_regl(port, USI_OPTION, val); 8<--------------------------------------------------------------------->8 > > + udelay(1); > > + > > + /* > > + * Set the HWACG option bit in case of UART Rx mode. > > + * CLKREQ_ON = 1, CLKSTOP_ON = 0 (set USI_OPTION[2:1] = 0x1). > > + */ > > + wr_regl(port, USI_OPTION, USI_HWACG_CLKREQ_ON); > > +} > > + > > /* power power management control */ > > > > static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level, > > @@ -1379,6 +1403,7 @@ static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level, > > if (!IS_ERR(ourport->baudclk)) > > clk_prepare_enable(ourport->baudclk); > > > > + exynos_usi_init(port); > > break; > > default: > > dev_err(port->dev, "s3c24xx_serial: unknown pm %d\n", level); > > @@ -2102,6 +2127,8 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport, > > if (ret) > > pr_warn("uart: failed to enable baudclk\n"); > > > > + exynos_usi_init(port); > > + > > /* Keep all interrupts masked and cleared */ > > switch (ourport->info->type) { > > case TYPE_S3C6400: > > @@ -2750,10 +2777,11 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = { > > #endif > > > > #if defined(CONFIG_ARCH_EXYNOS) > > -#define EXYNOS_COMMON_SERIAL_DRV_DATA \ > > +#define EXYNOS_COMMON_SERIAL_DRV_DATA_USI(_has_usi) \ > > .info = &(struct s3c24xx_uart_info) { \ > > .name = "Samsung Exynos UART", \ > > .type = TYPE_S3C6400, \ > > + .has_usi = _has_usi, \ > > .port_type = PORT_S3C6400, \ > > .has_divslot = 1, \ > > .rx_fifomask = S5PV210_UFSTAT_RXMASK, \ > > @@ -2773,6 +2801,9 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = { > > .has_fracval = 1, \ > > } \ > > > > +#define EXYNOS_COMMON_SERIAL_DRV_DATA \ > > + EXYNOS_COMMON_SERIAL_DRV_DATA_USI(0) > > + > > static struct s3c24xx_serial_drv_data exynos4210_serial_drv_data = { > > EXYNOS_COMMON_SERIAL_DRV_DATA, > > .fifosize = { 256, 64, 16, 16 }, > > diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h > > index f6c3323fc4c5..013c2646863e 100644 > > --- a/include/linux/serial_s3c.h > > +++ b/include/linux/serial_s3c.h > > @@ -28,6 +28,15 @@ > > #define S3C2410_UFSTAT (0x18) > > #define S3C2410_UMSTAT (0x1C) > > > > +/* USI Control Register offset */ > > +#define USI_CON (0xC4) > > +/* USI Option Register offset */ > > +#define USI_OPTION (0xC8) > > +/* USI_CON[0] = 0b0: clear USI global software reset (Active High) */ > > +#define USI_RESET (0<<0) > > Just 0x0. I understand you wanted to hint it is a bit field, but the > shift of 0 actually creates more questions. > After some consideration I decided to adhere to existing style and do something like this (in v2): 8<--------------------------------------------------------------------->8 #define USI_CON (0xC4) #define USI_OPTION (0xC8) #define USI_CON_RESET_CLEAR (0<<0) #define USI_CON_RESET_SET (1<<0) #define USI_CON_RESET_MASK (1<<0) #define USI_OPTION_HWACG_CLKREQ_ON (1<<1) #define USI_OPTION_HWACG_CLKSTOP_ON (1<<2) #define USI_OPTION_HWACG_MASK (3<<1) 8<--------------------------------------------------------------------->8 The whole reason for those comments was missing public TRM. But in the end I decided it just looks ugly. Also, this way I can do RMW operation (discussed above) in more logical way. Please let me know if code snippets above look good to you. > > Best regards, > Krzysztof 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=-14.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 87F03C4338F for ; Mon, 2 Aug 2021 23:08:16 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 4D30B60EBB for ; Mon, 2 Aug 2021 23:08:16 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 4D30B60EBB Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From: In-Reply-To:References:MIME-Version:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=5SF4hGId/yL75pMIzFpwOuBIV/e/A8q0mKWL+dnSw6c=; b=eUMFOIVJJWfOn8 dqcCpvNHXRK52XCgyTVOCD3mF8sk1pY0mgV6zdsweU7PyhEhTNX7m8DfDdAl4E7cO0QJOSA9qIvME yRV7oKDIhXO9zZQWiVU50XpL9nPmx7ayZMIq95E44Nd6NyyzmgtAyhWBehqOZdTOPR0GBgApnUNEj KoL21/cGy3umFdt+XCSuZzyWnvfV3C4pl9HL435hCSfTusSyNMq3wYxap9pk2+oVCEd5q9gw62r8R 6kCV1Ftd6tW3ffNOVTV2FhtRgnDTdoqVYsXdisaoWA7kskFMgjRxBVT1Fnjp56sV2xJF/bJiLAs2W 5MHYC+oj2KV+GSvxnk2w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAh0p-000XgM-UX; Mon, 02 Aug 2021 23:06:24 +0000 Received: from mail-vs1-xe33.google.com ([2607:f8b0:4864:20::e33]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mAh0k-000Xf1-Iw for linux-arm-kernel@lists.infradead.org; Mon, 02 Aug 2021 23:06:20 +0000 Received: by mail-vs1-xe33.google.com with SMTP id b138so4242997vsd.2 for ; Mon, 02 Aug 2021 16:06:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=xpyVU3ei/AP0KmybjlqX88a6HrifRHguk/VSuzK6yLo=; b=ohEu439CYwzdTzfT11BrJ44b15ggEvl6dUJjbnhwnZVt5QmMPMHbfZX+Y4vMm5gv2x MBIpC1bQ+TY5lKTRmdNBt+pW38dNcGDFpfFiy/EnwvOvW+uomleQyojIFn6NIDyDghgR XwH7Ud7MvQ2uk7HCNkeYZb8G4Zpa4COEHhPw4zGqht5omFkvUnxcLaOqJ0KA2Ohi/Tgz A1k5o3iiTu7IF+YkXGofinmq0KwAhxekEOP9ivHeh0DJZIBoe/2/dUc/xbHK7oVNhdno YMIy56Zp5BRQFzJAJhqmPmnbJ3o+TeAJntwXv7E8QGwHBbCjaDq2BY2doN6FCTdEWg/2 92GQ== 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=xpyVU3ei/AP0KmybjlqX88a6HrifRHguk/VSuzK6yLo=; b=B5Lt8xSdDiD5N7AFjGttB7ZJID3tWdd7rTQ1RekjKGkeGX+tvypoeGANFQFl5Qwrbi D5mq946fslYOFhGIsvNgfo2BReFl1z5Whnoz6i3DZSGqS/DJiUV13X1q3lx6xveMUj11 YushQzXGUkRIyoxiGxTRMNf1dhWzQtNY2Trwzkp+v5MZxRY9c+HUf3yEEX2p5J/Ks5uX VEarGrOYzzqfzy8Oiqj8iL8sJn/L5g9rJ7uDIX3rhLzukht9Vsts3ltqbd6eLxoXRYZG sf06yFwRU463sbV8/0PZMimK4PtDRy9SatnHs25uT5xP4pDIC0UBjATtejFJEg056Ig4 vwlQ== X-Gm-Message-State: AOAM533akJaNPjD2CYrx60SuLgknREhpcKo+PPomsIPNOzjCBWW1isaR 6jpSf9nwMgZGExqgytpvq/r6JCuClukDH2Nkk1mqjKLCs6ezQSvh1Kg= X-Google-Smtp-Source: ABdhPJy4VIZWnscV1Z5U3ENe5sVJSmubutu5e/54rLKoUQ9TLsjSTGstTYauXexlME0HYczpKQNgpC7vEN1H5rzIAmc= X-Received: by 2002:a05:6102:21b:: with SMTP id z27mr12281035vsp.27.1627945576875; Mon, 02 Aug 2021 16:06:16 -0700 (PDT) MIME-Version: 1.0 References: <20210730144922.29111-1-semen.protsenko@linaro.org> <20210730144922.29111-5-semen.protsenko@linaro.org> In-Reply-To: From: Sam Protsenko Date: Tue, 3 Aug 2021 02:06:05 +0300 Message-ID: Subject: Re: [PATCH 04/12] tty: serial: samsung: Init USI to keep clocks running To: Krzysztof Kozlowski Cc: Sylwester Nawrocki , Chanwoo Choi , Linus Walleij , Tomasz Figa , Rob Herring , Stephen Boyd , Michael Turquette , Jiri Slaby , Greg Kroah-Hartman , Charles Keepax , Ryu Euiyoul , Tom Gall , Sumit Semwal , John Stultz , Amit Pundir , devicetree , linux-arm Mailing List , linux-clk , "open list:GPIO SUBSYSTEM" , Linux Kernel Mailing List , Linux Samsung SOC , "open list:SERIAL DRIVERS" X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210802_160618_678590_AA0AD6C4 X-CRM114-Status: GOOD ( 45.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, 30 Jul 2021 at 19:32, Krzysztof Kozlowski wrote: > > On 30/07/2021 16:49, Sam Protsenko wrote: > > UART block is a part of USI (Universal Serial Interface) IP-core in > > Samsung SoCs since Exynos9810 (e.g. in Exynos850). USI allows one to > > enable one of three types of serial interface: UART, SPI or I2C. That's > > possible because USI shares almost all internal circuits within each > > protocol. USI also provides some additional registers so it's possible > > to configure it. > > > > One USI register called USI_OPTION has reset value of 0x0. Because of > > this the clock gating behavior is controlled by hardware (HWACG = > > Hardware Auto Clock Gating), which simply means the serial won't work > > after reset as is. In order to make it work, USI_OPTION[2:1] bits must > > be set to 0b01, so that HWACG is controlled manually (by software). > > Bits meaning: > > - CLKREQ_ON = 1: clock is continuously provided to IP > > - CLKSTOP_ON = 0: drive IP_CLKREQ to High (needs to be set along with > > CLKREQ_ON = 1) > > > > USI is not present on older chips, like s3c2410, s3c2412, s3c2440, > > s3c6400, s5pv210, exynos5433, exynos4210. So the new boolean field > > '.has_usi' was added to struct s3c24xx_uart_info. USI registers will be > > only actually accessed when '.has_usi' field is set to "1". > > > > This feature is needed for further serial enablement on Exynos850, but > > some other new Exynos chips (like Exynos9810) may benefit from this > > feature as well. > > > > Signed-off-by: Sam Protsenko > > --- > > drivers/tty/serial/samsung_tty.c | 33 +++++++++++++++++++++++++++++++- > > include/linux/serial_s3c.h | 9 +++++++++ > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serial/samsung_tty.c b/drivers/tty/serial/samsung_tty.c > > index 9fbc61151c2e..0f3cbd0b37e3 100644 > > --- a/drivers/tty/serial/samsung_tty.c > > +++ b/drivers/tty/serial/samsung_tty.c > > @@ -65,6 +65,7 @@ enum s3c24xx_port_type { > > struct s3c24xx_uart_info { > > char *name; > > enum s3c24xx_port_type type; > > + unsigned int has_usi; > > unsigned int port_type; > > unsigned int fifosize; > > unsigned long rx_fifomask; > > @@ -1352,6 +1353,29 @@ static int apple_s5l_serial_startup(struct uart_port *port) > > return ret; > > } > > > > +static void exynos_usi_init(struct uart_port *port) > > +{ > > + struct s3c24xx_uart_port *ourport = to_ourport(port); > > + struct s3c24xx_uart_info *info = ourport->info; > > + > > + if (!info->has_usi) > > + return; > > + > > + /* > > + * USI_RESET is an active High signal. Reset value of USI_RESET is 0x1 > > + * to drive stable value to PAD. Due to this feature, the USI_RESET must > > + * be cleared (set as 0x0) before starting a transaction. > > "before starting a transaction" suggests it is related with transaction > or something before starting it. Don't you need it simply after reset or > resume? > Not sure what you are suggesting. USI_RESET is set to "1" at start up (means USI block hangs in reset state), so we have to make it "0" (that's what this code does); only then UART becomes functional and UART transactions can be performed. And exynos_usi_init() is called exactly where you hinted: at init and on resume. Anyway, the whole comment is confusing, I'll simplify and rework it in v2. Please let me know if I'm missing the point though. > > + */ > > + wr_regl(port, USI_CON, USI_RESET); > > You are clearing entire register, not only USI_RESET bitfield. Is it > really what you want? > Yeah, USI_CON[31:1] bits are reserved, and the reset value of this register is 0x00000001. But anyway, I'm going to rework that code like this, for clarity and consistence: 8<--------------------------------------------------------------------->8 /* Clear the software reset of USI block (it's set at startup) */ val = rd_regl(port, USI_CON); val &= ~(USI_RESET_MASK) wr_regl(port, USI_CON, val); udelay(1); /* Continuously provide the clock to USI IP w/o gating (for Rx mode) */ val = rd_regl(port, USI_OPTION); val &= ~USI_HWACG_MASK; val |= USI_HWACG_CLKREQ_ON; wr_regl(port, USI_OPTION, val); 8<--------------------------------------------------------------------->8 > > + udelay(1); > > + > > + /* > > + * Set the HWACG option bit in case of UART Rx mode. > > + * CLKREQ_ON = 1, CLKSTOP_ON = 0 (set USI_OPTION[2:1] = 0x1). > > + */ > > + wr_regl(port, USI_OPTION, USI_HWACG_CLKREQ_ON); > > +} > > + > > /* power power management control */ > > > > static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level, > > @@ -1379,6 +1403,7 @@ static void s3c24xx_serial_pm(struct uart_port *port, unsigned int level, > > if (!IS_ERR(ourport->baudclk)) > > clk_prepare_enable(ourport->baudclk); > > > > + exynos_usi_init(port); > > break; > > default: > > dev_err(port->dev, "s3c24xx_serial: unknown pm %d\n", level); > > @@ -2102,6 +2127,8 @@ static int s3c24xx_serial_init_port(struct s3c24xx_uart_port *ourport, > > if (ret) > > pr_warn("uart: failed to enable baudclk\n"); > > > > + exynos_usi_init(port); > > + > > /* Keep all interrupts masked and cleared */ > > switch (ourport->info->type) { > > case TYPE_S3C6400: > > @@ -2750,10 +2777,11 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = { > > #endif > > > > #if defined(CONFIG_ARCH_EXYNOS) > > -#define EXYNOS_COMMON_SERIAL_DRV_DATA \ > > +#define EXYNOS_COMMON_SERIAL_DRV_DATA_USI(_has_usi) \ > > .info = &(struct s3c24xx_uart_info) { \ > > .name = "Samsung Exynos UART", \ > > .type = TYPE_S3C6400, \ > > + .has_usi = _has_usi, \ > > .port_type = PORT_S3C6400, \ > > .has_divslot = 1, \ > > .rx_fifomask = S5PV210_UFSTAT_RXMASK, \ > > @@ -2773,6 +2801,9 @@ static struct s3c24xx_serial_drv_data s5pv210_serial_drv_data = { > > .has_fracval = 1, \ > > } \ > > > > +#define EXYNOS_COMMON_SERIAL_DRV_DATA \ > > + EXYNOS_COMMON_SERIAL_DRV_DATA_USI(0) > > + > > static struct s3c24xx_serial_drv_data exynos4210_serial_drv_data = { > > EXYNOS_COMMON_SERIAL_DRV_DATA, > > .fifosize = { 256, 64, 16, 16 }, > > diff --git a/include/linux/serial_s3c.h b/include/linux/serial_s3c.h > > index f6c3323fc4c5..013c2646863e 100644 > > --- a/include/linux/serial_s3c.h > > +++ b/include/linux/serial_s3c.h > > @@ -28,6 +28,15 @@ > > #define S3C2410_UFSTAT (0x18) > > #define S3C2410_UMSTAT (0x1C) > > > > +/* USI Control Register offset */ > > +#define USI_CON (0xC4) > > +/* USI Option Register offset */ > > +#define USI_OPTION (0xC8) > > +/* USI_CON[0] = 0b0: clear USI global software reset (Active High) */ > > +#define USI_RESET (0<<0) > > Just 0x0. I understand you wanted to hint it is a bit field, but the > shift of 0 actually creates more questions. > After some consideration I decided to adhere to existing style and do something like this (in v2): 8<--------------------------------------------------------------------->8 #define USI_CON (0xC4) #define USI_OPTION (0xC8) #define USI_CON_RESET_CLEAR (0<<0) #define USI_CON_RESET_SET (1<<0) #define USI_CON_RESET_MASK (1<<0) #define USI_OPTION_HWACG_CLKREQ_ON (1<<1) #define USI_OPTION_HWACG_CLKSTOP_ON (1<<2) #define USI_OPTION_HWACG_MASK (3<<1) 8<--------------------------------------------------------------------->8 The whole reason for those comments was missing public TRM. But in the end I decided it just looks ugly. Also, this way I can do RMW operation (discussed above) in more logical way. Please let me know if code snippets above look good to you. > > Best regards, > Krzysztof _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel