linux-kernel - Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal to system freq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170905052031.GB2774@Asurada-CZ80>
Date:   Mon, 4 Sep 2017 22:20:32 -0700
From:   Nicolin Chen <nicoleotsuka@...il.com>
To:     Łukasz Majewski <lukma@...x.de>
Cc:     Fabio Estevam <fabio.estevam@....com>, Timur Tabi <timur@...i.org>,
        Xiubo Li <Xiubo.Lee@...il.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        "festevam@...il.com" <festevam@...il.com>,
        "alsa-devel@...a-project.org" <alsa-devel@...a-project.org>,
        "linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sound: soc: fsl: Do not set DAI sysclk when it is equal
 to system freq

On Sun, Sep 03, 2017 at 04:40:21PM +0200, Łukasz Majewski wrote:
> >/*
> >* Hardware limitation: The bclk rate must be
> >* never greater than 1/5 IPG clock rate
> >*/
> >if (freq * 5 > clk_get_rate(ssi_private->clk)) {
> >dev_err(cpu_dai->dev, "bitclk > ipgclk/5\n");
> >return -EINVAL;
> >}
> >
> 
> Unfortunately not.
> 
> This is the part of fsl_ssi_set_bclk() function which is called after
> fsl_ssi_set_dai_sysclk() (which sets ssi_private->bitclk_freq = freq;).
> 
> Before the aforementioned check we do have:
> 
> 	if (ssi_private->bitclk_freq)
> 		freq = ssi_private->bitclk_freq;
> 	else
> 		freq = params_channels(hw_params) * 32 * 	params_rate(hw_params);
> 
> 
> Which assigns freq = bitclk_freq (66 MHz)
> 
[...]
> And then we break on this particular check:
> 66MHz * 5 > 66 MHz.
[...]

Does the check fail and return an error here, or not?

> The culprit IMHO is the  ssi_private->bitclk_freq = freq; in the
> fsl_ssi_set_dai_sysclk(), since we _should_ set SSI's IP block clock
> (ssi_private->clk), not the bit clock (BCLK).

No. We should not set the IP block clock. That's from IPG bus
on certain IMX SoCs. Setting it might change IPG bus clocks
which might break the system.

And apparently, we shouldn't set bitclk to 66MHz either. Can
you help to find where this 66MHz comes from?

> This patch just quits early if it detects change, which don't need to be
> done.

I kinda see your point is to error out before hw_params(). So
I feel it would be better to have a similar 5-times-check in
the set_sysclk() too, if it's really necessary.

Btw, I don't see simple card calling set_sysclk() function in
any earlier stage than hw_params(). I am wondering how did you
encounter the problem?

Thanks
Nicolin