On Tue, Mar 06, 2012 at 02:48:05PM +0900, Tomoya MORINAGA wrote:
2012年3月3日1:39 Mark Brown broonie@opensource.wolfsonmicro.com:
I just merged Lars-Peter's dmaengine library code which has been on the list for a week or so - this should be updated to use that next time it's posted. That should save a lot of code from the driver and make sure it's following best practices for dmaengine use.
Sorry, we can't use this library.
This sort of response really isn't on. Do you have some reason for saying this? Flat out refusing to do things with no reason is not useful.
+static struct ioh_i2s_data *i2s_data; +static struct ioh_i2s_dma dmadata[MAX_I2S_CH];
Why are these needed, aren't they dynamically allocated by the driver?
You mean ASoC driver can't use global variable ?
In general no Linux driver should be using a global variable except for things like this.
case SNDRV_PCM_FORMAT_S32_LE:
byte = 24;
break;
That looks wrong... are you sure you don't support S24_LE or something?
This is correct.
Because maximum transmit size of ML7213's I2S hw is 24bit.
Note we often lay out 24 bit audio in 32 bit blocks.
+static struct platform_driver ioh_i2s_driver_plat = {
+static struct platform_driver ioh_dai_driver_plat = {
+static int ioh_i2s_pci_probe(struct pci_dev *pdev,
const struct pci_device_id *id)
Why are you creating these platform devices? I don't understand the function they serve. The code handling them looks to have quite a few problems but I'm not clear they should be there in the first place.
if these platform devices aren't used, device detection doesn't work correctly. So, I added these.
You've not actually mentioned the problem you were seeing...
rv = request_irq(pdev->irq, ioh_i2s_irq, IRQF_SHARED, "ml7213_ioh",
pdev);
if (rv != 0) {
printk(KERN_ERR "Failed to allocate irq\n");
goto out_irq;
}
Are you *sure* you're ready to handle interrupts at this point?
This is just registering interrupt handler. As long as interrupt register is not enabled, the interrupt handler is not called. This is common request_irq description. What's is your concern ?
Apart from anything else you've got the interrupt requested as IRQF_SHARED so the interrupt could get called at any time. It's also not clear that you've got the hardware in a known good state.