From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755854AbcHVRXQ (ORCPT ); Mon, 22 Aug 2016 13:23:16 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:23613 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753805AbcHVRXP (ORCPT ); Mon, 22 Aug 2016 13:23:15 -0400 X-AuditID: cbfec7f5-f792a6d000001302-4b-57bb34ffe895 Subject: Re: [alsa-devel] [RFC PATCH] mfd: arizona: Add gating of external MCLKn clocks To: Charles Keepax , broonie@kernel.org References: <1471627036-19465-1-git-send-email-s.nawrocki@samsung.com> <20160822092233.GN21682@localhost.localdomain> Cc: alsa-devel@alsa-project.org, lee.jones@linaro.org, linux-kernel@vger.kernel.org From: Sylwester Nawrocki Message-id: Date: Mon, 22 Aug 2016 19:22:47 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-version: 1.0 In-reply-to: <20160822092233.GN21682@localhost.localdomain> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFjrDLMWRmVeSWpSXmKPExsVy+t/xK7r/TXaHG5z5qWpx5eIhJoupD5+w WfybcoPd4v7Xo4wWl3fNYXNg9djwuYnNY9OqTjaPO9f2sHm8nPibzePzJrkA1igum5TUnMyy 1CJ9uwSujOUN71kL1ohVbLth1cB4WrCLkZNDQsBE4kXvUWYIW0ziwr31bCC2kMBSRokZHf5d jFxA9nNGicZ1R1lBEsICkRKLl34CKxIR8JFoPjCDsYuRA6ioXOLq/WQQk1kgVGL2cn2QCjYB Q4neo32MIDavgJ3E63t9TCA2i4CqxPutTewgtqhAhMStVR+hagQlfky+xwJicwpYS1yeBrIV ZKSexP2LWiBhZgF5ic1r3jJPYBSYhaRjFkLVLCRVCxiZVzGKppYmFxQnpeca6RUn5haX5qXr JefnbmKEhPHXHYxLj1kdYhTgYFTi4d3BvjtciDWxrLgy9xCjBAezkgivrC5QiDclsbIqtSg/ vqg0J7X4EKM0B4uSOO/MXe9DhATSE0tSs1NTC1KLYLJMHJxSDYxz7J9s8LA67fVA8seXC3c3 Pvjtp5r94Pi18ONHdhz/N+F28vTGiPy4Ve4Lf2y4xqD8Q+jiOpPH5fMrAlpmzXixZ/e19vCd jBEZxqebmIzchIx/FKzv55t3z3CR595bv2J1/ApFgln370hpys254VpseqvVQl6f+6o2lz1L bJTefGHVqvUH/jxWYinOSDTUYi4qTgQAyo5T+F8CAAA= Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/22/2016 11:22 AM, Charles Keepax wrote: > Yeah I am not sure this is quite the correct approach, there are > quite a few corner cases that would not be covered well here. For > example an internally divided down 32k in which case both the 32k > and MCLK would come from the same pin, or using the 32k to feed > an FLL in which case we are trying to enable MCLK1 unnecessarily. > > I think we could request the 32k clock source from this part > of the code, but without implementing clock drivers for the > chips internal clocking I think the main MCLK would need to be > requested from the machine driver for now. > > On that note, I have been working on a patch chain that adds an > actual clock driver for the chip unfortunately this has been > delayed somewhat due to issues interfacing SPI backed clocks to > the clock framework. Krzysztof Kozlowski has sent a series that > appears to resolve these issues for me so hopefully once the > clock guys have had a look at that I can send my clock driver. > My current implementation mostly just implements the 32k clock > but we can start building that out to include the SYSCLKs and > FLLs etc. fairly quickly. The best thing might be to wait for > that and then build additional features onto that. > > Let me know if you want to get an early look at that code. Thanks for the feedback. I would really like to avoid touching this code and messing up anything in code shared by multiple CODECs. You certainly know it much better than than I do. I got suggestion from Mark not to request the main MCLK clock in the machine driver. But even if gating of that clock was added to the CODEC driver I would need to get hold of it in the machine driver to get rate of MCLK. So I thought about exporting a helper from the MFD for requesting MCLK clock or specifying MCLK clock back in the sound DT node. IIUC, you are suggesting to leave the 32k parts of the patch and just drop the MCLK part? If we provided an interface like: struct clk * arizona_get_mclk(struct arizona *arizona, int id); void arizona_put_mclk(struct clk *clk); the machine driver would need to get hold of struct arizona*, which is not that straightforward if we are given on CODEC of_node (it can be a child of I2S or SPI bus). Then how about specifying MCLK in the sound node and using regular devm_clk_get()? Regarding the clock locking patches, I think it needs some more discussion and should rather be merged early in the development cycle to have good exposure in -next as it's quite an invasive change. I'd be happy to look at your code, if only to have a better overview and to avoid interfering with you work. Anyway, my main goal is only to get the tm2_wm5110 sound card upstream, with as little casualties as possible;) -- Thanks, Sylwester