Messages in this thread | | | Date | Fri, 12 Dec 2014 17:15:36 +0100 | From | Nikolaus Schulz <> | Subject | Re: [PATCH v2 2/2] iio: add driver for the TI DAC8554 |
| |
On Fri, Dec 12, 2014 at 11:57:08AM +0000, Jonathan Cameron wrote: > On 06/12/14 11:36, Hartmut Knaack wrote: > > Nikolaus Schulz schrieb am 24.11.2014 um 20:50: > >> The TI DAC8554 is a quad-channel Digital-to-Analog Converter with an SPI > >> interface. > >> > >> Changes in v2: > >> * Use DMA-safe buffer for SPI transfer > >> * Normalize powerdown_mode name "hi-z" to "three_state" as per > >> ABI/testing/sysfs-bus-iio > >> * Register device late in probe function > >> * Avoid powerdown broadcast update, which touches all DAC on the bus > > There are a few issues left, please see my comments inline. > 1 little one from me... > >> > >> Signed-off-by: Nikolaus Schulz <nikolaus.schulz@avionic-design.de> > >> --- > >> drivers/iio/dac/Kconfig | 10 ++ > >> drivers/iio/dac/Makefile | 1 + > >> drivers/iio/dac/ti-dac8554.c | 374 +++++++++++++++++++++++++++++++++++++++++++ > >> 3 files changed, 385 insertions(+) > >> create mode 100644 drivers/iio/dac/ti-dac8554.c [...] > >> diff --git a/drivers/iio/dac/ti-dac8554.c b/drivers/iio/dac/ti-dac8554.c > >> new file mode 100644 > >> index 0000000..fca751f > >> --- /dev/null > >> +++ b/drivers/iio/dac/ti-dac8554.c [...] > >> +static int dac8554_powerdown(struct dac8554_state *st, > >> + u8 powerdown_mode) > >> +{ > >> + int chan, cmd, ret; > >> + > >> + for (chan = DAC8554_DAC_CHANNELS-1; chan >= 0; --chan) { > > Please mind spaces around operators. > >> + cmd = chan ? DAC8554_CMD_STORE_DAC_N > >> + : DAC8554_CMD_STORE_DAC_N_UPDATE_ALL; > >> + ret = dac8554_spi_write(st, cmd, chan, > >> + DAC8554_PWRDN_TO_SR(powerdown_mode)); > >> + if (ret) > >> + return ret; > >> + } > >> + > >> + memset(st->powerdown_mode, powerdown_mode, sizeof(st->powerdown_mode)); > >> + memset(st->powerdown, true, sizeof(st->powerdown)); > Whilst this probably works, I think for the boolean case it is less > than entirely obvious, so probably better to have a simple loop.
Yeah, I agree it's better to keep it as simple and obvious as possible. Fixed in v3.
> >> + > >> + return 0; > >> +}
-- Avionic Design GmbH Nikolaus Schulz Wragekamp 10 D-22397 Hamburg Germany
Tel.: +49 40 88187-163 Fax: +49 40 88187-150 Email: nikolaus.schulz@avionic-design.de
Avionic Design GmbH Amtsgericht Hamburg HRB 82598 Geschäftsführung: Cornelis Broers Ust.-Ident-Nr.: DE813378254
| |