linux-kernel - RE: [PATCH v2] ASoC: da7219: Allow pdata to specify a VDDIO

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 23 Jul 2018 14:41:26 +0000
From:   Adam Thomson <Adam.Thomson.Opensource@...semi.com>
To:     Daniel Kurtz <djkurtz@...omium.org>,
        Mark Brown <broonie@...nel.org>
CC:     Akshu Agrawal <akshu.agrawal@....com>,
        Support Opensource <Support.Opensource@...semi.com>,
        Jaroslav Kysela <perex@...ex.cz>,
        "Takashi Iwai" <tiwai@...e.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        "moderated list:SOUND" <alsa-devel@...a-project.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v2] ASoC: da7219: Allow pdata to specify a VDDIO

On 23 July 2018 00:28, Daniel Kurtz wrote:

> Some systems do not have software controllable regulators driving the
> DA7219's supplies, nor can they use device tree to create "always-on fixed
> regulators" to easily pretend like they do.
> 
> On these systems the call to devm_regulator_bulk_get() just creates
> a set of dummy registers.  Calling regulator_get_voltage() on a dummy
> regulator just returns -EINVAL, in which case the DA7219 is always set up
> to use the default VDDIO voltage range of 2.5-3.6V.
> 
> Provide a new device property to let such systems specify a different
> VDDIO if needed (e.g., 1.8V).

I'm not sure what the general view on this is. In the past it was suggested
the regulator framework was the way to go to pass this kind of information,
but obviously ACPI platforms don't tend to use it.

Mark, what is your feeling on this? Would you be in favour of some kind of 
fixed voltage regulator representation, similar to the patch for the AMD
platform (ASoC: AMD: Add a fix voltage regulator for DA7219 and ADAU7002),
albeit tweaked to avoid asynchronous probe() issues, or is this a reasonable
route? Personally in my mind, and in an ideal world, I'd prefer just one method
for retrieving this data in the codec driver, but that may not be sensible.

> 
> Signed-off-by: Daniel Kurtz <djkurtz@...omium.org>
> ---
> Changes for v2:
>  - fix to use device_property_read_u32()
> 
>  include/sound/da7219.h    |  2 ++
>  sound/soc/codecs/da7219.c | 19 +++++++++++++------
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/include/sound/da7219.h b/include/sound/da7219.h
> index 1bfcb16f2d10ab..16ab125ad4adbf 100644
> --- a/include/sound/da7219.h
> +++ b/include/sound/da7219.h
> @@ -38,6 +38,8 @@ struct da7219_pdata {
> 
>  	const char *dai_clks_name;
> 
> +	u32 vddio;
> +
>  	/* Mic */
>  	enum da7219_micbias_voltage micbias_lvl;
>  	enum da7219_mic_amp_in_sel mic_amp_in_sel;
> diff --git a/sound/soc/codecs/da7219.c b/sound/soc/codecs/da7219.c
> index 980a6a8bf56d38..9893920b26f41f 100644
> --- a/sound/soc/codecs/da7219.c
> +++ b/sound/soc/codecs/da7219.c
> @@ -1634,6 +1634,9 @@ static struct da7219_pdata *da7219_fw_to_pdata(struct
> snd_soc_component *compone
>  	else
>  		pdata->mic_amp_in_sel = DA7219_MIC_AMP_IN_SEL_DIFF;
> 
> +	if (device_property_read_u32(dev, "dlg,vddio", &of_val32) >= 0)
> +		pdata->vddio = of_val32;
> +
>  	return pdata;
>  }
> 
> @@ -1717,8 +1720,12 @@ static int da7219_handle_supplies(struct
> snd_soc_component *component)
>  	/* Determine VDDIO voltage provided */
>  	vddio = da7219->supplies[DA7219_SUPPLY_VDDIO].consumer;
>  	ret = regulator_get_voltage(vddio);
> +	/* If regulator_get_voltage() fails, try to use vddio from pdata. */
> +	if (ret < 0 && da7219->pdata)
> +		ret = da7219->pdata->vddio;
>  	if (ret < 1200000)
> -		dev_warn(component->dev, "Invalid VDDIO voltage\n");
> +		dev_warn(component->dev, "Invalid VDDIO voltage: %d mV\n",
> +			 ret);
>  	else if (ret < 2800000)
>  		io_voltage_lvl = DA7219_IO_VOLTAGE_LEVEL_1_2V_2_8V;
> 
> @@ -1872,6 +1879,11 @@ static int da7219_probe(struct snd_soc_component
> *component)
>  	mutex_init(&da7219->ctrl_lock);
>  	mutex_init(&da7219->pll_lock);
> 
> +	/* Handle DT/ACPI/Platform data */
> +	da7219->pdata = dev_get_platdata(component->dev);
> +	if (!da7219->pdata)
> +		da7219->pdata = da7219_fw_to_pdata(component);
> +
>  	/* Regulator configuration */
>  	ret = da7219_handle_supplies(component);
>  	if (ret)
> @@ -1897,11 +1909,6 @@ static int da7219_probe(struct snd_soc_component
> *component)
>  		break;
>  	}
> 
> -	/* Handle DT/ACPI/Platform data */
> -	da7219->pdata = dev_get_platdata(component->dev);
> -	if (!da7219->pdata)
> -		da7219->pdata = da7219_fw_to_pdata(component);
> -
>  	da7219_handle_pdata(component);
> 
>  	/* Check if MCLK provided */
> --
> 2.18.0.233.g985f88cf7e-goog