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?
>
> 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
