All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Łukasz Majewski" <lukma@denx.de>
To: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: Timur Tabi <timur@tabi.org>, Xiubo Li <Xiubo.Lee@gmail.com>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Jaroslav Kysela <perex@perex.cz>,
	Takashi Iwai <tiwai@suse.com>,
	alsa-devel@alsa-project.org, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq
Date: Tue, 5 Sep 2017 10:19:05 +0200	[thread overview]
Message-ID: <deb40031-9db0-d667-7314-352cf548c295@denx.de> (raw)
In-Reply-To: <20170905075247.GA6112@Asurada>

On 09/05/2017 09:52 AM, Nicolin Chen wrote:
> On Tue, Sep 05, 2017 at 09:37:43AM +0200, Łukasz Majewski wrote:
> 
>>>> The last call is changing the bit clock (BCLK) frequency to SSI's IP
>>>> block clock (ipg = 66 MHz) [1].
>>>
>>> I think a bigger question here is why the routine sets BCLK to 66MHz.
>>
>> Yes, exactly.
>>
>> In my case the bclk is set to ipg clock, which is the SSI IP block clock
>> (ipg).
> 
> Can you elaborate why you set ipg clock as bclk? I don't remember SSI could
> derive bitclock from ipg clock.

Just to be clear:

What clock shall be set with:

struct snd_soc_dai_ops {

	int (*set_sysclk)(struct snd_soc_dai *dai,
		int clk_id, unsigned int freq, int dir);
}

callback?

The SSI IP block or BCLK ?


> 
>>>> This is wrong, since IMX SSI block requires the I2S BCLK to be less
>>>> than 1/5 of [1].
>>>>
>>>> As a result the driver initialization passes without any errors, but the
>>>> speaker-test test case breaks.
>>>>
>>>> This commit checks if the fsl_ssi_set_dai_sysclk() frequency passed is
>>>> not equal to [1].
>>>
>>> I don't feel it's quite comprehensive...what if it's being set to 67MHz.
>>
>> I think that this clock is not changing for the SoC. It should be 66 MHz
>> fixed.
> 
> What I mean is that we cannot just look at this SoC. Today is 66MHz for this
> SoC. Tomorrow could be 133MHz for another one. We should put a check that none
> of these shall pass -- the 1/5 limit.
> 

Ok. Good point.


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de

WARNING: multiple messages have this Message-ID (diff)
From: "Łukasz Majewski" <lukma@denx.de>
To: Nicolin Chen <nicoleotsuka@gmail.com>
Cc: alsa-devel@alsa-project.org, Timur Tabi <timur@tabi.org>,
	Xiubo Li <Xiubo.Lee@gmail.com>,
	linux-kernel@vger.kernel.org, Takashi Iwai <tiwai@suse.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Fabio Estevam <fabio.estevam@nxp.com>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq
Date: Tue, 5 Sep 2017 10:19:05 +0200	[thread overview]
Message-ID: <deb40031-9db0-d667-7314-352cf548c295@denx.de> (raw)
In-Reply-To: <20170905075247.GA6112@Asurada>

On 09/05/2017 09:52 AM, Nicolin Chen wrote:
> On Tue, Sep 05, 2017 at 09:37:43AM +0200, Łukasz Majewski wrote:
> 
>>>> The last call is changing the bit clock (BCLK) frequency to SSI's IP
>>>> block clock (ipg = 66 MHz) [1].
>>>
>>> I think a bigger question here is why the routine sets BCLK to 66MHz.
>>
>> Yes, exactly.
>>
>> In my case the bclk is set to ipg clock, which is the SSI IP block clock
>> (ipg).
> 
> Can you elaborate why you set ipg clock as bclk? I don't remember SSI could
> derive bitclock from ipg clock.

Just to be clear:

What clock shall be set with:

struct snd_soc_dai_ops {

