We need to set minimum bclk 64x of wclk as this is hw constraint in one
of the component used.
Since, clk_set_rate and clk is enabled in machine driver the
clk_set_rate in hw_params of da7219 fails and errors out and when it
tries to override the value.
In cases like these not only clk_set_rate of da7219 codec should fail
and not override the value but also should not error out.
Signed-off-by: Akshu Agrawal <[email protected]>
---
sound/soc/codecs/da7219.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
index 5f5fa3416af3..a041dbf442af 100644
--- a/sound/soc/codecs/da7219.c
+++ b/sound/soc/codecs/da7219.c
@@ -1621,13 +1621,7 @@ static int da7219_hw_params(struct snd_pcm_substream *substream,
if (bclk) {
bclk_rate = frame_size * sr;
- ret = clk_set_rate(bclk, bclk_rate);
- if (ret) {
- dev_err(component->dev,
- "Failed to set BCLK rate %lu: %d\n",
- bclk_rate, ret);
- return ret;
- }
+ clk_set_rate(bclk, bclk_rate);
} else {
ret = da7219_set_bclks_per_wclk(component, frame_size);
if (ret) {
--
2.17.1
Hi Akshu,
On Mon, Apr 22, 2019 at 2:15 PM Agrawal, Akshu <[email protected]> wrote:
>
> We need to set minimum bclk 64x of wclk as this is hw constraint in one
> of the component used.
> Since, clk_set_rate and clk is enabled in machine driver the
> clk_set_rate in hw_params of da7219 fails and errors out and when it
> tries to override the value.
>
> In cases like these not only clk_set_rate of da7219 codec should fail
> and not override the value but also should not error out.
>
> Signed-off-by: Akshu Agrawal <[email protected]>
> ---
> sound/soc/codecs/da7219.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
> index 5f5fa3416af3..a041dbf442af 100644
> --- a/sound/soc/codecs/da7219.c
> +++ b/sound/soc/codecs/da7219.c
> @@ -1621,13 +1621,7 @@ static int da7219_hw_params(struct snd_pcm_substream *substream,
>
> if (bclk) {
> bclk_rate = frame_size * sr;
> - ret = clk_set_rate(bclk, bclk_rate);
> - if (ret) {
> - dev_err(component->dev,
> - "Failed to set BCLK rate %lu: %d\n",
> - bclk_rate, ret);
> - return ret;
> - }
> + clk_set_rate(bclk, bclk_rate);
I think this might mask the real issue when setting rate fails.
A more fundamental problem might be what are the acceptable ratios
between bclk and wclk.
From existing code of da7219 I see there are only two choices of
frame_size: 32 or 64.
In da7219_hw_params:
...
if (da7219->master && !da7219->tdm_en) {
if ((word_len * DA7219_DAI_CH_NUM_MAX) <= 32)
frame_size = 32;
else
frame_size = 64;
And in https://lkml.org/lkml/2019/4/17/322
the choice from machine driver is 64 or higher.
In cz_da7219_params:
...
if (bclk < (wclk * 64))
bclk = wclk * 64;
return da7219_clk_enable(substream, wclk, bclk);
I don't know what is the best solution to avoid this conflict.
Maybe just set a fixed ratio from device property that can best work
for the combination of codecs on the machine?
> } else {
> ret = da7219_set_bclks_per_wclk(component, frame_size);
> if (ret) {
> --
> 2.17.1
>
> _______________________________________________
> Alsa-devel mailing list
> [email protected]
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On 4/22/2019 1:09 PM, Cheng-yi Chiang wrote:
> Hi Akshu,
>
>
> On Mon, Apr 22, 2019 at 2:15 PM Agrawal, Akshu <[email protected]> wrote:
>>
>> We need to set minimum bclk 64x of wclk as this is hw constraint in one
>> of the component used.
>> Since, clk_set_rate and clk is enabled in machine driver the
>> clk_set_rate in hw_params of da7219 fails and errors out and when it
>> tries to override the value.
>>
>> In cases like these not only clk_set_rate of da7219 codec should fail
>> and not override the value but also should not error out.
>>
>> Signed-off-by: Akshu Agrawal <[email protected]>
>> ---
>> sound/soc/codecs/da7219.c | 8 +-------
>> 1 file changed, 1 insertion(+), 7 deletions(-)
>>
>> diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
>> index 5f5fa3416af3..a041dbf442af 100644
>> --- a/sound/soc/codecs/da7219.c
>> +++ b/sound/soc/codecs/da7219.c
>> @@ -1621,13 +1621,7 @@ static int da7219_hw_params(struct snd_pcm_substream *substream,
>>
>> if (bclk) {
>> bclk_rate = frame_size * sr;
>> - ret = clk_set_rate(bclk, bclk_rate);
>> - if (ret) {
>> - dev_err(component->dev,
>> - "Failed to set BCLK rate %lu: %d\n",
>> - bclk_rate, ret);
>> - return ret;
>> - }
>> + clk_set_rate(bclk, bclk_rate);
>
> I think this might mask the real issue when setting rate fails.
Not doing an error check for clk_set_rate is not rare because
clk_set_rate fails if the clk which you are trying to set is already
enabled. Now we can add a is_prepared check before calling clk_set_rate
but that would be redundant as clk_set_rate does the same check.
> A more fundamental problem might be what are the acceptable ratios
> between bclk and wclk.
>
acceptable ratios for da7219 from spec are:
00 = 32 BCLKS per WCLK
01 = 64 BCLKS per WCLK
10 = 128 BCLK per WCLK
11 = 256 BCLK per WCLK
da7219 derives this ration from SR for a wclk. The same we do in machine
driver.
Conflict comes only in case of an exception where there is constraint
from a hw component. Else when the audio is played on da7219 codec we
could have let da7219 only set rate and enable clks.
Thanks,
Akshu
> From existing code of da7219 I see there are only two choices of
> frame_size: 32 or 64.
>
> In da7219_hw_params:
> ...
> if (da7219->master && !da7219->tdm_en) {
> if ((word_len * DA7219_DAI_CH_NUM_MAX) <= 32)
> frame_size = 32;
> else
> frame_size = 64;
>
> And in https://lkml.org/lkml/2019/4/17/322
> the choice from machine driver is 64 or higher.
>
> In cz_da7219_params:
> ...
> if (bclk < (wclk * 64))
> bclk = wclk * 64;
> return da7219_clk_enable(substream, wclk, bclk);
>
> I don't know what is the best solution to avoid this conflict.
> Maybe just set a fixed ratio from device property that can best work
> for the combination of codecs on the machine?
>
>
>
>
>
>> } else {
>> ret = da7219_set_bclks_per_wclk(component, frame_size);
>> if (ret) {
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Alsa-devel mailing list
>> [email protected]
>> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel