add ad3530r driver #2680
add ad3530r driver #2680
Conversation
kseerp
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
kseerp
force-pushed
the
dev/ad353xr
branch
4 times, most recently
from
de62301 to
fc99356
Compare
|
v2
|
drivers/iio/dac/ad3530r.c
Outdated
| st = iio_priv(indio_dev); | ||
| st->spi = spi; | ||
|
|
||
| mutex_init(&st->lock); |
There was a problem hiding this comment.
devm_mutex_init() when upstreaming
|
v3
bindings:
ad3530r:
|
kseerp
force-pushed
the
dev/ad353xr
branch
2 times, most recently
from
1a76933 to
46485a3
Compare
|
v4
will appreciate any reviews and feedback |
|
v5
|
There was a problem hiding this comment.
Some more notes from me... I would say to take my inputs, change what you agree with and send this upstream.
I would also recommend to improve your commit message on the bindings patch. Give a small description of the HW. Like it is now you'll most likely get some comments from maintainers because the message only state the obvious.
| required: | ||
| - compatible | ||
| - reg | ||
| - spi-max-frequency |
There was a problem hiding this comment.
odd to make this mandatory...
| const: 1 | ||
|
|
||
| '#size-cells': | ||
| const: 0 |
There was a problem hiding this comment.
drop the above... you don't have any child node
drivers/iio/dac/Kconfig
Outdated
| @@ -6,6 +6,16 @@ | |||
|
|
|||
| menu "Digital to analog converters" | |||
|
|
|||
| config AD3530R | |||
| tristate "Analog Devices AD3530R DAC driver" | |||
| depends on SPI_MASTER | |||
There was a problem hiding this comment.
you also need to select REGMAP_SPI
| { | ||
| struct ad3530r_state *st = iio_priv(indio_dev); | ||
|
|
||
| guard(mutex)(&st->lock); |
There was a problem hiding this comment.
need to include cleanup.h
drivers/iio/dac/ad3530r.c
Outdated
| ret = regmap_update_bits(st->regmap, AD3530R_OUTPUT_OPERATING_MODE_0, | ||
| GENMASK(chan->channel * 2 + 1, chan->channel * 2), | ||
| (powerdown ? st->chan[chan->channel].powerdown_mode : 0) << | ||
| (chan->channel * 2)); |
There was a problem hiding this comment.
not pretty... I would simplify this with some helper variables or even consider an helper function
| ((chan->channel - 4) * 2); | ||
| if (ret) | ||
| return ret; | ||
|
|
| struct ad3530r_state *st = iio_priv(indio_dev); | ||
|
|
||
| return regmap_write(st->regmap, AD3530R_MUX_OUT_SELECT, val); | ||
| } |
There was a problem hiding this comment.
up to you but I thought debugfd was going to be the path forward
|
|
||
| return regmap_update_bits(st->regmap, st->chip_info->sw_ldac_trig_reg, | ||
| AD3530R_SW_LDAC_TRIG_MASK, | ||
| FIELD_PREP(AD3530R_SW_LDAC_TRIG_MASK, 1)); |
drivers/iio/dac/ad3530r.c
Outdated
| for (i = 0; i < chip_info->num_channels; i++) { | ||
| st->channels[i] = ad3530r_channel_template; | ||
| st->channels[i].channel = i; | ||
| } |
There was a problem hiding this comment.
I wonder about the template usage... I'm not seeing anything being changed dynamically so it seems to be you could have a static channel definition.
There was a problem hiding this comment.
Ohh I have in mind another device in ad353xr series that will be released soon. It has specific feature that can be included as extended info and can be changed dynamically through that implementation above
Define muxout_select and muxout_select_available sysfs interface for the AD3530R and AD3531R DAC. Signed-off-by: Kim Seer Paller <[email protected]>
Add a new powerdown mode for DACs with 7.7kohm and 32kohm resistor to GND. Signed-off-by: Kim Seer Paller <[email protected]>
Document the AD3530R/AD3530, an 8-Channel, 16-bit Voltage Output DAC, and the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC. These devices include software-programmable gain controls that provide full-scale output spans of 2.5V or 5V for reference voltages of 2.5V. They operate from a single supply voltage range of 2.7V to 5.5V and are guaranteed to be monotonic by design. Additionally, these devices features a 2.5V, 5ppm/°C internal reference, which is disabled by default. Signed-off-by: Kim Seer Paller <[email protected]>
The AD3530R/AD3530 is an 8-Channel, 16-Bit Voltage Output DAC, while the AD3531R/AD3531 is a 4-Channel, 16-Bit Voltage Output DAC. These devices include software-programmable gain controls that provide full-scale output spans of 2.5V or 5V for reference voltages of 2.5V. They operate from a single supply voltage range of 2.7V to 5.5V and are guaranteed to be monotonic by design. Additionally, these devices features a 2.5V, 5ppm/°C internal reference, which is disabled by default. Signed-off-by: Kim Seer Paller <[email protected]>
Add entry for the AD3530R driver. Signed-off-by: Kim Seer Paller <[email protected]>
PR Description
AD3530R/AD3530 8-Channel, 16-Bit Voltage Output DAC
AD3531R/AD3531 4-Channel, 16-Bit Voltage Output DAC (unreleased)
PR Type
PR Checklist