* [PATCH 0/2] iio: mxs-lradc: fixes @ 2014-06-10 13:41 Robert Hodaszi 2014-06-10 13:41 ` [PATCH 1/2] iio: mxs-lradc: fix divider Robert Hodaszi 2014-06-10 13:41 ` [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 Robert Hodaszi 0 siblings, 2 replies; 9+ messages in thread From: Robert Hodaszi @ 2014-06-10 13:41 UTC (permalink / raw) To: jic23 Cc: linux-iio, linux-kernel, gregkh, marex, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars, Robert Hodaszi Hello, The following patches are fixing some bugs in the MXS LRADC driver. Note, that I tried the patches on a custom i.MX28 based module. I did not try them on i.MX23. The first patch fixes the analog voltage divider. The second patch adds back the channel 9 to fix the LRADC channel indexes. (E.g. currently the <&lradc 12> refers to the channel 13.) Robert Hodaszi (2): iio: mxs-lradc: fix divider iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 drivers/staging/iio/adc/mxs-lradc.c | 73 ++++++++++++++++++++++--------------- 1 file changed, 43 insertions(+), 30 deletions(-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] iio: mxs-lradc: fix divider 2014-06-10 13:41 [PATCH 0/2] iio: mxs-lradc: fixes Robert Hodaszi @ 2014-06-10 13:41 ` Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:47 ` Marek Vasut 2014-06-10 13:41 ` [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 Robert Hodaszi 1 sibling, 2 replies; 9+ messages in thread From: Robert Hodaszi @ 2014-06-10 13:41 UTC (permalink / raw) To: jic23 Cc: linux-iio, linux-kernel, gregkh, marex, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars, Robert Hodaszi All channels' single measurement are happening on CH 0. So enabling / disabling the divider once is not enough, because it has impact on all channels. Set only a flag, then check this on each measurement, and enable / disable the divider as required. Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> --- drivers/staging/iio/adc/mxs-lradc.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c index dae8d1a..52d7517 100644 --- a/drivers/staging/iio/adc/mxs-lradc.c +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -846,6 +846,14 @@ static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) LRADC_CTRL1); mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0); + /* Enable / disable the divider per requirement */ + if (test_bit(chan, &lradc->is_divided)) + mxs_lradc_reg_set(lradc, 1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, + LRADC_CTRL2); + else + mxs_lradc_reg_clear(lradc, + 1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, LRADC_CTRL2); + /* Clean the slot's previous content, then set new one. */ mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(0), LRADC_CTRL4); @@ -961,15 +969,11 @@ static int mxs_lradc_write_raw(struct iio_dev *iio_dev, if (val == scale_avail[MXS_LRADC_DIV_DISABLED].integer && val2 == scale_avail[MXS_LRADC_DIV_DISABLED].nano) { /* divider by two disabled */ - writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, - lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR); clear_bit(chan->channel, &lradc->is_divided); ret = 0; } else if (val == scale_avail[MXS_LRADC_DIV_ENABLED].integer && val2 == scale_avail[MXS_LRADC_DIV_ENABLED].nano) { /* divider by two enabled */ - writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, - lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET); set_bit(chan->channel, &lradc->is_divided); ret = 0; } ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: mxs-lradc: fix divider 2014-06-10 13:41 ` [PATCH 1/2] iio: mxs-lradc: fix divider Robert Hodaszi @ 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:47 ` Marek Vasut 1 sibling, 0 replies; 9+ messages in thread From: Alexandre Belloni @ 2014-06-10 15:19 UTC (permalink / raw) To: Robert Hodaszi Cc: jic23, linux-iio, linux-kernel, gregkh, marex, jbe, hector.palacios, fabio.estevam, lars On 10/06/2014 at 15:41:34 +0200, Robert Hodaszi wrote : > All channels' single measurement are happening on CH 0. So enabling / disabling > the divider once is not enough, because it has impact on all channels. > > Set only a flag, then check this on each measurement, and enable / disable the > divider as required. > > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> It makes perfect sense. Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > --- > drivers/staging/iio/adc/mxs-lradc.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c > index dae8d1a..52d7517 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -846,6 +846,14 @@ static int mxs_lradc_read_single(struct iio_dev *iio_dev, int chan, int *val) > LRADC_CTRL1); > mxs_lradc_reg_clear(lradc, 0xff, LRADC_CTRL0); > > + /* Enable / disable the divider per requirement */ > + if (test_bit(chan, &lradc->is_divided)) > + mxs_lradc_reg_set(lradc, 1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, > + LRADC_CTRL2); > + else > + mxs_lradc_reg_clear(lradc, > + 1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, LRADC_CTRL2); > + > /* Clean the slot's previous content, then set new one. */ > mxs_lradc_reg_clear(lradc, LRADC_CTRL4_LRADCSELECT_MASK(0), > LRADC_CTRL4); > @@ -961,15 +969,11 @@ static int mxs_lradc_write_raw(struct iio_dev *iio_dev, > if (val == scale_avail[MXS_LRADC_DIV_DISABLED].integer && > val2 == scale_avail[MXS_LRADC_DIV_DISABLED].nano) { > /* divider by two disabled */ > - writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, > - lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_CLR); > clear_bit(chan->channel, &lradc->is_divided); > ret = 0; > } else if (val == scale_avail[MXS_LRADC_DIV_ENABLED].integer && > val2 == scale_avail[MXS_LRADC_DIV_ENABLED].nano) { > /* divider by two enabled */ > - writel(1 << LRADC_CTRL2_DIVIDE_BY_TWO_OFFSET, > - lradc->base + LRADC_CTRL2 + STMP_OFFSET_REG_SET); > set_bit(chan->channel, &lradc->is_divided); > ret = 0; > } -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: mxs-lradc: fix divider 2014-06-10 13:41 ` [PATCH 1/2] iio: mxs-lradc: fix divider Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni @ 2014-06-10 22:47 ` Marek Vasut 2014-06-14 14:01 ` Jonathan Cameron 1 sibling, 1 reply; 9+ messages in thread From: Marek Vasut @ 2014-06-10 22:47 UTC (permalink / raw) To: Robert Hodaszi Cc: jic23, linux-iio, linux-kernel, gregkh, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars On Tuesday, June 10, 2014 at 03:41:34 PM, Robert Hodaszi wrote: > All channels' single measurement are happening on CH 0. So enabling / > disabling the divider once is not enough, because it has impact on all > channels. > > Set only a flag, then check this on each measurement, and enable / disable > the divider as required. > > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> Acked-by: Marek Vasut <marex@denx.de> Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: mxs-lradc: fix divider 2014-06-10 22:47 ` Marek Vasut @ 2014-06-14 14:01 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2014-06-14 14:01 UTC (permalink / raw) To: Marek Vasut, Robert Hodaszi Cc: linux-iio, linux-kernel, gregkh, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars On 10/06/14 23:47, Marek Vasut wrote: > On Tuesday, June 10, 2014 at 03:41:34 PM, Robert Hodaszi wrote: >> All channels' single measurement are happening on CH 0. So enabling / >> disabling the divider once is not enough, because it has impact on all >> channels. >> >> Set only a flag, then check this on each measurement, and enable / disable >> the divider as required. >> >> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> > > Acked-by: Marek Vasut <marex@denx.de> Applied to the fixes-togreg branch of iio.git Also marked for stable. > > Best regards, > Marek Vasut > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 2014-06-10 13:41 [PATCH 0/2] iio: mxs-lradc: fixes Robert Hodaszi 2014-06-10 13:41 ` [PATCH 1/2] iio: mxs-lradc: fix divider Robert Hodaszi @ 2014-06-10 13:41 ` Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:51 ` Marek Vasut 1 sibling, 2 replies; 9+ messages in thread From: Robert Hodaszi @ 2014-06-10 13:41 UTC (permalink / raw) To: jic23 Cc: linux-iio, linux-kernel, gregkh, marex, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars, Robert Hodaszi Commit c8231a9af8147f8a401fc55931ec44abfb937660 ("iio: mxs-lradc: compute temperature from channel 8 and 9") merged channel 8 and channel 9 to create an IIO_TEMP channel. It changed the number of LRADC channels, which could cause incompatibility with previous device-tree declarations, and also makes it illogical (e.g. channel 15 is <&lradc 14>). Add channel 9 as a copy of channel 8. Reading channel 9 has the same output as reading channel 8. Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> --- drivers/staging/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c index 52d7517..25c0519 100644 --- a/drivers/staging/iio/adc/mxs-lradc.c +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -1376,8 +1376,8 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { * Driver initialization */ -#define MXS_ADC_CHAN(idx, chan_type) { \ - .type = (chan_type), \ +#define MXS_ADC_VOLTAGE_CHAN(idx) { \ + .type = IIO_VOLTAGE, \ .indexed = 1, \ .scan_index = (idx), \ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ @@ -1391,32 +1391,41 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { }, \ } +#define MXS_ADC_TEMP_CHAN(idx) { \ + .type = IIO_TEMP, \ + .indexed = 1, \ + .scan_index = (idx), \ + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ + BIT(IIO_CHAN_INFO_SCALE) | \ + BIT(IIO_CHAN_INFO_OFFSET), \ + .channel = (idx), \ + .address = (idx), \ + .scan_type = { \ + .sign = 'u', \ + .realbits = 18, \ + .storagebits = 32, \ + }, \ +} + static const struct iio_chan_spec mxs_lradc_chan_spec[] = { - MXS_ADC_CHAN(0, IIO_VOLTAGE), - MXS_ADC_CHAN(1, IIO_VOLTAGE), - MXS_ADC_CHAN(2, IIO_VOLTAGE), - MXS_ADC_CHAN(3, IIO_VOLTAGE), - MXS_ADC_CHAN(4, IIO_VOLTAGE), - MXS_ADC_CHAN(5, IIO_VOLTAGE), - MXS_ADC_CHAN(6, IIO_VOLTAGE), - MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ + MXS_ADC_VOLTAGE_CHAN(0), + MXS_ADC_VOLTAGE_CHAN(1), + MXS_ADC_VOLTAGE_CHAN(2), + MXS_ADC_VOLTAGE_CHAN(3), + MXS_ADC_VOLTAGE_CHAN(4), + MXS_ADC_VOLTAGE_CHAN(5), + MXS_ADC_VOLTAGE_CHAN(6), + MXS_ADC_VOLTAGE_CHAN(7), /* VBATT */ /* Combined Temperature sensors */ - { - .type = IIO_TEMP, - .indexed = 1, - .scan_index = 8, - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | - BIT(IIO_CHAN_INFO_OFFSET) | - BIT(IIO_CHAN_INFO_SCALE), - .channel = 8, - .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, - }, - MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ - MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ - MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ - MXS_ADC_CHAN(13, IIO_VOLTAGE), /* VDDD */ - MXS_ADC_CHAN(14, IIO_VOLTAGE), /* VBG */ - MXS_ADC_CHAN(15, IIO_VOLTAGE), /* VDD5V */ + MXS_ADC_TEMP_CHAN(8), + /* CH 9 is the same as CH 8. Declared to fix following channel indexes */ + MXS_ADC_TEMP_CHAN(9), + MXS_ADC_VOLTAGE_CHAN(10), /* VDDIO */ + MXS_ADC_VOLTAGE_CHAN(11), /* VTH */ + MXS_ADC_VOLTAGE_CHAN(12), /* VDDA */ + MXS_ADC_VOLTAGE_CHAN(13), /* VDDD */ + MXS_ADC_VOLTAGE_CHAN(14), /* VBG */ + MXS_ADC_VOLTAGE_CHAN(15), /* VDD5V */ }; static int mxs_lradc_hw_init(struct mxs_lradc *lradc) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 2014-06-10 13:41 ` [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 Robert Hodaszi @ 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:51 ` Marek Vasut 1 sibling, 0 replies; 9+ messages in thread From: Alexandre Belloni @ 2014-06-10 15:19 UTC (permalink / raw) To: Robert Hodaszi Cc: jic23, linux-iio, linux-kernel, gregkh, marex, jbe, hector.palacios, fabio.estevam, lars On 10/06/2014 at 15:41:35 +0200, Robert Hodaszi wrote : > Commit c8231a9af8147f8a401fc55931ec44abfb937660 ("iio: mxs-lradc: compute > temperature from channel 8 and 9") merged channel 8 and channel 9 to create an > IIO_TEMP channel. It changed the number of LRADC channels, which could cause > incompatibility with previous device-tree declarations, and also makes it > illogical (e.g. channel 15 is <&lradc 14>). > > Add channel 9 as a copy of channel 8. Reading channel 9 has the same output as > reading channel 8. > > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > --- > drivers/staging/iio/adc/mxs-lradc.c | 61 +++++++++++++++++++++---------------- > 1 file changed, 35 insertions(+), 26 deletions(-) > > diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c > index 52d7517..25c0519 100644 > --- a/drivers/staging/iio/adc/mxs-lradc.c > +++ b/drivers/staging/iio/adc/mxs-lradc.c > @@ -1376,8 +1376,8 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { > * Driver initialization > */ > > -#define MXS_ADC_CHAN(idx, chan_type) { \ > - .type = (chan_type), \ > +#define MXS_ADC_VOLTAGE_CHAN(idx) { \ > + .type = IIO_VOLTAGE, \ > .indexed = 1, \ > .scan_index = (idx), \ > .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > @@ -1391,32 +1391,41 @@ static const struct iio_buffer_setup_ops mxs_lradc_buffer_ops = { > }, \ > } > > +#define MXS_ADC_TEMP_CHAN(idx) { \ > + .type = IIO_TEMP, \ > + .indexed = 1, \ > + .scan_index = (idx), \ > + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | \ > + BIT(IIO_CHAN_INFO_SCALE) | \ > + BIT(IIO_CHAN_INFO_OFFSET), \ > + .channel = (idx), \ > + .address = (idx), \ > + .scan_type = { \ > + .sign = 'u', \ > + .realbits = 18, \ > + .storagebits = 32, \ > + }, \ > +} > + > static const struct iio_chan_spec mxs_lradc_chan_spec[] = { > - MXS_ADC_CHAN(0, IIO_VOLTAGE), > - MXS_ADC_CHAN(1, IIO_VOLTAGE), > - MXS_ADC_CHAN(2, IIO_VOLTAGE), > - MXS_ADC_CHAN(3, IIO_VOLTAGE), > - MXS_ADC_CHAN(4, IIO_VOLTAGE), > - MXS_ADC_CHAN(5, IIO_VOLTAGE), > - MXS_ADC_CHAN(6, IIO_VOLTAGE), > - MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ > + MXS_ADC_VOLTAGE_CHAN(0), > + MXS_ADC_VOLTAGE_CHAN(1), > + MXS_ADC_VOLTAGE_CHAN(2), > + MXS_ADC_VOLTAGE_CHAN(3), > + MXS_ADC_VOLTAGE_CHAN(4), > + MXS_ADC_VOLTAGE_CHAN(5), > + MXS_ADC_VOLTAGE_CHAN(6), > + MXS_ADC_VOLTAGE_CHAN(7), /* VBATT */ > /* Combined Temperature sensors */ > - { > - .type = IIO_TEMP, > - .indexed = 1, > - .scan_index = 8, > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > - BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE), > - .channel = 8, > - .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, > - }, > - MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ > - MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ > - MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ > - MXS_ADC_CHAN(13, IIO_VOLTAGE), /* VDDD */ > - MXS_ADC_CHAN(14, IIO_VOLTAGE), /* VBG */ > - MXS_ADC_CHAN(15, IIO_VOLTAGE), /* VDD5V */ > + MXS_ADC_TEMP_CHAN(8), > + /* CH 9 is the same as CH 8. Declared to fix following channel indexes */ > + MXS_ADC_TEMP_CHAN(9), > + MXS_ADC_VOLTAGE_CHAN(10), /* VDDIO */ > + MXS_ADC_VOLTAGE_CHAN(11), /* VTH */ > + MXS_ADC_VOLTAGE_CHAN(12), /* VDDA */ > + MXS_ADC_VOLTAGE_CHAN(13), /* VDDD */ > + MXS_ADC_VOLTAGE_CHAN(14), /* VBG */ > + MXS_ADC_VOLTAGE_CHAN(15), /* VDD5V */ > }; > > static int mxs_lradc_hw_init(struct mxs_lradc *lradc) -- Alexandre Belloni, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 2014-06-10 13:41 ` [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni @ 2014-06-10 22:51 ` Marek Vasut 2014-06-14 13:58 ` Jonathan Cameron 1 sibling, 1 reply; 9+ messages in thread From: Marek Vasut @ 2014-06-10 22:51 UTC (permalink / raw) To: Robert Hodaszi Cc: jic23, linux-iio, linux-kernel, gregkh, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars On Tuesday, June 10, 2014 at 03:41:35 PM, Robert Hodaszi wrote: > Commit c8231a9af8147f8a401fc55931ec44abfb937660 ("iio: mxs-lradc: compute > temperature from channel 8 and 9") merged channel 8 and channel 9 to create > an IIO_TEMP channel. It changed the number of LRADC channels, which could > cause incompatibility with previous device-tree declarations, and also > makes it illogical (e.g. channel 15 is <&lradc 14>). > > Add channel 9 as a copy of channel 8. Reading channel 9 has the same output > as reading channel 8. > > Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> > --- [...] > static const struct iio_chan_spec mxs_lradc_chan_spec[] = { > - MXS_ADC_CHAN(0, IIO_VOLTAGE), > - MXS_ADC_CHAN(1, IIO_VOLTAGE), > - MXS_ADC_CHAN(2, IIO_VOLTAGE), > - MXS_ADC_CHAN(3, IIO_VOLTAGE), > - MXS_ADC_CHAN(4, IIO_VOLTAGE), > - MXS_ADC_CHAN(5, IIO_VOLTAGE), > - MXS_ADC_CHAN(6, IIO_VOLTAGE), > - MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ > + MXS_ADC_VOLTAGE_CHAN(0), > + MXS_ADC_VOLTAGE_CHAN(1), > + MXS_ADC_VOLTAGE_CHAN(2), > + MXS_ADC_VOLTAGE_CHAN(3), > + MXS_ADC_VOLTAGE_CHAN(4), > + MXS_ADC_VOLTAGE_CHAN(5), > + MXS_ADC_VOLTAGE_CHAN(6), > + MXS_ADC_VOLTAGE_CHAN(7), /* VBATT */ > /* Combined Temperature sensors */ > - { > - .type = IIO_TEMP, > - .indexed = 1, > - .scan_index = 8, > - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | > - BIT(IIO_CHAN_INFO_OFFSET) | > - BIT(IIO_CHAN_INFO_SCALE), > - .channel = 8, > - .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, > - }, I wonder, shouldn't the IIO framework handle this kind of a "hole" in the iio_chan_spec structure, where one entry has .channel = N and the subsequent one has .channel = N + 2 somehow ? > - MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ > - MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ > - MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ > - MXS_ADC_CHAN(13, IIO_VOLTAGE), /* VDDD */ > - MXS_ADC_CHAN(14, IIO_VOLTAGE), /* VBG */ > - MXS_ADC_CHAN(15, IIO_VOLTAGE), /* VDD5V */ [...] ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 2014-06-10 22:51 ` Marek Vasut @ 2014-06-14 13:58 ` Jonathan Cameron 0 siblings, 0 replies; 9+ messages in thread From: Jonathan Cameron @ 2014-06-14 13:58 UTC (permalink / raw) To: Marek Vasut, Robert Hodaszi Cc: linux-iio, linux-kernel, gregkh, alexandre.belloni, jbe, hector.palacios, fabio.estevam, lars On 10/06/14 23:51, Marek Vasut wrote: > On Tuesday, June 10, 2014 at 03:41:35 PM, Robert Hodaszi wrote: >> Commit c8231a9af8147f8a401fc55931ec44abfb937660 ("iio: mxs-lradc: compute >> temperature from channel 8 and 9") merged channel 8 and channel 9 to create >> an IIO_TEMP channel. It changed the number of LRADC channels, which could >> cause incompatibility with previous device-tree declarations, and also >> makes it illogical (e.g. channel 15 is <&lradc 14>). The binding is current just against an index of available channels. If the device does something faintly crazy with its numbering then holes will occur. Anyhow, suggestion below on how to create a 'gap' in the channel index. Honestly the binding is pretty hideous, but we'll have to maintain it for a long time when someone (i.e. not me ;) comes up with a better binding for iio consumer channels. The non device tree one is much nicer in that everything is done by name rather than artificial indexes. >> >> Add channel 9 as a copy of channel 8. Reading channel 9 has the same output >> as reading channel 8. >> >> Signed-off-by: Robert Hodaszi <robert.hodaszi@digi.com> >> --- > > [...] > >> static const struct iio_chan_spec mxs_lradc_chan_spec[] = { >> - MXS_ADC_CHAN(0, IIO_VOLTAGE), >> - MXS_ADC_CHAN(1, IIO_VOLTAGE), >> - MXS_ADC_CHAN(2, IIO_VOLTAGE), >> - MXS_ADC_CHAN(3, IIO_VOLTAGE), >> - MXS_ADC_CHAN(4, IIO_VOLTAGE), >> - MXS_ADC_CHAN(5, IIO_VOLTAGE), >> - MXS_ADC_CHAN(6, IIO_VOLTAGE), >> - MXS_ADC_CHAN(7, IIO_VOLTAGE), /* VBATT */ >> + MXS_ADC_VOLTAGE_CHAN(0), >> + MXS_ADC_VOLTAGE_CHAN(1), >> + MXS_ADC_VOLTAGE_CHAN(2), >> + MXS_ADC_VOLTAGE_CHAN(3), >> + MXS_ADC_VOLTAGE_CHAN(4), >> + MXS_ADC_VOLTAGE_CHAN(5), >> + MXS_ADC_VOLTAGE_CHAN(6), >> + MXS_ADC_VOLTAGE_CHAN(7), /* VBATT */ >> /* Combined Temperature sensors */ >> - { >> - .type = IIO_TEMP, >> - .indexed = 1, >> - .scan_index = 8, >> - .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | >> - BIT(IIO_CHAN_INFO_OFFSET) | >> - BIT(IIO_CHAN_INFO_SCALE), >> - .channel = 8, >> - .scan_type = {.sign = 'u', .realbits = 18, .storagebits = 32,}, >> - }, > > I wonder, shouldn't the IIO framework handle this kind of a "hole" in the > iio_chan_spec structure, where one entry has .channel = N and the subsequent one > has .channel = N + 2 somehow ? Hmm. This binding isn't all that heavily used as yet so this is the first time I've come across anyone wanting to jump a channel. Anyhow, simply having a channel with scan_index = -1 (not available for buffered capture) and nothing in any of the info_mask elements should instantiate a channel with no actual interfaces or apparent existence. (not that I've actually tested both of these being true as we only have either channels that are not buffered, or devices with channels that are only buffered using these two bits of functionality). I'd rather this solution than adding an artificial bonus channel. > >> - MXS_ADC_CHAN(10, IIO_VOLTAGE), /* VDDIO */ >> - MXS_ADC_CHAN(11, IIO_VOLTAGE), /* VTH */ >> - MXS_ADC_CHAN(12, IIO_VOLTAGE), /* VDDA */ >> - MXS_ADC_CHAN(13, IIO_VOLTAGE), /* VDDD */ >> - MXS_ADC_CHAN(14, IIO_VOLTAGE), /* VBG */ >> - MXS_ADC_CHAN(15, IIO_VOLTAGE), /* VDD5V */ > > [...] > -- > To unsubscribe from this list: send the line "unsubscribe linux-iio" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-06-14 13:59 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-06-10 13:41 [PATCH 0/2] iio: mxs-lradc: fixes Robert Hodaszi 2014-06-10 13:41 ` [PATCH 1/2] iio: mxs-lradc: fix divider Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:47 ` Marek Vasut 2014-06-14 14:01 ` Jonathan Cameron 2014-06-10 13:41 ` [PATCH 2/2] iio: mxs-lradc: add ADC channel 9 as a copy of channel 8 Robert Hodaszi 2014-06-10 15:19 ` Alexandre Belloni 2014-06-10 22:51 ` Marek Vasut 2014-06-14 13:58 ` Jonathan Cameron
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.