Re: [PATCH 3/7] iio: adc: sc27xx: structure adjuststment and optimization

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jan 13, 2022 at 9:54 AM Cixi Geng <gengcixi@xxxxxxxxx> wrote:
>
> Baolin Wang <baolin.wang7@xxxxxxxxx> 于2022年1月7日周五 15:03写道:
> >
> > On Thu, Jan 6, 2022 at 9:00 PM Cixi Geng <gengcixi@xxxxxxxxx> wrote:
> > >
> > > From: Cixi Geng <cixi.geng1@xxxxxxxxxx>
> > >
> > > Introduce one variant device data structure to be compatible
> > > with SC2731 PMIC since it has different scale and ratio calculation
> > > and so on.
> > >
> > > Signed-off-by: Yuming Zhu <yuming.zhu1@xxxxxxxxxx>
> > > Signed-off-by: Cixi Geng <cixi.geng1@xxxxxxxxxx>
> > > ---
> > >  drivers/iio/adc/sc27xx_adc.c | 94 ++++++++++++++++++++++++++++++------
> > >  1 file changed, 79 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/iio/adc/sc27xx_adc.c b/drivers/iio/adc/sc27xx_adc.c
> > > index aee076c8e2b1..d2712e54ee79 100644
> > > --- a/drivers/iio/adc/sc27xx_adc.c
> > > +++ b/drivers/iio/adc/sc27xx_adc.c
> > > @@ -12,9 +12,9 @@
> > >  #include <linux/slab.h>
> > >
> > >  /* PMIC global registers definition */
> > > -#define SC27XX_MODULE_EN               0xc08
> > > +#define SC2731_MODULE_EN               0xc08
> > >  #define SC27XX_MODULE_ADC_EN           BIT(5)
> > > -#define SC27XX_ARM_CLK_EN              0xc10
> > > +#define SC2731_ARM_CLK_EN              0xc10
> > >  #define SC27XX_CLK_ADC_EN              BIT(5)
> > >  #define SC27XX_CLK_ADC_CLK_EN          BIT(6)
> > >
> > > @@ -78,6 +78,23 @@ struct sc27xx_adc_data {
> > >         int channel_scale[SC27XX_ADC_CHANNEL_MAX];
> > >         u32 base;
> > >         int irq;
> > > +       const struct sc27xx_adc_variant_data *var_data;
> > > +};
> > > +
> > > +/*
> > > + * Since different PMICs of SC27xx series can have different
> > > + * address and ratio, we should save ratio config and base
> > > + * in the device data structure.
> > > + */
> > > +struct sc27xx_adc_variant_data {
> > > +       u32 module_en;
> > > +       u32 clk_en;
> > > +       u32 scale_shift;
> > > +       u32 scale_mask;
> > > +       const struct sc27xx_adc_linear_graph *bscale_cal;
> > > +       const struct sc27xx_adc_linear_graph *sscale_cal;
> > > +       void (*init_scale)(struct sc27xx_adc_data *data);
> > > +       int (*get_ratio)(int channel, int scale);
> > >  };
> > >
> > >  struct sc27xx_adc_linear_graph {
> > > @@ -103,6 +120,16 @@ static struct sc27xx_adc_linear_graph small_scale_graph = {
> > >         100, 341,
> > >  };
> > >
> > > +static const struct sc27xx_adc_linear_graph sc2731_big_scale_graph_calib = {
> > > +       4200, 850,
> > > +       3600, 728,
> > > +};
> > > +
> > > +static const struct sc27xx_adc_linear_graph sc2731_small_scale_graph_calib = {
> > > +       1000, 838,
> > > +       100, 84,
> > > +};
> >
> > The original big_scale_graph_calib and small_scale_graph_calib are for
> > SC2731 PMIC, why add new structure definition for SC2731?
> >
> > > +
> > >  static const struct sc27xx_adc_linear_graph big_scale_graph_calib = {
> > >         4200, 856,
> > >         3600, 733,
> > > @@ -130,11 +157,11 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
> > >         size_t len;
> > >
> > >         if (big_scale) {
> > > -               calib_graph = &big_scale_graph_calib;
> > > +               calib_graph = data->var_data->bscale_cal;
> > >                 graph = &big_scale_graph;
> > >                 cell_name = "big_scale_calib";
> > >         } else {
> > > -               calib_graph = &small_scale_graph_calib;
> > > +               calib_graph = data->var_data->sscale_cal;
> > >                 graph = &small_scale_graph;
> > >                 cell_name = "small_scale_calib";
> > >         }
> > > @@ -160,7 +187,7 @@ static int sc27xx_adc_scale_calibration(struct sc27xx_adc_data *data,
> > >         return 0;
> > >  }
> > >
> > > -static int sc27xx_adc_get_ratio(int channel, int scale)
> > > +static int sc2731_adc_get_ratio(int channel, int scale)
> > >  {
> > >         switch (channel) {
> > >         case 1:
> > > @@ -185,6 +212,21 @@ static int sc27xx_adc_get_ratio(int channel, int scale)
> > >         return SC27XX_VOLT_RATIO(1, 1);
> > >  }
> > >
> > > +/*
> > > + * According to the datasheet set specific value on some channel.
> > > + */
> > > +static void sc2731_adc_scale_init(struct sc27xx_adc_data *data)
> > > +{
> > > +       int i;
> > > +
> > > +       for (i = 0; i < SC27XX_ADC_CHANNEL_MAX; i++) {
> > > +               if (i == 5)
> > > +                       data->channel_scale[i] = 1;
> > > +               else
> > > +                       data->channel_scale[i] = 0;
> > > +       }
> > > +}
> >
> > This is unnecessary I think, please see sc27xx_adc_write_raw() that
> > can set the channel scale.
> Did you mean that all the PMIC's scale_init function should put into
> the sc27xx_adc_write_raw?

No.

> but the scale_init is all different by each PMIC, if implemented in
> the write_raw, will add a lot of
> if or switch_case branch

What I mean is we should follow the original method to set the channel
scale by iio_info. Please also refer to other drivers how ot handle
the channel scale.

-- 
Baolin Wang




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux