lkml.org 
[lkml]   [2012]   [Jun]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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

\
 
 \ /
  Last update: 2012-06-28 05:01    [W:0.103 / U:0.276 seconds]
©2003-2014 Jasper Spaans. Advertise on this site