* [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen @ 2020-02-04 9:22 Marian-Cristian Rotariu 2020-03-02 14:13 ` Geert Uytterhoeven 0 siblings, 1 reply; 5+ messages in thread From: Marian-Cristian Rotariu @ 2020-02-04 9:22 UTC (permalink / raw) To: linux-renesas-soc Cc: Chris Paterson, prabhakar.mahadev-lad.rj, Marian-Cristian Rotariu In one of the iWave-G22D development board variants, called Generic SODIMM Development Platform, we have an LCD with touchscreen. The resistive touch controller, STMPE811 is on the development board and is connected through the i2c5 of the RZ-G1E. Additionally, this controller should generate an interrupt to the CPU and it is connected through GPIO4,4 to the GIC. Touch was tested with one of our iW-RainboW-G22D-SODIMM RZ/G1E development platforms. More details on the iWave website: https://www.iwavesystems.com/rz-g1e-sodimm-development-kit.html Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com> Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts | 46 +++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts index ce6603b..1051d82 100644 --- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts +++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts @@ -128,6 +128,47 @@ status = "okay"; clock-frequency = <400000>; + stmpe811@44 { + compatible = "st,stmpe811"; + #address-cells = <1>; + #size-cells = <0>; + reg = <0x44>; + interrupt-parent = <&gpio4>; + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; + irq-gpio = <&gpio4 4 IRQ_TYPE_LEVEL_LOW>; + pinctrl-0 = <&touch>; + pinctrl-names = "default"; + id = <0>; + blocks = <0x5>; + irq-trigger = <0x1>; + + stmpe_touchscreen { + compatible = "st,stmpe-ts"; + reg = <0>; + /* 3.25 MHz ADC clock speed */ + st,adc-freq = <3>; + /* 8 sample average control */ + st,ave-ctrl = <2>; + /* 7 length fractional part in z */ + st,fraction-z = <7>; + /* + * 50 mA typical 80 mA max touchscreen drivers + * current limit value + */ + st,i-drive = <0>; + /* 12-bit ADC */ + st,mod-12b = <1>; + /* internal ADC reference */ + st,ref-sel = <0>; + /* ADC converstion time: 80 clocks */ + st,sample-time = <4>; + /* 1 ms panel driver settling time */ + st,settling = <3>; + /* 5 ms touch detect interrupt delay */ + st,touch-det-delay = <4>; + }; + }; + sgtl5000: codec@a { compatible = "fsl,sgtl5000"; #sound-dai-cells = <0>; @@ -181,6 +222,11 @@ function = "ssi"; }; + touch: stmpe811 { + groups = "intc_irq0"; + function = "intc"; + }; + usb0_pins: usb0 { groups = "usb0"; function = "usb0"; -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen 2020-02-04 9:22 [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen Marian-Cristian Rotariu @ 2020-03-02 14:13 ` Geert Uytterhoeven 2020-03-04 12:38 ` Marian-Cristian Rotariu 0 siblings, 1 reply; 5+ messages in thread From: Geert Uytterhoeven @ 2020-03-02 14:13 UTC (permalink / raw) To: Marian-Cristian Rotariu Cc: Linux-Renesas, Chris Paterson, Lad, Prabhakar, Dmitry Torokhov, linux-input, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Marian-Cristian, On Tue, Feb 4, 2020 at 10:23 AM Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com> wrote: > In one of the iWave-G22D development board variants, called Generic SODIMM > Development Platform, we have an LCD with touchscreen. The resistive touch > controller, STMPE811 is on the development board and is connected through > the i2c5 of the RZ-G1E. > > Additionally, this controller should generate an interrupt to the CPU and > it is connected through GPIO4,4 to the GIC. > > Touch was tested with one of our iW-RainboW-G22D-SODIMM RZ/G1E development > platforms. > > More details on the iWave website: > https://www.iwavesystems.com/rz-g1e-sodimm-development-kit.html > > Signed-off-by: Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> Thanks for your patch! > --- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > +++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > @@ -128,6 +128,47 @@ > status = "okay"; > clock-frequency = <400000>; > > + stmpe811@44 { > + compatible = "st,stmpe811"; According to the DT bindings, this must be "st,stmpe-ts", but the example also uses "st,stmpe811"? > + #address-cells = <1>; > + #size-cells = <0>; Not documented in the DT bindings (but used in the example). > + reg = <0x44>; > + interrupt-parent = <&gpio4>; > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; This should be "<4 IRQ_TYPE_LEVEL_LOW>", to refer to GP4_4. > + irq-gpio = <&gpio4 4 IRQ_TYPE_LEVEL_LOW>; "irq-gpio" is not documented in the DT bindings. > + pinctrl-0 = <&touch>; > + pinctrl-names = "default"; > + id = <0>; > + blocks = <0x5>; > + irq-trigger = <0x1>; Above 3 are not documented. > + > + stmpe_touchscreen { Missing unit-address. > + compatible = "st,stmpe-ts"; > + reg = <0>; > + /* 3.25 MHz ADC clock speed */ > + st,adc-freq = <3>; > + /* 8 sample average control */ > + st,ave-ctrl = <2>; > + /* 7 length fractional part in z */ > + st,fraction-z = <7>; > + /* > + * 50 mA typical 80 mA max touchscreen drivers > + * current limit value > + */ > + st,i-drive = <0>; Bindings say <0> corresponds to 20 mA. Either the comment is wrong, or this should be <1>. > + /* 12-bit ADC */ > + st,mod-12b = <1>; > + /* internal ADC reference */ > + st,ref-sel = <0>; > + /* ADC converstion time: 80 clocks */ > + st,sample-time = <4>; > + /* 1 ms panel driver settling time */ > + st,settling = <3>; > + /* 5 ms touch detect interrupt delay */ > + st,touch-det-delay = <4>; Bindings say <4> corresponds to 1 ms. Either the comment is wrong, or this should be <5>. > + }; > + }; > + > sgtl5000: codec@a { > compatible = "fsl,sgtl5000"; > #sound-dai-cells = <0>; > @@ -181,6 +222,11 @@ > function = "ssi"; > }; > > + touch: stmpe811 { > + groups = "intc_irq0"; > + function = "intc"; This does not match using GP4_4 for the interrupt. Either you use GP4_4: interrupt-parent = <&gpio4>; interrupts = <4 IRQ_TYPE_LEVEL_LOW>; which doesn't require any explicit pin control setup (the gpio-rcar driver takes care of that), or you use IRQ0: interrupt-parent = <&irqc>; interrupts = <0 IRQ_TYPE_LEVEL_LOW>; The latter does require explicit pin control setup. > + }; > + > usb0_pins: usb0 { > groups = "usb0"; > function = "usb0"; Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen 2020-03-02 14:13 ` Geert Uytterhoeven @ 2020-03-04 12:38 ` Marian-Cristian Rotariu 2020-03-04 13:01 ` Geert Uytterhoeven 0 siblings, 1 reply; 5+ messages in thread From: Marian-Cristian Rotariu @ 2020-03-04 12:38 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Linux-Renesas, Chris Paterson, Prabhakar Mahadev Lad, Dmitry Torokhov, linux-input, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Geert, Thank you for the review. > > --- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > > +++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > > @@ -128,6 +128,47 @@ > > status = "okay"; > > clock-frequency = <400000>; > > > > + stmpe811@44 { > > + compatible = "st,stmpe811"; > > According to the DT bindings, this must be "st,stmpe-ts", but the example > also uses "st,stmpe811"? The device is a MFD and the bindings doc is here: Documentation/devicetree/bindings/mfd/stmpe.txt You need to add its specific function as a child node of the mfd dt node. In our case it is a touchscreen: Documentation/devicetree/bindings/input/touchscreen/stmpe.txt > > > + #address-cells = <1>; > > + #size-cells = <0>; > > Not documented in the DT bindings (but used in the example). In the child node you do not need the reg property, therefore the example is not applicable. I will remove the above in the v2 patch. > > > + reg = <0x44>; > > + interrupt-parent = <&gpio4>; > > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > This should be "<4 IRQ_TYPE_LEVEL_LOW>", to refer to GP4_4. Indeed, I will fix it in v2. > > > + irq-gpio = <&gpio4 4 IRQ_TYPE_LEVEL_LOW>; > > "irq-gpio" is not documented in the DT bindings. See "Documentation/devicetree/bindings/mfd/stmpe.txt" > > > + pinctrl-0 = <&touch>; > > + pinctrl-names = "default"; > > + id = <0>; > > + blocks = <0x5>; > > + irq-trigger = <0x1>; > > Above 3 are not documented. These must be relics from an old driver that we have for an older kernel version. I will remove all 3 as the current driver is not using them. > > > + > > + stmpe_touchscreen { > > Missing unit-address. I will remove the reg property, therefore no unit-address requirement. > > > + compatible = "st,stmpe-ts"; > > + reg = <0>; > > + /* 3.25 MHz ADC clock speed */ > > + st,adc-freq = <3>; > > + /* 8 sample average control */ > > + st,ave-ctrl = <2>; > > + /* 7 length fractional part in z */ > > + st,fraction-z = <7>; > > + /* > > + * 50 mA typical 80 mA max touchscreen drivers > > + * current limit value > > + */ > > + st,i-drive = <0>; > > Bindings say <0> corresponds to 20 mA. > Either the comment is wrong, or this should be <1>. I will add the value that matches the one from the comment. > > > + /* 12-bit ADC */ > > + st,mod-12b = <1>; > > + /* internal ADC reference */ > > + st,ref-sel = <0>; > > + /* ADC converstion time: 80 clocks */ > > + st,sample-time = <4>; > > + /* 1 ms panel driver settling time */ > > + st,settling = <3>; > > + /* 5 ms touch detect interrupt delay */ > > + st,touch-det-delay = <4>; > > Bindings say <4> corresponds to 1 ms. > Either the comment is wrong, or this should be <5>. As above. > > > + }; > > + }; > > + > > sgtl5000: codec@a { > > compatible = "fsl,sgtl5000"; > > #sound-dai-cells = <0>; @@ -181,6 +222,11 @@ > > function = "ssi"; > > }; > > > > + touch: stmpe811 { > > + groups = "intc_irq0"; > > + function = "intc"; > > This does not match using GP4_4 for the interrupt. > > Either you use GP4_4: > > interrupt-parent = <&gpio4>; > interrupts = <4 IRQ_TYPE_LEVEL_LOW>; > > which doesn't require any explicit pin control setup (the gpio-rcar driver > takes care of that), or you use IRQ0: > > interrupt-parent = <&irqc>; > interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > The latter does require explicit pin control setup. I will use the first approach as the patch looks more compact. Best regards, Marian ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen 2020-03-04 12:38 ` Marian-Cristian Rotariu @ 2020-03-04 13:01 ` Geert Uytterhoeven 2020-03-04 14:27 ` Marian-Cristian Rotariu 0 siblings, 1 reply; 5+ messages in thread From: Geert Uytterhoeven @ 2020-03-04 13:01 UTC (permalink / raw) To: Marian-Cristian Rotariu Cc: Linux-Renesas, Chris Paterson, Prabhakar Mahadev Lad, Dmitry Torokhov, linux-input, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Marian, On Wed, Mar 4, 2020 at 1:38 PM Marian-Cristian Rotariu <marian-cristian.rotariu.rb@bp.renesas.com> wrote: > > > --- a/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > > > +++ b/arch/arm/boot/dts/r8a7745-iwg22d-sodimm.dts > > > @@ -128,6 +128,47 @@ > > > status = "okay"; > > > clock-frequency = <400000>; > > > > > > + stmpe811@44 { > > > + compatible = "st,stmpe811"; > > > > According to the DT bindings, this must be "st,stmpe-ts", but the example > > also uses "st,stmpe811"? > > The device is a MFD and the bindings doc is here: > Documentation/devicetree/bindings/mfd/stmpe.txt Thanks, I hadn't considered that file when looking for "st,stmpe811", due to the regex used in the document. > You need to add its specific function as a child node of the mfd dt node. In our > case it is a touchscreen: > Documentation/devicetree/bindings/input/touchscreen/stmpe.txt OK. > > > + reg = <0x44>; > > > + interrupt-parent = <&gpio4>; > > > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > > > This should be "<4 IRQ_TYPE_LEVEL_LOW>", to refer to GP4_4. > > Indeed, I will fix it in v2. > > > > > > + irq-gpio = <&gpio4 4 IRQ_TYPE_LEVEL_LOW>; > > > > "irq-gpio" is not documented in the DT bindings. > > See "Documentation/devicetree/bindings/mfd/stmpe.txt" I believe you cannot use the same GPIO as an interrupt and as a GPIO at the same time. Don't you get a -EBUSY somewhere? Perhaps it worked due to the typo above? As both interrupts and irq-gpio are documented to be optional properties, probably they are mutually exclusive, and you can drop irq-gpio? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen 2020-03-04 13:01 ` Geert Uytterhoeven @ 2020-03-04 14:27 ` Marian-Cristian Rotariu 0 siblings, 0 replies; 5+ messages in thread From: Marian-Cristian Rotariu @ 2020-03-04 14:27 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Linux-Renesas, Chris Paterson, Prabhakar Mahadev Lad, Dmitry Torokhov, linux-input, Rob Herring, open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS Hi Geert, > > > > + reg = <0x44>; > > > > + interrupt-parent = <&gpio4>; > > > > + interrupts = <0 IRQ_TYPE_LEVEL_LOW>; > > > > > > This should be "<4 IRQ_TYPE_LEVEL_LOW>", to refer to GP4_4. > > > > Indeed, I will fix it in v2. > > > > > > > > > + irq-gpio = <&gpio4 4 IRQ_TYPE_LEVEL_LOW>; > > > > > > "irq-gpio" is not documented in the DT bindings. > > > > See "Documentation/devicetree/bindings/mfd/stmpe.txt" > > I believe you cannot use the same GPIO as an interrupt and as a GPIO at the > same time. Don't you get a -EBUSY somewhere? > Perhaps it worked due to the typo above? > > As both interrupts and irq-gpio are documented to be optional properties, > probably they are mutually exclusive, and you can drop irq-gpio? Yes, this is redundant. They are mutually exclusive in the driver code with irq-gpio having precedence over the interrupts/interrupt-parent registration. Therefore, I will remove the irq-gpio property as interrupts/interrupt-parent pair does the job. Thank you, Marian ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-03-04 14:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-02-04 9:22 [PATCH] ARM: dts: iwg22d-sodimm: Enable touchscreen Marian-Cristian Rotariu 2020-03-02 14:13 ` Geert Uytterhoeven 2020-03-04 12:38 ` Marian-Cristian Rotariu 2020-03-04 13:01 ` Geert Uytterhoeven 2020-03-04 14:27 ` Marian-Cristian Rotariu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).