Re: [PATCH 7/8] staging:iio:dac:ad5446: Consolidate store_sample and store_pwr_down functions

Jonathan Cameron <jic23@xxxxxxxxx> · Tue, 24 Apr 2012 10:10:37 +0100

On 4/24/2012 10:11 AM, Lars-Peter Clausen wrote:
On 04/24/2012 10:06 AM, Jonathan Cameron wrote:
On 4/23/2012 6:51 PM, Lars-Peter Clausen wrote:
The devices supported by this drivers only have a single shift register,
which
contains both the power down mode and the output sample. So writing the power
down mode and the output sample can be done by the same function. The two
power
down bits are always placed ontop of the msb of the output sample, so we can
easily calculate their position by adding the channels shift to the channels
realbits.
I wonder if the change in logic here slightly hides what is going on.
Perhaps simply renaming store_sample to something that indicates it's dual
purpose
will make things clearer in the resulting code?
I had initially renamed it to simply 'write', but well reverted it again. Do
you think renaming it to 'write' will work?
yeah.

Also does this mean you can kill of store_pwr_down in the structure?
Given nothing sets it this should be fine!

Uhm, yes, that was actually the plan, looks like I missed it though. Good catch.

Thanks,
- Lars

Signed-off-by: Lars-Peter Clausen<lars@xxxxxxxxxx>
---
   drivers/staging/iio/dac/ad5446.c |   38
++++++++++----------------------------
   1 file changed, 10 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/iio/dac/ad5446.c
b/drivers/staging/iio/dac/ad5446.c
index c767024..90461b2 100644
--- a/drivers/staging/iio/dac/ad5446.c
+++ b/drivers/staging/iio/dac/ad5446.c
@@ -31,21 +31,6 @@ static void ad5446_store_sample(struct ad5446_state
*st, unsigned val)

   static void ad5660_store_sample(struct ad5446_state *st, unsigned val)
   {
-    val |= AD5660_LOAD;
-    st->data.d24[0] = (val>>   16)&   0xFF;
-    st->data.d24[1] = (val>>   8)&   0xFF;
-    st->data.d24[2] = val&   0xFF;
-}
-
-static void ad5620_store_pwr_down(struct ad5446_state *st, unsigned mode)
-{
-    st->data.d16 = cpu_to_be16(mode<<   14);
-}
-
-static void ad5660_store_pwr_down(struct ad5446_state *st, unsigned mode)
-{
-    unsigned val = mode<<   16;
-
       st->data.d24[0] = (val>>   16)&   0xFF;
       st->data.d24[1] = (val>>   8)&   0xFF;
       st->data.d24[2] = val&   0xFF;
@@ -105,6 +90,8 @@ static ssize_t ad5446_write_dac_powerdown(struct
iio_dev *indio_dev,
                           const char *buf, size_t len)
   {
       struct ad5446_state *st = iio_priv(indio_dev);
+    unsigned int shift;
+    unsigned int val;
       bool powerdown;
       int ret;

@@ -115,10 +102,14 @@ static ssize_t ad5446_write_dac_powerdown(struct
iio_dev *indio_dev,
       mutex_lock(&indio_dev->mlock);
       st->pwr_down = powerdown;

-    if (st->pwr_down)
-        st->chip_info->store_pwr_down(st, st->pwr_down_mode);
-    else
-        st->chip_info->store_sample(st, st->cached_val);
+    if (st->pwr_down) {
+        shift = chan->scan_type.realbits + chan->scan_type.shift;
+        val = st->pwr_down_mode<<   shift;
+    } else {
+        val = st->cached_val;
+    }
+
+    st->chip_info->store_sample(st, val);

       ret = spi_sync(st->spi,&st->msg);
       mutex_unlock(&indio_dev->mlock);
@@ -186,53 +177,44 @@ static const struct ad5446_chip_info
ad5446_chip_info_tbl[] = {
       [ID_AD5601] = {
           .channel = AD5446_CHANNEL_POWERDOWN(8, 16, 6),
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5611] = {
           .channel = AD5446_CHANNEL_POWERDOWN(10, 16, 4),
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5621] = {
           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5620_2500] = {
           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
           .int_vref_mv = 2500,
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5620_1250] = {
           .channel = AD5446_CHANNEL_POWERDOWN(12, 16, 2),
           .int_vref_mv = 1250,
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5640_2500] = {
           .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
           .int_vref_mv = 2500,
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5640_1250] = {
           .channel = AD5446_CHANNEL_POWERDOWN(14, 16, 0),
           .int_vref_mv = 1250,
           .store_sample = ad5446_store_sample,
-        .store_pwr_down = ad5620_store_pwr_down,
       },
       [ID_AD5660_2500] = {
           .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
           .int_vref_mv = 2500,
           .store_sample = ad5660_store_sample,
-        .store_pwr_down = ad5660_store_pwr_down,
       },
       [ID_AD5660_1250] = {
           .channel = AD5446_CHANNEL_POWERDOWN(16, 16, 0),
           .int_vref_mv = 1250,
           .store_sample = ad5660_store_sample,
-        .store_pwr_down = ad5660_store_pwr_down,
       },
   };


--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html





