From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752918AbcGGPUt (ORCPT ); Thu, 7 Jul 2016 11:20:49 -0400 Received: from mailout3.w1.samsung.com ([210.118.77.13]:26115 "EHLO mailout3.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751154AbcGGPUm (ORCPT ); Thu, 7 Jul 2016 11:20:42 -0400 X-AuditID: cbfec7f5-f792a6d000001302-fb-577e73461881 Subject: Re: [PATCH v4 1/2] clk: exynos5433: do not use CLK_IGNORE_UNUSED for SPI clocks To: Andi Shyti References: <1467893637-12573-1-git-send-email-andi.shyti@samsung.com> <1467893637-12573-2-git-send-email-andi.shyti@samsung.com> <577E4F1C.8070004@samsung.com> <20160707132738.GI23620@samsunx.samsung> Cc: Krzysztof Kozlowski , Chanwoo Choi , Jaehoon Chung , Tomasz Figa , Michael Turquette , Stephen Boyd , Kukjin Kim , linux-samsung-soc@vger.kernel.org, linux-clk@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Andi Shyti From: Sylwester Nawrocki Message-id: <577E7343.9090002@samsung.com> Date: Thu, 07 Jul 2016 17:20:35 +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: <20160707132738.GI23620@samsunx.samsung> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFmplkeLIzCtJLcpLzFFi42I5/e/4NV234rpwg8V75Sy2H3nGarH4x3Mm i+tfnrNa3PjVxmrx+oWhRf/j18wWmx5fY7X42HOP1eLyrjlsFjPO72OyuHjK1eLHmW4Wi1W7 /jA68Hq8v9HK7nG5r5fJ4/qST8weO2fdZffYtKqTzWPzknqPvi2rGD0+b5IL4IjisklJzcks Sy3St0vgyljx6TNrwRO+it+XvRsYb3N3MXJySAiYSCw9cpkdwhaTuHBvPVsXIxeHkMBSRonm nouMEM5zRom5G6aBVQkLRElse/aIBcQWEVCXWLJ3KzNE0UImiTPH1rKCOMwCR5glrv88CdbB JmAo0Xu0jxHE5hXQkth69xKYzSKgKvF8XysTiC0qECHxZO5JqBpBiR+T74Ft4BQwlTi2eDXQ UA6goXoS9y9qgYSZBeQlNq95yzyBUWAWko5ZCFWzkFQtYGRexSiaWppcUJyUnmukV5yYW1ya l66XnJ+7iRESOV93MC49ZnWIUYCDUYmHd0FObbgQa2JZcWXuIUYJDmYlEd7peXXhQrwpiZVV qUX58UWlOanFhxilOViUxHln7nofIiSQnliSmp2aWpBaBJNl4uCUamBcPCXCiOF10fNJIe0z zz/K+eHMw37r0fGI/bkN0oYTj/85uWXL5y6WtGcaao0eaxYa6RttmpHA1fPle2/5Yq3f/F/S gstChSZ/mqlsw5RfwbugorxUyifY6PXHxRPdT6eU/Hpa+FampvTl0adJ58XF3fgbORY/NI9t bpZ1s/8U33PU9d0rexMlluKMREMt5qLiRAAVtzqOmAIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/07/2016 03:27 PM, Andi Shyti wrote: >>> > > The CLK_IGNORE_UNUSED flag has to be avoided whenever possible. >>> > > Use the CLK_IS_CRITICAL flag instead for critical SPI1 clocks, >>> > > which enables the clock line during boot time. >> > >> > I don't agree. Both flags should be avoided. Clk is critical does not >> > solve the problem. It is just a better workaround for lack of proper >> > clock consumers. >> > >> > The IOCLK is not a critical clock. It can be disabled (e.g. when SoC >> > is used on a board without any SPI device connected). > > As we discussed offline there is no driver which is requesting > this clock. We cannot ask to the spi driver to handle three > clocks because the exynos5433 has its own peculiarities. > > If we want this on the spi driver's side, we need to add a new > compatible and check it everytime. To me it looks uglier than > just keep it alive. I took a closer look at what those IOCLK clocks exactly are and unfortunately I must agree with Krzysztof. These clock gates are closely related to the IP blocks and to me proper approach is to have them listed in DT and controlled by the SPI bus driver. I checked and there is similar pattern for other IPs like I2S and other SoCs, e.g. exynos7420. Additionally parents of those IOCLK_SPI?_CLK clocks are currently wrongly modelled as fixed rate clocks. These clocks really don't have a parent until some clock is fed externally to the SoC's I/O pin. But this issue could be addressed later. I think it is not a big deal to add "samsung-exynos5433-spi" compatible to the SPI driver along with a new variant data and a flag like "has_cmu_ioclk" to indicate whether a third clock should be handled or not. Presumably for now the ioclk clock can just simply be enabled in probe(), this way it will be enabled only for SPI controllers actually in use.