diff mbox series

[v3,1/2] dt-bindings: input/touchscreen: add bindings for msg26xx

Message ID 20210121174359.1455393-1-vincent.knecht@mailoo.org
State Changes Requested, archived
Headers show
Series [v3,1/2] dt-bindings: input/touchscreen: add bindings for msg26xx | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema success
robh/dtbs-check success

Commit Message

Vincent Knecht Jan. 21, 2021, 5:43 p.m. UTC
This adds dts bindings for the mstar msg26xx touchscreen.

Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
---
Changed in v3:
- added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties
Changed in v2:
- changed M-Star to MStar in title line
- changed reset gpio to active-low in example section
---
 .../input/touchscreen/mstar,msg26xx.yaml      | 69 +++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml

Comments

Rob Herring Feb. 9, 2021, 4:13 p.m. UTC | #1
On Thu, Jan 21, 2021 at 06:43:47PM +0100, Vincent Knecht wrote:
> This adds dts bindings for the mstar msg26xx touchscreen.
> 
> Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> ---
> Changed in v3:
> - added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties
> Changed in v2:
> - changed M-Star to MStar in title line
> - changed reset gpio to active-low in example section
> ---
>  .../input/touchscreen/mstar,msg26xx.yaml      | 69 +++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> new file mode 100644
> index 000000000000..5d26a1008bf1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> @@ -0,0 +1,69 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg26xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MStar msg26xx touchscreen controller Bindings
> +
> +maintainers:
> +  - Vincent Knecht <vincent.knecht@mailoo.org>
> +
> +allOf:
> +  - $ref: touchscreen.yaml#
> +
> +properties:
> +  compatible:
> +    const: mstar,msg26xx

Don't use wildcards in compatible strings.
Vincent Knecht Feb. 9, 2021, 6:58 p.m. UTC | #2
Le mardi 09 février 2021 à 10:13 -0600, Rob Herring a écrit :
> On Thu, Jan 21, 2021 at 06:43:47PM +0100, Vincent Knecht wrote:
> > This adds dts bindings for the mstar msg26xx touchscreen.
> > 
> > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> > ---
> > Changed in v3:
> > - added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties
> > Changed in v2:
> > - changed M-Star to MStar in title line
> > - changed reset gpio to active-low in example section
> > ---
> >  .../input/touchscreen/mstar,msg26xx.yaml      | 69 +++++++++++++++++++
> >  1 file changed, 69 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > new file mode 100644
> > index 000000000000..5d26a1008bf1
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > @@ -0,0 +1,69 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg26xx.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MStar msg26xx touchscreen controller Bindings
> > +
> > +maintainers:
> > +  - Vincent Knecht <vincent.knecht@mailoo.org>
> > +
> > +allOf:
> > +  - $ref: touchscreen.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    const: mstar,msg26xx
> 
> Don't use wildcards in compatible strings.

Thank you for the input...

Let's say I set it to "mstar,msg2638", is it better to rename the driver file and functions too ?
According to downstream source file naming, msg2638 is the model I have and test this driver with.


There's a possibility this driver works as-is or with minor mods for msg2633 too,
and a more remote one for msg21xx and msg22xx...
Jeff LaBundy Feb. 10, 2021, 3:10 a.m. UTC | #3
Hi Vincent,

On Tue, Feb 09, 2021 at 07:58:33PM +0100, Vincent Knecht wrote:
> Le mardi 09 février 2021 à 10:13 -0600, Rob Herring a écrit :
> > On Thu, Jan 21, 2021 at 06:43:47PM +0100, Vincent Knecht wrote:
> > > This adds dts bindings for the mstar msg26xx touchscreen.
> > > 
> > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> > > ---
> > > Changed in v3:
> > > - added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties
> > > Changed in v2:
> > > - changed M-Star to MStar in title line
> > > - changed reset gpio to active-low in example section
> > > ---
> > >  .../input/touchscreen/mstar,msg26xx.yaml      | 69 +++++++++++++++++++
> > >  1 file changed, 69 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > new file mode 100644
> > > index 000000000000..5d26a1008bf1
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > @@ -0,0 +1,69 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg26xx.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: MStar msg26xx touchscreen controller Bindings
> > > +
> > > +maintainers:
> > > +  - Vincent Knecht <vincent.knecht@mailoo.org>
> > > +
> > > +allOf:
> > > +  - $ref: touchscreen.yaml#
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: mstar,msg26xx
> > 
> > Don't use wildcards in compatible strings.
> 
> Thank you for the input...
> 
> Let's say I set it to "mstar,msg2638", is it better to rename the driver file and functions too ?
> According to downstream source file naming, msg2638 is the model I have and test this driver with.

This is ultimately Dmitry's call, but it's fairly common to use wildcards
for driver names and function calls if the driver is known to work across
all devices that fit in the wildcard (see iqs5xx and many others).

The risk with wildcards, however, is that vendors can introduce different
devices later with similar part numbers. Therefore, some subsystems (e.g.
iio) tend to frown upon wildcards for that reason.

You should try and make the driver cover as many devices as possible. But
if the driver is only known to work for one device then I don't think you
can use a wildcard in the name unless you support all other devices (just
my opinion).

In either case, however, compatible strings must be unique just as with a
part number in a schematic or bill of materials. As such, it is perfectly
fine to have multiple compatible strings in a single driver.

> 
> 
> There's a possibility this driver works as-is or with minor mods for msg2633 too,
> and a more remote one for msg21xx and msg22xx...
> 

Kind regards,
Jeff LaBundy
Vincent Knecht Feb. 10, 2021, 5:48 p.m. UTC | #4
Le mardi 09 février 2021 à 21:10 -0600, Jeff LaBundy a écrit :
> Hi Vincent,
> 
> On Tue, Feb 09, 2021 at 07:58:33PM +0100, Vincent Knecht wrote:
> > Le mardi 09 février 2021 à 10:13 -0600, Rob Herring a écrit :
> > > On Thu, Jan 21, 2021 at 06:43:47PM +0100, Vincent Knecht wrote:
> > > > This adds dts bindings for the mstar msg26xx touchscreen.
> > > > 
> > > > Signed-off-by: Vincent Knecht <vincent.knecht@mailoo.org>
> > > > ---
> > > > Changed in v3:
> > > > - added `touchscreen-size-x: true` and `touchscreen-size-y: true` properties
> > > > Changed in v2:
> > > > - changed M-Star to MStar in title line
> > > > - changed reset gpio to active-low in example section
> > > > ---
> > > >  .../input/touchscreen/mstar,msg26xx.yaml      | 69 +++++++++++++++++++
> > > >  1 file changed, 69 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > > b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > > new file mode 100644
> > > > index 000000000000..5d26a1008bf1
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
> > > > @@ -0,0 +1,69 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg26xx.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: MStar msg26xx touchscreen controller Bindings
> > > > +
> > > > +maintainers:
> > > > +  - Vincent Knecht <vincent.knecht@mailoo.org>
> > > > +
> > > > +allOf:
> > > > +  - $ref: touchscreen.yaml#
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: mstar,msg26xx
> > > 
> > > Don't use wildcards in compatible strings.
> > 
> > Thank you for the input...
> > 
> > Let's say I set it to "mstar,msg2638", is it better to rename the driver file and functions too ?
> > According to downstream source file naming, msg2638 is the model I have and test this driver with.
> 
> This is ultimately Dmitry's call, but it's fairly common to use wildcards
> for driver names and function calls if the driver is known to work across
> all devices that fit in the wildcard (see iqs5xx and many others).
> 
> The risk with wildcards, however, is that vendors can introduce different
> devices later with similar part numbers. Therefore, some subsystems (e.g.
> iio) tend to frown upon wildcards for that reason.
> 
> You should try and make the driver cover as many devices as possible. But
> if the driver is only known to work for one device then I don't think you
> can use a wildcard in the name unless you support all other devices (just
> my opinion).
> 
> In either case, however, compatible strings must be unique just as with a
> part number in a schematic or bill of materials. As such, it is perfectly
> fine to have multiple compatible strings in a single driver.
> 
> > 
> > 
> > There's a possibility this driver works as-is or with minor mods for msg2633 too,
> > and a more remote one for msg21xx and msg22xx...
> > 
> 
> Kind regards,
> Jeff LaBundy

Thank you Jeff for the insight.

Since I can't test it with any other model, I've renamed it to msg2638 in v4:
https://lore.kernel.org/linux-input/20210210173403.667482-1-vincent.knecht@mailoo.org/T/#t
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
new file mode 100644
index 000000000000..5d26a1008bf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/mstar,msg26xx.yaml
@@ -0,0 +1,69 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/mstar,msg26xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MStar msg26xx touchscreen controller Bindings
+
+maintainers:
+  - Vincent Knecht <vincent.knecht@mailoo.org>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  compatible:
+    const: mstar,msg26xx
+
+  reg:
+    const: 0x26
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vdd-supply:
+    description: Power supply regulator for the chip
+
+  vddio-supply:
+    description: Power supply regulator for the I2C bus
+
+  touchscreen-size-x: true
+  touchscreen-size-y: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - touchscreen-size-x
+  - touchscreen-size-y
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      touchscreen@26 {
+        compatible = "mstar,msg26xx";
+        reg = <0x26>;
+        interrupt-parent = <&msmgpio>;
+        interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+        reset-gpios = <&msmgpio 100 GPIO_ACTIVE_LOW>;
+        pinctrl-names = "default";
+        pinctrl-0 = <&ts_int_active>;
+        vdd-supply = <&pm8916_l17>;
+        vddio-supply = <&pm8916_l5>;
+        touchscreen-size-x = <720>;
+        touchscreen-size-y = <1280>;
+      };
+    };
+
+...