| Message ID | 1358798722-25102-1-git-send-email-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2013/1/21 Marek Vasut <marex@denx.de>: > 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. [...] > +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)); > } > > +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
Dear Micha? Miros?aw, > 2013/1/21 Marek Vasut <marex@denx.de>: > > 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. > > [...] > > > +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)); > > > > } > > > > +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. From my point of view, using the number (IMX23_LRADC / IMX28_LRADC) is much more convenient in the long run. Best regards, Marek Vasut
2013/1/21 Marek Vasut <marex@denx.de>: > Dear Micha? Miros?aw, > >> 2013/1/21 Marek Vasut <marex@denx.de>: >> > 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. >> >> [...] >> >> > +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)); >> > >> > } >> > >> > +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. > > 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
Dear Micha? Miros?aw, > 2013/1/21 Marek Vasut <marex@denx.de>: > > Dear Micha? Miros?aw, > > > >> 2013/1/21 Marek Vasut <marex@denx.de>: > >> > 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. > >> > >> [...] > >> > >> > +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)); > >> > > >> > } > >> > > >> > +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. > > > > 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. > Sure, it's just an insn or two, so no strong opinion here --- just curious. I tried to put it down above ;-) > Best Regards, > Micha? Miros?aw Best regards, Marek Vasut
On 01/21/2013 10:32 PM, Marek Vasut wrote: > Dear Micha? Miros?aw, > >> 2013/1/21 Marek Vasut <marex@denx.de>: >>> Dear Micha? Miros?aw, >>> >>>> 2013/1/21 Marek Vasut <marex@denx.de>: >>>>> 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. >>>> >>>> [...] >>>> >>>>> +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)); >>>>> >>>>> } >>>>> >>>>> +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. >>> >>> 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. 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
2013/1/21 Marek Vasut <marex@denx.de>: > Dear Micha? Miros?aw, >> 2013/1/21 Marek Vasut <marex@denx.de>: >> > Dear Micha? Miros?aw, >> >> 2013/1/21 Marek Vasut <marex@denx.de>: >> >> > 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. >> >> >> >> [...] >> >> >> >> > +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)); >> >> > >> >> > } >> >> > >> >> > +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. >> > >> > 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. 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
Dear Lars-Peter Clausen, > On 01/21/2013 10:32 PM, Marek Vasut wrote: > > Dear Micha? Miros?aw, > > > >> 2013/1/21 Marek Vasut <marex@denx.de>: > >>> Dear Micha? Miros?aw, > >>> > >>>> 2013/1/21 Marek Vasut <marex@denx.de>: > >>>>> 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. > >>>> > >>>> [...] > >>>> > >>>>> +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)); > >>>>> > >>>>> } > >>>>> > >>>>> +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. > >>> > >>> 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. > > 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. Certainly. All that is needed is in place now. Best regards, Marek Vasut
Dear Micha? Miros?aw, [...] > >> 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? 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
On 01/21/2013 09:49 PM, Marek Vasut wrote: > Dear Lars-Peter Clausen, > >> On 01/21/2013 10:32 PM, Marek Vasut wrote: >>> Dear Micha? Miros?aw, >>> >>>> 2013/1/21 Marek Vasut <marex@denx.de>: >>>>> Dear Micha? Miros?aw, >>>>> >>>>>> 2013/1/21 Marek Vasut <marex@denx.de>: >>>>>>> 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. >>>>>> >>>>>> [...] >>>>>> >>>>>>> +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)); >>>>>>> >>>>>>> } >>>>>>> >>>>>>> +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. >>>>> >>>>> 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. >> >> 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. > > 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 ;)
diff --git a/drivers/staging/iio/adc/Kconfig b/drivers/staging/iio/adc/Kconfig index fb8c239..7b2a01d 100644 --- a/drivers/staging/iio/adc/Kconfig +++ b/drivers/staging/iio/adc/Kconfig @@ -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 diff --git a/drivers/staging/iio/adc/mxs-lradc.c b/drivers/staging/iio/adc/mxs-lradc.c index 829d043..bea8372 100644 --- a/drivers/staging/iio/adc/mxs-lradc.c +++ b/drivers/staging/iio/adc/mxs-lradc.c @@ -76,7 +76,24 @@ */ #define LRADC_TS_SAMPLE_AMOUNT 4 -static const char * const mxs_lradc_irq_name[] = { +enum mxs_lradc_id { + IMX23_LRADC, + IMX28_LRADC, +}; + +static const char * const mx23_lradc_irq_names[] = { + "mxs-lradc-touchscreen", + "mxs-lradc-channel0", + "mxs-lradc-channel1", + "mxs-lradc-channel2", + "mxs-lradc-channel3", + "mxs-lradc-channel4", + "mxs-lradc-channel5", + "mxs-lradc-channel6", + "mxs-lradc-channel7", +}; + +static const char * const mx28_lradc_irq_names[] = { "mxs-lradc-touchscreen", "mxs-lradc-thresh0", "mxs-lradc-thresh1", @@ -92,6 +109,22 @@ static const char * const mxs_lradc_irq_name[] = { "mxs-lradc-button1", }; +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)); } +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); + static int mxs_lradc_probe(struct platform_device *pdev) { + const struct of_device_id *of_id = + of_match_device(mxs_lradc_dt_ids, &pdev->dev); + const struct mxs_lradc_of_config *of_cfg = + &mxs_lradc_of_config[(enum mxs_lradc_id)of_id->data]; struct device *dev = &pdev->dev; struct device_node *node = dev->of_node; struct mxs_lradc *lradc; @@ -902,7 +946,7 @@ static int mxs_lradc_probe(struct platform_device *pdev) ts_wires); /* Grab all IRQ sources */ - for (i = 0; i < 13; i++) { + for (i = 0; i < of_cfg->irq_count; i++) { lradc->irq[i] = platform_get_irq(pdev, i); if (lradc->irq[i] < 0) { ret = -EINVAL; @@ -911,7 +955,7 @@ static int mxs_lradc_probe(struct platform_device *pdev) ret = devm_request_irq(dev, lradc->irq[i], mxs_lradc_handle_irq, 0, - mxs_lradc_irq_name[i], iio); + of_cfg->irq_name[i], iio); if (ret) goto err_addr; } @@ -983,12 +1027,6 @@ static int mxs_lradc_remove(struct platform_device *pdev) return 0; } -static const struct of_device_id mxs_lradc_dt_ids[] = { - { .compatible = "fsl,imx28-lradc", }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(of, mxs_lradc_dt_ids); - static struct platform_driver mxs_lradc_driver = { .driver = { .name = DRIVER_NAME,
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. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Fabio Estevam <fabio.estevam@freescale.com> Cc: Jonathan Cameron <jic23@kernel.org> Cc: Shawn Guo <shawn.guo@linaro.org> --- drivers/staging/iio/adc/Kconfig | 4 +-- drivers/staging/iio/adc/mxs-lradc.c | 56 +++++++++++++++++++++++++++++------ 2 files changed, 49 insertions(+), 11 deletions(-)