> > Add GPIO support to jack reporting framework in ASoC using gpiolib calls.
> > The gpio support exports two new functions: snd_soc_jack_add_gpios and
> > snd_soc_jack_free_gpios.
> I prefer GPIO stuff built in conditionally.
> The jack layer is used also for non-ASoC codes, and GPIO
> isn't always present.
Is it ok if I enclose all gpio functionality with #ifdef CONFIG_GPIOLIB?
> > +/* irq handler for gpio pin */
> > +static irqreturn_t gpio_handler(int irq, void *data) {
> > + struct snd_soc_jack_gpio *gpio = data;
> > +
> > + return IRQ_RETVAL(schedule_work(&gpio->work));
> Is this really correct?
Changed to:
static irqreturn_t gpio_handler(int irq, void *data)
{
struct snd_soc_jack_gpio *gpio = data;
schedule_work(&gpio->work);
return IRQ_HANDLED;
}
> > +int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> > + struct snd_soc_jack_gpio *gpios)
> This error path doesn't look good. It seems leaking /
> keeping some resources.
Changed to:
int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
struct snd_soc_jack_gpio *gpios)
{
int i, ret;
for (i = 0; i < count; i++) {
if (!gpio_is_valid(gpios[i].gpio)) {
printk(KERN_ERR "Invalid gpio %d\n",
gpios[i].gpio);
return -EINVAL;
}
if (!gpios[i].name) {
printk(KERN_ERR "No name for gpio %d\n",
gpios[i].gpio);
return -EINVAL;
}
ret = gpio_request(gpios[i].gpio, gpios[i].name);
if (ret)
return ret;
ret = gpio_direction_input(gpios[i].gpio);
if (ret) {
gpio_free(gpios[i].gpio);
return ret;
}
ret = request_irq(gpio_to_irq(gpios[i].gpio),
gpio_handler,
IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
jack->card->dev->driver->name,
&gpios[i]);
if (ret) {
gpio_free(gpios[i].gpio);
return ret;
}
INIT_WORK(&gpios[i].work, gpio_work);
gpios[i].jack = jack;
}
return 0;
}
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
