linux-kernel - Re: [PATCH 7/8] iio: add sd modulator generic iio backend

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 for Android: free password hash cracker in your pocket

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

Date: Mon, 24 Jun 2024 18:41:29 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Olivier MOYSAN <olivier.moysan@...s.st.com>
CC: Jonathan Cameron <jic23@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>,
	Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
	<linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>
Subject: Re: [PATCH 7/8] iio: add sd modulator generic iio backend


> >> +	}
> >> +
> >> +	return -EINVAL;
> >> +};
> >> +
> >> +static const struct iio_backend_ops sd_backend_ops = {
> >> +	.enable = sd_backend_enable,
> >> +	.disable = sd_backend_disable,
> >> +	.read_raw = sd_backend_read,
> >> +};
> >> +
> >> +static int iio_sd_backend_probe(struct platform_device *pdev)
> >> +{
> >> +	struct device *dev = &pdev->dev;
> >> +	struct regulator *vref;
> >> +	struct iio_sd_backend_priv *priv;
> >> +	int ret;
> >> +
> >> +	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> >> +	if (!priv)
> >> +		return -ENOMEM;
> >> +
> >> +	vref = devm_regulator_get_optional(dev, "vref");  
> > 
> > New devm_regulator_get_enable_read_voltage() slightly simplifies this
> > and means you don't need to keep vref around.
> >   
> >> +	if (IS_ERR(vref)) {
> >> +		if (PTR_ERR(vref) != -ENODEV)
> >> +			return dev_err_probe(dev, PTR_ERR(vref), "Failed to get vref\n");
> >> +	} else {
> >> +		ret = regulator_get_voltage(vref);  
> > You haven't turned it on so it's not guaranteed to give you a useful
> > answer.
> >   
> 
> My understanding is that regulator_get_voltage() always returns the 
> regulator voltage, whatever the regulator state, as documented in the 
> API description:
> "* NOTE: If the regulator is disabled it will return the voltage value.
> * This function should not be used to determine regulator state."
> 
> So, my logic was to enable the regulator only when requested, through 
> enable/disable callbacks to manage power.
> 
> Please, let me know if I missed something here.

Ah ok. I had a vague and it seems incorrect recollection that you had
to turn the regs on to get voltage in some cases.  Ah well. Clearly not :)
What you have is fine.  Add a comment though just so no one replaces
this code with the helper.  

Jonathan