lkml.org 
[lkml]   [2009]   [May]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] regulator/max1586: support increased V3 voltage range
On Tue, May 26, 2009 at 09:39:08PM +0200, Philipp Zabel wrote:

Please don't use my @wolfsonmicro.com address for anything kernel
related - use @opensource.wolfsonmicro.com or @sirena.org.uk.

> -#define MAX1586_V3_MIN_UV 700000
> -#define MAX1586_V3_MAX_UV 1475000
> -#define MAX1586_V3_STEP_UV 25000

I don't see these values being re-added elsewhere in the code?

> + selector = (min_uV - max1586->min_uV) * 31 / range_uV;
> + if (max1586_v3_calc_voltage(max1586, selector) > max_uV)
> return -EINVAL;

Might be nice to have a #define for the 31 but then it's the only
reference to it.

> rdev = kzalloc(sizeof(struct regulator_dev *) * (MAX1586_V6 + 1),
> GFP_KERNEL);
> if (!rdev)
> - return -ENOMEM;
> + goto out;
> +
> + max1586 = kzalloc(sizeof(struct max1586_data *), GFP_KERNEL);
> + if (!max1586)
> + goto out_unmap;

Could perhaps make these one struct? But either way is fine.

> + switch (pdata->r24) {
> + case MAX1586_R24_3k32:

This switch statement doesn't reject invalid values which I'd expect it
to and...

> + * @r24: external regulator to increase V3 range
> + * (0, MAX1586_R24_3k32, MAX1586_R24_5k11 or MAX1586_R24_7k5)

...I think I'd be more comfortable if an explicit configuration were
required for the no feedback case since otherwise the driver will
happily start up and begin producing the wrong output voltages by
default on platforms which have this feedback resistor fitted. This
could make the system non-obviously unstable and in the worst case
possibly lead to long term hardware damage if something is consistently
driven over voltage.


\
 
 \ /
  Last update: 2009-05-26 22:45    [from the cache]
©2003-2011 Jasper Spaans