linux-kernel - Re: [PATCH] regulator: add MAX8907 driver

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: <20120804101918.GD9248@opensource.wolfsonmicro.com>
Date:	Sat, 4 Aug 2012 11:19:18 +0100
From:	Mark Brown <broonie@...nsource.wolfsonmicro.com>
To:	Stephen Warren <swarren@...dotorg.org>
Cc:	Liam Girdwood <lrg@...com>, linux-kernel@...r.kernel.org,
	Laxman Dewangan <ldewangan@...dia.com>,
	Gyungoh Yoo <jack.yoo@...im-ic.com>,
	Stephen Warren <swarren@...dia.com>
Subject: Re: [PATCH] regulator: add MAX8907 driver

On Thu, Aug 02, 2012 at 12:27:13PM -0600, Stephen Warren wrote:

> The MAX8907 is an I2C-based power-management IC containing voltage
> regulators, a reset controller, a real-time clock, and a touch-screen
> controller.

> * Reworked the regulator driver to be represented as a single device that
>   provides multiple regulators, rather than as a device per regulator.  It
>   seems like this is more common?

This is mostly a reflection of the poor reuse available with most
hardware.  If you've got a bunch of regulators which can usefully
be instantiated and work out where they are by just getting a
register range or two, and especially if you've got a bunch of PMICs
with different arrangements of these things, then it's useful to
split into multiple drivers.  If each regulator needs a bunch of custom
data then there's not much point.

> +static int max8907_regulator_list_voltage(struct regulator_dev *rdev,
> +					  unsigned index)
> +{
> +	struct max8907_regulator *pmic = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	const struct max8907_regulator_info *info = &pmic->info[id];
> +
> +	return info->min_uV + info->step_uV * index;
> +}

regulator_list_voltage_linear().

> +static int max8907_regulator_ldo_set_voltage(struct regulator_dev *rdev,
> +					     int min_uV, int max_uV,
> +					     unsigned *selector)

Should use regulator_set_voltage_sel_regmap() and
regulator_map_voltage_linear().

> +static int max8907_regulator_bbat_set_voltage(struct regulator_dev *rdev,
> +					      int min_uV, int max_uV,
> +					      unsigned *selector)

Similarly for this regulator, use a linear mapping.

> +static int max8907_regulator_fixed_get_voltage(struct regulator_dev *rdev)

This one too.

> +static int max8907_regulator_wled_set_current_limit(struct regulator_dev *rdev,
> +						    int min_uA, int max_uA)

I'm really not convinced it makes much sense to represent the backlight
driver current regulators as regulators, they only get used as part of
the backlight and are usually tightly coupled to their boosts.

> +static int max8907_regulator_ldo_enable(struct regulator_dev *rdev)
> +{
> +	struct max8907_regulator *pmic = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	const struct max8907_regulator_info *info = &pmic->info[id];
> +	unsigned int reg = MAX8907_MASK_LDO_EN | MAX8907_MASK_LDO_SEQ;
> +
> +	return regmap_update_bits(rdev->regmap, info->reg_base + MAX8907_CTL,
> +				  reg, reg);

regulator_enable_regmap() and friends (and similarly for a bunch of the
others).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/