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


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

2010-06-12 Thread Marek Vasut
This device is very simple, it supports only one LDO. This single LDO is
programmed over I2C to 16 possible voltages.

Signed-off-by: Marek Vasut marek.va...@gmail.com
---
 drivers/regulator/Kconfig  |6 +
 drivers/regulator/Makefile |1 +
 drivers/regulator/isl6271a-regulator.c |  199 
 3 files changed, 206 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/isl6271a-regulator.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 04f2e08..6772ca7 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -201,5 +201,11 @@ config REGULATOR_88PM8607
help
  This driver supports 88PM8607 voltage regulator chips.
 
+config REGULATOR_ISL6271A
+   tristate Intersil ISL6271A Power regulator
+   depends on I2C
+   help
+ This driver supports ISL6271A voltage regulator chip.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 4e7feec..e2191bf 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -31,5 +31,6 @@ obj-$(CONFIG_REGULATOR_AB3100) += ab3100.o
 obj-$(CONFIG_REGULATOR_TPS65023) += tps65023-regulator.o
 obj-$(CONFIG_REGULATOR_TPS6507X) += tps6507x-regulator.o
 obj-$(CONFIG_REGULATOR_88PM8607) += 88pm8607.o
+obj-$(CONFIG_REGULATOR_ISL6271A) += isl6271a-regulator.o
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/isl6271a-regulator.c 
b/drivers/regulator/isl6271a-regulator.c
new file mode 100644
index 000..41cddc7
--- /dev/null
+++ b/drivers/regulator/isl6271a-regulator.c
@@ -0,0 +1,199 @@
+/*
+ * isl6271a-regulator.c
+ *
+ * Support for Intersil ISL6271A voltage regulator
+ *
+ * Copyright (C) 2010 Marek Vasut marek.va...@gmail.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed as is WITHOUT ANY WARRANTY of any kind,
+ * whether express or implied; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/init.h
+#include linux/err.h
+#include linux/platform_device.h
+#include linux/regulator/driver.h
+#include linux/i2c.h
+#include linux/delay.h
+#include linux/slab.h
+
+/* 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,
+};
+
+/* PMIC details */
+struct isl_pmic {
+   struct regulator_desc   desc;
+   struct i2c_client   *client;
+   struct regulator_dev*rdev;
+   struct mutexmtx;
+};
+
+static int isl6271a_get_voltage(struct regulator_dev *dev)
+{
+   struct isl_pmic *pmic = rdev_get_drvdata(dev);
+   int idx, data;
+
+   mutex_lock(pmic-mtx);
+
+   idx = i2c_smbus_read_byte(pmic-client);
+   if (idx  0) {
+   dev_err(pmic-client-dev, Error getting voltage\n);
+   data = idx;
+   goto out;
+   }
+
+   data = core_buck_table[idx  0xf];
+
+out:
+   mutex_unlock(pmic-mtx);
+   return data;
+}
+
+static int isl6271a_set_voltage(struct regulator_dev *dev, int minuV, int 
maxuV)
+{
+   struct isl_pmic *pmic = rdev_get_drvdata(dev);
+   int vsel, err;
+   int pmic_minuV = core_buck_table[0];
+   int pmic_maxuV = core_buck_table[ARRAY_SIZE(core_buck_table) - 1];
+
+   if (minuV  pmic_minuV || minuV  pmic_maxuV)
+   return -EINVAL;
+   if (maxuV  pmic_minuV || maxuV  pmic_maxuV)
+   return -EINVAL;
+
+   for (vsel = 0; vsel  ARRAY_SIZE(core_buck_table); vsel++) {
+   int uV = core_buck_table[vsel];
+
+   if (minuV = uV  uV = maxuV)
+   break;
+   }
+
+   if (vsel == ARRAY_SIZE(core_buck_table))
+   return -EINVAL;
+
+   mutex_lock(pmic-mtx);
+
+   err = i2c_smbus_write_byte(pmic-client, vsel);
+   if (err  0)
+   dev_err(pmic-client-dev, Error setting voltage\n);
+
+   mutex_unlock(pmic-mtx);
+   return err;
+}
+
+static int isl6271a_list_voltage(struct regulator_dev *dev, unsigned selector)
+{
+   return core_buck_table[selector];
+}
+
+static struct regulator_ops isl_core_ops = {
+   .get_voltage= isl6271a_get_voltage,
+   .set_voltage= isl6271a_set_voltage,
+   .list_voltage   = isl6271a_list_voltage,
+};
+
+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;
+

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