Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
On Fri, Jun 13, 2014 at 11:00:01AM +0200, Takashi Iwai wrote: > At Thu, 12 Jun 2014 22:24:55 -0500, > Timur Tabi wrote: > > > > On Thu, Dec 12, 2013 at 4:44 AM, Nicolin Chen > > wrote: > > > > > > +static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > > > + int clk_id, unsigned int freq, int dir) > > > +{ > > > + struct fsl_ssi_private *ssi_private = > > > snd_soc_dai_get_drvdata(cpu_dai); > > > + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; > > > + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; > > > + u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; > > > + unsigned long flags, clkrate, baudrate, tmprate; > > > + u64 sub, savesub = 10; > > > + > > > + /* Don't apply it to any non-baudclk circumstance */ > > > + if (IS_ERR(ssi_private->baudclk)) > > > + return -EINVAL; > > > + > > > + /* It should be already enough to divide clock by setting pm > > > alone */ > > > + psr = 0; > > > + div2 = 0; > > > + > > > + factor = (div2 + 1) * (7 * psr + 1) * 2; > > > + > > > + for (i = 0; i < 255; i++) { > > > + /* The bclk rate must be smaller than 1/5 sysclk rate */ > > > + if (factor * (i + 1) < 5) > > > + continue; > > > + > > > + tmprate = freq * factor * (i + 2); > > > + clkrate = clk_round_rate(ssi_private->baudclk, tmprate); > > > + > > > + do_div(clkrate, factor); > > > > This do_div() call causes this warning on PowerPC: > > > > CC sound/soc/fsl/fsl_ssi.o > > sound/soc/fsl/fsl_ssi.c: In function 'fsl_ssi_set_bclk': > > sound/soc/fsl/fsl_ssi.c:593:3: warning: comparison of distinct pointer > > types lacks a cast > > sound/soc/fsl/fsl_ssi.c:593:3: warning: right shift count >= width of type > > sound/soc/fsl/fsl_ssi.c:593:3: warning: passing argument 1 of > > '__div64_32' from incompatible pointer type > > include/asm-generic/div64.h:35:17: note: expected 'uint64_t *' but > > argument is of type 'long unsigned int *' > > > > The comments in do_div() say that clkrate should be 64-bits. Changing > > clkrate to a u64 does fix this problem, but I'm wondering if anyone > > else has seen this. This code has been around for months. > > Using do_div() for unsigned long makes no sense. > And, you *must* pass uint64_t there as an argument, since it's no > inline function and there is no implicit conversion in the generic > code. > > In short, use the normal division operator instead: > clkrate /= factor; Yes. I forgot why I used do_div() for clkrate at the first place but it's definitely a mistake here. Sorry, Nicolin > > > Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
At Thu, 12 Jun 2014 22:24:55 -0500, Timur Tabi wrote: > > On Thu, Dec 12, 2013 at 4:44 AM, Nicolin Chen > wrote: > > > > +static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > > + int clk_id, unsigned int freq, int dir) > > +{ > > + struct fsl_ssi_private *ssi_private = > > snd_soc_dai_get_drvdata(cpu_dai); > > + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; > > + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; > > + u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; > > + unsigned long flags, clkrate, baudrate, tmprate; > > + u64 sub, savesub = 10; > > + > > + /* Don't apply it to any non-baudclk circumstance */ > > + if (IS_ERR(ssi_private->baudclk)) > > + return -EINVAL; > > + > > + /* It should be already enough to divide clock by setting pm alone > > */ > > + psr = 0; > > + div2 = 0; > > + > > + factor = (div2 + 1) * (7 * psr + 1) * 2; > > + > > + for (i = 0; i < 255; i++) { > > + /* The bclk rate must be smaller than 1/5 sysclk rate */ > > + if (factor * (i + 1) < 5) > > + continue; > > + > > + tmprate = freq * factor * (i + 2); > > + clkrate = clk_round_rate(ssi_private->baudclk, tmprate); > > + > > + do_div(clkrate, factor); > > This do_div() call causes this warning on PowerPC: > > CC sound/soc/fsl/fsl_ssi.o > sound/soc/fsl/fsl_ssi.c: In function 'fsl_ssi_set_bclk': > sound/soc/fsl/fsl_ssi.c:593:3: warning: comparison of distinct pointer > types lacks a cast > sound/soc/fsl/fsl_ssi.c:593:3: warning: right shift count >= width of type > sound/soc/fsl/fsl_ssi.c:593:3: warning: passing argument 1 of > '__div64_32' from incompatible pointer type > include/asm-generic/div64.h:35:17: note: expected 'uint64_t *' but > argument is of type 'long unsigned int *' > > The comments in do_div() say that clkrate should be 64-bits. Changing > clkrate to a u64 does fix this problem, but I'm wondering if anyone > else has seen this. This code has been around for months. Using do_div() for unsigned long makes no sense. And, you *must* pass uint64_t there as an argument, since it's no inline function and there is no implicit conversion in the generic code. In short, use the normal division operator instead: clkrate /= factor; Takashi ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
On Thu, Jun 12, 2014 at 10:44:48PM -0500, Timur Tabi wrote: > On Thu, Jun 12, 2014 at 10:21 PM, Nicolin Chen > wrote: > > > It compiles well with my ARM cross compiler. I guess it might be related > > to the compiler's version? But we should fix it anyway. Would you mind if > > I submit a patch? Or you do it directly? > > I just submitted a patch, but I can't test the code since I don't have > any boards that configure the SSI into master mode, so this function > is never called. I just tested it with i.MX platform. It still works. Thank you, Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
On Thu, Jun 12, 2014 at 10:21 PM, Nicolin Chen wrote: > It compiles well with my ARM cross compiler. I guess it might be related > to the compiler's version? But we should fix it anyway. Would you mind if > I submit a patch? Or you do it directly? I just submitted a patch, but I can't test the code since I don't have any boards that configure the SSI into master mode, so this function is never called. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
Sir, On Thu, Jun 12, 2014 at 10:24:55PM -0500, Timur Tabi wrote: > On Thu, Dec 12, 2013 at 4:44 AM, Nicolin Chen > wrote: > > > > +static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > > + int clk_id, unsigned int freq, int dir) > > +{ > > + struct fsl_ssi_private *ssi_private = > > snd_soc_dai_get_drvdata(cpu_dai); > > + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; > > + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; > > + u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; > > + unsigned long flags, clkrate, baudrate, tmprate; > > + u64 sub, savesub = 10; > > + > > + /* Don't apply it to any non-baudclk circumstance */ > > + if (IS_ERR(ssi_private->baudclk)) > > + return -EINVAL; > > + > > + /* It should be already enough to divide clock by setting pm alone > > */ > > + psr = 0; > > + div2 = 0; > > + > > + factor = (div2 + 1) * (7 * psr + 1) * 2; > > + > > + for (i = 0; i < 255; i++) { > > + /* The bclk rate must be smaller than 1/5 sysclk rate */ > > + if (factor * (i + 1) < 5) > > + continue; > > + > > + tmprate = freq * factor * (i + 2); > > + clkrate = clk_round_rate(ssi_private->baudclk, tmprate); > > + > > + do_div(clkrate, factor); > > This do_div() call causes this warning on PowerPC: > > CC sound/soc/fsl/fsl_ssi.o > sound/soc/fsl/fsl_ssi.c: In function 'fsl_ssi_set_bclk': > sound/soc/fsl/fsl_ssi.c:593:3: warning: comparison of distinct pointer > types lacks a cast > sound/soc/fsl/fsl_ssi.c:593:3: warning: right shift count >= width of type > sound/soc/fsl/fsl_ssi.c:593:3: warning: passing argument 1 of > '__div64_32' from incompatible pointer type > include/asm-generic/div64.h:35:17: note: expected 'uint64_t *' but > argument is of type 'long unsigned int *' > > The comments in do_div() say that clkrate should be 64-bits. Changing > clkrate to a u64 does fix this problem, but I'm wondering if anyone > else has seen this. This code has been around for months. It compiles well with my ARM cross compiler. I guess it might be related to the compiler's version? But we should fix it anyway. Would you mind if I submit a patch? Or you do it directly? Thank you, Nicolin ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [alsa-devel] [RFC][PATCH v1] ASoC: fsl_ssi: Add DAI master mode support for SSI on i.MX series
On Thu, Dec 12, 2013 at 4:44 AM, Nicolin Chen wrote: > > +static int fsl_ssi_set_dai_sysclk(struct snd_soc_dai *cpu_dai, > + int clk_id, unsigned int freq, int dir) > +{ > + struct fsl_ssi_private *ssi_private = > snd_soc_dai_get_drvdata(cpu_dai); > + struct ccsr_ssi __iomem *ssi = ssi_private->ssi; > + int synchronous = ssi_private->cpu_dai_drv.symmetric_rates, ret; > + u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i; > + unsigned long flags, clkrate, baudrate, tmprate; > + u64 sub, savesub = 10; > + > + /* Don't apply it to any non-baudclk circumstance */ > + if (IS_ERR(ssi_private->baudclk)) > + return -EINVAL; > + > + /* It should be already enough to divide clock by setting pm alone */ > + psr = 0; > + div2 = 0; > + > + factor = (div2 + 1) * (7 * psr + 1) * 2; > + > + for (i = 0; i < 255; i++) { > + /* The bclk rate must be smaller than 1/5 sysclk rate */ > + if (factor * (i + 1) < 5) > + continue; > + > + tmprate = freq * factor * (i + 2); > + clkrate = clk_round_rate(ssi_private->baudclk, tmprate); > + > + do_div(clkrate, factor); This do_div() call causes this warning on PowerPC: CC sound/soc/fsl/fsl_ssi.o sound/soc/fsl/fsl_ssi.c: In function 'fsl_ssi_set_bclk': sound/soc/fsl/fsl_ssi.c:593:3: warning: comparison of distinct pointer types lacks a cast sound/soc/fsl/fsl_ssi.c:593:3: warning: right shift count >= width of type sound/soc/fsl/fsl_ssi.c:593:3: warning: passing argument 1 of '__div64_32' from incompatible pointer type include/asm-generic/div64.h:35:17: note: expected 'uint64_t *' but argument is of type 'long unsigned int *' The comments in do_div() say that clkrate should be 64-bits. Changing clkrate to a u64 does fix this problem, but I'm wondering if anyone else has seen this. This code has been around for months. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev