| Message ID | 20220712110232.329164-1-francesco.dolcini@toradex.com |
|---|---|
| Headers | show |
| Series | mfd: stmpe: Probe sub-function by compatible | expand |
On Tue, 12 Jul 2022, Francesco Dolcini wrote: > Use sub-function of_compatible during probe, instead of using the node > name. The code should not rely on the node names during probe, in > addition to that the previously hard-coded node names are not compliant > to the latest naming convention (they are not generic and they use > underscores), and it was broken by mistake already once [1]. > > While doing this change `rotator` entry was removed, it is not > used in any device tree file, there is no cell defined, it's just dead > non-working code with no of_compatible for it. > > [1] commit 56086b5e804f ("ARM: dts: imx6qdl-apalis: Avoid underscore in node name") > > Suggested-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com> > --- > drivers/mfd/stmpe.c | 31 +++++++++++++++++-------------- > 1 file changed, 17 insertions(+), 14 deletions(-) > > diff --git a/drivers/mfd/stmpe.c b/drivers/mfd/stmpe.c > index aeb9ea55f97d..90a07a94455f 100644 > --- a/drivers/mfd/stmpe.c > +++ b/drivers/mfd/stmpe.c > @@ -23,6 +23,12 @@ > #include <linux/regulator/consumer.h> > #include "stmpe.h" > > +#define STMPE_GPIO_COMPATIBLE "st,stmpe-gpio" > +#define STMPE_KEYPAD_COMPATIBLE "st,stmpe-keypad" > +#define STMPE_PWM_COMPATIBLE "st,stmpe-pwm" > +#define STMPE_TS_COMPATIBLE "st,stmpe-ts" > +#define STMPE_ADC_COMPATIBLE "st,stmpe-adc" This is horrible. Please refrain from defining device/compatible strings. > /** > * struct stmpe_platform_data - STMPE platform data > * @id: device id to distinguish between multiple STMPEs on the same board > @@ -321,14 +327,14 @@ static struct resource stmpe_gpio_resources[] = { > > static const struct mfd_cell stmpe_gpio_cell = { > .name = "stmpe-gpio", > - .of_compatible = "st,stmpe-gpio", > + .of_compatible = STMPE_GPIO_COMPATIBLE, > .resources = stmpe_gpio_resources, > .num_resources = ARRAY_SIZE(stmpe_gpio_resources), > }; > > static const struct mfd_cell stmpe_gpio_cell_noirq = { > .name = "stmpe-gpio", > - .of_compatible = "st,stmpe-gpio", > + .of_compatible = STMPE_GPIO_COMPATIBLE, > /* gpio cell resources consist of an irq only so no resources here */ > }; > > @@ -350,7 +356,7 @@ static struct resource stmpe_keypad_resources[] = { > > static const struct mfd_cell stmpe_keypad_cell = { > .name = "stmpe-keypad", > - .of_compatible = "st,stmpe-keypad", > + .of_compatible = STMPE_KEYPAD_COMPATIBLE, > .resources = stmpe_keypad_resources, > .num_resources = ARRAY_SIZE(stmpe_keypad_resources), > }; > @@ -376,7 +382,7 @@ static struct resource stmpe_pwm_resources[] = { > > static const struct mfd_cell stmpe_pwm_cell = { > .name = "stmpe-pwm", > - .of_compatible = "st,stmpe-pwm", > + .of_compatible = STMPE_PWM_COMPATIBLE, > .resources = stmpe_pwm_resources, > .num_resources = ARRAY_SIZE(stmpe_pwm_resources), > }; > @@ -461,7 +467,7 @@ static struct resource stmpe_ts_resources[] = { > > static const struct mfd_cell stmpe_ts_cell = { > .name = "stmpe-ts", > - .of_compatible = "st,stmpe-ts", > + .of_compatible = STMPE_TS_COMPATIBLE, > .resources = stmpe_ts_resources, > .num_resources = ARRAY_SIZE(stmpe_ts_resources), > }; > @@ -484,7 +490,7 @@ static struct resource stmpe_adc_resources[] = { > > static const struct mfd_cell stmpe_adc_cell = { > .name = "stmpe-adc", > - .of_compatible = "st,stmpe-adc", > + .of_compatible = STMPE_ADC_COMPATIBLE, > .resources = stmpe_adc_resources, > .num_resources = ARRAY_SIZE(stmpe_adc_resources), > }; > @@ -1362,19 +1368,16 @@ static void stmpe_of_probe(struct stmpe_platform_data *pdata, > pdata->autosleep = (pdata->autosleep_timeout) ? true : false; > > for_each_available_child_of_node(np, child) { > - if (of_node_name_eq(child, "stmpe_gpio")) { > + if (of_device_is_compatible(child, STMPE_GPIO_COMPATIBLE)) > pdata->blocks |= STMPE_BLOCK_GPIO; > - } else if (of_node_name_eq(child, "stmpe_keypad")) { > + else if (of_device_is_compatible(child, STMPE_KEYPAD_COMPATIBLE)) > pdata->blocks |= STMPE_BLOCK_KEYPAD; > - } else if (of_node_name_eq(child, "stmpe_touchscreen")) { > + else if (of_device_is_compatible(child, STMPE_TS_COMPATIBLE)) > pdata->blocks |= STMPE_BLOCK_TOUCHSCREEN; > - } else if (of_node_name_eq(child, "stmpe_adc")) { > + else if (of_device_is_compatible(child, STMPE_ADC_COMPATIBLE)) > pdata->blocks |= STMPE_BLOCK_ADC; > - } else if (of_node_name_eq(child, "stmpe_pwm")) { > + else if (of_device_is_compatible(child, STMPE_PWM_COMPATIBLE)) > pdata->blocks |= STMPE_BLOCK_PWM; > - } else if (of_node_name_eq(child, "stmpe_rotator")) { > - pdata->blocks |= STMPE_BLOCK_ROTATOR; > - } This should be a separate patch. > } > } >
Hi all, This series update the STMPE MFD driver to use of_compatible to probe for sub-functions instead of using hardcoded names. Matching by name does not seems in general a good idea, in this specific case it is even worst since the node name are not compliant to the current naming convention (they are not generic and they do include underscores), and because of that recently we had a regression introduced [1]. This change was suggested by Ahmad Fatoum [2]. [1] commit 56086b5e ("ARM: dts: imx6qdl-apalis: Avoid underscore in node name") [2] https://lore.kernel.org/all/86815346-209e-304e-3565-b4160afa48e8@pengutronix.de/ Francesco Dolcini (4): mfd: stmpe: Probe sub-function by compatible dt-bindings: gpio: stmpe: Remove node name requirement dt-bindings: iio: adc: stmpe: Remove node name requirement dt-bindings: input: touchscreen: stmpe: Remove node name requirement .../devicetree/bindings/gpio/gpio-stmpe.txt | 3 +- .../bindings/iio/adc/st,stmpe-adc.yaml | 3 +- .../bindings/input/touchscreen/stmpe.txt | 3 +- drivers/mfd/stmpe.c | 31 ++++++++++--------- 4 files changed, 20 insertions(+), 20 deletions(-)