	int (*set_sysclk)(struct snd_soc_dai *dai,
		int clk_id, unsigned int freq, int dir);
}

callback?

The SSI IP block or BCLK ?


> 
>>>> This is wrong, since IMX SSI block requires the I2S BCLK to be less
>>>> than 1/5 of [1].
>>>>
>>>> As a result the driver initialization passes without any errors, but the
>>>> speaker-test test case breaks.
>>>>
>>>> This commit checks if the fsl_ssi_set_dai_sysclk() frequency passed is
>>>> not equal to [1].
>>>
>>> I don't feel it's quite comprehensive...what if it's being set to 67MHz.
>>
>> I think that this clock is not changing for the SoC. It should be 66 MHz
>> fixed.
> 
> What I mean is that we cannot just look at this SoC. Today is 66MHz for this
> SoC. Tomorrow could be 133MHz for another one. We should put a check that none
> of these shall pass -- the 1/5 limit.
> 

Ok. Good point.


-- 
Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply	other threads:[~2017-09-05  8:19 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-03 11:05 [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq Lukasz Majewski
2017-09-03 12:44 ` Fabio Estevam
2017-09-03 12:44   ` Fabio Estevam
2017-09-03 14:40   ` Łukasz Majewski
2017-09-03 15:29     ` Fabio Estevam
2017-09-03 15:29       ` Fabio Estevam
2017-09-03 15:29       ` Fabio Estevam
2017-09-05  5:20     ` Nicolin Chen
2017-09-05  8:35       ` Łukasz Majewski
2017-09-05 18:11         ` Nicolin Chen
2017-09-05 21:13           ` Łukasz Majewski
2017-09-05 21:13             ` Łukasz Majewski
2017-09-05 22:52             ` Nicolin Chen
2017-09-05 22:52               ` Nicolin Chen
2017-09-06  9:22               ` Łukasz Majewski
2017-09-06  9:22                 ` Łukasz Majewski
2017-09-06 17:33                 ` Nicolin Chen
2017-09-06 17:33                   ` Nicolin Chen
2017-09-06 18:35                   ` Łukasz Majewski
2017-09-06 19:47                     ` Nicolin Chen
2017-09-06 19:47                       ` Nicolin Chen
2017-09-06 21:18                       ` Łukasz Majewski
2017-09-06 21:18                         ` Łukasz Majewski
2017-09-07 23:10                       ` Łukasz Majewski
2017-09-07 23:10                         ` Łukasz Majewski
2017-09-08  0:39                         ` Nicolin Chen
2017-09-08  0:39                           ` Nicolin Chen
2017-09-05 23:20             ` Fabio Estevam
2017-09-05 23:20               ` Fabio Estevam
2017-09-05 23:20               ` Fabio Estevam
2017-09-06  8:44               ` Łukasz Majewski
2017-09-05 20:14         ` Fabio Estevam
2017-09-05 20:14           ` Fabio Estevam
2017-09-05 21:14           ` Łukasz Majewski
2017-09-05  5:06 ` Nicolin Chen
2017-09-05  7:37   ` Łukasz Majewski
2017-09-05  7:52     ` Nicolin Chen
2017-09-05  8:19       ` Łukasz Majewski [this message]
2017-09-05  8:19         ` Łukasz Majewski
2017-09-05 15:15         ` Mark Brown
2017-09-05 15:15           ` Mark Brown
2017-09-05 17:45           ` Nicolin Chen
2017-09-07 13:44             ` Mark Brown
2017-09-07 13:44               ` Mark Brown
2017-09-07 23:03               ` Nicolin Chen

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=deb40031-9db0-d667-7314-352cf548c295@denx.de \
    --to=lukma@denx.de \
    --cc=Xiubo.Lee@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=fabio.estevam@nxp.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=nicoleotsuka@gmail.com \
    --cc=perex@perex.cz \
    --cc=timur@tabi.org \
    --cc=tiwai@suse.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.