linux-kernel - Re: [PATCH v2 1/3] input: cygnus-update touchscreen dt node document

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAAFb_vqozomcuZ_OaB+d_HWsPqr1VPSTjfYc_Bh9a+vbaLPBpA@mail.gmail.com>
Date:	Sat, 27 Feb 2016 12:19:00 +0530
From:	Raveendra Padasalagi <raveendra.padasalagi@...adcom.com>
To:	Ray Jui <ray.jui@...adcom.com>
Cc:	Scott Branden <scott.branden@...adcom.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Rob Herring <robh@...nel.org>,
	Russell King <linux@....linux.org.uk>,
	Arnd Bergmann <arnd@...db.de>, devicetree@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-input@...r.kernel.org,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Jonathan Richardson <jonathar@...adcom.com>,
	Jon Mason <jonmason@...adcom.com>,
	Florian Fainelli <f.fainelli@...il.com>,
	Anup Patel <anup.patel@...adcom.com>,
	Ray Jui <rjui@...adcom.com>,
	Scott Branden <sbranden@...adcom.com>,
	linux-kernel@...r.kernel.org, bcm-kernel-feedback-list@...adcom.com
Subject: Re: [PATCH v2 1/3] input: cygnus-update touchscreen dt node document

Thanks Scott and Ray for the inputs. I will implement syscon only
register access and send out the changes in patch set - v4.

Regards,
Raveendra

On Tue, Feb 23, 2016 at 1:18 AM, Ray Jui <ray.jui@...adcom.com> wrote:
>
>
> On 2/22/2016 11:41 AM, Scott Branden wrote:
>>
>> My comments below
>>
>> On 16-02-22 11:36 AM, Dmitry Torokhov wrote:
>>>
>>> On Fri, Feb 19, 2016 at 11:43:50AM +0530, Raveendra Padasalagi wrote:
>>>>
>>>> On Thu, Feb 18, 2016 at 8:06 PM, Rob Herring <robh@...nel.org> wrote:
>>>>>
>>>>> On Wed, Feb 17, 2016 at 03:13:44PM +0530, Raveendra Padasalagi wrote:
>>>>>>
>>>>>> In Cygnus SOC touch screen controller registers are shared
>>>>>> with ADC and flex timer. Using readl/writel could lead to
>>>>>> race condition. So touch screen driver is enhanced to support
>>>>>>
>>>>>> 1. If touchscreen register's are not shared. Register access
>>>>>> is handled through readl/writel if "brcm,iproc-touchscreen"
>>>>>> compatible is provided in touchscreen dt node. This will help
>>>>>> for future SOC's if comes with dedicated touchscreen IP register's.
>>>>>>
>>>>>> 2. If touchscreen register's are shared with other IP's, register
>>>>>> access is handled through syscon framework API's to take care of
>>>>>> mutually exclusive access. This feature can be enabled by selecting
>>>>>> "brcm,iproc-touchscreen-syscon" compatible string in the touchscreen
>>>>>> dt node.
>>>>>>
>>>>>> Hence touchscreen dt node bindings document is updated to take care
>>>>>> of above changes in the touchscreen driver.
>>>>>>
>>>>>> Signed-off-by: Raveendra Padasalagi
>>>>>> <raveendra.padasalagi@...adcom.com>
>>>>>> Reviewed-by: Ray Jui <ray.jui@...adcom.com>
>>>>>> Reviewed-by: Scott Branden <scott.branden@...adcom.com>
>>>>>> ---
>>>>>>   .../input/touchscreen/brcm,iproc-touchscreen.txt   | 57
>>>>>> +++++++++++++++++++---
>>>>>>   1 file changed, 51 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git
>>>>>>
>>>>>> a/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
>>>>>>
>>>>>> b/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
>>>>>>
>>>>>> index 34e3382..f530c25 100644
>>>>>> ---
>>>>>>
>>>>>> a/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
>>>>>>
>>>>>> +++
>>>>>>
>>>>>> b/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
>>>>>>
>>>>>> @@ -1,12 +1,30 @@
>>>>>>   * Broadcom's IPROC Touchscreen Controller
>>>>>>
>>>>>>   Required properties:
>>>>>> -- compatible: must be "brcm,iproc-touchscreen"
>>>>>> -- reg: physical base address of the controller and length of
>>>>>> memory mapped
>>>>>> -  region.
>>>>>> +- compatible: should be one of
>>>>>> +        "brcm,iproc-touchscreen"
>>>>>> +        "brcm,iproc-touchscreen-syscon"
>>>>>
>>>>>
>>>>> More specific and this is not how you do syscon. Either the block is or
>>>>> isn't. You can't have it both ways.
>>>>
>>>>
>>>> Existing driver has support for reg, if we modify now to support only
>>>> syscon
>>>> then this driver will not work if some one wishes to use previous
>>>> kernel version's
>>>> dt and vice versa. Basically this breaks dt compatibility. Is that ok ?
>>>
>>>
>>> But the issue is that the driver does not actually work correctly with
>>> direct register access on those systems, since the registers are
>>> actually shared with other components. I am not quite sure if it is OK
>>> to break DT binding in this case...
>>
>>
>> The driver does work correctly with direct register access on those
>> systems because the other components using those registers are not
>> active in those systems - so syscon is not needed in those cases.
>>
>> I'm ok with not containing backwards compatibility though and always
>> using syscon.  There are no deployed systems using older versions of the
>> upstreamed kernel.
>>>
>>>
>>> Thanks.
>>>
>>
>> Regards,
>> Scott
>
>
> The iproc touchscreen is currently activated in the "bcm9hmidc.dtsi" that
> represents the optional daughter card installed on reference boards
> bcm958300k and bcm958305k. While not maintaining backwards compatibility
> *might not* be a serious issue, it would be nice if we can at least make
> sure the driver change and DT are merged into the same kernel version so
> they stay in sync.
>
> Going forward, if we are only going to support syscon based implementation,
> the existing compatible string "brcm,iproc-touchscreen" is preferred over
> "brcm,iproc-touchscreen-syscon".
>
> Thanks,
>
> Ray