iio: mxs: Add MX23 support into the IIO driver
Subsystems (5)
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
--- drivers/staging/iio/adc/Kconfig | 4 +-- drivers/staging/iio/adc/mxs-lradc.c | 56 +++++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 11 deletions(-)
drivers/staging/iio/adc/Kconfig +2 -2
@@ -119,12 +119,12 @@ config LPC32XX_ADC via sysfs. config MXS_LRADC - tristate "Freescale i.MX28 LRADC" + tristate "Freescale i.MX23/i.MX28 LRADC" depends on ARCH_MXS select IIO_BUFFER select IIO_TRIGGERED_BUFFER help - Say yes here to build support for i.MX28 LRADC convertor + Say yes here to build support for i.MX23/i.MX28 LRADC convertor built into these chips. To compile this driver as a module, choose M here: the
drivers/staging/iio/adc/mxs-lradc.c +47 -9
94 lines not on this page · raw message
Signature · 1 line
1.7.10.4
PATCH 2/2 ARM: mxs: Add OF props for MX23 LRADC
Add interrupt mapping and compatible string for MX23 LRADC.
--- arch/arm/boot/dts/imx23.dtsi | 2 ++ 1 file changed, 2 insertions(+)
arch/arm/boot/dts/imx23.dtsi +2 -0
@@ -391,7 +391,9 @@ }; lradc at 80050000 { + compatible = "fsl,imx23-lradc"; reg = <0x80050000 0x2000>; + interrupts = <36 37 38 39 40 41 42 43 44>; status = "disabled"; };
Signature · 1 line
1.7.10.4
Jonathan Cameron
maintainerPATCH 2/2 ARM: mxs: Add OF props for MX23 LRADC
Quoted 1 line · Marek Vasut
Add interrupt mapping and compatible string for MX23 LRADC.
Series looks fine to me, but obviously I'd like an ACK for this one from Shawn before taking it.
Thanks,
Jonathan
Quoted 23 lines
Signed-off-by Marek Vasut <[email protected]>Cc Fabio Estevam <[email protected]>Cc Shawn Guo <[email protected]>--- arch/arm/boot/dts/imx23.dtsi | 2 ++ 1 file changed, 2 insertions(+)arch/arm/boot/dts/imx23.dtsi +2
-0@@ -391,7 +391,9 @@ }; lradc at 80050000 { + compatible = "fsl,imx23-lradc"; reg = <0x80050000 0x2000>; + interrupts = <36 37 38 39 40 41 42 43 44>; status = "disabled"; };
PATCH 2/2 ARM: mxs: Add OF props for MX23 LRADC
Quoted 5 lines · Jonathan Cameron
Quoted 1 line · Marek Vasut
Add interrupt mapping and compatible string for MX23 LRADC.
Series looks fine to me, but obviously I'd like an ACK for this one from Shawn before taking it.
Jonathan Cameron
maintainerPATCH 2/2 ARM: mxs: Add OF props for MX23 LRADC
Quoted 9 lines · Shawn Guo
Quoted 5 lines · Jonathan Cameron
Quoted 1 line · Marek Vasut
Add interrupt mapping and compatible string for MX23 LRADC.
Series looks fine to me, but obviously I'd like an ACK for this one from Shawn before taking it.
Acked-by Shawn Guo <[email protected]>
Thanks,
both added to togreg branch of iio.git
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
In reply to Marek Vasut2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 20 lines
drivers/staging/iio/adc/mxs-lradc.c +16
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] = { + [IMX23_LRADC ] = { + .irq_count =ARRAY_SIZE (mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC ] = { + .irq_count =ARRAY_SIZE (mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 ,MXS_LRADC_TOUCHSCREEN_4WIRE ,
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c +7
-0@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); } +static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible ="fsl,imx23-lradc" , .data = (void *)IMX23_LRADC , }, + { .compatible ="fsl,imx28-lradc" , .data = (void *)IMX28_LRADC , }, + {/* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +
Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Best Regards,
Micha? Miros?aw
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +16
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] = { + [IMX23_LRADC ] = { + .irq_count =ARRAY_SIZE (mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC ] = { + .irq_count =ARRAY_SIZE (mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 ,MXS_LRADC_TOUCHSCREEN_4WIRE ,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }
Quoted 9 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 7 lines
drivers/staging/iio/adc/mxs-lradc.c +5
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible ="fsl,imx23-lradc" , .data = (void *)IMX23_LRADC , }, + { .compatible = "fsl,imx28-lradc", .data = (void *)IMX28_LRADC, }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
Quoted 1 line
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more
convenient in the long run.
Best regards,
Marek Vasut
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
2013/1/21 Marek Vasut <[email protected]>:
Quoted 52 lines
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +16
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] = { + [IMX23_LRADC ] = { + .irq_count =ARRAY_SIZE (mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC ] = { + .irq_count =ARRAY_SIZE (mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 ,MXS_LRADC_TOUCHSCREEN_4WIRE ,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }Quoted 9 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 7 lines
drivers/staging/iio/adc/mxs-lradc.c +5
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible ="fsl,imx23-lradc" , .data = (void *)IMX23_LRADC , }, + { .compatible = "fsl,imx28-lradc", .data = (void *)IMX28_LRADC, }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Quoted 6 lines
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.
I'm asking, because you don't use this number anywhere other than in mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.
Sure, it's just an insn or two, so no strong opinion here --- just curious.
Best Regards,
Micha? Miros?aw
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Dear Micha? Miros?aw,
Quoted 59 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 58 lines
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +6
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] ={ + [
IMX23_LRADC] = {drivers/staging/iio/adc/mxs-lradc.c +9
-0+ .irq_count = ARRAY_SIZE(mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC] = { + .irq_count = ARRAY_SIZE(mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 , MXS_LRADC_TOUCHSCREEN_4WIRE,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 8 lines
(quoted diff) +2
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible = "fsl,imx23-lradc", .data = (void *)IMX23_LRADC, }, + { .compatible = "fsl,imx28-lradc", .data =(void
*)IMX28_LRADC, }, + { /* sentinel */ }drivers/staging/iio/adc/mxs-lradc.c +3
-0+}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
Quoted 8 lines
Quoted 2 lines
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.
Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
Quoted 1 line
Sure, it's just an insn or two, so no strong opinion here --- just curious.
I tried to put it down above ;-)
Quoted 2 lines
Best Regards,
Micha? Miros?aw
Best regards,
Marek Vasut
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Quoted 64 lines · Marek Vasut
Dear Michał Mirosław,
Quoted 62 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 60 lines
Dear Michał Mirosław,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +6
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] ={ + [
IMX23_LRADC] = {drivers/staging/iio/adc/mxs-lradc.c +9
-0+ .irq_count = ARRAY_SIZE(mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC] = { + .irq_count = ARRAY_SIZE(mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 , MXS_LRADC_TOUCHSCREEN_4WIRE,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 8 lines
(quoted diff) +2
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible = "fsl,imx23-lradc", .data = (void *)IMX23_LRADC, }, + { .compatible = "fsl,imx28-lradc", .data =(void
*)IMX28_LRADC, }, + { /* sentinel */ }drivers/staging/iio/adc/mxs-lradc.c +3
-0+}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.
Quoted 8 lines
Quoted 5 lines
I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
You'll probably be better of by putting these differences into the mxs_lradc_of_config struct as well, instead of adding switch statements here and there throughout the code.
- Lars
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Dear Lars-Peter Clausen,
Quoted 71 lines · Marek Vasut
Quoted 70 lines
Dear Micha? Miros?aw,
Quoted 67 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 53 lines
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +6
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[]= { + [
IMX23_LRADC] = {drivers/staging/iio/adc/mxs-lradc.c +9
-0+ .irq_count = ARRAY_SIZE(mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC] = { + .irq_count = ARRAY_SIZE(mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 , MXS_LRADC_TOUCHSCREEN_4WIRE,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 8 lines
(quoted diff) +2
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible = "fsl,imx23-lradc", .data = (void *)IMX23_LRADC, }, + { .compatible = "fsl,imx28-lradc", .data =(void
*)IMX28_LRADC, }, + { /* sentinel */ }drivers/staging/iio/adc/mxs-lradc.c +3
-0+}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Quoted 7 lines
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.
Quoted 6 lines
Quoted 2 lines
Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
You'll probably be better of by putting these differences into the
mxs_lradc_of_configstruct as well, instead of adding switch statements here and there throughout the code.
Certainly. All that is needed is in place now.
Best regards,
Marek Vasut
Jonathan Cameron
maintainerPATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Quoted 73 lines · Marek Vasut
Dear Lars-Peter Clausen,
Quoted 71 lines · Marek Vasut
Quoted 70 lines
Dear Micha? Miros?aw,
Quoted 67 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 53 lines
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
(quoted diff) +6
-0+struct mxs_lradc_of_config { + const int irq_count; + const char * const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[]= { + [
IMX23_LRADC] = {(quoted diff) +9
-0+ .irq_count = ARRAY_SIZE(mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC] = { + .irq_count = ARRAY_SIZE(mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; + enum mxs_lradc_ts { MXS_LRADC_TOUCHSCREEN_NONE = 0, MXS_LRADC_TOUCHSCREEN_4WIRE,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) writel(0, lradc->base + LRADC_DELAY(i)); }Quoted 10 lines
Review
Quoted 8 lines
(quoted diff) +2
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible = "fsl,imx23-lradc", .data = (void *)IMX23_LRADC, }, + { .compatible = "fsl,imx28-lradc", .data =(void
*)IMX28_LRADC, }, + { /* sentinel */ }(quoted diff) +3
-0+}; +MODULE_DEVICE_TABLE(of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Quoted 7 lines
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.
Quoted 9 lines
Quoted 6 lines
Quoted 2 lines
Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
You'll probably be better of by putting these differences into the
mxs_lradc_of_configstruct as well, instead of adding switch statements here and there throughout the code.Certainly. All that is needed is in place now.
All look sane to me and Marek has answered all the questions as far as I can see. I'll take these once I get a response from Shawn (unless someone convinces me otherwise ;)
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
In reply to Marek Vasut2013/1/21 Marek Vasut <[email protected]>:
Quoted 62 lines
Dear Micha? Miros?aw,
Quoted 61 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 59 lines
Dear Micha? Miros?aw,
Quoted 36 lines
2013/1/21 Marek Vasut <[email protected]>:
Quoted 3 lines
This patch adds support for i.MX23 into the LRADC driver. The LRADC block on MX23 is not much different from the one on MX28, thus this is only a few changes fixing the parts that are specific to MX23.
[...]
Quoted 29 lines
drivers/staging/iio/adc/mxs-lradc.c +6
-0+struct mxs_lradc_of_config { +const int irq_count; +const char *const *irq_name; +}; + +static const struct mxs_lradc_of_config const mxs_lradc_of_config[] ={ + [
IMX23_LRADC] = {drivers/staging/iio/adc/mxs-lradc.c +9
-0+ .irq_count = ARRAY_SIZE(mx23_lradc_irq_names), + .irq_name = mx23_lradc_irq_names, + }, + [IMX28_LRADC] = { + .irq_count = ARRAY_SIZE(mx28_lradc_irq_names), + .irq_name = mx28_lradc_irq_names, + }, +}; +enum mxs_lradc_ts {MXS_LRADC_TOUCHSCREEN_NONE =0 , MXS_LRADC_TOUCHSCREEN_4WIRE,@@ -857,8 +890,19 @@ static void mxs_lradc_hw_stop(struct mxs_lradc *lradc)writel (0 , lradc->base +LRADC_DELAY (i)); }Quoted 10 lines
drivers/staging/iio/adc/mxs-lradc.c static void mxs_lradc_hw_stop(struct mxs_lradc *lradc) · line 899
Quoted 8 lines
(quoted diff) +2
-0+static const struct of_device_id mxs_lradc_dt_ids[] = { + { .compatible = "fsl,imx23-lradc", .data = (void *)IMX23_LRADC, }, + { .compatible = "fsl,imx28-lradc", .data =(void
*)IMX28_LRADC, }, + { /* sentinel */ }drivers/staging/iio/adc/mxs-lradc.c +3
-0+}; +MODULE_DEVICE_TABLE (of, mxs_lradc_dt_ids); +Why not s/(void \*)\(IMX.._LRADC\)/\&mxs_lradc_of_config[\1]/ ?
Check the register layout, it differs between MX23 and MX28, that's one reason, since were we to access differently placed registers, we can do it easily as in the SSP/I2C drivers.
Moreover, there are some features on the MX28 that are not on the MX23 (like voltage treshold triggers and touchbuttons), with this setup, we can easily check what we're running at at runtime and determine to disallow these.
From my point of view, using the number (
IMX23_LRADC/IMX28_LRADC) is much more convenient in the long run.
Quoted 8 lines
Quoted 5 lines
I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
Hmm. Then is IMX23 LRADC going to just work after this series (assuming I don't use unsupported features) or this needs more patches to be usable?
Best Regards,
Micha? Miros?aw
PATCH 1/2 iio: mxs: Add MX23 support into the IIO driver
Dear Micha? Miros?aw,
[...]
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
I'm asking, because you don't use this number anywhere other than in
mxs_lradc_probe()
and there only to dereference the irq-names table. After that the structure and number
are forgotten.
Quoted 6 lines
Quoted 2 lines
Certainly, so far it's used only this way. But please see my argument about register layout, that's why I went down this road of abstraction.
Hmm. Then is IMX23 LRADC going to just work after this series (assuming I don't use unsupported features) or this needs more patches to be usable?
It's gonna work, touchscreen will work as well. You won't have the touchbuttons, but when these're gonna be implemented, we will need to be careful to enable them only for mx28.
Best regards,
Marek Vasut