add ltc2672 and ltc2664 #2456
add ltc2672 and ltc2664 #2456
Conversation
kseerp
requested review from
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
Add documentation for ltc2664 and ltc2672. Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC LTC2672 5 channel, 16 bit Current Output Softspan DAC Co-developed-by: Michael Hennerich <michael.hennerich@analog.com> Signed-off-by: Michael Hennerich <michael.hennerich@analog.com> Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
Add entry for the LTC2664 driver. Signed-off-by: Kim Seer Paller <kimseer.paller@analog.com>
|
appreciate all the reviews, here are the changelogs for v2 and changes have been tested on relevant hardware
|
| Set the channel as a toggle enabled channel. Toggle operation | ||
| enables fast switching of a DAC output between two different DAC | ||
| codes without any SPI transaction. | ||
| type: boolean |
There was a problem hiding this comment.
The toggle mode seems to be similar to both devices. Maybe something like this will make things better.
Just something you may wanna try before sending upstream.
|
|
||
| /* internal reference */ | ||
| if (chip_info->id == LTC2672) | ||
| st->vref = 1250; |
There was a problem hiding this comment.
Hmm and in this case why is vref_reg not NULL? We did not got one and are we still going to configure it in ltc2664_setup()?
| /* | ||
| * Duplicate the default channel configuration as it can change during | ||
| * @ltc2664_channel_config() | ||
| */ |
There was a problem hiding this comment.
nit: no need for this new line
|
|
||
| st->rfsadj = 20000; | ||
| ret = device_property_read_u32(dev, "adi,rfsadj-ohms", &st->rfsadj); | ||
| if (!ret) { |
There was a problem hiding this comment.
After a second thought, I think (to be on the safe side) having a second check on the device id would make sense... So, we make sure no one skips bindings validations and uses properties o devices that don't support them . I would just another if() before checking the property values. Ditto for all specific properties...
| if (ret) | ||
| break; | ||
| st->global_toggle = en; | ||
| break; |
There was a problem hiding this comment.
I would just return len in here..
| case LTC2664_GLOBAL_TOGGLE: | ||
| ret = regmap_write(st->regmap, LTC2664_CMD_GLOBAL_TOGGLE, en); | ||
| if (ret) | ||
| break; |
There was a problem hiding this comment.
And return ret here so the last return len at the end of the function is not needed
| return -EINVAL; | ||
| } | ||
|
|
||
| return sysfs_emit(buf, "%u\n", val); |
There was a problem hiding this comment.
Same thing... No need for val. Return sysfs_emit() on every case directly
| static int ltc2664_offset_get(const struct ltc2664_state *st, int c, int *val) | ||
| { | ||
| const struct ltc2664_chan *chan = &st->channels[c]; | ||
| const int (*span_helper)[2] = st->chip_info->span_helper; |
There was a problem hiding this comment.
nit: I would likely use st->chip_info->span_helper directly
| ltc2664_reg_bool_get, ltc2664_reg_bool_set), | ||
| LTC2664_CHAN_EXT_INFO("toggle_en", LTC2664_TOGGLE_EN, | ||
| IIO_SEPARATE, ltc2664_reg_bool_get, | ||
| ltc2664_reg_bool_set), |
There was a problem hiding this comment.
You're missing the ABI docs. I guess you can copy it from ltc2668.
LTC2664 4 channel, 16 bit Voltage Output SoftSpan DAC
LTC2672 5 channel, 16 bit Current Output Softspan DAC
since ltc2664 and ltc2672 have the same registers/softspan, thus leveraging the use of this PR #2408 and there's plenty of shared codes
i'll plan to utilize also the automatic cleanup for mutex but my current kernel used does not yet support that