Re: [PATCH 3/9] ASoC: ipq806x: add native LPAIF driver

["Kenneth Westfield" <kwestfie@codeaurora.org>]

On Tue, November 25, 2014 1:44 pm, Mark Brown wrote:
> On Wed, Nov 19, 2014 at 10:52:43AM -0800, Kenneth Westfield wrote:
>> +static int lpaif_pcm_int_enable(uint8_t dma_ch)
>> +{
>> +	uint32_t intr_val;
>> +	uint32_t status_val;
>> +	unsigned long flags;
>> +
>> +	if (dma_ch >= LPAIF_MAX_CHANNELS) {
>> +		pr_err("%s: invalid DMA channel given: %hhu\n",
>> +				__func__, dma_ch);
> dev_err().

Now using it in most cases, except for two or three functions that would need to have a handle
passed to it solely for the purpose of using dev_*.

>> +	while (intrsrc) {
>> +		dma_ch = lpaif_dai_find_dma_channel(intrsrc);
>> +		if (dma_ch != -1) {
>> +			if (lpaif_dai[dma_ch]->callback) {
>> +
>> +				ret = lpaif_dai[dma_ch]->callback(intrsrc,
>> +					lpaif_dai[dma_ch]->private_data);
>> +			}
>> +			intrsrc &= ~LPAIF_PER_CH(dma_ch);
>> +		} else {
>> +			pr_err("%s: error getting channel\n", __func__);
>> +			break;
>> +		}
>> +	}
>> +	return ret;
>> +}
> This all looks like a very simple demux of a single register - don't we have a generic irqchip
that can handle it?  It looks like that's what you're trying to implement here, each DMA channel
could be requesting the three interrupts it has separately rather than open coding an interrupt
controller here.
> I'm not actually immediately seeing one right now but it'd be simple to add (or regmap-irq could
do it if we used a regmap, though it assumes threading and isn't a great fit).

I will look into this.  If one fits perfectly, great. If not, then I will add one.


-- 
Kenneth Westfield
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,  a Linux Foundation
Collaborative Project