Hi Jonathan,
On Tue, Dec 3, 2024 at 9:09 PM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
> On Tue, 3 Dec 2024 13:13:08 +0200
> Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
> > The ADC IP available on the RZ/G3S differs slightly from the one found on
> > the RZ/G2L. The identified differences are as follows:
> > - different number of channels (one being used for temperature conversion);
> > consequently, various registers differ
> > - different default sampling periods
> > - the RZ/G3S variant lacks the ADVIC register.
> >
> > To accommodate these differences, the rzg2l_adc driver has been updated by
> > introducing the struct rzg2l_adc_hw_params, which encapsulates the
> > hardware-specific differences between the IP variants. A pointer to an
> > object of type struct rzg2l_adc_hw_params is embedded in
> > struct rzg2l_adc_data.
> >
> > Additionally, the completion member of struct rzg2l_adc_data was relocated
> > to avoid potential padding, if any.
> >
> > The code has been adjusted to utilize hardware-specific parameters stored
> > in the new structure instead of relying on plain macros.
> >
> > The check of chan->channel in rzg2l_adc_read_raw() function, against the
> > driver specific mask was removed as the subsystem should have already
> > been done this before reaching the rzg2l_adc_read_raw() function.
> >
> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> > ---
> > drivers/iio/adc/rzg2l_adc.c | 92 ++++++++++++++++++++++++++-----------
> > 1 file changed, 64 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/iio/adc/rzg2l_adc.c b/drivers/iio/adc/rzg2l_adc.c
> > index fda8b42ded81..aff41152ebf8 100644
> > --- a/drivers/iio/adc/rzg2l_adc.c
> > +++ b/drivers/iio/adc/rzg2l_adc.c
> > @@ -32,20 +32,15 @@
> > #define RZG2L_ADM1_MS BIT(2)
> > #define RZG2L_ADM1_BS BIT(4)
> > #define RZG2L_ADM1_EGA_MASK GENMASK(13, 12)
> > -#define RZG2L_ADM2_CHSEL_MASK GENMASK(7, 0)
> > #define RZG2L_ADM3_ADIL_MASK GENMASK(31, 24)
> > #define RZG2L_ADM3_ADCMP_MASK GENMASK(23, 16)
> > -#define RZG2L_ADM3_ADCMP_E FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, 0xe)
> > -#define RZG2L_ADM3_ADSMP_MASK GENMASK(15, 0)
> >
> > #define RZG2L_ADINT 0x20
> > -#define RZG2L_ADINT_INTEN_MASK GENMASK(7, 0)
> > #define RZG2L_ADINT_CSEEN BIT(16)
> > #define RZG2L_ADINT_INTS BIT(31)
> >
> > #define RZG2L_ADSTS 0x24
> > #define RZG2L_ADSTS_CSEST BIT(16)
> > -#define RZG2L_ADSTS_INTST_MASK GENMASK(7, 0)
> >
> > #define RZG2L_ADIVC 0x28
> > #define RZG2L_ADIVC_DIVADC_MASK GENMASK(8, 0)
> > @@ -56,12 +51,26 @@
> > #define RZG2L_ADCR(n) (0x30 + ((n) * 0x4))
> > #define RZG2L_ADCR_AD_MASK GENMASK(11, 0)
> >
> > -#define RZG2L_ADSMP_DEFAULT_SAMPLING 0x578
> > -
> > -#define RZG2L_ADC_MAX_CHANNELS 8
> > -#define RZG2L_ADC_CHN_MASK 0x7
> > #define RZG2L_ADC_TIMEOUT usecs_to_jiffies(1 * 4)
> >
> > +/**
> > + * struct rzg2l_adc_hw_params - ADC hardware specific parameters
> > + * @default_adsmp: default ADC sampling period (see ADM3 register)
> > + * @adsmp_mask: ADC sampling period mask (see ADM3 register)
> > + * @adint_inten_mask: conversion end interrupt mask (see ADINT register)
> > + * @default_adcmp: default ADC cmp (see ADM3 register)
> > + * @num_channels: number of supported channels
> > + * @adivc: specifies if ADVIC register is available
> > + */
> > +struct rzg2l_adc_hw_params {
> > + u16 default_adsmp;
> > + u16 adsmp_mask;
> > + u16 adint_inten_mask;
> > + u8 default_adcmp;
> > + u8 num_channels;
> > + bool adivc;
> > +};
> > +
> > struct rzg2l_adc_data {
> > const struct iio_chan_spec *channels;
> > u8 num_channels;
> > @@ -71,10 +80,11 @@ struct rzg2l_adc {
> > void __iomem *base;
> > struct reset_control *presetn;
> > struct reset_control *adrstn;
> > - struct completion completion;
> > const struct rzg2l_adc_data *data;
> > + const struct rzg2l_adc_hw_params *hw_params;
> > + u16 *last_val;
> > + struct completion completion;
> > struct mutex lock;
> > - u16 last_val[RZG2L_ADC_MAX_CHANNELS];
>
> Just make this big enough for the max device. Chances are it will make little or
> no difference to this allocation and nice to avoid the dynamic part.
>
> Feel free to add a runtime check to make sure this is big enough to avoid any
> future problems with forgetting to update it.
Flexible array member and the new __counted_by() attribute?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
