From mboxrd@z Thu Jan 1 00:00:00 1970 From: Martin Fuzzey Subject: Re: [PATCH] Sound: sgtl5000 Allow codec clock frequency to be set. Date: Thu, 21 Mar 2013 18:46:17 +0100 Message-ID: <514B4769.30109@parkeon.com> References: <20130319170322.27828.7691.stgit@localhost> <514A63EE.7000707@tabi.org> <514AC738.6050900@parkeon.com> Reply-To: mfuzzey@parkeon.com Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Timur Tabi Cc: alsa-devel@alsa-project.org, linux-arm-kernel@lists.infradead.org List-Id: alsa-devel@alsa-project.org On 21/03/13 17:11, Timur Tabi wrote: > On Thu, Mar 21, 2013 at 3:39 AM, Martin Fuzzey wrote: > >> With this patch and that DT example the frequency of clock 162 will be _set_ >> to 20MHz > Does that mean that it ignores the ' clocks' parameter? No, in this case the clocks parameter must contain the phandle of a clock whose rate is configurable. > If clock-frequency is omitted the binding is still correct (hence the > optional) but the frequency of clock 162 would not be modified. > > In the documentation I wrote "If both a clock and clock-frequency are > provided the clock's rate will be set. " maybe this is not clear enough? > It's not clear what it will be set *to*. It will be set to the frequency specified by the clock-frquency attribute. Would "If both a clock and clock-frequency are provided clock must support the set_rate operation and its frequency will be set to the value specified by clock-frequency" be better? The possible configurations and their use cases are: 1) only 'clocks' specified clock points to a clock specified in the DT which already has an appropriate frequency and is configured by some other means external to the sgtl5000 driver (bootloader, board setup code, or just a fixed rate clock) 2) only 'clock-frequency' specified The chip is assumed to be clocked by a signal having the given frequency, which may even be generated by a clock unknown to linux. This could actually be represented as a special case of 1) by defining a fixed-rate clock in the DT. 3) Both 'clocks' and 'clock-frequency' specified The chip is assumed to be clocked by a rate programmable clock defined in the clock tree. clk_set_rate() will be called for this clock to set its rate to that specified by clock-frequency Cases 1 and 2 exist in the current code, this patch adds support for case 3. Prior to commit 81e8e4926167ab32593bbb915b45a42024ca1020 "ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000" only case 2 was supported This explains why 2 is not implemented as a special case of 1) plain 'clock-frequency' was supported before the driver learnt how to get a clock from the DT Regards, Martin From mboxrd@z Thu Jan 1 00:00:00 1970 From: mfuzzey@parkeon.com (Martin Fuzzey) Date: Thu, 21 Mar 2013 18:46:17 +0100 Subject: [PATCH] Sound: sgtl5000 Allow codec clock frequency to be set. In-Reply-To: References: <20130319170322.27828.7691.stgit@localhost> <514A63EE.7000707@tabi.org> <514AC738.6050900@parkeon.com> Message-ID: <514B4769.30109@parkeon.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 21/03/13 17:11, Timur Tabi wrote: > On Thu, Mar 21, 2013 at 3:39 AM, Martin Fuzzey wrote: > >> With this patch and that DT example the frequency of clock 162 will be _set_ >> to 20MHz > Does that mean that it ignores the ' clocks' parameter? No, in this case the clocks parameter must contain the phandle of a clock whose rate is configurable. > If clock-frequency is omitted the binding is still correct (hence the > optional) but the frequency of clock 162 would not be modified. > > In the documentation I wrote "If both a clock and clock-frequency are > provided the clock's rate will be set. " maybe this is not clear enough? > It's not clear what it will be set *to*. It will be set to the frequency specified by the clock-frquency attribute. Would "If both a clock and clock-frequency are provided clock must support the set_rate operation and its frequency will be set to the value specified by clock-frequency" be better? The possible configurations and their use cases are: 1) only 'clocks' specified clock points to a clock specified in the DT which already has an appropriate frequency and is configured by some other means external to the sgtl5000 driver (bootloader, board setup code, or just a fixed rate clock) 2) only 'clock-frequency' specified The chip is assumed to be clocked by a signal having the given frequency, which may even be generated by a clock unknown to linux. This could actually be represented as a special case of 1) by defining a fixed-rate clock in the DT. 3) Both 'clocks' and 'clock-frequency' specified The chip is assumed to be clocked by a rate programmable clock defined in the clock tree. clk_set_rate() will be called for this clock to set its rate to that specified by clock-frequency Cases 1 and 2 exist in the current code, this patch adds support for case 3. Prior to commit 81e8e4926167ab32593bbb915b45a42024ca1020 "ASoC: fsl: add sgtl5000 clock support for imx-sgtl5000" only case 2 was supported This explains why 2 is not implemented as a special case of 1) plain 'clock-frequency' was supported before the driver learnt how to get a clock from the DT Regards, Martin