Re: [PATCH v3 10/13] ASoC: arizona-jack: Use snd_soc_jack to report jack events

[Andy Shevchenko <andy.shevchenko@gmail.com>]

On Fri, Jan 22, 2021 at 6:41 PM Hans de Goede <hdegoede@redhat.com> wrote:
>
> Use the snd_soc_jack code to report jack events, instead of using extcon
> for reporting the cable-type + an input_dev for reporting the button
> presses.
>
> The snd_soc_jack code will report the cable-type through both input_dev
> events and through ALSA controls and the button-presses through input_dev
> events.
>
> Note that this means that when the codec drivers are moved over to use
> the new arizona-jack.c library code instead of having a separate MFD
> extcon cell with the extcon-arizona.c driver, we will no longer report
> extcon events to userspace for cable-type changes. This should not be
> a problem since "standard" Linux distro userspace does not (and has
> never) used the extcon class interface for this. Android does have
> support for the extcon class interface, but that was introduced in
> the same release as support for input_dev cable-type events, so this
> should not be a problem for Android either.
>
> Note this also reduces ARIZONA_MAX_MICD_RANGE from 8 to 6, this is
> ok to do since this info is always provided through pdata (or defaults)
> and cannot be overridden from devicetree. All in kernel users of the

in-kernel

> pdata (and the fallback defaults) define 6 or less buttons/ranges.
>

It makes a lot of sense to me. It might be that we can optimize
arizona_button_reading() even more in the future.

Reviewed-by: Andy Shevchenko <andy.shevchenko@gmal.com>

> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> ---
>  sound/soc/codecs/arizona-jack.c | 149 +++++++++-----------------------
>  sound/soc/codecs/arizona.h      |   7 +-
>  2 files changed, 47 insertions(+), 109 deletions(-)
>
> diff --git a/sound/soc/codecs/arizona-jack.c b/sound/soc/codecs/arizona-jack.c
> index e121490eb379..268d2a44d891 100644
> --- a/sound/soc/codecs/arizona-jack.c
> +++ b/sound/soc/codecs/arizona-jack.c
> @@ -16,8 +16,8 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/property.h>
>  #include <linux/regulator/consumer.h>
> -#include <linux/extcon-provider.h>
>
> +#include <sound/jack.h>
>  #include <sound/soc.h>
>
>  #include <linux/mfd/arizona/core.h>
> @@ -29,6 +29,12 @@
>
>  #define ARIZONA_MAX_MICD_RANGE 8
>
> +/*
> + * The hardware supports 8 ranges / buttons, but the snd-jack interface
> + * only supports 6 buttons (button 0-5).
> + */
> +#define ARIZONA_MAX_MICD_BUTTONS 6
> +
>  #define ARIZONA_MICD_CLAMP_MODE_JDL      0x4
>  #define ARIZONA_MICD_CLAMP_MODE_JDH      0x5
>  #define ARIZONA_MICD_CLAMP_MODE_JDL_GP5H 0x9
> @@ -86,14 +92,6 @@ static const int arizona_micd_levels[] = {
>         1257, 30000,
>  };
>
> -static const unsigned int arizona_cable[] = {
> -       EXTCON_MECHANICAL,
> -       EXTCON_JACK_MICROPHONE,
> -       EXTCON_JACK_HEADPHONE,
> -       EXTCON_JACK_LINE_OUT,
> -       EXTCON_NONE,
> -};
> -
>  static void arizona_start_hpdet_acc_id(struct arizona_priv *info);
>
>  static void arizona_extcon_hp_clamp(struct arizona_priv *info,
> @@ -559,8 +557,7 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>         struct arizona_priv *info = data;
>         struct arizona *arizona = info->arizona;
>         int id_gpio = arizona->pdata.hpdet_id_gpio;
> -       unsigned int report = EXTCON_JACK_HEADPHONE;
> -       int ret, reading, state;
> +       int ret, reading, state, report;
>         bool mic = false;
>
>         mutex_lock(&info->lock);
> @@ -573,11 +570,8 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>         }
>
>         /* If the cable was removed while measuring ignore the result */
> -       state = extcon_get_state(info->edev, EXTCON_MECHANICAL);
> -       if (state < 0) {
> -               dev_err(arizona->dev, "Failed to check cable state: %d\n", state);
> -               goto out;
> -       } else if (!state) {
> +       state = info->jack->status & SND_JACK_MECHANICAL;
> +       if (!state) {
>                 dev_dbg(arizona->dev, "Ignoring HPDET for removed cable\n");
>                 goto done;
>         }
> @@ -603,14 +597,11 @@ static irqreturn_t arizona_hpdet_irq(int irq, void *data)
>
>         /* Report high impedence cables as line outputs */
>         if (reading >= 5000)
> -               report = EXTCON_JACK_LINE_OUT;
> +               report = SND_JACK_LINEOUT;
>         else
> -               report = EXTCON_JACK_HEADPHONE;
> +               report = SND_JACK_HEADPHONE;
>
> -       ret = extcon_set_state_sync(info->edev, report, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report HP/line: %d\n",
> -                       ret);
> +       snd_soc_jack_report(info->jack, report, SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>  done:
>         /* Reset back to starting range */
> @@ -686,9 +677,8 @@ static void arizona_identify_headphone(struct arizona_priv *info)
>         pm_runtime_put_autosuspend(arizona->dev);
>
>         /* Just report headphone */
> -       ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
> +       snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
> +                           SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>         if (info->mic)
>                 arizona_start_mic(info);
> @@ -740,9 +730,8 @@ static void arizona_start_hpdet_acc_id(struct arizona_priv *info)
>
>  err:
>         /* Just report headphone */
> -       ret = extcon_set_state_sync(info->edev, EXTCON_JACK_HEADPHONE, true);
> -       if (ret != 0)
> -               dev_err(arizona->dev, "Failed to report headphone: %d\n", ret);
> +       snd_soc_jack_report(info->jack, SND_JACK_HEADPHONE,
> +                           SND_JACK_LINEOUT | SND_JACK_HEADPHONE);
>
>         info->hpdet_active = false;
>  }
> @@ -863,11 +852,7 @@ static int arizona_micdet_reading(void *priv)
>
>                 arizona_identify_headphone(info);
>
> -               ret = extcon_set_state_sync(info->edev,
> -                                             EXTCON_JACK_MICROPHONE, true);
> -               if (ret != 0)
> -                       dev_err(arizona->dev, "Headset report failed: %d\n",
> -                               ret);
> +               snd_soc_jack_report(info->jack, SND_JACK_MICROPHONE, SND_JACK_MICROPHONE);
>
>                 /* Don't need to regulate for button detection */
>                 ret = regulator_allow_bypass(info->micvdd, true);
> @@ -930,7 +915,7 @@ static int arizona_button_reading(void *priv)
>  {
>         struct arizona_priv *info = priv;
>         struct arizona *arizona = info->arizona;
> -       int val, key, lvl, i;
> +       int val, key, lvl;
>
>         val = arizona_micd_read(info);
>         if (val < 0)
> @@ -947,14 +932,11 @@ static int arizona_button_reading(void *priv)
>                         lvl = val & ARIZONA_MICD_LVL_MASK;
>                         lvl >>= ARIZONA_MICD_LVL_SHIFT;
>
> -                       for (i = 0; i < info->num_micd_ranges; i++)
> -                               input_report_key(info->input,
> -                                                info->micd_ranges[i].key, 0);
> -
>                         if (lvl && ffs(lvl) - 1 < info->num_micd_ranges) {
> -                               key = info->micd_ranges[ffs(lvl) - 1].key;
> -                               input_report_key(info->input, key, 1);
> -                               input_sync(info->input);
> +                               key = ffs(lvl) - 1;
> +                               snd_soc_jack_report(info->jack,
> +                                                   SND_JACK_BTN_0 >> key,
> +                                                   info->micd_button_mask);
>                         } else {
>                                 dev_err(arizona->dev, "Button out of range\n");
>                         }
> @@ -964,10 +946,7 @@ static int arizona_button_reading(void *priv)
>                 }
>         } else {
>                 dev_dbg(arizona->dev, "Mic button released\n");
> -               for (i = 0; i < info->num_micd_ranges; i++)
> -                       input_report_key(info->input,
> -                                        info->micd_ranges[i].key, 0);
> -               input_sync(info->input);
> +               snd_soc_jack_report(info->jack, 0, info->micd_button_mask);
>                 arizona_extcon_pulse_micbias(info);
>         }
>
> @@ -980,20 +959,13 @@ static void arizona_micd_detect(struct work_struct *work)
>                                                 struct arizona_priv,
>                                                 micd_detect_work.work);
>         struct arizona *arizona = info->arizona;
> -       int ret;
>
>         cancel_delayed_work_sync(&info->micd_timeout_work);
>
>         mutex_lock(&info->lock);
>
>         /* If the cable was removed while measuring ignore the result */
> -       ret = extcon_get_state(info->edev, EXTCON_MECHANICAL);
> -       if (ret < 0) {
> -               dev_err(arizona->dev, "Failed to check cable state: %d\n",
> -                               ret);
> -               mutex_unlock(&info->lock);
> -               return;
> -       } else if (!ret) {
> +       if (!(info->jack->status & SND_JACK_MECHANICAL)) {
>                 dev_dbg(arizona->dev, "Ignoring MICDET for removed cable\n");
>                 mutex_unlock(&info->lock);
>                 return;
> @@ -1134,12 +1106,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>
>         if (info->last_jackdet == present) {
>                 dev_dbg(arizona->dev, "Detected jack\n");
> -               ret = extcon_set_state_sync(info->edev,
> -                                             EXTCON_MECHANICAL, true);
> -
> -               if (ret != 0)
> -                       dev_err(arizona->dev, "Mechanical report failed: %d\n",
> -                               ret);
> +               snd_soc_jack_report(info->jack, SND_JACK_MECHANICAL, SND_JACK_MECHANICAL);
>
>                 info->detecting = true;
>                 info->mic = false;
> @@ -1170,18 +1137,7 @@ static irqreturn_t arizona_jackdet(int irq, void *data)
>                 info->hpdet_done = false;
>                 info->hpdet_retried = false;
>
> -               for (i = 0; i < info->num_micd_ranges; i++)
> -                       input_report_key(info->input,
> -                                        info->micd_ranges[i].key, 0);
> -               input_sync(info->input);
> -
> -               for (i = 0; i < ARRAY_SIZE(arizona_cable) - 1; i++) {
> -                       ret = extcon_set_state_sync(info->edev,
> -                                       arizona_cable[i], false);
> -                       if (ret != 0)
> -                               dev_err(arizona->dev,
> -                                       "Removal report failed: %d\n", ret);
> -               }
> +               snd_soc_jack_report(info->jack, 0, ARIZONA_JACK_MASK | info->micd_button_mask);
>
>                 /*
>                  * If the jack was removed during a headphone detection we
> @@ -1389,29 +1345,6 @@ int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev)
>                 break;
>         }
>
> -       info->edev = devm_extcon_dev_allocate(dev, arizona_cable);
> -       if (IS_ERR(info->edev)) {
> -               dev_err(arizona->dev, "failed to allocate extcon device\n");
> -               return -ENOMEM;
> -       }
> -
> -       ret = devm_extcon_dev_register(dev, info->edev);
> -       if (ret < 0) {
> -               dev_err(arizona->dev, "extcon_dev_register() failed: %d\n",
> -                       ret);
> -               return ret;
> -       }
> -
> -       info->input = devm_input_allocate_device(dev);
> -       if (!info->input) {
> -               dev_err(arizona->dev, "Can't allocate input dev\n");
> -               ret = -ENOMEM;
> -               return ret;
> -       }
> -
> -       info->input->name = "Headset";
> -       info->input->phys = "arizona/extcon";
> -
>         if (!pdata->micd_timeout)
>                 pdata->micd_timeout = DEFAULT_MICD_TIMEOUT;
>
> @@ -1535,9 +1468,9 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>                 info->num_micd_ranges = ARRAY_SIZE(micd_default_ranges);
>         }
>
> -       if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_RANGE) {
> -               dev_err(arizona->dev, "Too many MICD ranges: %d\n",
> -                       arizona->pdata.num_micd_ranges);
> +       if (arizona->pdata.num_micd_ranges > ARIZONA_MAX_MICD_BUTTONS) {
> +               dev_err(arizona->dev, "Too many MICD ranges: %d > %d\n",
> +                       arizona->pdata.num_micd_ranges, ARIZONA_MAX_MICD_BUTTONS);
>                 return -EINVAL;
>         }
>
> @@ -1571,8 +1504,11 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>                         arizona_micd_levels[j], i);
>
>                 arizona_micd_set_level(arizona, i, j);
> -               input_set_capability(info->input, EV_KEY,
> -                                    info->micd_ranges[i].key);
> +
> +               /* SND_JACK_BTN_# masks start with the most significant bit */
> +               info->micd_button_mask |= SND_JACK_BTN_0 >> i;
> +               snd_jack_set_key(jack->jack, SND_JACK_BTN_0 >> i,
> +                                info->micd_ranges[i].key);
>
>                 /* Enable reporting of that range */
>                 regmap_update_bits(arizona->regmap, ARIZONA_MIC_DETECT_2,
> @@ -1620,6 +1556,8 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>
>         arizona_extcon_set_mode(info, 0);
>
> +       info->jack = jack;
> +
>         pm_runtime_get_sync(arizona->dev);
>
>         if (info->micd_clamp) {
> @@ -1680,18 +1618,10 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>         if (ret != 0)
>                 dev_warn(arizona->dev, "Failed to set MICVDD to bypass: %d\n", ret);
>
> -       ret = input_register_device(info->input);
> -       if (ret) {
> -               dev_err(arizona->dev, "Can't register input device: %d\n", ret);
> -               goto err_hpdet;
> -       }
> -
>         pm_runtime_put(arizona->dev);
>
>         return 0;
>
> -err_hpdet:
> -       arizona_free_irq(arizona, ARIZONA_IRQ_HPDET, info);
>  err_micdet:
>         arizona_free_irq(arizona, ARIZONA_IRQ_MICDET, info);
>  err_fall_wake:
> @@ -1704,6 +1634,7 @@ static int arizona_jack_enable_jack_detect(struct arizona_priv *info,
>         arizona_free_irq(arizona, jack_irq_rise, info);
>  err_pm:
>         pm_runtime_put(arizona->dev);
> +       info->jack = NULL;
>         return ret;
>  }
>
> @@ -1714,6 +1645,9 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
>         bool change;
>         int ret;
>
> +       if (!info->jack)
> +               return 0;
> +
>         if (info->micd_clamp) {
>                 jack_irq_rise = ARIZONA_IRQ_MICD_CLAMP_RISE;
>                 jack_irq_fall = ARIZONA_IRQ_MICD_CLAMP_FALL;
> @@ -1748,6 +1682,7 @@ static int arizona_jack_disable_jack_detect(struct arizona_priv *info)
>         regmap_update_bits(arizona->regmap, ARIZONA_JACK_DETECT_ANALOGUE,
>                            ARIZONA_JD1_ENA, 0);
>         arizona_clk32k_disable(arizona);
> +       info->jack = NULL;
>
>         return 0;
>  }
> diff --git a/sound/soc/codecs/arizona.h b/sound/soc/codecs/arizona.h
> index fc515845a3e6..173ebd0bf8c9 100644
> --- a/sound/soc/codecs/arizona.h
> +++ b/sound/soc/codecs/arizona.h
> @@ -97,9 +97,8 @@ struct arizona_priv {
>         struct delayed_work hpdet_work;
>         struct delayed_work micd_detect_work;
>         struct delayed_work micd_timeout_work;
> +       struct snd_soc_jack *jack;
>         struct regulator *micvdd;
> -       struct input_dev *input;
> -       struct extcon_dev *edev;
>         struct gpio_desc *micd_pol_gpio;
>
>         u16 last_jackdet;
> @@ -108,6 +107,7 @@ struct arizona_priv {
>         const struct arizona_micd_config *micd_modes;
>         int micd_num_modes;
>
> +       int micd_button_mask;
>         const struct arizona_micd_range *micd_ranges;
>         int num_micd_ranges;
>
> @@ -257,6 +257,9 @@ extern unsigned int arizona_mixer_values[ARIZONA_NUM_MIXER_INPUTS];
>  #define ARIZONA_RATE_ENUM_SIZE 4
>  #define ARIZONA_SAMPLE_RATE_ENUM_SIZE 14
>
> +/* SND_JACK_* mask for supported cable/switch types */
> +#define ARIZONA_JACK_MASK  (SND_JACK_HEADSET | SND_JACK_LINEOUT | SND_JACK_MECHANICAL)
> +
>  extern const char * const arizona_rate_text[ARIZONA_RATE_ENUM_SIZE];
>  extern const unsigned int arizona_rate_val[ARIZONA_RATE_ENUM_SIZE];
>  extern const char * const arizona_sample_rate_text[ARIZONA_SAMPLE_RATE_ENUM_SIZE];
> --
> 2.28.0
>


-- 
With Best Regards,
Andy Shevchenko