Re: [Zaurus-devel] [PATCH 1/3] ISL6271A voltage regulator support.

2010-06-13 Thread Marek Vasut 
Dne So 12. června 2010 19:21:14 Mark Brown napsal(a):
 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.

Is this necessary? I'd only increase the kernel size without any gain ... or am 
I wrong ?
 
  Signed-off-by: Marek Vasut marek.va...@gmail.com
  
  +/* Supported voltage values for regulators */
  +static const u32 core_buck_table[] = {
  +85,  90,  95, 100,
  +   105, 110, 115, 120,
  +   125, 130, 135, 140,
  +   145, 150, 155, 160,
  +};
 
 This looks like it could be replaced with a simple function rather than
 a lookup table which would simplify the code.

Will do.
 
 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

That's what's written in the ISL6271a TRM. On the first page, under the 
Features section.
 
  +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.

Ok.

Thanks!

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

Re: [Zaurus-devel] [PATCH 1/3] ISL6271A voltage regulator support.

2010-06-12 Thread Mark Brown 
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[] = {
 +  85,  90,  95, 100,
 + 105, 110, 115, 120,
 + 125, 130, 135, 140,
 + 145, 150, 155, 160,
 +};

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