| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <30036799.190541340849289982.JavaMail.weblogic@epml02>
Date: Thu, 28 Jun 2012 02:08:10 +0000 (GMT)
From: MyungJoo Ham <myungjoo.ham@...sung.com>
To: Mark Brown <broonie@...nsource.wolfsonmicro.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
¹Ú°æ¹Î <kyungmin.park@...sung.com>,
"patches@...nsource.wolfsonmicro.com"
<patches@...nsource.wolfsonmicro.com>,
"cw00.choi@...sung.com" <cw00.choi@...sung.com>
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@...nsource.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
Powered by blists - more mailing lists