| Message ID | 20250319-wip-bl-ad3552r-fixes-v2-1-2656bdd6778e@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2] iio: dac: ad3552r-hs: add debugfs reg access | expand |
On 3/19/25 8:30 AM, Angelo Dureghello wrote: > From: Angelo Dureghello <adureghello@baylibre.com> > > Add debugfs register access. > Forgot to pick up Nuno's review tag or explain why not. > Signed-off-by: Angelo Dureghello <adureghello@baylibre.com> > --- > Changes in v2: > - set reg size setup as inline. > - Link to v1: https://lore.kernel.org/r/20250319-wip-bl-ad3552r-fixes-v1-1-cf10d6fae52a@baylibre.com > --- > drivers/iio/dac/ad3552r-hs.c | 26 ++++++++++++++++++++++++++ > drivers/iio/dac/ad3552r.h | 2 ++ > 2 files changed, 28 insertions(+) > > diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c > index cd8dabb60c5548780f0fce5d1b68c494cd71321d..fdea9984547ae338a51c4671024133be82ed854f 100644 > --- a/drivers/iio/dac/ad3552r-hs.c > +++ b/drivers/iio/dac/ad3552r-hs.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/bitfield.h> > +#include <linux/debugfs.h> Is this header actually needed? > #include <linux/delay.h> > #include <linux/gpio/consumer.h> > #include <linux/iio/backend.h> > @@ -464,6 +465,30 @@ static int ad3552r_hs_setup_custom_gain(struct ad3552r_hs_state *st, > gain, 1); > } > > +static int ad3552r_hs_reg_access(struct iio_dev *indio_dev, unsigned int reg, > + unsigned int writeval, unsigned int *readval) > +{ > + struct ad3552r_hs_state *st = iio_priv(indio_dev); > + int size_xfer, max_reg_addr; > + > + max_reg_addr = (st->model_data->num_hw_channels == 2) ? > + AD3552R_REG_ADDR_MAX : AD3551R_REG_ADDR_MAX; Might as well add max reg to the model_data struct and read it directly instead of inferring it from other info. > + > + if (reg > max_reg_addr) > + return -EINVAL; > + > + /* > + * There is no 3 or 4 bytes r/w len possible in HDL, so keeping 2 > + * also for the 24bit area. > + */ > + size_xfer = (reg > AD3552R_SECONDARY_REGION_START) ? 2 : 1; If we are reading both bytes of a 16-bit register at the same time, we should only allow reading the lower of the two addresses, otherwise reading the high register address could return 1 byte from one register and 1 byte from another register. And if we can't read the 24-bit and 32-bit registers all at once, I think we should read them as 8-bit instead of 16-bit because the 24-bit registers are not 16-bit aligned. Or to keep it consistent, just allow accessing everything as 8-bit registers. > + > + if (readval) > + return ad3552r_hs_reg_read(st, reg, readval, size_xfer); > + > + return st->data->bus_reg_write(st->back, reg, writeval, size_xfer); > +}
diff --git a/drivers/iio/dac/ad3552r-hs.c b/drivers/iio/dac/ad3552r-hs.c index cd8dabb60c5548780f0fce5d1b68c494cd71321d..fdea9984547ae338a51c4671024133be82ed854f 100644 --- a/drivers/iio/dac/ad3552r-hs.c +++ b/drivers/iio/dac/ad3552r-hs.c @@ -7,6 +7,7 @@ */ #include <linux/bitfield.h> +#include <linux/debugfs.h> #include <linux/delay.h> #include <linux/gpio/consumer.h> #include <linux/iio/backend.h> @@ -464,6 +465,30 @@ static int ad3552r_hs_setup_custom_gain(struct ad3552r_hs_state *st, gain, 1); } +static int ad3552r_hs_reg_access(struct iio_dev *indio_dev, unsigned int reg, + unsigned int writeval, unsigned int *readval) +{ + struct ad3552r_hs_state *st = iio_priv(indio_dev); + int size_xfer, max_reg_addr; + + max_reg_addr = (st->model_data->num_hw_channels == 2) ? + AD3552R_REG_ADDR_MAX : AD3551R_REG_ADDR_MAX; + + if (reg > max_reg_addr) + return -EINVAL; + + /* + * There is no 3 or 4 bytes r/w len possible in HDL, so keeping 2 + * also for the 24bit area. + */ + size_xfer = (reg > AD3552R_SECONDARY_REGION_START) ? 2 : 1; + + if (readval) + return ad3552r_hs_reg_read(st, reg, readval, size_xfer); + + return st->data->bus_reg_write(st->back, reg, writeval, size_xfer); +} + static int ad3552r_hs_setup(struct ad3552r_hs_state *st) { u16 id; @@ -639,6 +664,7 @@ static const struct iio_chan_spec ad3552r_hs_channels[] = { static const struct iio_info ad3552r_hs_info = { .read_raw = &ad3552r_hs_read_raw, .write_raw = &ad3552r_hs_write_raw, + .debugfs_reg_access = &ad3552r_hs_reg_access, }; static int ad3552r_hs_probe(struct platform_device *pdev) diff --git a/drivers/iio/dac/ad3552r.h b/drivers/iio/dac/ad3552r.h index 768fa264d39e9e6d517aeb4098382e072f153543..69ce96f132cdb353d2f140939c534586cb791aee 100644 --- a/drivers/iio/dac/ad3552r.h +++ b/drivers/iio/dac/ad3552r.h @@ -113,6 +113,8 @@ #define AD3552R_REG_ADDR_INPUT_PAGE_MASK_24B 0x44 #define AD3552R_REG_ADDR_SW_LDAC_24B 0x45 #define AD3552R_REG_ADDR_CH_INPUT_24B(ch) (0x4B - (1 - (ch)) * 3) +#define AD3551R_REG_ADDR_MAX 0x46 +#define AD3552R_REG_ADDR_MAX 0x49 #define AD3552R_MAX_CH 2 #define AD3552R_MASK_CH(ch) BIT(ch)