Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Matt Flax]

I have realised that this second patch may not be necessary. It is also 
likely that it will skew the actual sample rate.

Charles, can you please advise me on the following...

I notice that I can probably get the correct sample rates by spoofing 
the crystal frequencies in my machine driver. For example ...
The ratio between the USB and Normal mode fs multipliers with BOSR=0 is :
250/256 = 0.97656
Similarly, I can match this with the following crystal frequency ratio 
(crystal f in MHz) :
12/12.288 = 0.97656
By induction, I assume that I can spoof a 12.288 clock with BOSR=0 for 
sample rate = 48kHz, 96 kHz when using a 12 MHz crystal.

Again, I can match ratios between the following crystal frequecies (in 
MHz) :
12/16.9344 = 0.70862
With BOSR = 1, I assume that can get 44.1 kHz and 88.2 kHz because the 
fs ratio between normal and usb mode is :
272/384 = 0.70833

Now these two ratios are skew by 0.41% which is equvalent to the skew in 
12 MHz USB mode clocks of 44.1kHz to 44.118kHz as reported in the  
wm8731 datasheet.

Can you advise me whether my idea of spoofing clocks in the machine 
driver is perhaps a better way of acquiring the correct sample rates, 
rather then this patch ?

thanks
Matt

On 10/06/16 10:11, Matt Flax wrote:
> This patch allows a machine to set normal mode when using a
> 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on
> the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin.
> The previously ignored direction variable input to the
> wm8731_set_dai_sysclk function is used to indicate whether crystal
> or bit clock is output on the BCLK codec pin.
> I have ensured that this does not effect the db1200.c machine driver
> (the only other driver to use a 12 MHz crystal in USB_MODE). It also
> does not effect the other machine drivers which use the wm8731 codec
> as they don't use USB_MODE nor 12 MHz crystals.
>
> When driving the wm8731 in normal mode with a 12 MHz crystal, the
> DSP filters are expected to be effected. If the filters are activated
> then it is expected that the filter cut offs are shifted by 2.3% at
> 250 fs and 29.1% at 272 fs. The assumption is that the filters are
> linear, which implies a linear shift in rate scaling effects the
> filter cut off frequencies linearly. If the filters are not active,
> there will be no effects.
>
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
>   sound/soc/codecs/wm8731.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
> index d18261a..ec98d5a 100644
> --- a/sound/soc/codecs/wm8731.c
> +++ b/sound/soc/codecs/wm8731.c
> @@ -5,6 +5,7 @@
>    * Copyright 2006-12 Wolfson Microelectronics, plc
>    *
>    * Author: Richard Purdie <richard@openedhand.com>
> + * Author: Matt Flax <flatmax@flatmax.com> 12Mhz XTAL normal mode and 32bit mode.
>    *
>    * Based on wm8753.c by Liam Girdwood
>    *
> @@ -53,6 +54,7 @@ struct wm8731_priv {
>   	int sysclk_type;
>   	int playback_fs;
>   	bool deemph;
> +	bool bclk_12_mhz;
>   
>   	struct mutex lock;
>   };
> @@ -341,8 +343,11 @@ static int wm8731_hw_params(struct snd_pcm_substream *substream,
>   	struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
>   	u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
>   	int i = get_coeff(wm8731->sysclk, params_rate(params));
> -	u16 srate = (coeff_div[i].sr << 2) |
> -		(coeff_div[i].bosr << 1) | coeff_div[i].usb;
> +	u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
> +
> +	/* this determines whether to output bit or crystal clk */
> +	if (wm8731->bclk_12_mhz)
> +		srate |= coeff_div[i].usb;
>   
>   	wm8731->playback_fs = params_rate(params);
>   
> @@ -420,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct snd_soc_dai *codec_dai,
>   
>   	wm8731->sysclk = freq;
>   
> +	if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
> +		wm8731->bclk_12_mhz = 1;
> +	else /* SND_SOC_CLOCK_OUT, output the bit clock */
> +		wm8731->bclk_12_mhz = 0;
> +
>   	snd_soc_dapm_sync(dapm);
>   
>   	return 0;

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Matt Flax]

