On Sat, Jun 12, 2010 at 02:44:37PM +0200, Marek Vasut wrote:
> This device is very simple, it supports only one LDO. This single LDO is
> programmed over I2C to 16 possible voltages.

Google tells me that the device has three regulators on it - two LDOs
and one DCDC buck convertor:

   http://www.intersil.com/products/deviceinfo.asp?pn=ISL6271A

While the LDOs look like they have no software control it'd be best to
provide hookup for them, even if that's just a case of instantiating the
appropriate fixed voltage regulators.

> Signed-off-by: Marek Vasut <marek.va...@gmail.com>

> +/* Supported voltage values for regulators */
> +static const u32 core_buck_table[] = {
> +      850000,  900000,  950000, 1000000,
> +     1050000, 1100000, 1150000, 1200000,
> +     1250000, 1300000, 1350000, 1400000,
> +     1450000, 1500000, 1550000, 1600000,
> +};

This looks like it could be replaced with a simple function rather than
a lookup table which would simplify the code.

Also, are you sure this is a buck?  That's a specific technical term
usually only applied to DCDC regulators - LDOs are a different type of
regulator 

> +static int __devinit isl6271a_probe(struct i2c_client *client,
> +                                  const struct i2c_device_id *id)
> +{
> +     struct regulator_init_data *init_data   = client->dev.platform_data;
> +     struct isl_pmic *pmic;
> +     int err;
> +
> +     if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA))
> +             return -EIO;
> +
> +     if (!init_data)
> +             return -EIO;

It'd be better to print an error message here so users know what's going
on when the device fails to appear.

_______________________________________________
Zaurus-devel mailing list
Zaurus-devel@lists.linuxtogo.org
http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/zaurus-devel

Reply via email to