Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 18 Jan 2021 19:24:20 +0200 | Subject | Re: [PATCH v2 08/12] ASoC: arizona-jack: convert into a helper library for codec drivers |
| |
On Sun, Jan 17, 2021 at 6:06 PM Hans de Goede <hdegoede@redhat.com> wrote: > > Convert the arizona extcon driver into a helper library for direct use > from the arizona codec-drivers, rather then being bound to a separate > MFD cell. > > Note the probe (and remove) sequence is split into 2 parts: > > 1. The arizona_jack_codec_dev_probe() function inits a bunch of > jack-detect specific variables in struct arizona_priv and tries to get > a number of resources where getting them may fail with -EPROBE_DEFER. > > 2. Then once the machine driver has create a snd_sock_jack through > snd_soc_card_jack_new() it calls snd_soc_component_set_jack() on > the codec component, which will call the new arizona_jack_set_jack(), > which sets up jack-detection and requests the IRQs. > > This split is necessary, because the IRQ handlers need access to the > arizona->dapm pointer and the snd_sock_jack which are not available > when the codec-driver's probe function runs. > > Note this requires that machine-drivers for codecs which are converted > to use the new helper functions from arizona-jack.c are modified to > create a snd_soc_jack through snd_soc_card_jack_new() and register > this jack with the codec through snd_soc_component_set_jack().
...
> +int arizona_jack_codec_dev_probe(struct arizona_priv *info, struct device *dev) > { > - struct arizona *arizona = dev_get_drvdata(pdev->dev.parent); > + struct arizona *arizona = info->arizona; > struct arizona_pdata *pdata = &arizona->pdata;
> + int ret, mode; > > if (!dev_get_platdata(arizona->dev)) > - arizona_extcon_device_get_pdata(&pdev->dev, arizona); > + arizona_extcon_device_get_pdata(dev, arizona); > > - info->micvdd = devm_regulator_get(&pdev->dev, "MICVDD"); > + info->micvdd = devm_regulator_get(arizona->dev, "MICVDD");
I'm wondering if arizona->dev == dev here. if no, can this function get a comment / kernel-doc explaining what dev is?
> if (IS_ERR(info->micvdd)) {
> ret = PTR_ERR(info->micvdd); > dev_err(arizona->dev, "Failed to get MICVDD: %d\n", ret);
Side note: at some point perhaps consider to use dev_err_probe() with functions which may return deferred probe error code.
...
> + info->edev = devm_extcon_dev_allocate(dev, arizona_cable); > if (IS_ERR(info->edev)) { > - dev_err(&pdev->dev, "failed to allocate extcon device\n"); > + dev_err(arizona->dev, "failed to allocate extcon device\n");
Ditto about dev.
> return -ENOMEM; > }
...
> + ret = devm_gpio_request_one(dev, arizona->pdata.hpdet_id_gpio, > GPIOF_OUT_INIT_LOW, > "HPDET"); > if (ret != 0) { > dev_err(arizona->dev, "Failed to request GPIO%d: %d\n", > arizona->pdata.hpdet_id_gpio, ret); > - goto err_gpio; > + gpiod_put(info->micd_pol_gpio);
Perhaps move before dev_err() ? Side comment: Do we need dev_err_probe() here?
> + return ret; > }
-- With Best Regards, Andy Shevchenko
| |