[PATCH v1 0/4] mfd: stmpe: Probe sub-function by compatible

[PATCH v1 0/4] mfd: stmpe: Probe sub-function by compatible

[Francesco Dolcini]

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(-)

-- 
2.25.1

[PATCH v1 1/4] mfd: stmpe: Probe sub-function by compatible

[Francesco Dolcini]

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"
+
 /**
  * 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;
-		}
 	}
 }
 
-- 
2.25.1

Re: [PATCH v1 1/4] mfd: stmpe: Probe sub-function by compatible

[Lee Jones]

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.

>  	}
>  }
>  

-- 
Lee Jones [李琼斯]
Principal Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v1 2/4] dt-bindings: gpio: stmpe: Remove node name requirement

[Francesco Dolcini]

STMPE driver does not require a specific node name anymore, only the
compatible is checked, update binding according to this.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 Documentation/devicetree/bindings/gpio/gpio-stmpe.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
index a0e4cf885213..b33f8f02c0d7 100644
--- a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
+++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
@@ -8,8 +8,7 @@ Optional properties:
  - st,norequest-mask: bitmask specifying which GPIOs should _not_ be requestable
    due to different usage (e.g. touch, keypad)
 
-Node name must be stmpe_gpio and should be child node of stmpe node to which it
-belongs.
+Node should be child node of stmpe node to which it belongs.
 
 Example:
 	stmpe_gpio {
-- 
2.25.1

Re: [PATCH v1 2/4] dt-bindings: gpio: stmpe: Remove node name requirement

[Bartosz Golaszewski]

On Tue, Jul 12, 2022 at 1:02 PM Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:
>
> STMPE driver does not require a specific node name anymore, only the
> compatible is checked, update binding according to this.
>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
> ---
>  Documentation/devicetree/bindings/gpio/gpio-stmpe.txt | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
> index a0e4cf885213..b33f8f02c0d7 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-stmpe.txt
> @@ -8,8 +8,7 @@ Optional properties:
>   - st,norequest-mask: bitmask specifying which GPIOs should _not_ be requestable
>     due to different usage (e.g. touch, keypad)
>
> -Node name must be stmpe_gpio and should be child node of stmpe node to which it
> -belongs.
> +Node should be child node of stmpe node to which it belongs.
>
>  Example:
>         stmpe_gpio {
> --
> 2.25.1
>

Acked-by: Bartosz Golaszewski <brgl@bgdev.pl>

Re: [PATCH v1 2/4] dt-bindings: gpio: stmpe: Remove node name requirement

[Linus Walleij]

On Tue, Jul 12, 2022 at 1:02 PM Francesco Dolcini
<francesco.dolcini@toradex.com> wrote:

> STMPE driver does not require a specific node name anymore, only the
> compatible is checked, update binding according to this.
>
> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>

Another leftover from the early days of device tree. (This was one
of the first ARM drivers converted to DT).
Thanks for fixing!

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH v1 3/4] dt-bindings: iio: adc: stmpe: Remove node name requirement

[Francesco Dolcini]

STMPE driver does not require a specific node name anymore, only the
compatible is checked, update binding according to this.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 Documentation/devicetree/bindings/iio/adc/st,stmpe-adc.yaml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/st,stmpe-adc.yaml b/Documentation/devicetree/bindings/iio/adc/st,stmpe-adc.yaml
index 9049c699152f..333744a2159c 100644
--- a/Documentation/devicetree/bindings/iio/adc/st,stmpe-adc.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/st,stmpe-adc.yaml
@@ -13,8 +13,7 @@ description:
   This ADC forms part of an ST microelectronics STMPE multifunction device .
   The ADC is shared with the STMPE touchscreen. As a result some ADC related
   settings are specified in the parent node.
-  The node name myst be stmpe_adc and should be a child node of the stmpe node
-  to which it belongs.
+  The node should be a child node of the stmpe node to which it belongs.
 
 properties:
   compatible:
-- 
2.25.1

[PATCH v1 4/4] dt-bindings: input: touchscreen: stmpe: Remove node name requirement

[Francesco Dolcini]

STMPE driver does not require a specific node name anymore, only the
compatible is checked, update binding according to this.

Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
 Documentation/devicetree/bindings/input/touchscreen/stmpe.txt | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
index c549924603d2..238b51555c04 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/stmpe.txt
@@ -54,8 +54,7 @@ Optional properties common with MFD (deprecated):
 				1 -> 3.25 MHz
 				2 || 3 -> 6.5 MHz
 
-Node name must be stmpe_touchscreen and should be child node of stmpe node to
-which it belongs.
+Node should be child node of stmpe node to which it belongs.
 
 Note that common ADC settings of stmpe_touchscreen (child) will take precedence
 over the settings done in MFD.
-- 
2.25.1