It appears that my logic here is flawed.
 From what I can tell, it isn't possible to get a suitable bit clock out 
of the codec when using a 12 MHz crystal. For that reason, in my 
hardware I will change crystals from 12 MHz.

I just want to check ... are there any versions of this codec which 
output the bit clock when in USB mode on the BCLK pin ?

thanks
Matt

On 11/06/16 19:16, Matt Flax wrote:
> I have realised that this second patch may not be necessary. It is 
> also likely that it will skew the actual sample rate.
>
> Charles, can you please advise me on the following...
>
> I notice that I can probably get the correct sample rates by spoofing 
> the crystal frequencies in my machine driver. For example ...
> The ratio between the USB and Normal mode fs multipliers with BOSR=0 is :
> 250/256 = 0.97656
> Similarly, I can match this with the following crystal frequency ratio 
> (crystal f in MHz) :
> 12/12.288 = 0.97656
> By induction, I assume that I can spoof a 12.288 clock with BOSR=0 for 
> sample rate = 48kHz, 96 kHz when using a 12 MHz crystal.
>
> Again, I can match ratios between the following crystal frequecies (in 
> MHz) :
> 12/16.9344 = 0.70862
> With BOSR = 1, I assume that can get 44.1 kHz and 88.2 kHz because the 
> fs ratio between normal and usb mode is :
> 272/384 = 0.70833
>
> Now these two ratios are skew by 0.41% which is equvalent to the skew 
> in 12 MHz USB mode clocks of 44.1kHz to 44.118kHz as reported in the  
> wm8731 datasheet.
>
> Can you advise me whether my idea of spoofing clocks in the machine 
> driver is perhaps a better way of acquiring the correct sample rates, 
> rather then this patch ?
>
> thanks
> Matt
>
> On 10/06/16 10:11, Matt Flax wrote:
>> This patch allows a machine to set normal mode when using a
>> 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on
>> the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin.
>> The previously ignored direction variable input to the
>> wm8731_set_dai_sysclk function is used to indicate whether crystal
>> or bit clock is output on the BCLK codec pin.
>> I have ensured that this does not effect the db1200.c machine driver
>> (the only other driver to use a 12 MHz crystal in USB_MODE). It also
>> does not effect the other machine drivers which use the wm8731 codec
>> as they don't use USB_MODE nor 12 MHz crystals.
>>
>> When driving the wm8731 in normal mode with a 12 MHz crystal, the
>> DSP filters are expected to be effected. If the filters are activated
>> then it is expected that the filter cut offs are shifted by 2.3% at
>> 250 fs and 29.1% at 272 fs. The assumption is that the filters are
>> linear, which implies a linear shift in rate scaling effects the
>> filter cut off frequencies linearly. If the filters are not active,
>> there will be no effects.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
>>   sound/soc/codecs/wm8731.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
>> index d18261a..ec98d5a 100644
>> --- a/sound/soc/codecs/wm8731.c
>> +++ b/sound/soc/codecs/wm8731.c
>> @@ -5,6 +5,7 @@
>>    * Copyright 2006-12 Wolfson Microelectronics, plc
>>    *
>>    * Author: Richard Purdie <richard@openedhand.com>
>> + * Author: Matt Flax <flatmax@flatmax.com> 12Mhz XTAL normal mode 
>> and 32bit mode.
>>    *
>>    * Based on wm8753.c by Liam Girdwood
>>    *
>> @@ -53,6 +54,7 @@ struct wm8731_priv {
>>       int sysclk_type;
>>       int playback_fs;
>>       bool deemph;
>> +    bool bclk_12_mhz;
>>         struct mutex lock;
>>   };
>> @@ -341,8 +343,11 @@ static int wm8731_hw_params(struct 
>> snd_pcm_substream *substream,
>>       struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
>>       u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
>>       int i = get_coeff(wm8731->sysclk, params_rate(params));
>> -    u16 srate = (coeff_div[i].sr << 2) |
>> -        (coeff_div[i].bosr << 1) | coeff_div[i].usb;
>> +    u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
>> +
>> +    /* this determines whether to output bit or crystal clk */
>> +    if (wm8731->bclk_12_mhz)
>> +        srate |= coeff_div[i].usb;
>>         wm8731->playback_fs = params_rate(params);
>>   @@ -420,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct 
>> snd_soc_dai *codec_dai,
>>         wm8731->sysclk = freq;
>>   +    if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
>> +        wm8731->bclk_12_mhz = 1;
>> +    else /* SND_SOC_CLOCK_OUT, output the bit clock */
>> +        wm8731->bclk_12_mhz = 0;
>> +
>>       snd_soc_dapm_sync(dapm);
>>         return 0;
>

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Charles Keepax]

