RE: [PATCH v5 2/2] iio: dac: ad5706r: Add support for AD5706R DAC

[Torreno, Alexis Czezar]

> > +static int ad5706r_regmap_write(void *context, const void *data,
> > +size_t count) {
> > +	struct ad5706r_state *st = context;
> > +	unsigned int num_bytes, val;
> > +	u16 reg;
> > +
> > +	reg = get_unaligned_be16(data);
> > +	num_bytes = ad5706r_reg_len(reg);
> > +
> > +	struct spi_transfer xfer = {
> > +		.tx_buf = st->tx_buf,
> > +		.len = num_bytes + 2,
> > +	};
> 
> > +	val = get_unaligned_be32(data);
> 
> Is it safe? The data is void *, no size of it is counted here...
> 

The regmap regbit and valbits are both 16, so data here should always be
4 bytes. Though I do agree putting this

if (count != 4) return -EINVAL;

is still safer.

> > +	put_unaligned_be32(val, st->tx_buf);

...

> > +	/* Full duplex, device responds immediately after command */
> > +	struct spi_transfer xfer = {
> > +		.tx_buf = st->tx_buf,
> > +		.rx_buf = st->rx_buf,
> > +		.len = 2 + num_bytes,
> > +	};
> > +
> > +	cmd = AD5706R_RD_MASK | (reg & AD5706R_ADDR_MASK);
> > +	put_unaligned_be16(cmd, st->tx_buf);
> 
> For the consistency's sake use &st->tx_buf[0].

Ok, I'll also apply this to the write func above with:
put_unaligned_be32(val, &st->tx_buf[0]);

> 
> > +	put_unaligned_be16(0, &st->tx_buf[2]);
 
...

> > +#define AD5706R_CHAN(_channel) {				\
> > +	.type = IIO_CURRENT,					\
> > +	.info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |		\
> > +			      BIT(IIO_CHAN_INFO_SCALE),	\
> 
> Missing indentation at the end with tabs.
> 

Got it, 

Regards,
Alexis