On 27/02/17 10:31, Arnaud Pouliquen wrote:
>
>
> On 02/19/2017 03:56 PM, Jonathan Cameron wrote:
>> On 14/02/17 17:45, Mark Brown wrote:
>>> On Mon, Feb 13, 2017 at 05:38:27PM +0100, Arnaud Pouliquen wrote:
>>>
>>> This looks basically fine as a system specific driver but as some of the
>>> comments in here say there's bits of it could perhaps be genericised but
>>> I'm not sure we need to do that right now. I'm not sure the abstraction
>>> is exactly comfortable but having another bit of hardware doing a bridge
>>> to IIO might be the best way to figure out something better.
>> Agreed. To an extent we are fishing around in the dark at the moment.
>> Lets wait until we have a few more cases of similar hardware before trying
>> too much generalization. This is acting as a good exploration of what
>> is needed.
>
> So for now i keep like this the API between ASOC and IIO, means not
> generic API, and DMA handled in ASOC?
>
> Then when some other hardwares come with same kind of requirements, we
> will re-discuss a more generic way to do it...
Exactly what I was thinking.
>
>>
>> Ideally Lars might upstream some of the other bits he has in his tree
>> to do with DSP processing on ADC streams and that might provide us with
>> more clues on generality (at least at the lowest layers)
>> (Apparently he's busy - though he always makes that excuse :)
>> Joking aside, the exploration Analog and in particular Lars does around
>> pushing the limits of how things interact is always useful!)
>>
>>>
>>>> + .period_bytes_min = 40, /* 8 khz 5 ms */
>>>> + .period_bytes_max = 4 * PAGE_SIZE,
>>>> + .buffer_bytes_max = 16 * PAGE_SIZE
>>>
>>> What's the physical minimum period limit? The comment makes this sound
>>> like it's just made up.
>>>
>>>> + unsigned int shift = 24 -priv->max_scaling;
>>>> +
>>>
>>> Missing space after -.
>>>
>>>> + dev_dbg(dai->dev, "%s: enter\n", __func__);
>>>> + return 0;
>>>> + return snd_pcm_hw_constraint_list(substream->runtime, 0,
>>>> + SNDRV_PCM_HW_PARAM_RATE,
>>>> + &priv->rates_const);
>>>
>>> Looks like debug changes got left in?
>>>
>>>> +static int stm32_adfsdm_set_sysclk(struct snd_soc_dai *dai, int clk_id,
>>>> + unsigned int freq, int dir)
>>>> +{
>>>> + struct stm32_adfsdm_priv *priv = snd_soc_dai_get_drvdata(dai);
>>>> + struct stm32_adfsdm_pdata *pdata = priv->pdata;
>>>> +
>>>> + dev_dbg(dai->dev, "%s: enter for dai %d\n", __func__, dai->id);
>>>> + if (dir == SND_SOC_CLOCK_IN) {
>>>> + pdata->ops->set_sysclk(pdata->adc, freq);
>>>> + priv->dmic_clk = freq;
>>>> + }
>>>> +
>>>> + /* Determine supported rate which depends on SPI/manchester clock */
>>>> + return stm32_adfsdm_get_supported_rates(dai, &priv->rates_const.mask);
>>>
>>> Since the DAI is unidirectional it doesn't matter but obviously if it
>>> weren't then the fact that getting the supported rates involves setting
>>> the hwparams means this could become disruptive. If we're going to
>>> genericise this to be a more general IIO/ASoC bridge that could matter.
>>>
>>>> +static int stm32_adfsdm_dai_remove(struct snd_soc_dai *dai)
>>>> +{
>>>> + dev_dbg(dai->dev, "%s: enter for dai %d\n", __func__, dai->id);
>>>> +
>>>> + return 0;
>>>> +}
>>>
>>> Remove empty functions, though in this case I think you want to add
>>> something to disconnect the XRUN callback just in order to be sure it
>>> can't be mistakenly called.
>>>
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
