linux-kernel - Re: [PATCH v2] regulator: add new regulator driver for lp872x

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]

Message-ID: <20120615132130.GH4482@opensource.wolfsonmicro.com>
Date:	Fri, 15 Jun 2012 14:21:31 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	"Kim, Milo" <Milo.Kim@...com>
Cc:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Girdwood, Liam" <lrg@...com>
Subject: Re: [PATCH v2] regulator: add new regulator driver for lp872x

On Fri, Jun 15, 2012 at 08:46:34AM +0000, Kim, Milo wrote:
> This driver supports TI/National Semiconductor LP8720, LP8725 PMIC.

Overall looks very good, just a few fairly minor things below but
nothing substantial.

> 
> patch v2
> --------

See Documentation/SubmittingPatches for how to format stuff like this -
put it after the ---

> +	if (chip == LP8720) {
> +		mask = LP8720_TIMESTEP_M;
> +		shift = LP8720_TIMESTEP_S;
> +		time_usec = &lp8720_time_usec[0];
> +		size = ARRAY_SIZE(lp8720_time_usec);
> +	} else if (chip == LP8725) {
> +		mask = LP8725_TIMESTEP_M;
> +		shift = LP8725_TIMESTEP_S;
> +		time_usec = &lp8725_time_usec[0];
> +		size = ARRAY_SIZE(lp8725_time_usec);
> +	} else {
> +		return -EINVAL;
> +	}

This should be a switch statement.

> +static int lp8725_buck_set_current_limit(struct regulator_dev *rdev,
> +					int min_uA, int max_uA)
> +{
> +	struct lp872x *lp = rdev_get_drvdata(rdev);
> +	enum lp872x_regulator_id buck = rdev_get_id(rdev);
> +	int i, max = ARRAY_SIZE(lp8725_buck_uA);
> +	u8 addr, val;
> +
> +	if (buck == LP8725_ID_BUCK1)
> +		addr = LP8725_BUCK1_VOUT2;
> +	else if (buck == LP8725_ID_BUCK2)
> +		addr = LP8725_BUCK2_VOUT2;
> +	else
> +		return -EINVAL;

Again, switch statement.

> +static const struct regmap_config lp872x_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.max_register = MAX_REGISTERS,
> +};

Probably worth enabling cache here, it'll cut out read/modify/write
cycles which will improve the performance as the reads should be free.
Not essential but worth considering.

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)