Re: [PATCH 5/5] ASoC: Intel: bytcr_rt5640: Add support for HP Elite Pad 1000G2 jack-detect

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

> But I guess things won't be so bad wrt err-exit-path complexity as for
> them to really be a problem, so if you prefer I can also:
> 
> 1. Remove the second acpi_dev_add_driver_gpios + acpi_dev_remove_driver_gpios
> pair from the dai_link .init/.exit.
> 2. Remove the acpi_dev_remove_driver_gpios(ACPI_COMPANION(priv->codec_dev) call here
> moving it to snd_byt_rt5640_mc_remove()
> 3. Introduce a acpi_dev_remove_driver_gpios() remove in the error-exit paths
> of snd_byt_rt5640_mc_probe where necessary.

that sounds good to me, it's probably better to do things once with a
bit of additional error handling.

>>> +		if (IS_ERR(priv->hsmic_detect)) {
>>> +			ret_val = PTR_ERR(priv->hsmic_detect);
>>> +			dev_err_probe(&pdev->dev, ret_val, "getting hsmic-detect GPIO\n");
>>> +			goto err_device;
>>> +		}
>>> +	}
>> Does this part need to be part of the machine driver probe, or could it
>> be moved in the dailink .init function?
> 
> The idea here is that the gpiod_get may fail with -EPROBE_DEFER and then I want
> to fail as early as possible, so right in the probe function.
> 
> This is also why the error is logged with dev_err_probe() which does not
> log anything for EPROBE_DEFER as retval.
> 
>> Is this because you wanted to use devm_ function?
> 
> No, I did consider adding the gpiod_get() for priv->hsmic_detect to the
> dai_link .init function and then I would just use a normal get, combined
> with an explicit _put in the dailink exit. I put this gpiod_get in
> the platform_driver probe to handle EPROBE_DEFER early on, rather then
> having it happen deep inside the devm_snd_soc_register_card() call-graph
> (when it calls the dailink .init).
> 
> I would prefer to keep the gpiod_get inside snd_byt_rt5640_mc_probe for this
> reason, but as mentioned I can removed the second acpi_dev_add_driver_gpios +
> acpi_dev_remove_driver_gpios call pair from the dai_link .init/.exit.
> 
> Please let me know how you want to proceed with this.

ok with the suggestion above. Thanks Hans!