diff mbox

[v2,3/3] tps6507x-ts: add DT bindings description

Message ID 1489069869-3849-4-git-send-email-yegorslists@googlemail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Yegor Yefremov March 9, 2017, 2:31 p.m. UTC
From: Yegor Yefremov <yegorslists@googlemail.com>

Provide description for following properties:

- ti,poll-period
- ti,min-pressure

Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
---
 Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Rob Herring (Arm) March 9, 2017, 2:49 p.m. UTC | #1
On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> From: Yegor Yefremov <yegorslists@googlemail.com>

This needs to go to DT list.

>
> Provide description for following properties:
>
> - ti,poll-period
> - ti,min-pressure
>
> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> ---
>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> index 8fffa3c..8875662 100644
> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> @@ -1,4 +1,8 @@
> -TPS6507x Power Management Integrated Circuit
> +TPS6507x Multifunctional Device.
> +
> +Features provided by TPS6507x:
> +        1. Power Management Integrated Circuit.
> +        2. Touch-Screen.
>
>  Required properties:
>  - compatible: "ti,tps6507x"
> @@ -30,6 +34,12 @@ Regulator Optional properties:
>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>    If this property is not defined, it defaults to 0 (not enabled).
>
> +Touchscreen Optional properties:
> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> +                 Default value: 30 ms.

Isn't there a standard property for this?

> +- ti,min-pressure: Minimum pressure value to trigger touch.
> +                  Default value: 0x30.
> +
>  Example:
>
>         pmu: tps6507x@48 {
> @@ -40,6 +50,9 @@ Example:
>                 vindcdc3-supply = <...>;
>                 vinldo1_2-supply = <...>;
>
> +               ti,poll-period = <30>;
> +               ti,min-pressure = <0x30>;
> +
>                 regulators {
>                         #address-cells = <1>;
>                         #size-cells = <0>;
> @@ -87,5 +100,4 @@ Example:
>                                 regulator-boot-on;
>                         };
>                 };
> -
>         };
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Yegor Yefremov March 9, 2017, 3:18 p.m. UTC | #2
On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> From: Yegor Yefremov <yegorslists@googlemail.com>
>
> This needs to go to DT list.

Will do.

>>
>> Provide description for following properties:
>>
>> - ti,poll-period
>> - ti,min-pressure
>>
>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> ---
>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> index 8fffa3c..8875662 100644
>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> @@ -1,4 +1,8 @@
>> -TPS6507x Power Management Integrated Circuit
>> +TPS6507x Multifunctional Device.
>> +
>> +Features provided by TPS6507x:
>> +        1. Power Management Integrated Circuit.
>> +        2. Touch-Screen.
>>
>>  Required properties:
>>  - compatible: "ti,tps6507x"
>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>    If this property is not defined, it defaults to 0 (not enabled).
>>
>> +Touchscreen Optional properties:
>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> +                 Default value: 30 ms.
>
> Isn't there a standard property for this?

Such a value will be already used in [1]. I've looked at [2], but it
doesn't have such a field.

