On Fri, Oct 03, 2014 at 02:37:26PM -0700, Ben Zhang wrote:
This patch adds support for jack detection using GPIOs on the codec. Plug detect signal and mic present signal can be routed from the physical audio jack to GPIOs on the codec. The codec is configured so that upon signal change, an IRQ(GPIO1) is fired. The codec interrupt handler reads
This then assumes that GPIO1 was configured as the IRQ - what happens if it was connected with some other function on the board and some other GPIO was assigned as IRQ?
- desc = devm_gpiod_get_index(&i2c->dev, "RT5677_GPIO",
RT5677_GPIO_PLUG_DET);
- if (!IS_ERR(desc) && !gpiod_direction_input(desc))
rt5677->gpio_plug_det = desc_to_gpio(desc)
- rt5677->gpio_chip.base + 1;
I'm having trouble reading this, I think other reasonable users would too. It's not entirely clear to me what it's supposed to be doing or why we are mixing the descriptor and number based GPIO APIs.
/*
* Loop to handle interrupts until the last i2c read shows no pending
* irqs with a safeguard of 20 loops
*/
for (i = 0; i < 20; i++) {
Eh? Why are we doing this and why are we so sure that 20 is a good value?
static int rt5677_probe(struct snd_soc_codec *codec) {
rt5677_setup_jack_detect(codec, rt5677); return 0;
This sets up GPIO1 as an interrupt without verifying that we have an interrupt assigned for it successfully and only in the ASoC level probe, not in the chip level probe. This means we might not have an interrupt at all if the board configuration was buggy and means that we'll start paying attention to the interrupt (which is requested in the i2c level probe as it should be) without configuring the chip to be in the appropriate state which means that it might start falsely flagging interrupts. I'd expect the configuration to be closer together, and to have the CODEC set up ready to generate interrupts prior to starting to listen to them.