Message ID | 20241203111314.2420473-9-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
---|---|
State | New |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Series | iio: adc: rzg2l_adc: Add support for RZ/G3S | expand |
On Tue, 3 Dec 2024 13:13:08 +0200 Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > 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@bp.renesas.com> > --- > 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. > }; > > @@ -392,6 +410,15 @@ static int rzg2l_adc_probe(struct platform_device *pdev) > > adc = iio_priv(indio_dev); > > + adc->hw_params = device_get_match_data(dev); > + if (!adc->hw_params) > + return -EINVAL; > + > + adc->last_val = devm_kcalloc(dev, adc->hw_params->num_channels, > + sizeof(*adc->last_val), GFP_KERNEL); > + if (!adc->last_val) > + return -ENOMEM; > + > ret = rzg2l_adc_parse_properties(pdev, adc); > if (ret) > return ret; > @@ -449,8 +476,17 @@ static int rzg2l_adc_probe(struct platform_device *pdev) > return devm_iio_device_register(dev, indio_dev); > }
Hi Jonathan, On Tue, Dec 3, 2024 at 9:09 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Tue, 3 Dec 2024 13:13:08 +0200 > Claudiu <claudiu.beznea@tuxon.dev> 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@bp.renesas.com> > > --- > > 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
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]; }; static const char * const rzg2l_adc_channel_name[] = { @@ -153,6 +163,7 @@ static void rzg2l_set_trigger(struct rzg2l_adc *adc) static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch) { + const struct rzg2l_adc_hw_params *hw_params = adc->hw_params; u32 reg; if (rzg2l_adc_readl(adc, RZG2L_ADM(0)) & RZG2L_ADM0_ADBSY) @@ -162,7 +173,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch) /* Select analog input channel subjected to conversion. */ reg = rzg2l_adc_readl(adc, RZG2L_ADM(2)); - reg &= ~RZG2L_ADM2_CHSEL_MASK; + reg &= ~GENMASK(hw_params->num_channels - 1, 0); reg |= BIT(ch); rzg2l_adc_writel(adc, RZG2L_ADM(2), reg); @@ -174,7 +185,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch) */ reg = rzg2l_adc_readl(adc, RZG2L_ADINT); reg &= ~RZG2L_ADINT_INTS; - reg &= ~RZG2L_ADINT_INTEN_MASK; + reg &= ~hw_params->adint_inten_mask; reg |= (RZG2L_ADINT_CSEEN | BIT(ch)); rzg2l_adc_writel(adc, RZG2L_ADINT, reg); @@ -183,6 +194,7 @@ static int rzg2l_adc_conversion_setup(struct rzg2l_adc *adc, u8 ch) static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc, u8 ch) { + const struct rzg2l_adc_hw_params *hw_params = adc->hw_params; struct device *dev = indio_dev->dev.parent; int ret; @@ -200,7 +212,7 @@ static int rzg2l_adc_conversion(struct iio_dev *indio_dev, struct rzg2l_adc *adc if (!wait_for_completion_timeout(&adc->completion, RZG2L_ADC_TIMEOUT)) { rzg2l_adc_writel(adc, RZG2L_ADINT, - rzg2l_adc_readl(adc, RZG2L_ADINT) & ~RZG2L_ADINT_INTEN_MASK); + rzg2l_adc_readl(adc, RZG2L_ADINT) & ~hw_params->adint_inten_mask); ret = -ETIMEDOUT; } @@ -217,8 +229,8 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev, int *val, int *val2, long mask) { struct rzg2l_adc *adc = iio_priv(indio_dev); + u8 ch = chan->channel; int ret; - u8 ch; switch (mask) { case IIO_CHAN_INFO_RAW: @@ -226,7 +238,6 @@ static int rzg2l_adc_read_raw(struct iio_dev *indio_dev, return -EINVAL; mutex_lock(&adc->lock); - ch = chan->channel & RZG2L_ADC_CHN_MASK; ret = rzg2l_adc_conversion(indio_dev, adc, ch); if (!ret) *val = adc->last_val[ch]; @@ -254,6 +265,7 @@ static const struct iio_info rzg2l_adc_iio_info = { static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id) { struct rzg2l_adc *adc = dev_id; + const struct rzg2l_adc_hw_params *hw_params = adc->hw_params; unsigned long intst; u32 reg; int ch; @@ -266,11 +278,11 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id) return IRQ_HANDLED; } - intst = reg & RZG2L_ADSTS_INTST_MASK; + intst = reg & GENMASK(hw_params->num_channels - 1, 0); if (!intst) return IRQ_NONE; - for_each_set_bit(ch, &intst, RZG2L_ADC_MAX_CHANNELS) + for_each_set_bit(ch, &intst, hw_params->num_channels) adc->last_val[ch] = rzg2l_adc_readl(adc, RZG2L_ADCR(ch)) & RZG2L_ADCR_AD_MASK; /* clear the channel interrupt */ @@ -283,6 +295,7 @@ static irqreturn_t rzg2l_adc_isr(int irq, void *dev_id) static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l_adc *adc) { + const struct rzg2l_adc_hw_params *hw_params = adc->hw_params; struct iio_chan_spec *chan_array; struct rzg2l_adc_data *data; unsigned int channel; @@ -300,7 +313,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l return -ENODEV; } - if (num_channels > RZG2L_ADC_MAX_CHANNELS) { + if (num_channels > hw_params->num_channels) { dev_err(&pdev->dev, "num of channel children out of range\n"); return -EINVAL; } @@ -316,7 +329,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l if (ret) return ret; - if (channel >= RZG2L_ADC_MAX_CHANNELS) + if (channel >= hw_params->num_channels) return -EINVAL; chan_array[i].type = IIO_VOLTAGE; @@ -336,6 +349,7 @@ static int rzg2l_adc_parse_properties(struct platform_device *pdev, struct rzg2l static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc) { + const struct rzg2l_adc_hw_params *hw_params = adc->hw_params; u32 reg; int ret; @@ -353,11 +367,13 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc) if (ret) goto exit_hw_init; - /* Only division by 4 can be set */ - reg = rzg2l_adc_readl(adc, RZG2L_ADIVC); - reg &= ~RZG2L_ADIVC_DIVADC_MASK; - reg |= RZG2L_ADIVC_DIVADC_4; - rzg2l_adc_writel(adc, RZG2L_ADIVC, reg); + if (hw_params->adivc) { + /* Only division by 4 can be set */ + reg = rzg2l_adc_readl(adc, RZG2L_ADIVC); + reg &= ~RZG2L_ADIVC_DIVADC_MASK; + reg |= RZG2L_ADIVC_DIVADC_4; + rzg2l_adc_writel(adc, RZG2L_ADIVC, reg); + } /* * Setup AMD3 @@ -368,8 +384,10 @@ static int rzg2l_adc_hw_init(struct device *dev, struct rzg2l_adc *adc) reg = rzg2l_adc_readl(adc, RZG2L_ADM(3)); reg &= ~RZG2L_ADM3_ADIL_MASK; reg &= ~RZG2L_ADM3_ADCMP_MASK; - reg &= ~RZG2L_ADM3_ADSMP_MASK; - reg |= (RZG2L_ADM3_ADCMP_E | RZG2L_ADSMP_DEFAULT_SAMPLING); + reg &= ~hw_params->adsmp_mask; + reg |= FIELD_PREP(RZG2L_ADM3_ADCMP_MASK, hw_params->default_adcmp) | + hw_params->default_adsmp; + rzg2l_adc_writel(adc, RZG2L_ADM(3), reg); exit_hw_init: @@ -392,6 +410,15 @@ static int rzg2l_adc_probe(struct platform_device *pdev) adc = iio_priv(indio_dev); + adc->hw_params = device_get_match_data(dev); + if (!adc->hw_params) + return -EINVAL; + + adc->last_val = devm_kcalloc(dev, adc->hw_params->num_channels, + sizeof(*adc->last_val), GFP_KERNEL); + if (!adc->last_val) + return -ENOMEM; + ret = rzg2l_adc_parse_properties(pdev, adc); if (ret) return ret; @@ -449,8 +476,17 @@ static int rzg2l_adc_probe(struct platform_device *pdev) return devm_iio_device_register(dev, indio_dev); } +static const struct rzg2l_adc_hw_params rzg2l_hw_params = { + .num_channels = 8, + .default_adcmp = 0xe, + .default_adsmp = 0x578, + .adsmp_mask = GENMASK(15, 0), + .adint_inten_mask = GENMASK(7, 0), + .adivc = true +}; + static const struct of_device_id rzg2l_adc_match[] = { - { .compatible = "renesas,rzg2l-adc",}, + { .compatible = "renesas,rzg2l-adc", .data = &rzg2l_hw_params }, { /* sentinel */ } }; MODULE_DEVICE_TABLE(of, rzg2l_adc_match);