On Sun, Jun 12, 2016 at 01:22:26PM +1000, Matt Flax wrote:
> It appears that my logic here is flawed.
> From what I can tell, it isn't possible to get a suitable bit clock out of
> the codec when using a 12 MHz crystal. For that reason, in my hardware I
> will change crystals from 12 MHz.

This looks correct the chip only has simple dividers so you won't
be able to get a good audio rate clock from a 12MHz crystal.

> 
> I just want to check ... are there any versions of this codec which output
> the bit clock when in USB mode on the BCLK pin ?
> 

Apologies I am not quite sure I follow what USB mode is in this
context, and the bit clock surely always comes out of the BCLK
pin. Do you simply mean a system with a clock speed that
relates to the 48MHz USB clock, such as your 12MHz? Basically any
part that has a PLL/FLL should work for that, for example the
wm8998:

http://www.cirrus.com/en/products/pro/detail/P1265.html

Thanks,
Charles

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Matt Flax]

On 13/06/16 18:37, Charles Keepax wrote:
> On Sun, Jun 12, 2016 at 01:22:26PM +1000, Matt Flax wrote:
>> It appears that my logic here is flawed.
>>  From what I can tell, it isn't possible to get a suitable bit clock out of
>> the codec when using a 12 MHz crystal. For that reason, in my hardware I
>> will change crystals from 12 MHz.
> This looks correct the chip only has simple dividers so you won't
> be able to get a good audio rate clock from a 12MHz crystal.
>
>> I just want to check ... are there any versions of this codec which output
>> the bit clock when in USB mode on the BCLK pin ?
>>
> Apologies I am not quite sure I follow what USB mode is in this
> context, and the bit clock surely always comes out of the BCLK
> pin. Do you simply mean a system with a clock speed that
> relates to the 48MHz USB clock, such as your 12MHz? Basically any
> part that has a PLL/FLL should work for that, for example the
> wm8998:
>
> http://www.cirrus.com/en/products/pro/detail/P1265.html

Literally, with the chip set to usb mode (register 0b0001000 bit 0, page 
41 of the data sheet) it appears that the 12 MHz clock comes out of the 
BCLK pin, rather then the bit clock.

Matt

> Thanks,
> Charles

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Charles Keepax]

On Mon, Jun 13, 2016 at 07:45:22PM +1000, Matt Flax wrote:
> On 13/06/16 18:37, Charles Keepax wrote:
> >On Sun, Jun 12, 2016 at 01:22:26PM +1000, Matt Flax wrote:
> >>It appears that my logic here is flawed.
> >> From what I can tell, it isn't possible to get a suitable bit clock out of
> >>the codec when using a 12 MHz crystal. For that reason, in my hardware I
> >>will change crystals from 12 MHz.
> >This looks correct the chip only has simple dividers so you won't
> >be able to get a good audio rate clock from a 12MHz crystal.
> >
> >>I just want to check ... are there any versions of this codec which output
> >>the bit clock when in USB mode on the BCLK pin ?
> >>
> >Apologies I am not quite sure I follow what USB mode is in this
> >context, and the bit clock surely always comes out of the BCLK
> >pin. Do you simply mean a system with a clock speed that
> >relates to the 48MHz USB clock, such as your 12MHz? Basically any
> >part that has a PLL/FLL should work for that, for example the
> >wm8998:
> >
> >http://www.cirrus.com/en/products/pro/detail/P1265.html
> 
> Literally, with the chip set to usb mode (register 0b0001000 bit 0, page 41
> of the data sheet) it appears that the 12 MHz clock comes out of the BCLK
> pin, rather then the bit clock.

