Messages in this thread | | | Date | Thu, 28 Jun 2012 02:08:10 +0000 (GMT) | From | MyungJoo Ham <> | Subject | Re: [PATCH] Extcon: Arizona: Add driver for Wolfson Arizona class devices |
| |
> Most Wolfson Arizona class audio hub CODECs include a flexible ultra low > power accessory detection subsystem. This driver exposes initial support > for this subsystem via the Extcon framework, implementing support for > ultra low power detection of headphone and headset with the ability to > detect the polarity of a headset. > > The functionality of the devices is much richer and more flexible than > the current driver, future patches will extend the features of the > driver to more fully exploit this. > > Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
The driver looks good.
I only have some performance concerns that may be ignored if you don't care of it for this device.
> --- > drivers/extcon/Kconfig | 8 + > drivers/extcon/Makefile | 1 + > drivers/extcon/extcon-arizona.c | 491 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 500 insertions(+) > create mode 100644 drivers/extcon/extcon-arizona.c >
[]
> + > +#define ARIZONA_CABLE_MECHANICAL "Mechanical" > +#define ARIZONA_CABLE_HEADPHONE "Headphone" > +#define ARIZONA_CABLE_HEADSET "Headset" > + > +static const char *arizona_cable[] = { > + ARIZONA_CABLE_MECHANICAL, > + ARIZONA_CABLE_HEADSET, > + ARIZONA_CABLE_HEADPHONE, > + NULL, > +};
For ARIZONA_CABLE_HEADPHONE and ARIZONA_CABLE_MECHANICAL, you can use extcon_cable_name[EXTCON_HEADPHONE_OUT] and extcon_cable_name[EXTCON_MECHANICAL].
It appears that I need to rephrase line 38-41 of extcon_class.c. Anyway, it is not recommended to import the whole list. However, it is strongly recommended to reuse the corresponding entries from the list.
Anyway, the HEADSET appears to be a pair of HEADPHONE and MIC. You may replace HEADSET with MIC in arizona_cable and remove exclusive[] and regard HEADPHONE | MIC as "HEADSET".
> + > +static irqreturn_t arizona_micdet(int irq, void *data) > +{ > + struct arizona_extcon_info *info = data; > + struct arizona *arizona = info->arizona; > + unsigned int val; > + int ret; > + > + mutex_lock(&info->lock); > + > + ret = regmap_read(arizona->regmap, ARIZONA_MIC_DETECT_3, &val); > + if (ret != 0) { > + dev_err(arizona->dev, "Failed to read MICDET: %d\n", ret); > + return IRQ_NONE; > + } > + > + dev_dbg(arizona->dev, "MICDET: %x\n", val); > + > + if (!(val & ARIZONA_MICD_VALID)) { > + dev_warn(arizona->dev, "Microphone detection state invalid\n"); > + mutex_unlock(&info->lock); > + return IRQ_NONE; > + } > + > + /* Due to jack detect this should never happen */ > + if (!(val & ARIZONA_MICD_STS)) { > + dev_warn(arizona->dev, "Detected open circuit\n"); > + info->detecting = false; > + goto handled; > + } > + > + /* If we got a high impedence we should have a headset, report it. */ > + if (info->detecting && (val & 0x400)) { > + ret = extcon_set_cable_state(&info->edev, > + ARIZONA_CABLE_HEADSET, true);
You may use extcon_set_cable_state_ for the performance as you already know the index of HEADSET. Or extcon_update_state();
> + > + if (ret != 0) > + dev_err(arizona->dev, "Headset report failed: %d\n", > + ret); > + > + info->mic = true; > + info->detecting = false; > + goto handled; > + } > + > + /* If we detected a lower impedence during initial startup > + * then we probably have the wrong polarity, flip it. Don't > + * do this for the lowest impedences to speed up detection of > + * plain headphones. If both polarities report a low > + * impedence then give up and report headphones. > + */ > + if (info->detecting && (val & 0x3f8)) { > + info->jack_flips++; > + > + if (info->jack_flips >= info->micd_num_modes) { > + dev_dbg(arizona->dev, "Detected headphone\n"); > + info->detecting = false; > + ret = extcon_set_cable_state(&info->edev, > + ARIZONA_CABLE_HEADPHONE, > + true);
Same for here.
[]
> + > + /* > + * If we're still detecting and we detect a short then we've > + * got a headphone. Otherwise it's a button press, the > + * button reporting is stubbed out for now. > + */ > + if (val & 0x3fc) { > + if (info->mic) { > + dev_dbg(arizona->dev, "Mic button detected\n"); > + > + } else if (info->detecting) { > + dev_dbg(arizona->dev, "Headphone detected\n"); > + info->detecting = false; > + arizona_stop_mic(info); > + > + ret = extcon_set_cable_state(&info->edev, > + ARIZONA_CABLE_HEADPHONE, > + true);
Here, too.
[]
> + > +static irqreturn_t arizona_jackdet(int irq, void *data) > +{ > + struct arizona_extcon_info *info = data; > + struct arizona *arizona = info->arizona; > + unsigned int val; > + int ret; > + > + pm_runtime_get_sync(info->dev); > + > + mutex_lock(&info->lock); > + > + ret = regmap_read(arizona->regmap, ARIZONA_AOD_IRQ_RAW_STATUS, &val); > + if (ret != 0) { > + dev_err(arizona->dev, "Failed to read jackdet status: %d\n", > + ret); > + mutex_unlock(&info->lock); > + pm_runtime_put_autosuspend(info->dev); > + return IRQ_NONE; > + } > + > + if (val & ARIZONA_JD1_STS) { > + dev_dbg(arizona->dev, "Detected jack\n"); > + ret = extcon_set_cable_state(&info->edev, > + ARIZONA_CABLE_MECHANICAL, true);
Here.
Cheers! MyungJoo
| |