[2/2] dt-bindings: input: Update documentation for clps711x-keypad
diff mbox series

Message ID 20181222155520.8156-1-shc_work@mail.ru
State New
Headers show
Series
  • [1/2] input: clps711x-keypad: Switch to use syscon_regmap_lookup_by_phandle()
Related show

Commit Message

Alexander Shiyan Dec. 22, 2018, 3:55 p.m. UTC
Since the driver switched to using the syscon_regmap_lookup_by_phandle()
function, we need to specify the phandle for the driver. This patch
updates the driver binding documentation for these changes.

Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
---
 Documentation/devicetree/bindings/input/clps711x-keypad.txt | 2 ++
 1 file changed, 2 insertions(+)

Comments

Rob Herring Jan. 3, 2019, 8:59 p.m. UTC | #1
On Sat, Dec 22, 2018 at 06:55:20PM +0300, Alexander Shiyan wrote:
> Since the driver switched to using the syscon_regmap_lookup_by_phandle()
> function, we need to specify the phandle for the driver. This patch
> updates the driver binding documentation for these changes.

That's not a backwards compatible change. Is that okay for all users of 
this platform(s)? If so, say why.

> 
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
>  Documentation/devicetree/bindings/input/clps711x-keypad.txt | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> index 3eed881..aebedc3 100644
> --- a/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> +++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> @@ -6,6 +6,7 @@ Required Properties:
>  - poll-interval: Poll interval time in milliseconds.
>  - linux,keymap:  The definition can be found at
>                   bindings/input/matrix-keymap.txt.
> +- syscon:        Phandle that points to the CPU syscon1 node.

What does this provide? If the keypad registers are a sub-block of the 
syscon1, then you should make this node a child node.

>  
>  Optional Properties:
>  - autorepeat:    Enable autorepeat feature.
> @@ -17,6 +18,7 @@ Example:
>  		poll-interval = <120>;
>  		row-gpios = <&porta 0 0>,
>  			    <&porta 1 0>;
> +		syscon = <&syscon1>;
>  
>  		linux,keymap = <
>  			MATRIX_KEY(0, 0, KEY_UP)
> -- 
> 2.10.2
>
Alexander Shiyan Jan. 16, 2019, 12:26 p.m. UTC | #2
>Четверг,  3 января 2019, 23:59 +03:00 от Rob Herring <robh@kernel.org>:
>
>On Sat, Dec 22, 2018 at 06:55:20PM +0300, Alexander Shiyan wrote:
>> Since the driver switched to using the syscon_regmap_lookup_by_phandle()
>> function, we need to specify the phandle for the driver. This patch
>> updates the driver binding documentation for these changes.
...
>> diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> index 3eed881..aebedc3 100644
>> --- a/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> +++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> @@ -6,6 +6,7 @@ Required Properties:
>>  - poll-interval: Poll interval time in milliseconds.
>>  - linux,keymap:  The definition can be found at
>>                   bindings/input/matrix-keymap.txt.
>> +- syscon:        Phandle that points to the CPU syscon1 node.
>
>What does this provide? If the keypad registers are a sub-block of the 
>syscon1, then you should make this node a child node.

Syscon does not populate child devices.

---
Rob Herring Jan. 21, 2019, 4:25 p.m. UTC | #3
On Wed, Jan 16, 2019 at 6:26 AM Alexander Shiyan <shc_work@mail.ru> wrote:
>
> >Четверг,  3 января 2019, 23:59 +03:00 от Rob Herring <robh@kernel.org>:
> >
> >On Sat, Dec 22, 2018 at 06:55:20PM +0300, Alexander Shiyan wrote:
> >> Since the driver switched to using the syscon_regmap_lookup_by_phandle()
> >> function, we need to specify the phandle for the driver. This patch
> >> updates the driver binding documentation for these changes.
> ...
> >> diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> >> index 3eed881..aebedc3 100644
> >> --- a/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> >> +++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
> >> @@ -6,6 +6,7 @@ Required Properties:
> >>  - poll-interval: Poll interval time in milliseconds.
> >>  - linux,keymap:  The definition can be found at
> >>                   bindings/input/matrix-keymap.txt.
> >> +- syscon:        Phandle that points to the CPU syscon1 node.
> >
> >What does this provide? If the keypad registers are a sub-block of the
> >syscon1, then you should make this node a child node.
>
> Syscon does not populate child devices.

I don't have any context of what syscon1 looks like. Maybe it needs
simple-mfd or a driver for the syscon.

Rob
Alexander Shiyan Jan. 22, 2019, 7:54 a.m. UTC | #4
>Понедельник, 21 января 2019, 19:25 +03:00 от Rob Herring <robh@kernel.org>:
...
>> >On Sat, Dec 22, 2018 at 06:55:20PM +0300, Alexander Shiyan wrote:
>> >> Since the driver switched to using the syscon_regmap_lookup_by_phandle()
>> >> function, we need to specify the phandle for the driver. This patch
>> >> updates the driver binding documentation for these changes.
>> ...
>> >> diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> >> index 3eed881..aebedc3 100644
>> >> --- a/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> >> +++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
>> >> @@ -6,6 +6,7 @@ Required Properties:
>> >>  - poll-interval: Poll interval time in milliseconds.
>> >>  - linux,keymap:  The definition can be found at
>> >>                   bindings/input/matrix-keymap.txt.
>> >> +- syscon:        Phandle that points to the CPU syscon1 node.
>> >
>> >What does this provide? If the keypad registers are a sub-block of the
>> >syscon1, then you should make this node a child node.
>>
>> Syscon does not populate child devices.
>
>I don't have any context of what syscon1 looks like. Maybe it needs
>simple-mfd or a driver for the syscon.

The SYSCON1 on this processor is already being processed by the
mfd/syscon driver, so it is represents in DTS, as shown below:
...
		syscon1: syscon@80000100 {
			compatible = "syscon";
			reg = <0x80000100 0x80>;
		};
...
	keypad: keypad {
		compatible = "cirrus,ep7209-keypad";
		syscon = <&syscon1>;
		status = "disabled";
	};

Maybe I need to expand the example section in the DT-bindings so that we can see it?

---

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/input/clps711x-keypad.txt b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
index 3eed881..aebedc3 100644
--- a/Documentation/devicetree/bindings/input/clps711x-keypad.txt
+++ b/Documentation/devicetree/bindings/input/clps711x-keypad.txt
@@ -6,6 +6,7 @@  Required Properties:
 - poll-interval: Poll interval time in milliseconds.
 - linux,keymap:  The definition can be found at
                  bindings/input/matrix-keymap.txt.
+- syscon:        Phandle that points to the CPU syscon1 node.
 
 Optional Properties:
 - autorepeat:    Enable autorepeat feature.
@@ -17,6 +18,7 @@  Example:
 		poll-interval = <120>;
 		row-gpios = <&porta 0 0>,
 			    <&porta 1 0>;
+		syscon = <&syscon1>;
 
 		linux,keymap = <
 			MATRIX_KEY(0, 0, KEY_UP)