Ah seems there are two versions of the chip the MC and L I was
looking at the datasheet for the version that doesn't have that
mode.

>From that datasheet it looks like it should be possible to
support audio rate clocks from a 12MHz input. Although as you say
the 44.1 clock is not exactly 44.1. I am not sure why you are
getting 12MHz coming out of the BCLK pin the datasheet certainly
does not appear to suggest this should happen. Are you sure the
sample rate and clock divider registers are set sensibly? My
first guess would be if things are in an unsupported state then
perhaps it just fires the clock straight through. I assume we are
very sure the input clock is 12MHz? Only other thing I can
imagine is for some reason we have a higher clock rate than we
expected and on the scope it would be hard to differenciate
between 12.288MHz and 12MHz dead. So for example if it was a
48MHz USB clock coming in rather than 12, then the chip tries to
generate a 3.072MHz bit clock that would look like 12MHz.

Again I guess a newer part with an PLL might be simpler as you
can simply use the PLL to generate audio rate clocks from the
12MHz.

Thanks,
Charles

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Matt Flax]

OK - I will look into that futher. Thanks for the info.

Is it possible to look at patch 1/2 in this series ?

thanks
Matt

On 13/06/16 21:59, Charles Keepax wrote:
> On Mon, Jun 13, 2016 at 07:45:22PM +1000, Matt Flax wrote:
>> On 13/06/16 18:37, Charles Keepax wrote:
>>> On Sun, Jun 12, 2016 at 01:22:26PM +1000, Matt Flax wrote:
>>>> It appears that my logic here is flawed.
>>>>  From what I can tell, it isn't possible to get a suitable bit clock out of
>>>> the codec when using a 12 MHz crystal. For that reason, in my hardware I
>>>> will change crystals from 12 MHz.
>>> This looks correct the chip only has simple dividers so you won't
>>> be able to get a good audio rate clock from a 12MHz crystal.
>>>
>>>> I just want to check ... are there any versions of this codec which output
>>>> the bit clock when in USB mode on the BCLK pin ?
>>>>
>>> Apologies I am not quite sure I follow what USB mode is in this
>>> context, and the bit clock surely always comes out of the BCLK
>>> pin. Do you simply mean a system with a clock speed that
>>> relates to the 48MHz USB clock, such as your 12MHz? Basically any
>>> part that has a PLL/FLL should work for that, for example the
>>> wm8998:
>>>
>>> http://www.cirrus.com/en/products/pro/detail/P1265.html
>> Literally, with the chip set to usb mode (register 0b0001000 bit 0, page 41
>> of the data sheet) it appears that the 12 MHz clock comes out of the BCLK
>> pin, rather then the bit clock.
> Ah seems there are two versions of the chip the MC and L I was
> looking at the datasheet for the version that doesn't have that
> mode.
>
>  From that datasheet it looks like it should be possible to
> support audio rate clocks from a 12MHz input. Although as you say
> the 44.1 clock is not exactly 44.1. I am not sure why you are
> getting 12MHz coming out of the BCLK pin the datasheet certainly
> does not appear to suggest this should happen. Are you sure the
> sample rate and clock divider registers are set sensibly? My
> first guess would be if things are in an unsupported state then
> perhaps it just fires the clock straight through. I assume we are
> very sure the input clock is 12MHz? Only other thing I can
> imagine is for some reason we have a higher clock rate than we
> expected and on the scope it would be hard to differenciate
> between 12.288MHz and 12MHz dead. So for example if it was a
> 48MHz USB clock coming in rather than 12, then the chip tries to
> generate a 3.072MHz bit clock that would look like 12MHz.
>
> Again I guess a newer part with an PLL might be simpler as you
> can simply use the PLL to generate audio rate clocks from the
> 12MHz.
>
> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Charles Keepax]

On Mon, Jun 13, 2016 at 10:39:22PM +1000, Matt Flax wrote:
> OK - I will look into that futher. Thanks for the info.
> 
> Is it possible to look at patch 1/2 in this series ?
> 

