The AD5398 and AD5821 are single 10-bit DAC with 120 mA output current
sink capability. They feature an internal reference and operates from
a single 2.7 V to 5.5 V supply.

This driver supports both the AD5398 and the AD5821.  It adapts into the
voltage and current framework.


Signed-off-by: Sonic Zhang <sonic.zhang@analog.com>
---
 drivers/regulator/Kconfig  |    6 +
 drivers/regulator/Makefile |    1 +
 drivers/regulator/ad5398.c |  285 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 292 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/ad5398.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 04f2e08..2ade09c 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_AD5398
+	tristate "Ananlog Devices AD5398/AD5821 regulators"
+	depends on I2C
+	help
+	  This driver supports AD5398 and AD5821 current regulator chips.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 4e7feec..c256668 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_REGULATOR_FIXED_VOLTAGE) += fixed.o
 obj-$(CONFIG_REGULATOR_VIRTUAL_CONSUMER) += virtual.o
 obj-$(CONFIG_REGULATOR_USERSPACE_CONSUMER) += userspace-consumer.o
 
+obj-$(CONFIG_REGULATOR_AD5398) += ad5398.o
 obj-$(CONFIG_REGULATOR_BQ24022) += bq24022.o
 obj-$(CONFIG_REGULATOR_DUMMY) += dummy.o
 obj-$(CONFIG_REGULATOR_LP3971) += lp3971.o
diff --git a/drivers/regulator/ad5398.c b/drivers/regulator/ad5398.c
new file mode 100644
index 0000000..4d6cb26
--- /dev/null
+++ b/drivers/regulator/ad5398.c
@@ -0,0 +1,285 @@
+/*
+ * ad5398.c  --  Voltage and current regulation for AD5398 and AD5821
+ *
+ * Copyright 2010 Analog Devices Inc.
+ *
+ * Enter bugs at http://blackfin.uclinux.org/
+ *
+ * Licensed under the GPL-2 or later.
+ */
+#include ...
[ message continues ]

the "From:" doesnt match your s-o-b tag ...

also, this dropped my s-o-b:






not specific to this driver, but it looks like regulator_ops and





does the regulator core take care of displaying a notice ?  if not,
i'd make this dev_info().  also, this should be "registered", not

should there be a MODULE_ALIAS() ?
-mike
--

[ message continues ]

Does this need to be matched? I prefer to discuss via gmail account


--

[ message continues ]

so set the From: in the body as the first line like git-send-email does it:
From: <author info>

<commit message>

<other tags>
---
  <diffstat>

how does it make any difference to the probe code what each id is
pointing to ?  it isnt comparing the private data pointers to any
other storage pointers.

from what i can see, this should give the same exact behavior:
static const struct ad5398_current_data_format df_10_4 = {10, 4};
static const struct i2c_device_id ad5398_id[] = {
       { "ad5398", (kernel_ulong_t)&df_10_4 },
       { "ad5821", (kernel_ulong_t)&df_10_4 },
-mike
--

[ message continues ]

Yes, the behavior is the same for ad5398 and ad5821. But, if more
chips are enabled in this driver, they may differ.
This line is used as an example for future chips.

Sonic
--

[ message continues ]

seems like a weak reason for otherwise useless overhead.  especially
considering my simpler example should also be pretty obvious to extend
for future drivers should the need arise.  you're a smart guy and dont
need things spelled out explicitly.
-mike
--

[ message continues ]

A message will be displayed if the device actually has any constraints
(and is therefore in use) so if it makes any difference to the system

Possibly, though it's unlikely that actual systems will ever build a
regulator they're using as a module.
--

[ message continues ]

Should this not be a be16_to_cpu() or similar rather than an
unconditional byte swap?  Presumably the byte swap is not going to be
needed if the CPU has the same endianness as the CPU that the system is

The reason I was querying this code on the original submission is that
it is more normal to write this as something like

    data = selector | (data & ~chip->current_mask);

ie, to write the code as "set these bits" rather than "preserve these
bits".  This is more clearly robust to the reader since it's clear that

This looks very wrong, especially given that you use the min_uA and
max_uA settings to scale the register values being written in to the
chip.  I would expect that either the limits would be fixed in the
silicon or (if they depend on things like the associated passives which
can be configured per-board) that there would be some other setting in
the platform data which specifies what's actually being changed.

The constraints being specified by the platform should not influence the
physical properties of the chip, they control which values are allowed
in a particular design (for example, saying that values over a given
limit are not allowed due to the limits of the hardware connected to the
regulator) and are separate to what the chip is capable of.
--

[ message continues ]

On Wed, Jun 2, 2010 at 6:33 PM, Mark Brown

I made a mistake to mix simple i2c transfer and smbus i2c transfer


I will predefine the chip physical min max values in the driver and add user
defined limitation based on both initial constraints and chip spec.


Sonic
--

[ message continues ]