From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751442AbcGFEvO (ORCPT ); Wed, 6 Jul 2016 00:51:14 -0400 Received: from mailout3.samsung.com ([203.254.224.33]:43203 "EHLO mailout3.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750864AbcGFEvM (ORCPT ); Wed, 6 Jul 2016 00:51:12 -0400 X-AuditID: cbfee68e-f79266d000001428-f1-577c8e3d5bc3 Date: Wed, 06 Jul 2016 13:51:09 +0900 From: Andi Shyti To: Sylwester Nawrocki Cc: Andi Shyti , Chanwoo Choi , Jaehoon Chung , Tomasz Figa , Michael Turquette , Stephen Boyd , Kukjin Kim , Krzysztof Kozlowski , linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andrzej Hajda Subject: Re: [PATCH v3 1/2] clk: exynos5433: do not use CLK_IGNORE_UNUSED for SPI clocks Message-id: <20160706045108.GD23620@samsunx.samsung> 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> <577A7D8D.8070004@samsung.com> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii Content-disposition: inline In-reply-to: <577A7D8D.8070004@samsung.com> User-Agent: Mutt/1.6.0 (2016-04-01) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrIIsWRmVeSWpSXmKPExsWyRsSkQNe2rybcYPYmcYtb686xWiz+8ZzJ 4vqX56wWN361sVq8fmFo0f/4NbPFpsfXWC0+9txjtbi8aw6bxYzz+5gsLp5ytTj8pp3V4seZ bhaLVbv+MDrweby/0crucbmvl8nj+pJPzB47Z91l99i0qpPNY/OSeo++LasYPT5vkgvgiOKy SUnNySxLLdK3S+DK+D3jGlvBLLGK60u/sDcwPuDrYuTkkBAwkVixeAsrhC0mceHeerYuRi4O IYEVjBJtxx+zwRQ1tv9kB7GFBGYxSsz4wAtR9JFR4tf+R2AJFgFVia/bH7CA2GwCmhJNt3+A NYsI6EssWXURzGYWeMkssaU7oouRg0NYIEriwfEMkDAv0Pz+nzcYIWbuZ5KY8nQnI0RCUOLH 5HssEL1aEut3HmeCsKUlHv2dAbaXU0Bb4uLFg4wgM0UFlCXuLgsCmSMhMJVDYsX5TVC3CUh8 m3yIBaRGQkBWYtMBZoi/JCUOrrjBMoFRbBaSbbOQbJuFZNsCRuZVjKKpBckFxUnpRUZ6xYm5 xaV56XrJ+bmbGIExffrfs74djDcPWB9iFOBgVOLhnfC8OlyINbGsuDL3EKMp0BUTmaVEk/OB iSOvJN7Q2MzIwtTE1NjI3NJMSZw3QepnsJBAemJJanZqakFqUXxRaU5q8SFGJg5OqQbGHh0X g0ertZQ/iB6vUE3YcX6N8T1fyQD7v8HfXFfFdug+F/1RMnmd+OL3uc2uL/c1HzTwu8cWH7Hs z+t8vRftHUKKIZ1Sx8PWnDII6hJtjmWN0yoU/FxY7pTV/Di3i1N4w/Htizd8+isrcy8loLJq yrzzrercf1uT9M7sn7fnrRen0BWTto9KLMUZiYZazEXFiQCcDhG35AIAAA== X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprHKsWRmVeSWpSXmKPExsVy+t9jQV3bvppwg5bDAha31p1jtVj84zmT xfUvz1ktbvxqY7V4/cLQov/xa2aLTY+vsVp87LnHanF51xw2ixnn9zFZXDzlanH4TTurxY8z 3SwWq3b9YXTg83h/o5Xd43JfL5PH9SWfmD12zrrL7rFpVSebx+Yl9R59W1YxenzeJBfAEdXA aJORmpiSWqSQmpecn5KZl26r5B0c7xxvamZgqGtoaWGupJCXmJtqq+TiE6DrlpkDdLaSQlli TilQKCCxuFhJ3w7ThNAQN10LmMYIXd+QILgeIwM0kLCGMeP3jGtsBbPEKq4v/cLewPiAr4uR k0NCwESisf0nO4QtJnHh3no2EFtIYBajxIwPvF2MXED2R0aJX/sfgRWxCKhKfN3+gAXEZhPQ lGi6/QOsQURAX2LJqotgNrPAS2aJLd0RXYwcHMICURIPjmeAhHmBdvX/vMEIMXM/k8SUpzsZ IRKCEj8m32OB6NWSWL/zOBOELS3x6O8MsL2cAtoSFy8eZASZKSqgLHF3WdAERqAjEbpnIeme haR7ASPzKkaJ1ILkguKk9FzDvNRyveLE3OLSvHS95PzcTYzgtPFMagfjwV3uhxgFOBiVeHgn PK8OF2JNLCuuzD3EKMHBrCTCG9NTEy7Em5JYWZValB9fVJqTWnyI0RQYHhOZpUST84EpLa8k 3tDYxMzI0sjc0MLI2FxJnPfx/3VhQgLpiSWp2ampBalFMH1MHJxSDYwH169ofrAp5G13f8DJ 5kDbMq8n3kdi7+dU28xePXtxVqNB5CLZuWIHT2Xcf3klZP7fc8tnH5xw5IXo7U8aGQbnf2rk vTS7ZeRscWHifP6rm78qqR6ua7C7+7BN7707U3HrFasdb9k29oa2SwkdN/fr3njs3aVL4nem 16QtMfsWYl/xNaSrJ1FBiaU4I9FQi7moOBEAJeh+HjEDAAA= DLP-Filter: Pass X-MTR: 20000000000000000@CPGS X-CFilter-Loop: Reflected Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Sylwester, > +#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 Yes, that's how it should be, indeed. > /* 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); I don't see anything wrong on the above. We could make it as: @@ -596,9 +597,7 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd) u32 val; /* Disable Clock */ - if (sdd->port_conf->clk_from_cmu) { - clk_disable_unprepare(sdd->src_clk); - } else { + if (!sdd->port_conf->clk_from_cmu) { val = readl(regs + S3C64XX_SPI_CLK_CFG); val &= ~S3C64XX_SPI_ENCLK_ENABLE; writel(val, regs + S3C64XX_SPI_CLK_CFG); @@ -640,13 +639,7 @@ static void s3c64xx_spi_config(struct s3c64xx_spi_driver_data *sdd) writel(val, regs + S3C64XX_SPI_MODE_CFG); - if (sdd->port_conf->clk_from_cmu) { - /* Configure Clock */ - /* 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); - } else { + if (!sdd->port_conf->clk_from_cmu) { /* Configure Clock */ val = readl(regs + S3C64XX_SPI_CLK_CFG); val &= ~S3C64XX_SPI_PSR_MASK; > 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. I think that if the clock comes from the cmu it's not necessary to disable it. I would like to avoid adding DTS properties because we have the clock disabling inherited from old code which it might not be required at all (in our tests, indeed it works). Thanks, Andi