From: Lars-Peter Clausen <lars@metafoo.de>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Mark Brown <broonie@kernel.org>,
Manuel Lauss <manuel.lauss@gmail.com>,
alsa-devel <alsa-devel@alsa-project.org>,
Richard Purdie <richard@openedhand.com>,
patches@opensource.wolfsonmicro.com,
LKML <linux-kernel@vger.kernel.org>,
linux-sound@vger.kernel.org,
Manuel Lauss <manuel.lauss@googlemail.com>,
Bo Shen <voice.shen@atmel.com>,
linux-arm-kernel@lists.infradead.org,
Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [alsa-devel] [RFC PATCH] ASoC: wm8731: let codec to manage clock by itself
Date: Tue, 03 Feb 2015 18:26:04 +0100 [thread overview]
Message-ID: <54D104AC.5010603@metafoo.de> (raw)
In-Reply-To: <20150203171733.GY8656@n2100.arm.linux.org.uk>
On 02/03/2015 06:17 PM, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 05:53:48PM +0100, Lars-Peter Clausen wrote:
>> On 02/03/2015 01:44 PM, Mark Brown wrote:
>>> On Tue, Feb 03, 2015 at 08:54:57AM +0100, Manuel Lauss wrote:
>>>
>>>> + wm8731->mclk = devm_clk_get(&spi->dev, "mclk");
>>>> + if (IS_ERR(wm8731->mclk)) {
>>>> + wm8731->mclk = NULL;
>>>> + dev_warn(&spi->dev, "assuming static MCLK\n");
>>>> + }
>>>
>>> This is broken for both deferred probe and in the case where the clock
>>> API genuinely returns a NULL clock. Other than that it's the kind of
>>> thing that we've done for some other drivers, though it's not good to
>>> have to do this. Check them for correct behaviour.
>>
>> Ideally we'd introduce a {devm_}clk_get_optional(), with the same semantics
>> as gpiod_get_optional(), which handles the finer details of differentiating
>> between clock specified, but not yet probed, clock specified, but
>> incorrectly and no clock specified, so this doesn't have to be done over and
>> over by each driver.
>
> No, we don't need to. It clk_get() already knows this distinction, and
> it appropriately returns -ENOENT vs -EPROBE_DEFER according to whether
> there's a clock specified in DT or not.
I know, but it returns a error when no clock is specified (-ENOENT), whereas
gpiod_get_optional()-like semantics mean, it would return no error.
WARNING: multiple messages have this Message-ID (diff)
From: Lars-Peter Clausen <lars@metafoo.de>
To: Russell King - ARM Linux <linux@arm.linux.org.uk>
Cc: Mark Brown <broonie@kernel.org>,
Manuel Lauss <manuel.lauss@gmail.com>,
alsa-devel <alsa-devel@alsa-project.org>,
Richard Purdie <richard@openedhand.com>,
patches@opensource.wolfsonmicro.com,
LKML <linux-kernel@vger.kernel.org>,
linux-sound@vger.kernel.org,
Manuel Lauss <manuel.lauss@googlemail.com>,
Bo Shen <voice.shen@atmel.com>,
linux-arm-kernel@lists.infradead.org,
Liam Girdwood <lrg@slimlogic.co.uk>
Subject: Re: [alsa-devel] [RFC PATCH] ASoC: wm8731: let codec to manage clock by itself
Date: Tue, 03 Feb 2015 17:26:04 +0000 [thread overview]
Message-ID: <54D104AC.5010603@metafoo.de> (raw)
In-Reply-To: <20150203171733.GY8656@n2100.arm.linux.org.uk>
On 02/03/2015 06:17 PM, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 05:53:48PM +0100, Lars-Peter Clausen wrote:
>> On 02/03/2015 01:44 PM, Mark Brown wrote:
>>> On Tue, Feb 03, 2015 at 08:54:57AM +0100, Manuel Lauss wrote:
>>>
>>>> + wm8731->mclk = devm_clk_get(&spi->dev, "mclk");
>>>> + if (IS_ERR(wm8731->mclk)) {
>>>> + wm8731->mclk = NULL;
>>>> + dev_warn(&spi->dev, "assuming static MCLK\n");
>>>> + }
>>>
>>> This is broken for both deferred probe and in the case where the clock
>>> API genuinely returns a NULL clock. Other than that it's the kind of
>>> thing that we've done for some other drivers, though it's not good to
>>> have to do this. Check them for correct behaviour.
>>
>> Ideally we'd introduce a {devm_}clk_get_optional(), with the same semantics
>> as gpiod_get_optional(), which handles the finer details of differentiating
>> between clock specified, but not yet probed, clock specified, but
>> incorrectly and no clock specified, so this doesn't have to be done over and
>> over by each driver.
>
> No, we don't need to. It clk_get() already knows this distinction, and
> it appropriately returns -ENOENT vs -EPROBE_DEFER according to whether
> there's a clock specified in DT or not.
I know, but it returns a error when no clock is specified (-ENOENT), whereas
gpiod_get_optional()-like semantics mean, it would return no error.
WARNING: multiple messages have this Message-ID (diff)
From: lars@metafoo.de (Lars-Peter Clausen)
To: linux-arm-kernel@lists.infradead.org
Subject: [alsa-devel] [RFC PATCH] ASoC: wm8731: let codec to manage clock by itself
Date: Tue, 03 Feb 2015 18:26:04 +0100 [thread overview]
Message-ID: <54D104AC.5010603@metafoo.de> (raw)
In-Reply-To: <20150203171733.GY8656@n2100.arm.linux.org.uk>
On 02/03/2015 06:17 PM, Russell King - ARM Linux wrote:
> On Tue, Feb 03, 2015 at 05:53:48PM +0100, Lars-Peter Clausen wrote:
>> On 02/03/2015 01:44 PM, Mark Brown wrote:
>>> On Tue, Feb 03, 2015 at 08:54:57AM +0100, Manuel Lauss wrote:
>>>
>>>> + wm8731->mclk = devm_clk_get(&spi->dev, "mclk");
>>>> + if (IS_ERR(wm8731->mclk)) {
>>>> + wm8731->mclk = NULL;
>>>> + dev_warn(&spi->dev, "assuming static MCLK\n");
>>>> + }
>>>
>>> This is broken for both deferred probe and in the case where the clock
>>> API genuinely returns a NULL clock. Other than that it's the kind of
>>> thing that we've done for some other drivers, though it's not good to
>>> have to do this. Check them for correct behaviour.
>>
>> Ideally we'd introduce a {devm_}clk_get_optional(), with the same semantics
>> as gpiod_get_optional(), which handles the finer details of differentiating
>> between clock specified, but not yet probed, clock specified, but
>> incorrectly and no clock specified, so this doesn't have to be done over and
>> over by each driver.
>
> No, we don't need to. It clk_get() already knows this distinction, and
> it appropriately returns -ENOENT vs -EPROBE_DEFER according to whether
> there's a clock specified in DT or not.
I know, but it returns a error when no clock is specified (-ENOENT), whereas
gpiod_get_optional()-like semantics mean, it would return no error.
next prev parent reply other threads:[~2015-02-03 17:26 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-02-03 3:33 [RFC PATCH] ASoC: wm8731: let codec to manage clock by itself Bo Shen
2015-02-03 3:33 ` Bo Shen
2015-02-03 3:33 ` Bo Shen
2015-02-03 3:33 ` Bo Shen
2015-02-03 7:54 ` Manuel Lauss
2015-02-03 7:54 ` Manuel Lauss
2015-02-03 7:54 ` Manuel Lauss
2015-02-03 12:44 ` Mark Brown
2015-02-03 12:44 ` Mark Brown
2015-02-03 12:44 ` Mark Brown
2015-02-03 12:44 ` Mark Brown
2015-02-03 14:40 ` Manuel Lauss
2015-02-03 14:40 ` Manuel Lauss
2015-02-03 14:40 ` Manuel Lauss
2015-02-03 14:40 ` Manuel Lauss
2015-02-03 16:21 ` Mark Brown
2015-02-03 16:21 ` Mark Brown
2015-02-03 16:21 ` Mark Brown
2015-02-03 16:21 ` Mark Brown
2015-02-04 3:45 ` Bo Shen
2015-02-04 3:45 ` Bo Shen
2015-02-04 3:45 ` Bo Shen
2015-02-04 3:45 ` Bo Shen
2015-02-04 11:13 ` Mark Brown
2015-02-04 11:13 ` Mark Brown
2015-02-04 11:13 ` Mark Brown
2015-02-03 16:53 ` [alsa-devel] " Lars-Peter Clausen
2015-02-03 16:53 ` Lars-Peter Clausen
2015-02-03 16:53 ` Lars-Peter Clausen
2015-02-03 17:17 ` Russell King - ARM Linux
2015-02-03 17:17 ` Russell King - ARM Linux
2015-02-03 17:17 ` Russell King - ARM Linux
2015-02-03 17:26 ` Lars-Peter Clausen [this message]
2015-02-03 17:26 ` Lars-Peter Clausen
2015-02-03 17:26 ` Lars-Peter Clausen
2015-02-03 17:49 ` Lars-Peter Clausen
2015-02-03 17:49 ` Lars-Peter Clausen
2015-02-03 17:49 ` Lars-Peter Clausen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54D104AC.5010603@metafoo.de \
--to=lars@metafoo.de \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=lrg@slimlogic.co.uk \
--cc=manuel.lauss@gmail.com \
--cc=manuel.lauss@googlemail.com \
--cc=patches@opensource.wolfsonmicro.com \
--cc=richard@openedhand.com \
--cc=voice.shen@atmel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.