At Mon, 2 Mar 2009 19:05:28 -0600,
Lopez Cruz, Misael wrote:
>
> 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.
>
> Client drivers using gpio feature must pass an array of jack_gpio pins
> belonging to a specific jack to the snd_soc_jack_add_gpios function. The
> framework will request the gpio, set the data direction and request irq.
> The framework will update power status of related jack_pins when an event on
> the gpio pins comes according to the reporting bits defined for each gpio.
>
> All gpio resources allocated when adding jack_gpio pins for a particular
> jack can be released using snd_soc_jack_free_gpios function.
>
> Signed-off-by: Misael Lopez Cruz <x0052729@xxxxxx>
I prefer GPIO stuff built in conditionally.
The jack layer is used also for non-ASoC codes, and GPIO isn't always
present.
> +/* 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?
> +int snd_soc_jack_add_gpios(struct snd_soc_jack *jack, int count,
> + struct snd_soc_jack_gpio *gpios)
> +{
> + int i, ret = 0;
> +
> + /* Link gpio array with the soc_jack */
> + if (count > 0)
> + jack->gpios = gpios;
> +
> + 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)
> + break;
> +
> + 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)
> + break;
> +
> + INIT_WORK(&gpios[i].work, gpio_work);
> + gpios[i].jack = jack;
> + }
> +
> + if (ret)
> + gpio_free(gpios[i].gpio);
This error path doesn't look good. It seems leaking / keeping some
resources.
thanks,
Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
