Re: [PATCH 1/4 v2] mfd: arizona: Export function to control subsystem DVFS

[Charles Keepax <ckeepax@opensource.wolfsonmicro.com>]

On Sat, Jun 21, 2014 at 09:45:49PM +0100, Mark Brown wrote:
> On Fri, Jun 20, 2014 at 03:41:10PM +0100, Charles Keepax wrote:
> 
> > +	int ret = 0;
> 
> > +			ret = regulator_set_voltage(arizona->dcvdd,
> > +						    1800000, 1800000);
> > +			if (ret != 0) {
> > +				dev_err(arizona->dev,
> > +					"Failed to raise dcvdd (%u)\n", ret);
> > +				goto err;
> > +			}
> 
> I'm really nitpicking here but the error code is a signed integer being
> printed for some reason with %u and while I'm at it DCVDD would normally
> be written in all caps for human consumption.

Yeah that should be fixed.

> 
> > +int arizona_dvfs_down(struct arizona *arizona, unsigned int flags)
> > +{
> > +	int ret = 0;
> > +
> > +	mutex_lock(&arizona->subsys_max_lock);
> > +
> > +	arizona->subsys_max_rq &= ~flags;
> 
> Is it worth checking to see if the request was asserted and logging a
> warning if it wasn't?  The lack of refcounting suggests that a bit of
> defensiveness might be in order.

I think it seems reasonable, I will add some code for this.

Thanks,
Charles