I can't seem to see patch 1/2 on the list, you may have to resend
that one.

Thanks,
Charles

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Rob Calhoun]

Hi Matt,

Based on my experience with the WM8737, the bitclock will always run
at 12 MHz on these devices in USB mode. While the carrying capacity of
the channel is always 12 Mbps, the data put on the wire by the codec
depend on the sample rate, number of channels, and bit depth. This
leaves some unused capacity in the channel. It is the responsibility
of the receiving digital audio interface to pull off the appropriate
number of bits every time the LRCLK/frameclock fires and ignore the
rest of it. For example in I2S mode, 2-channel, 32-bit, the DAI should
pull off the first 32 bits following each LRCLK transition.

Whether you can use USB mode or not depends on the capabilities of the
receiving digital audio interface. For example, we tried to use WM8737
USB mode on a TI AM335x-based board because the AM335x has a 24 MHz
master clock. This will not work because the MCASP unit on AM335x has
a hard limit of 32 bits per "slot" (thus 64 bits per frame with
two-channel acquisition) and there are 128 bits per frame at 96 kHz,
which is as fast as the WM8737 will go. (Oddly the MCASP would be fine
192 kHz x 2 or 96 KHz x 4 ; the limit is based on the size of the
XRBUF register MCASP uses to swizzle data, not the speed of the
interface.)

12.288 MHz (or 24.576 MHz) are much more commonly used a master clock
frequencies in the audio world than 12 MHz, so unless you are trying
to squeeze the last dollar out of your board BOM cost you will
probably find it easier to get everything working if you switch to a
12.288 MHz master clock.

Best regards,

Rob Calhoun

