From patchwork Mon Jul 4 15:15:25 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Sylwester Nawrocki/Kernel \\(PLT\\) /SRPOL/Staff Engineer/Samsung Electronics" X-Patchwork-Id: 9212861 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BBF9960752 for ; Mon, 4 Jul 2016 15:17:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A432D2873B for ; Mon, 4 Jul 2016 15:17:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 98ECB2873E; Mon, 4 Jul 2016 15:17:44 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 12D062873B for ; Mon, 4 Jul 2016 15:17:43 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.85_2 #1 (Red Hat Linux)) id 1bK5bT-0004Fq-RC; Mon, 04 Jul 2016 15:16:07 +0000 Received: from mailout4.w1.samsung.com ([210.118.77.14]) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1bK5bN-0004DX-LW for linux-arm-kernel@lists.infradead.org; Mon, 04 Jul 2016 15:16:02 +0000 Received: from eucpsbgm2.samsung.com (unknown [203.254.199.245]) by mailout4.w1.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTP id <0O9S00496QE0GC60@mailout4.w1.samsung.com> for linux-arm-kernel@lists.infradead.org; Mon, 04 Jul 2016 16:15:37 +0100 (BST) X-AuditID: cbfec7f5-f792a6d000001302-4e-577a7d981f2d Received: from eusync3.samsung.com ( [203.254.199.213]) by eucpsbgm2.samsung.com (EUCPMTA) with SMTP id 5B.E5.04866.89D7A775; Mon, 4 Jul 2016 16:15:36 +0100 (BST) Received: from [106.116.147.32] by eusync3.samsung.com (Oracle Communications Messaging Server 7.0.5.31.0 64bit (built May 5 2014)) with ESMTPA id <0O9S00JNBQE04O90@eusync3.samsung.com>; Mon, 04 Jul 2016 16:15:36 +0100 (BST) Subject: Re: [PATCH v3 1/2] clk: exynos5433: do not use CLK_IGNORE_UNUSED for SPI clocks References: <1467270911-10971-1-git-send-email-andi.shyti@samsung.com> <1467270911-10971-2-git-send-email-andi.shyti@samsung.com> <577A2EFF.2010306@samsung.com> <20160704102614.GD1257@jack.zhora.eu> From: Sylwester Nawrocki To: Andi Shyti Message-id: <577A7D8D.8070004@samsung.com> Date: Mon, 04 Jul 2016 17:15:25 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-version: 1.0 In-reply-to: <20160704102614.GD1257@jack.zhora.eu> X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFupkkeLIzCtJLcpLzFFi42I5/e/4Vd0ZtVXhBgvuGFncWneO1WL7kWes Fot/PGeyuP7lOavFjV9trBavXxha9D9+zWyx6fE1VouPPfdYLS7vmsNmMeP8PiaLi6dcLX6c 6WaxWLXrD6MDn8f7G63sHpf7epk8ri/5xOyxc9Zddo9NqzrZPDYvqffo27KK0ePzJrkAjigu m5TUnMyy1CJ9uwSujO2Hd7EU9BhXfFz9gK2BsVGpi5GTQ0LAROLtwaNMELaYxIV769m6GLk4 hASWMkpsnPKXGcJ5zijxZuMLIIeDQ1ggSuLB8QyI+ClGiQdHdrOAdDMLvGSW2NIdAWKzCRhK 9B7tYwSxRQTUJZbs3coMYvMKaEn8fdrEDmKzCKhKPPy3mxXEFhWIkHgy9yQjRI2gxI/J98Bm cgoYSaxdOZMRZC+zgJ7E/YtaEKvkJTavecs8gVFgFpKOWQhVs5BULWBkXsUomlqaXFCclJ5r pFecmFtcmpeul5yfu4kREkFfdzAuPWZ1iFGAg1GJh5chpjJciDWxrLgy9xCjBAezkgivTVVV uBBvSmJlVWpRfnxRaU5q8SFGaQ4WJXHembvehwgJpCeWpGanphakFsFkmTg4pRoY3aWuis5n 09pb5eF4cJHYbq2r/H/mcb8LiP6YMfnm0fKJH5aZ51qInNi58KXlTIlujY7c+ZLyqU03Fz+U L9llcM959ZsMk/mFlbPkp9hW2dZMEN9ZeTtPlvlM0J+9gX9rN39Ln5IV/VX++3EN44AW+6uO qtqvJp/ZcGBx8OLks7zTXh8OM03eqsRSnJFoqMVcVJwIAMCfqt+cAgAA X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160704_081601_890884_45F7EA39 X-CRM114-Status: GOOD ( 26.25 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Krzysztof Kozlowski , linux-samsung-soc@vger.kernel.org, Andrzej Hajda , Michael Turquette , Stephen Boyd , Tomasz Figa , linux-kernel@vger.kernel.org, Jaehoon Chung , Chanwoo Choi , Kukjin Kim , Andi Shyti , linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On 07/04/2016 12:26 PM, Andi Shyti wrote: > The single clock lines are not configured in the exynos5433 dts, > but in the drivers/clk/samsung/clk-exynos5433.c file and it's the > only place where we can set the flags. I meant we could amend which clocks are specified at the SPI bus device DT nodes and change handling of clocks in the spi-s3c64xx driver to model everything properly and get it all working. >> What is an exact problem here, are you perhaps testing suspend to RAM? >> I tested my sound support patches on top of v4.7-rc1 and everything >> seemed to work well, I didn't notice any issues with the audio codec >> which was the only slave on the SPI 1 bus. > > Yes, because the audio codec is on SPI1 and its bus line > (spi_busclk0) is CLK_SCLK_SPI1_PERIC while the CLK_SCLK_SPI1 is > set as CLK_IGNORE_UNUSED. That's true, looking at a downstream kernel I see that there is just plain div clock specified for spi_busclk0 (DIVsclk_spi1_b), i.e. SCLK_SPI1_PERIC and SCLK_SPI1 don't get disabled in s3c64xx_spi_config(). It seems SCLK_SPI1 in CMU_PERIC need to be kept enabled while accessing the SPI controller's registers. >> Doesn't it help when you specify CLK_SCLK_SPI1 as the second clock >> ("spi_busclk0") of the spi_1 bus controller instead of >> CLK_SCLK_SPI0_PERIC? CLK_SCLK_SPI0_PERIC seem to be parent of >> CLK_SCLK_SPI1 so the enable state would be propagated. > > nope! :( > > For some reasons, if you set in the DTS as spi_busclk0 the > CLK_SCLK_SPI1 from cmu_peric you get a synchronus abort in the > s3c64xx_spi_config (the first read performed on the device). Indeed, I also observed that, after removing CLK_IGNORE_UNUSED from the CLK_SCLK_SPI1 clock. After discussion with Krzysztof and Andrzej I came up with a patch as below where there is no aborts, the sound works and clocks are not kept always enabled: root@localhost:~# cat /sys/kernel/debug/clk/clk_summary | grep spi1 ioclk_spi1_clk_in 0 0 50000000 0 0 sclk_ioclk_spi1 0 0 50000000 0 0 pclk_isp_spi1 0 0 6000000 0 0 mout_sclk_isp_spi1_user 0 0 24000000 0 0 sclk_isp_spi1 0 0 24000000 0 0 mout_sclk_spi1 0 0 24000000 0 0 div_sclk_spi1_a 0 0 3000000 0 0 div_sclk_spi1_b 0 0 3000000 0 0 sclk_spi1_peric 0 0 3000000 0 0 sclk_spi1 0 0 3000000 0 0 mout_sclk_isp_spi1 0 0 24000000 0 0 div_sclk_isp_spi1_a 0 0 3000000 0 0 div_sclk_isp_spi1_b 0 0 25000 0 0 sclk_isp_spi1_cam1 0 0 25000 0 0 pclk_spi1 0 0 66666667 0 0 I'm not yet 100% sure if it is a correct approach, the downstream kernel uses "global-per-IP" gate clocks (ENABLE_IP_PERIC? registers), that gate all clocks to a given IP block and those clocks are not defined in mainline at all, but it seems we just need to amend the SPI controller driver to not be disabling sdd->src_clk clock before accessing registers. Or maybe to pass only DIVsclk_spi?_b as spi_busclk0 in DT nodes and add SCLK_SPI0 from CMU_PERIC as a third SPI device clock for exynos5433. -----------8<------------ -----------8<------------ diff --git a/arch/arm64/boot/dts/exynos/exynos5433.dtsi b/arch/arm64/boot/dts/exynos/exynos5433.dtsi index 8e124fc..f444c66 100644 --- a/arch/arm64/boot/dts/exynos/exynos5433.dtsi +++ b/arch/arm64/boot/dts/exynos/exynos5433.dtsi @@ -617,8 +617,13 @@ dma-names = "tx", "rx"; #address-cells = <1>; #size-cells = <0>; +#if 0 clocks = <&cmu_peric CLK_PCLK_SPI1>, <&cmu_top CLK_SCLK_SPI1_PERIC>; +#else + clocks = <&cmu_peric CLK_PCLK_SPI1>, + <&cmu_peric CLK_SCLK_SPI1>; +#endif clock-names = "spi", "spi_busclk0"; samsung,spi-src-clk = <0>; pinctrl-names = "default"; diff --git a/drivers/clk/samsung/clk-exynos5433.c b/drivers/clk/samsung/clk-exynos5433.c index e3cc935..61d5643 100644 --- a/drivers/clk/samsung/clk-exynos5433.c +++ b/drivers/clk/samsung/clk-exynos5433.c @@ -1675,7 +1675,7 @@ static struct samsung_gate_clock peric_gate_clks[] __initdata = { GATE(CLK_SCLK_SPI2, "sclk_spi2", "sclk_spi2_peric", ENABLE_SCLK_PERIC, 5, CLK_SET_RATE_PARENT, 0), GATE(CLK_SCLK_SPI1, "sclk_spi1", "sclk_spi1_peric", ENABLE_SCLK_PERIC, - 4, CLK_IGNORE_UNUSED | CLK_SET_RATE_PARENT, 0), + 4, CLK_SET_RATE_PARENT, 0), GATE(CLK_SCLK_SPI0, "sclk_spi0", "sclk_spi0_peric", ENABLE_SCLK_PERIC, 3, CLK_SET_RATE_PARENT, 0), GATE(CLK_SCLK_UART2, "sclk_uart2", "sclk_uart2_peric", diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c index 5a76a50..2cb965c 100644 --- a/drivers/spi/spi-s3c64xx.c +++ b/drivers/spi/spi-s3c64xx.c @@ -578,7 +578,7 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd) /* Disable Clock */ if (sdd->port_conf->clk_from_cmu) { - clk_disable_unprepare(sdd->src_clk); + /* clk_disable_unprepare(sdd->src_clk); */ } else { val = readl(regs + S3C64XX_SPI_CLK_CFG); val &= ~S3C64XX_SPI_ENCLK_ENABLE; @@ -626,7 +626,7 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd) /* There is half-multiplier before the SPI */ clk_set_rate(sdd->src_clk, sdd->cur_speed * 2); /* Enable Clock */ - clk_prepare_enable(sdd->src_clk); + /* clk_prepare_enable(sdd->src_clk); */ } else { /* Configure Clock */ val = readl(regs + S3C64XX_SPI_CLK_CFG);