Re: [PATCH 2/4] iio: mxs-lradc: add scale attribute to channels

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

 



Dear Hector Palacios,

> Dear Marek,
> 
> On 07/05/2013 01:41 PM, Marek Vasut wrote:
> > Dear Hector Palacios,
> > 
> >> Some LRADC channels have a fixed pre-divider, and all can have enabled
> >> an optional divisor by two which allows a maximum input voltage of
> >> VDDIO - 50mV.
> >> 
> >> This patch
> >> 
> >>   - adds the scaling info flag to all channels
> >>   - adds a lookup table with max reference voltage per channel
> >>   
> >>     (where the fixed pre-dividers apply)
> >>   
> >>   - allows to read the scaling attribute (computed from the Vref)
> >> 
> >> Signed-off-by: Hector Palacios <hector.palacios@xxxxxxxx>
> >> ---
> >> 
> >>   drivers/staging/iio/adc/mxs-lradc.c | 122
> >> 
> >> +++++++++++++++++++++++++++--------- 1 file changed, 93 insertions(+),
> >> 29 deletions(-)
> >> 
> >> diff --git a/drivers/staging/iio/adc/mxs-lradc.c
> >> b/drivers/staging/iio/adc/mxs-lradc.c index d65a594..012c42e 100644
> >> --- a/drivers/staging/iio/adc/mxs-lradc.c
> >> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> >> @@ -107,19 +107,63 @@ static const char * const mx28_lradc_irq_names[] =
> >> {
> >> 
> >>   	"mxs-lradc-button1",
> >>   
> >>   };
> >> 
> >> +/*
> >> + * Max reference voltage (mV) for each channel
> >> + */
> >> +static const int mx23_vref_mv[LRADC_MAX_TOTAL_CHANS] = {
> >> +	1850,		/* CH0 */
> >> +	1850,		/* CH1 */
> >> +	1850,		/* CH2 */
> >> +	1850,		/* CH3 */
> >> +	1850,		/* CH4 */
> >> +	1850,		/* CH5 */
> >> +	1850 * 2,	/* CH6 VDDIO */
> >> +	1850 * 4,	/* CH7 VBATT */
> >> +	1850,		/* CH8 Temp sense 0 */
> >> +	1850,		/* CH9 Temp sense 1 */
> >> +	1850,		/* CH10 */
> >> +	1850,		/* CH11 */
> >> +	1850,		/* CH12 USB_DP */
> >> +	1850,		/* CH13 USB_DN */
> >> +	1850,		/* CH14 VBG */
> >> +	1850 * 4,	/* CH15 VDD5V */
> >> +};
> >> +
> >> +static const int mx28_vref_mv[LRADC_MAX_TOTAL_CHANS] = {
> >> +	1850,		/* CH0 */
> >> +	1850,		/* CH1 */
> >> +	1850,		/* CH2 */
> >> +	1850,		/* CH3 */
> >> +	1850,		/* CH4 */
> >> +	1850,		/* CH5 */
> >> +	1850,		/* CH6 */
> >> +	1850 * 4,	/* CH7 VBATT */
> >> +	1850,		/* CH8 Temp sense 0 */
> >> +	1850,		/* CH9 Temp sense 1 */
> >> +	1850 * 2,	/* CH10 VDDIO */
> >> +	1850,		/* CH11 VTH */
> >> +	1850 * 2,	/* CH12 VDDA */
> >> +	1850,		/* CH13 VDDD */
> >> +	1850,		/* CH14 VBG */
> >> +	1850 * 4,	/* CH15 VDD5V */
> >> +};
> > 
> > Should the above not be in DT ?
> 
> Do you mean for example:
> 
> 	lradc@80050000 {
> 		compatible = "fsl,imx28-lradc";
> 		reg = <0x80050000 0x2000>;
> 		interrupts = <10 14 15 16 17 18 19
> 				20 21 22 23 24 25>;
> 		vref = <1850 1850 1850 1850
> 			1850 1850 1850 7400
> 			1850 1850 3700 1850
> 			3700 1850 1850 7400>
> 	};
> 
> I'm ok with it, but are there other examples of driver using a similar
> approach? I see for example the spear-adc which reads:
> 
> 	Optional properties:
> 	- vref-external: External voltage reference in milli-volts. If omitted
> 	  the internal voltage reference will be used.
> 
> which makes me believe the 'internal voltage reference' is hardcoded in the
> driver. More opinions?

I'd duke this one out on devicetree-discuss@ list. In general, I'd use something 
like fsl,vref prop, but this really better be discussed in a separate thread.

On the other hand, I'd hate to keep these patches here waiting , so maybe we 
should apply them as-is and remove this part from the driver in a subsequent 
patch? What do you say, Lars ?

Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[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