On Sat, Jun 11, 2016 at 11:22 PM, Matt Flax <flatmax@flatmax.org> wrote:
> It appears that my logic here is flawed.
> From what I can tell, it isn't possible to get a suitable bit clock out of
> the codec when using a 12 MHz crystal. For that reason, in my hardware I
> will change crystals from 12 MHz.
>
> I just want to check ... are there any versions of this codec which output
> the bit clock when in USB mode on the BCLK pin ?
>
> thanks
> Matt
>
> On 11/06/16 19:16, Matt Flax wrote:
>>
>> I have realised that this second patch may not be necessary. It is also
>> likely that it will skew the actual sample rate.
>>
>> Charles, can you please advise me on the following...
>>
>> I notice that I can probably get the correct sample rates by spoofing the
>> crystal frequencies in my machine driver. For example ...
>> The ratio between the USB and Normal mode fs multipliers with BOSR=0 is :
>> 250/256 = 0.97656
>> Similarly, I can match this with the following crystal frequency ratio
>> (crystal f in MHz) :
>> 12/12.288 = 0.97656
>> By induction, I assume that I can spoof a 12.288 clock with BOSR=0 for
>> sample rate = 48kHz, 96 kHz when using a 12 MHz crystal.
>>
>> Again, I can match ratios between the following crystal frequecies (in
>> MHz) :
>> 12/16.9344 = 0.70862
>> With BOSR = 1, I assume that can get 44.1 kHz and 88.2 kHz because the fs
>> ratio between normal and usb mode is :
>> 272/384 = 0.70833
>>
>> Now these two ratios are skew by 0.41% which is equvalent to the skew in
>> 12 MHz USB mode clocks of 44.1kHz to 44.118kHz as reported in the  wm8731
>> datasheet.
>>
>> Can you advise me whether my idea of spoofing clocks in the machine driver
>> is perhaps a better way of acquiring the correct sample rates, rather then
>> this patch ?
>>
>> thanks
>> Matt
>>
>> On 10/06/16 10:11, Matt Flax wrote:
>>>
>>> This patch allows a machine to set normal mode when using a
>>> 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on
>>> the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin.
>>> The previously ignored direction variable input to the
>>> wm8731_set_dai_sysclk function is used to indicate whether crystal
>>> or bit clock is output on the BCLK codec pin.
>>> I have ensured that this does not effect the db1200.c machine driver
>>> (the only other driver to use a 12 MHz crystal in USB_MODE). It also
>>> does not effect the other machine drivers which use the wm8731 codec
>>> as they don't use USB_MODE nor 12 MHz crystals.
>>>
>>> When driving the wm8731 in normal mode with a 12 MHz crystal, the
>>> DSP filters are expected to be effected. If the filters are activated
>>> then it is expected that the filter cut offs are shifted by 2.3% at
>>> 250 fs and 29.1% at 272 fs. The assumption is that the filters are
>>> linear, which implies a linear shift in rate scaling effects the
>>> filter cut off frequencies linearly. If the filters are not active,
>>> there will be no effects.
>>>
>>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>>> ---
>>>   sound/soc/codecs/wm8731.c | 14 ++++++++++++--
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
>>> index d18261a..ec98d5a 100644
>>> --- a/sound/soc/codecs/wm8731.c
>>> +++ b/sound/soc/codecs/wm8731.c
>>> @@ -5,6 +5,7 @@
>>>    * Copyright 2006-12 Wolfson Microelectronics, plc
>>>    *
>>>    * Author: Richard Purdie <richard@openedhand.com>
>>> + * Author: Matt Flax <flatmax@flatmax.com> 12Mhz XTAL normal mode and
>>> 32bit mode.
>>>    *
>>>    * Based on wm8753.c by Liam Girdwood
>>>    *
>>> @@ -53,6 +54,7 @@ struct wm8731_priv {
>>>       int sysclk_type;
>>>       int playback_fs;
>>>       bool deemph;
>>> +    bool bclk_12_mhz;
>>>         struct mutex lock;
>>>   };
>>> @@ -341,8 +343,11 @@ static int wm8731_hw_params(struct snd_pcm_substream
>>> *substream,
>>>       struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
>>>       u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
>>>       int i = get_coeff(wm8731->sysclk, params_rate(params));
>>> -    u16 srate = (coeff_div[i].sr << 2) |
>>> -        (coeff_div[i].bosr << 1) | coeff_div[i].usb;
>>> +    u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
>>> +
>>> +    /* this determines whether to output bit or crystal clk */
>>> +    if (wm8731->bclk_12_mhz)
>>> +        srate |= coeff_div[i].usb;
>>>         wm8731->playback_fs = params_rate(params);
>>>   @@ -420,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct
>>> snd_soc_dai *codec_dai,
>>>         wm8731->sysclk = freq;
>>>   +    if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
>>> +        wm8731->bclk_12_mhz = 1;
>>> +    else /* SND_SOC_CLOCK_OUT, output the bit clock */
>>> +        wm8731->bclk_12_mhz = 0;
>>> +
>>>       snd_soc_dapm_sync(dapm);
>>>         return 0;
>>
>>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Matt Flax]

Hi Rob,

Thanks for the information. I also discovered what you mention regarding 
bit clock in USB
mode. Charles mentioned it in one of his earlier mails as well,
it didn't sink in till I saw on the I2S what you are saying below !

Luckily the bcm2835 i2s controller is capable of shifting the R
channel bits (w.r.t. the LRCLK leading edge) with a resolution of
ten bits. Here is the patch which makes it all happen :
https://github.com/raspberrypi/linux/commit/19343c782de3ef73e5e4903906e658a9193eb177

I made a few versions of this hardware with a 12 MHz and 18.432 MHz 
crystal, I prefer the 12 MHz
version because it provides more usable sample rates ... as you can see 
from the patch, it can't
replicate 8 kHz because the I2S controller needs 11 bits resolution to 
make it work !

Matt

On 20/06/16 07:46, Rob Calhoun wrote:
> Hi Matt,
>
> Based on my experience with the WM8737, the bitclock will always run
> at 12 MHz on these devices in USB mode. While the carrying capacity of
> the channel is always 12 Mbps, the data put on the wire by the codec
> depend on the sample rate, number of channels, and bit depth. This
> leaves some unused capacity in the channel. It is the responsibility
> of the receiving digital audio interface to pull off the appropriate
> number of bits every time the LRCLK/frameclock fires and ignore the
> rest of it. For example in I2S mode, 2-channel, 32-bit, the DAI should
> pull off the first 32 bits following each LRCLK transition.
>
> Whether you can use USB mode or not depends on the capabilities of the
> receiving digital audio interface. For example, we tried to use WM8737
> USB mode on a TI AM335x-based board because the AM335x has a 24 MHz
> master clock. This will not work because the MCASP unit on AM335x has
> a hard limit of 32 bits per "slot" (thus 64 bits per frame with
> two-channel acquisition) and there are 128 bits per frame at 96 kHz,
> which is as fast as the WM8737 will go. (Oddly the MCASP would be fine
> 192 kHz x 2 or 96 KHz x 4 ; the limit is based on the size of the
> XRBUF register MCASP uses to swizzle data, not the speed of the
> interface.)
>
> 12.288 MHz (or 24.576 MHz) are much more commonly used a master clock
> frequencies in the audio world than 12 MHz, so unless you are trying
> to squeeze the last dollar out of your board BOM cost you will
> probably find it easier to get everything working if you switch to a
> 12.288 MHz master clock.
>
> Best regards,
>
> Rob Calhoun
>
> On Sat, Jun 11, 2016 at 11:22 PM, Matt Flax <flatmax@flatmax.org> wrote:
>> It appears that my logic here is flawed.
>>  From what I can tell, it isn't possible to get a suitable bit clock out of
>> the codec when using a 12 MHz crystal. For that reason, in my hardware I
>> will change crystals from 12 MHz.
>>
>> I just want to check ... are there any versions of this codec which output
>> the bit clock when in USB mode on the BCLK pin ?
>>
>> thanks
>> Matt
>>
>> On 11/06/16 19:16, Matt Flax wrote:
>>> I have realised that this second patch may not be necessary. It is also
>>> likely that it will skew the actual sample rate.
>>>
>>> Charles, can you please advise me on the following...
>>>
>>> I notice that I can probably get the correct sample rates by spoofing the
>>> crystal frequencies in my machine driver. For example ...
>>> The ratio between the USB and Normal mode fs multipliers with BOSR=0 is :
>>> 250/256 = 0.97656
>>> Similarly, I can match this with the following crystal frequency ratio
>>> (crystal f in MHz) :
>>> 12/12.288 = 0.97656
>>> By induction, I assume that I can spoof a 12.288 clock with BOSR=0 for
>>> sample rate = 48kHz, 96 kHz when using a 12 MHz crystal.
>>>
>>> Again, I can match ratios between the following crystal frequecies (in
>>> MHz) :
>>> 12/16.9344 = 0.70862
>>> With BOSR = 1, I assume that can get 44.1 kHz and 88.2 kHz because the fs
>>> ratio between normal and usb mode is :
>>> 272/384 = 0.70833
>>>
>>> Now these two ratios are skew by 0.41% which is equvalent to the skew in
>>> 12 MHz USB mode clocks of 44.1kHz to 44.118kHz as reported in the  wm8731
>>> datasheet.
>>>
>>> Can you advise me whether my idea of spoofing clocks in the machine driver
>>> is perhaps a better way of acquiring the correct sample rates, rather then
>>> this patch ?
>>>
>>> thanks
>>> Matt
>>>
>>> On 10/06/16 10:11, Matt Flax wrote:
>>>> This patch allows a machine to set normal mode when using a
>>>> 12 MHz crystal. In USB_MODE the 12 MHz crystal signal is output on
>>>> the BCLK pin. In NORMAL_MODE the bit clock is output on the BCLK pin.
>>>> The previously ignored direction variable input to the
>>>> wm8731_set_dai_sysclk function is used to indicate whether crystal
>>>> or bit clock is output on the BCLK codec pin.
>>>> I have ensured that this does not effect the db1200.c machine driver
>>>> (the only other driver to use a 12 MHz crystal in USB_MODE). It also
>>>> does not effect the other machine drivers which use the wm8731 codec
>>>> as they don't use USB_MODE nor 12 MHz crystals.
>>>>
>>>> When driving the wm8731 in normal mode with a 12 MHz crystal, the
>>>> DSP filters are expected to be effected. If the filters are activated
>>>> then it is expected that the filter cut offs are shifted by 2.3% at
>>>> 250 fs and 29.1% at 272 fs. The assumption is that the filters are
>>>> linear, which implies a linear shift in rate scaling effects the
>>>> filter cut off frequencies linearly. If the filters are not active,
>>>> there will be no effects.
>>>>
>>>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>>>> ---
>>>>    sound/soc/codecs/wm8731.c | 14 ++++++++++++--
>>>>    1 file changed, 12 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/sound/soc/codecs/wm8731.c b/sound/soc/codecs/wm8731.c
>>>> index d18261a..ec98d5a 100644
>>>> --- a/sound/soc/codecs/wm8731.c
>>>> +++ b/sound/soc/codecs/wm8731.c
>>>> @@ -5,6 +5,7 @@
>>>>     * Copyright 2006-12 Wolfson Microelectronics, plc
>>>>     *
>>>>     * Author: Richard Purdie <richard@openedhand.com>
>>>> + * Author: Matt Flax <flatmax@flatmax.com> 12Mhz XTAL normal mode and
>>>> 32bit mode.
>>>>     *
>>>>     * Based on wm8753.c by Liam Girdwood
>>>>     *
>>>> @@ -53,6 +54,7 @@ struct wm8731_priv {
>>>>        int sysclk_type;
>>>>        int playback_fs;
>>>>        bool deemph;
>>>> +    bool bclk_12_mhz;
>>>>          struct mutex lock;
>>>>    };
>>>> @@ -341,8 +343,11 @@ static int wm8731_hw_params(struct snd_pcm_substream
>>>> *substream,
>>>>        struct wm8731_priv *wm8731 = snd_soc_codec_get_drvdata(codec);
>>>>        u16 iface = snd_soc_read(codec, WM8731_IFACE) & 0xfff3;
>>>>        int i = get_coeff(wm8731->sysclk, params_rate(params));
>>>> -    u16 srate = (coeff_div[i].sr << 2) |
>>>> -        (coeff_div[i].bosr << 1) | coeff_div[i].usb;
>>>> +    u16 srate = (coeff_div[i].sr << 2) | (coeff_div[i].bosr << 1);
>>>> +
>>>> +    /* this determines whether to output bit or crystal clk */
>>>> +    if (wm8731->bclk_12_mhz)
>>>> +        srate |= coeff_div[i].usb;
>>>>          wm8731->playback_fs = params_rate(params);
>>>>    @@ -420,6 +425,11 @@ static int wm8731_set_dai_sysclk(struct
>>>> snd_soc_dai *codec_dai,
>>>>          wm8731->sysclk = freq;
>>>>    +    if (dir == SND_SOC_CLOCK_IN) /* output the 12 MHz clock */
>>>> +        wm8731->bclk_12_mhz = 1;
>>>> +    else /* SND_SOC_CLOCK_OUT, output the bit clock */
>>>> +        wm8731->bclk_12_mhz = 0;
>>>> +
>>>>        snd_soc_dapm_sync(dapm);
>>>>          return 0;
>>>
>> _______________________________________________
>> Alsa-devel mailing list
>> Alsa-devel@alsa-project.org
>> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 2/2] ASoc: wm8731: add normal mode with 12MHz XTAL

[Charles Keepax]

On Sun, Jun 19, 2016 at 05:46:02PM -0400, Rob Calhoun wrote:
> Hi Matt,
> 
> Based on my experience with the WM8737, the bitclock will always run
> at 12 MHz on these devices in USB mode. While the carrying capacity of
> the channel is always 12 Mbps, the data put on the wire by the codec
> depend on the sample rate, number of channels, and bit depth. This
> leaves some unused capacity in the channel. It is the responsibility
> of the receiving digital audio interface to pull off the appropriate
> number of bits every time the LRCLK/frameclock fires and ignore the
> rest of it. For example in I2S mode, 2-channel, 32-bit, the DAI should
> pull off the first 32 bits following each LRCLK transition.

Thanks very much for adding this, sounds like this is basically
how the part operates then. Although I feel the datasheet could
have been more clear, apologies for that.

Thanks,
Charles