[1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
[2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

>> +- ti,min-pressure: Minimum pressure value to trigger touch.
>> +                  Default value: 0x30.
>> +
>>  Example:
>>
>>         pmu: tps6507x@48 {
>> @@ -40,6 +50,9 @@ Example:
>>                 vindcdc3-supply = <...>;
>>                 vinldo1_2-supply = <...>;
>>
>> +               ti,poll-period = <30>;
>> +               ti,min-pressure = <0x30>;
>> +
>>                 regulators {
>>                         #address-cells = <1>;
>>                         #size-cells = <0>;
>> @@ -87,5 +100,4 @@ Example:
>>                                 regulator-boot-on;
>>                         };
>>                 };
>> -
>>         };
>> --
>> 2.1.4
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov March 9, 2017, 5:55 p.m. UTC | #3
On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> > On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >> From: Yegor Yefremov <yegorslists@googlemail.com>
> >
> > This needs to go to DT list.
> 
> Will do.
> 
> >>
> >> Provide description for following properties:
> >>
> >> - ti,poll-period
> >> - ti,min-pressure
> >>
> >> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >> ---
> >>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> index 8fffa3c..8875662 100644
> >> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> @@ -1,4 +1,8 @@
> >> -TPS6507x Power Management Integrated Circuit
> >> +TPS6507x Multifunctional Device.
> >> +
> >> +Features provided by TPS6507x:
> >> +        1. Power Management Integrated Circuit.
> >> +        2. Touch-Screen.
> >>
> >>  Required properties:
> >>  - compatible: "ti,tps6507x"
> >> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >>    If this property is not defined, it defaults to 0 (not enabled).
> >>
> >> +Touchscreen Optional properties:
> >> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >> +                 Default value: 30 ms.
> >
> > Isn't there a standard property for this?
> 
> Such a value will be already used in [1]. I've looked at [2], but it
> doesn't have such a field.
> 
> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt

I think the bigger question (since we are adding this to DT I assume it
is not longer "toy" driver) can we convert it to a proper
interrupt-driven scheme? Polling for real shipping devices is not
efficient, I do not think we should even try to allow this mode in DTS
bindings (note that tsc2007 use is different: it allows to specify
sampling interval once touch is detected).

Thanks.
Sekhar Nori March 10, 2017, 6:41 a.m. UTC | #4
On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>
>>> This needs to go to DT list.
>>
>> Will do.
>>
>>>>
>>>> Provide description for following properties:
>>>>
>>>> - ti,poll-period
>>>> - ti,min-pressure
>>>>
>>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>>> ---
>>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> index 8fffa3c..8875662 100644
>>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>> @@ -1,4 +1,8 @@
>>>> -TPS6507x Power Management Integrated Circuit
>>>> +TPS6507x Multifunctional Device.
>>>> +
>>>> +Features provided by TPS6507x:
>>>> +        1. Power Management Integrated Circuit.
>>>> +        2. Touch-Screen.
>>>>
>>>>  Required properties:
>>>>  - compatible: "ti,tps6507x"
>>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>>>    If this property is not defined, it defaults to 0 (not enabled).
>>>>
>>>> +Touchscreen Optional properties:
>>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>>>> +                 Default value: 30 ms.
>>>
>>> Isn't there a standard property for this?
>>
>> Such a value will be already used in [1]. I've looked at [2], but it
>> doesn't have such a field.
>>
>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> 
> I think the bigger question (since we are adding this to DT I assume it
> is not longer "toy" driver) can we convert it to a proper
> interrupt-driven scheme? Polling for real shipping devices is not
> efficient, I do not think we should even try to allow this mode in DTS
> bindings (note that tsc2007 use is different: it allows to specify
> sampling interval once touch is detected).

Unfortunately, the only current user of tps6507x in kernel (da850-evm)
does not have the touch interrupt connected to the processor. So if we
dont support polled mode in when using device-tree, da850-evm.dts wont
be able to use this driver.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring (Arm) March 10, 2017, 4:26 p.m. UTC | #5
On Thu, Mar 9, 2017 at 11:55 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>> > On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> >> From: Yegor Yefremov <yegorslists@googlemail.com>
>> >
>> > This needs to go to DT list.
>>
>> Will do.
>>
>> >>
>> >> Provide description for following properties:
>> >>
>> >> - ti,poll-period
>> >> - ti,min-pressure
>> >>
>> >> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> >> ---
>> >>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>> >>  1 file changed, 14 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> index 8fffa3c..8875662 100644
>> >> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >> @@ -1,4 +1,8 @@
>> >> -TPS6507x Power Management Integrated Circuit
>> >> +TPS6507x Multifunctional Device.
>> >> +
>> >> +Features provided by TPS6507x:
>> >> +        1. Power Management Integrated Circuit.
>> >> +        2. Touch-Screen.
>> >>
>> >>  Required properties:
>> >>  - compatible: "ti,tps6507x"
>> >> @@ -30,6 +34,12 @@ Regulator Optional properties:
>> >>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>> >>    If this property is not defined, it defaults to 0 (not enabled).
>> >>
>> >> +Touchscreen Optional properties:
>> >> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> >> +                 Default value: 30 ms.
>> >
>> > Isn't there a standard property for this?
>>
>> Such a value will be already used in [1]. I've looked at [2], but it
>> doesn't have such a field.
>>
>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>
> I think the bigger question (since we are adding this to DT I assume it
> is not longer "toy" driver) can we convert it to a proper
> interrupt-driven scheme? Polling for real shipping devices is not
> efficient, I do not think we should even try to allow this mode in DTS
> bindings (note that tsc2007 use is different: it allows to specify
> sampling interval once touch is detected).

I guess I was assuming that polling period corresponded to sampling
period or rate. The latter makes more sense for a binding and I would
think is fairly common whether the h/w has functionality or the driver
has to implement the sampling rate in s/w.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov March 10, 2017, 6 p.m. UTC | #6
On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
> >>>
> >>> This needs to go to DT list.
> >>
> >> Will do.
> >>
> >>>>
> >>>> Provide description for following properties:
> >>>>
> >>>> - ti,poll-period
> >>>> - ti,min-pressure
> >>>>
> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> index 8fffa3c..8875662 100644
> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >>>> @@ -1,4 +1,8 @@
> >>>> -TPS6507x Power Management Integrated Circuit
> >>>> +TPS6507x Multifunctional Device.
> >>>> +
> >>>> +Features provided by TPS6507x:
> >>>> +        1. Power Management Integrated Circuit.
> >>>> +        2. Touch-Screen.
> >>>>
> >>>>  Required properties:
> >>>>  - compatible: "ti,tps6507x"
> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >>>>    If this property is not defined, it defaults to 0 (not enabled).
> >>>>
> >>>> +Touchscreen Optional properties:
> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >>>> +                 Default value: 30 ms.
> >>>
> >>> Isn't there a standard property for this?
> >>
> >> Such a value will be already used in [1]. I've looked at [2], but it
> >> doesn't have such a field.
> >>
> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> > 
> > I think the bigger question (since we are adding this to DT I assume it
> > is not longer "toy" driver) can we convert it to a proper
> > interrupt-driven scheme? Polling for real shipping devices is not
> > efficient, I do not think we should even try to allow this mode in DTS
> > bindings (note that tsc2007 use is different: it allows to specify
> > sampling interval once touch is detected).
> 
> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
> does not have the touch interrupt connected to the processor. So if we
> dont support polled mode in when using device-tree, da850-evm.dts wont
> be able to use this driver.

Do we care? Right now the driver is unusable for any practical
application I think as it burns CPU cycles by constantly polling the
touch controller.

Yegor, your conversion to device tree: is it done because you have a
real product using this?

Thanks.
Yegor Yefremov March 10, 2017, 7:54 p.m. UTC | #7
On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
>> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
>> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>> >>>
>> >>> This needs to go to DT list.
>> >>
>> >> Will do.
>> >>
>> >>>>
>> >>>> Provide description for following properties:
>> >>>>
>> >>>> - ti,poll-period
>> >>>> - ti,min-pressure
>> >>>>
>> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>> >>>> ---
>> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>> >>>>
>> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> index 8fffa3c..8875662 100644
>> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>> >>>> @@ -1,4 +1,8 @@
>> >>>> -TPS6507x Power Management Integrated Circuit
>> >>>> +TPS6507x Multifunctional Device.
>> >>>> +
>> >>>> +Features provided by TPS6507x:
>> >>>> +        1. Power Management Integrated Circuit.
>> >>>> +        2. Touch-Screen.
>> >>>>
>> >>>>  Required properties:
>> >>>>  - compatible: "ti,tps6507x"
>> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>> >>>>    If this property is not defined, it defaults to 0 (not enabled).
>> >>>>
>> >>>> +Touchscreen Optional properties:
>> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>> >>>> +                 Default value: 30 ms.
>> >>>
>> >>> Isn't there a standard property for this?
>> >>
>> >> Such a value will be already used in [1]. I've looked at [2], but it
>> >> doesn't have such a field.
>> >>
>> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>> >
>> > I think the bigger question (since we are adding this to DT I assume it
>> > is not longer "toy" driver) can we convert it to a proper
>> > interrupt-driven scheme? Polling for real shipping devices is not
>> > efficient, I do not think we should even try to allow this mode in DTS
>> > bindings (note that tsc2007 use is different: it allows to specify
>> > sampling interval once touch is detected).
>>
>> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
>> does not have the touch interrupt connected to the processor. So if we
>> dont support polled mode in when using device-tree, da850-evm.dts wont
>> be able to use this driver.
>
> Do we care? Right now the driver is unusable for any practical
> application I think as it burns CPU cycles by constantly polling the
> touch controller.
>
> Yegor, your conversion to device tree: is it done because you have a
> real product using this?

Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html

It even seems to have interrupt pin connected. So we have to support both modes.

Yegor
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Dmitry Torokhov March 10, 2017, 8:08 p.m. UTC | #8
On Fri, Mar 10, 2017 at 08:54:51PM +0100, Yegor Yefremov wrote:
> On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
> >> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
> >> > On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
> >> >> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
> >> >>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
> >> >>>> From: Yegor Yefremov <yegorslists@googlemail.com>
> >> >>>
> >> >>> This needs to go to DT list.
> >> >>
> >> >> Will do.
> >> >>
> >> >>>>
> >> >>>> Provide description for following properties:
> >> >>>>
> >> >>>> - ti,poll-period
> >> >>>> - ti,min-pressure
> >> >>>>
> >> >>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
> >> >>>> ---
> >> >>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
> >> >>>>  1 file changed, 14 insertions(+), 2 deletions(-)
> >> >>>>
> >> >>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> index 8fffa3c..8875662 100644
> >> >>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
> >> >>>> @@ -1,4 +1,8 @@
> >> >>>> -TPS6507x Power Management Integrated Circuit
> >> >>>> +TPS6507x Multifunctional Device.
> >> >>>> +
> >> >>>> +Features provided by TPS6507x:
> >> >>>> +        1. Power Management Integrated Circuit.
> >> >>>> +        2. Touch-Screen.
> >> >>>>
> >> >>>>  Required properties:
> >> >>>>  - compatible: "ti,tps6507x"
> >> >>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
> >> >>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
> >> >>>>    If this property is not defined, it defaults to 0 (not enabled).
> >> >>>>
> >> >>>> +Touchscreen Optional properties:
> >> >>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
> >> >>>> +                 Default value: 30 ms.
> >> >>>
> >> >>> Isn't there a standard property for this?
> >> >>
> >> >> Such a value will be already used in [1]. I've looked at [2], but it
> >> >> doesn't have such a field.
> >> >>
> >> >> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> >> >> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
> >> >
> >> > I think the bigger question (since we are adding this to DT I assume it
> >> > is not longer "toy" driver) can we convert it to a proper
> >> > interrupt-driven scheme? Polling for real shipping devices is not
> >> > efficient, I do not think we should even try to allow this mode in DTS
> >> > bindings (note that tsc2007 use is different: it allows to specify
> >> > sampling interval once touch is detected).
> >>
> >> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
> >> does not have the touch interrupt connected to the processor. So if we
> >> dont support polled mode in when using device-tree, da850-evm.dts wont
> >> be able to use this driver.
> >
> > Do we care? Right now the driver is unusable for any practical
> > application I think as it burns CPU cycles by constantly polling the
> > touch controller.
> >
> > Yegor, your conversion to device tree: is it done because you have a
> > real product using this?
> 
> Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html
> 
> It even seems to have interrupt pin connected. So we have to support both modes.

Well, I'd rather supported interrupt only. Polling is a toy mode.

Thanks.
Sekhar Nori March 13, 2017, 2:15 p.m. UTC | #9
Hi Dmitry,

On Saturday 11 March 2017 01:38 AM, Dmitry Torokhov wrote:
> On Fri, Mar 10, 2017 at 08:54:51PM +0100, Yegor Yefremov wrote:
>> On Fri, Mar 10, 2017 at 7:00 PM, Dmitry Torokhov
>> <dmitry.torokhov@gmail.com> wrote:
>>> On Fri, Mar 10, 2017 at 12:11:08PM +0530, Sekhar Nori wrote:
>>>> On Thursday 09 March 2017 11:25 PM, Dmitry Torokhov wrote:
>>>>> On Thu, Mar 09, 2017 at 04:18:40PM +0100, Yegor Yefremov wrote:
>>>>>> On Thu, Mar 9, 2017 at 3:49 PM, Rob Herring <robh@kernel.org> wrote:
>>>>>>> On Thu, Mar 9, 2017 at 8:31 AM,  <yegorslists@googlemail.com> wrote:
>>>>>>>> From: Yegor Yefremov <yegorslists@googlemail.com>
>>>>>>>
>>>>>>> This needs to go to DT list.
>>>>>>
>>>>>> Will do.
>>>>>>
>>>>>>>>
>>>>>>>> Provide description for following properties:
>>>>>>>>
>>>>>>>> - ti,poll-period
>>>>>>>> - ti,min-pressure
>>>>>>>>
>>>>>>>> Signed-off-by: Yegor Yefremov <yegorslists@googlemail.com>
>>>>>>>> ---
>>>>>>>>  Documentation/devicetree/bindings/mfd/tps6507x.txt | 16 ++++++++++++++--
>>>>>>>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> index 8fffa3c..8875662 100644
>>>>>>>> --- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> +++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
>>>>>>>> @@ -1,4 +1,8 @@
>>>>>>>> -TPS6507x Power Management Integrated Circuit
>>>>>>>> +TPS6507x Multifunctional Device.
>>>>>>>> +
>>>>>>>> +Features provided by TPS6507x:
>>>>>>>> +        1. Power Management Integrated Circuit.
>>>>>>>> +        2. Touch-Screen.
>>>>>>>>
>>>>>>>>  Required properties:
>>>>>>>>  - compatible: "ti,tps6507x"
>>>>>>>> @@ -30,6 +34,12 @@ Regulator Optional properties:
>>>>>>>>                         1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
>>>>>>>>    If this property is not defined, it defaults to 0 (not enabled).
>>>>>>>>
>>>>>>>> +Touchscreen Optional properties:
>>>>>>>> +- ti,poll-period: Time at which touch input is getting sampled in ms.
>>>>>>>> +                 Default value: 30 ms.
>>>>>>>
>>>>>>> Isn't there a standard property for this?
>>>>>>
>>>>>> Such a value will be already used in [1]. I've looked at [2], but it
>>>>>> doesn't have such a field.
>>>>>>
>>>>>> [1] Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
>>>>>> [2] Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>>>>>
>>>>> I think the bigger question (since we are adding this to DT I assume it
>>>>> is not longer "toy" driver) can we convert it to a proper
>>>>> interrupt-driven scheme? Polling for real shipping devices is not
>>>>> efficient, I do not think we should even try to allow this mode in DTS
>>>>> bindings (note that tsc2007 use is different: it allows to specify
>>>>> sampling interval once touch is detected).
>>>>
>>>> Unfortunately, the only current user of tps6507x in kernel (da850-evm)
>>>> does not have the touch interrupt connected to the processor. So if we
>>>> dont support polled mode in when using device-tree, da850-evm.dts wont
>>>> be able to use this driver.
>>>
>>> Do we care? Right now the driver is unusable for any practical
>>> application I think as it burns CPU cycles by constantly polling the
>>> touch controller.
>>>
>>> Yegor, your conversion to device tree: is it done because you have a
>>> real product using this?
>>
>> Yes. It is VS-860 device http://www.visionsystems.de/produkte/vs-860.html
>>
>> It even seems to have interrupt pin connected. So we have to support both modes.
> 
> Well, I'd rather supported interrupt only. Polling is a toy mode.

On my DA850 EVM, running

$ evtest /dev/input/event0  > /dev/null &

and monitoring cpu load using top while continuously touching the screen
takes the CPU to about 90% idle. This is against 98% idle with no touch.
This running on ARM9 at 300Mhz. The processor can do about 450Mhz.

So, while I agree polling mode is not ideal, its not completely useless
as well. It will help the EVM to move to DT completely. Also, this
should not affect any core infrastructure or even interrupt driven users
of this driver.

Please do reconsider the stance on interrupt-only DT mode.

Thanks,
Sekhar
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mfd/tps6507x.txt b/Documentation/devicetree/bindings/mfd/tps6507x.txt
index 8fffa3c..8875662 100644
--- a/Documentation/devicetree/bindings/mfd/tps6507x.txt
+++ b/Documentation/devicetree/bindings/mfd/tps6507x.txt
@@ -1,4 +1,8 @@ 
-TPS6507x Power Management Integrated Circuit
+TPS6507x Multifunctional Device.
+
+Features provided by TPS6507x:
+        1. Power Management Integrated Circuit.
+        2. Touch-Screen.
 
 Required properties:
 - compatible: "ti,tps6507x"
@@ -30,6 +34,12 @@  Regulator Optional properties:
 			1: If defdcdc pin of DCDC2/DCDC3 is driven HIGH.
   If this property is not defined, it defaults to 0 (not enabled).
 
+Touchscreen Optional properties:
+- ti,poll-period: Time at which touch input is getting sampled in ms.
+		  Default value: 30 ms.
+- ti,min-pressure: Minimum pressure value to trigger touch.
+		   Default value: 0x30.
+
 Example:
 
 	pmu: tps6507x@48 {
@@ -40,6 +50,9 @@  Example:
 		vindcdc3-supply = <...>;
 		vinldo1_2-supply = <...>;
 
+		ti,poll-period = <30>;
+		ti,min-pressure = <0x30>;
+
 		regulators {
 			#address-cells = <1>;
 			#size-cells = <0>;
@@ -87,5 +100,4 @@  Example:
 				regulator-boot-on;
 			};
 		};
-
 	};