[PATCH 0/3] Syscon support for iProc touchscreen driver

[PATCH 0/3] Syscon support for iProc touchscreen driver

[Raveendra Padasalagi]

This patchset is based on v4.5-rc3 tag and its tested on
Broadcom Cygnus SoC.

The patches can be fetched from iproc-tsc-v1 branch of
https://github.com/Broadcom/arm64-linux.git

Raveendra Padasalagi (3):
  input: cygnus-update touchscreen dt node document
  input: syscon support in bcm_iproc_tsc driver
  ARM: dts: use syscon in cygnus touchscreen dt node

 .../input/touchscreen/brcm,iproc-touchscreen.txt   |  18 ++-
 arch/arm/boot/dts/bcm-cygnus.dtsi                  |   9 +-
 drivers/input/touchscreen/bcm_iproc_tsc.c          | 122 +++++++++++++++------
 3 files changed, 112 insertions(+), 37 deletions(-)

-- 
1.9.1

[PATCH 1/3] input: cygnus-update touchscreen dt node document

[Raveendra Padasalagi]

Touch screen driver is modified to use syscon framework
based register read/write API's. So added syscon node and
related changes in dt file.Updating the device tree documentation
file to reflect the changes.

Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 .../input/touchscreen/brcm,iproc-touchscreen.txt       | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 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..67599ae 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
+++ b/Documentation/devicetree/bindings/input/touchscreen/brcm,iproc-touchscreen.txt
@@ -2,8 +2,16 @@
 
 Required properties:
 - compatible: must be "brcm,iproc-touchscreen"
-- reg: physical base address of the controller and length of memory mapped
-  region.
+
+One of the below 2 property is mandatory.
+- ts_syscon: handler of syscon node defining physical base address
+  of the controller and length of memory mapped region.
+  If this property is selected please make sure MFD_SYSCON config
+  is enabled in the defconfig file.
+or
+- reg: physical base address of the controller and length of memory
+  mapped region.
+
 - clocks:  The clock provided by the SOC to driver the tsc
 - clock-name:  name for the clock
 - interrupts: The touchscreen controller's interrupt
@@ -54,12 +62,16 @@ Optional properties:
 - touchscreen-inverted-y: Y axis is inverted (boolean)
 
 Example:
+	ts_adc_syscon: ts_adc_syscon@0x180a6000 {
+		compatible = "syscon";
+		reg = <0x180a6000 0xc30>;
+	};
 
 	touchscreen: tsc@0x180A6000 {
 		compatible = "brcm,iproc-touchscreen";
 		#address-cells = <1>;
 		#size-cells = <1>;
-		reg = <0x180A6000 0x40>;
+		syscon = <&ts_adc_syscon>;
 		clocks = <&adc_clk>;
 		clock-names = "tsc_clk";
 		interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
-- 
1.9.1

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

[Arnd Bergmann]

On Tuesday 09 February 2016 14:27:55 Raveendra Padasalagi wrote:
> +
> +One of the below 2 property is mandatory.
> +- ts_syscon: handler of syscon node defining physical base address
> +  of the controller and length of memory mapped region.
> +  If this property is selected please make sure MFD_SYSCON config
> +  is enabled in the defconfig file.
> +or
> +- reg: physical base address of the controller and length of memory
> +  mapped region.
> +
>  - clocks:  The clock provided by the SOC to driver the tsc
>  - clock-name:  name for the clock
>  - interrupts: The touchscreen controller's interrupt
> @@ -54,12 +62,16 @@ Optional properties:
>  - touchscreen-inverted-y: Y axis is inverted (boolean)
>  
>  Example:
> +       ts_adc_syscon: ts_adc_syscon@0x180a6000 {
> +               compatible = "syscon";
> +               reg = <0x180a6000 0xc30>;
> +       };
>  
>         touchscreen: tsc@0x180A6000 {
>                 compatible = "brcm,iproc-touchscreen";
>                 #address-cells = <1>;
>                 #size-cells = <1>;
> -               reg = <0x180A6000 0x40>;
> +               syscon = <&ts_adc_syscon>;
>                 clocks = <&adc_clk>;
>                 clock-names = "tsc_clk";
>                 interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;

This doesn't seem right: For one thing, it's an incompatible binding
change which you should try to avoid.

Worse, the syscon has a name indicating that it really has a particular
function, but the name is completely generic (just "syscon"). Please
add a specific compatible string identifying the device that owns
the registers, and put any other associated properties (clocks,
interrupts, ...) that are for that device in general rather than just
the touchscreen in there.

Your changelog text fails to describe why you do this change in
the first place, and why you can't do it in a backward-compatible
way, so please rewrite that text.

	Arnd

RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document

[Raveendra Padasalagi]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: 09 February 2016 16:34
> To: Raveendra Padasalagi
> Cc: Dmitry Torokhov; Russell King; devicetree@vger.kernel.org;
linux-arm-
> kernel@lists.infradead.org; linux-input@vger.kernel.org; Rob Herring;
Pawel
> Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson; Jon
> Mason; Florian Fainelli; Ray Jui; Scott Branden;
linux-kernel@vger.kernel.org;
> bcm-kernel-feedback-list@broadcom.com
> Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
document
>
> On Tuesday 09 February 2016 14:27:55 Raveendra Padasalagi wrote:
> > +
> > +One of the below 2 property is mandatory.
> > +- ts_syscon: handler of syscon node defining physical base address
> > +  of the controller and length of memory mapped region.
> > +  If this property is selected please make sure MFD_SYSCON config
> > +  is enabled in the defconfig file.
> > +or
> > +- reg: physical base address of the controller and length of memory
> > +  mapped region.
> > +
> >  - clocks:  The clock provided by the SOC to driver the tsc
> >  - clock-name:  name for the clock
> >  - interrupts: The touchscreen controller's interrupt @@ -54,12 +62,16
> > @@ Optional properties:
> >  - touchscreen-inverted-y: Y axis is inverted (boolean)
> >
> >  Example:
> > +       ts_adc_syscon: ts_adc_syscon@0x180a6000 {
> > +               compatible = "syscon";
> > +               reg = <0x180a6000 0xc30>;
> > +       };
> >
> >         touchscreen: tsc@0x180A6000 {
> >                 compatible = "brcm,iproc-touchscreen";
> >                 #address-cells = <1>;
> >                 #size-cells = <1>;
> > -               reg = <0x180A6000 0x40>;
> > +               syscon = <&ts_adc_syscon>;
> >                 clocks = <&adc_clk>;
> >                 clock-names = "tsc_clk";
> >                 interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
>
> This doesn't seem right: For one thing, it's an incompatible binding
change which
> you should try to avoid.

> Worse, the syscon has a name indicating that it really has a particular
function,
> but the name is completely generic (just "syscon"). Please add a
specific
> compatible string identifying the device that owns the registers, and
put any
> other associated properties (clocks, interrupts, ...) that are for that
device in
> general rather than just the touchscreen in there.

By mistake "syscon" name is documented in "touchscreen" label above. In
the
touchscreen driver code and .dtsi file it's named and used as "ts_syscon".
Please refer https://lkml.org/lkml/2016/2/9/71 for the changes.
Let me know if this ok. I will correct the documentation in the next
patch.

> Your changelog text fails to describe why you do this change in the
first place,
> and why you can't do it in a backward-compatible way, so please rewrite
that
> text.

Yes, I will update the change log in the next patch.

In Cygnus SOC, Touch screen controller registers are shared with ADC and
flex timer IP's.
Using readl/writel could lead to race condition. So this patch is
introduced to take care
mutually exclusive access of register's in such a case.

In the platform's where touchscreen register's are not shared normal
readl/writel
could be used.

Based on the dt  property entry "reg" or "ts_syscon" register accesses are
handled.
if "reg" is provided normal readl/writel API's are used to access
register's else if
"ts_syscon" is provided then syscon API's are used.

> 	Arnd

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

[Dmitry Torokhov]

On Tue, Feb 09, 2016 at 10:25:26PM +0530, Raveendra Padasalagi wrote:
> > -----Original Message-----
> > From: Arnd Bergmann [mailto:arnd-r2nGTMty4D4@public.gmane.org]
> > Sent: 09 February 2016 16:34
> > To: Raveendra Padasalagi
> > Cc: Dmitry Torokhov; Russell King; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> linux-arm-
> > kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Rob Herring;
> Pawel
> > Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson; Jon
> > Mason; Florian Fainelli; Ray Jui; Scott Branden;
> linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org;
> > bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w@public.gmane.org
> > Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
> document
> >
> > On Tuesday 09 February 2016 14:27:55 Raveendra Padasalagi wrote:
> > > +
> > > +One of the below 2 property is mandatory.
> > > +- ts_syscon: handler of syscon node defining physical base address
> > > +  of the controller and length of memory mapped region.
> > > +  If this property is selected please make sure MFD_SYSCON config
> > > +  is enabled in the defconfig file.
> > > +or
> > > +- reg: physical base address of the controller and length of memory
> > > +  mapped region.
> > > +
> > >  - clocks:  The clock provided by the SOC to driver the tsc
> > >  - clock-name:  name for the clock
> > >  - interrupts: The touchscreen controller's interrupt @@ -54,12 +62,16
> > > @@ Optional properties:
> > >  - touchscreen-inverted-y: Y axis is inverted (boolean)
> > >
> > >  Example:
> > > +       ts_adc_syscon: ts_adc_syscon@0x180a6000 {
> > > +               compatible = "syscon";
> > > +               reg = <0x180a6000 0xc30>;
> > > +       };
> > >
> > >         touchscreen: tsc@0x180A6000 {
> > >                 compatible = "brcm,iproc-touchscreen";
> > >                 #address-cells = <1>;
> > >                 #size-cells = <1>;
> > > -               reg = <0x180A6000 0x40>;
> > > +               syscon = <&ts_adc_syscon>;
> > >                 clocks = <&adc_clk>;
> > >                 clock-names = "tsc_clk";
> > >                 interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> >
> > This doesn't seem right: For one thing, it's an incompatible binding
> change which
> > you should try to avoid.
> 
> > Worse, the syscon has a name indicating that it really has a particular
> function,
> > but the name is completely generic (just "syscon"). Please add a
> specific
> > compatible string identifying the device that owns the registers, and
> put any
> > other associated properties (clocks, interrupts, ...) that are for that
> device in
> > general rather than just the touchscreen in there.
> 
> By mistake "syscon" name is documented in "touchscreen" label above. In
> the
> touchscreen driver code and .dtsi file it's named and used as "ts_syscon".
> Please refer https://lkml.org/lkml/2016/2/9/71 for the changes.
> Let me know if this ok. I will correct the documentation in the next
> patch.
> 
> > Your changelog text fails to describe why you do this change in the
> first place,
> > and why you can't do it in a backward-compatible way, so please rewrite
> that
> > text.
> 
> Yes, I will update the change log in the next patch.
> 
> In Cygnus SOC, Touch screen controller registers are shared with ADC and
> flex timer IP's.
> Using readl/writel could lead to race condition. So this patch is
> introduced to take care
> mutually exclusive access of register's in such a case.
> 
> In the platform's where touchscreen register's are not shared normal
> readl/writel
> could be used.

Are there currently platforms where the registers are not shared? If we
have to use regmap to correctly update registers (and thus we need
syscon) then I wonder if we should even attempt to support direct
register access by the driver.

> 
> Based on the dt  property entry "reg" or "ts_syscon" register accesses are
> handled.
> if "reg" is provided normal readl/writel API's are used to access
> register's else if
> "ts_syscon" is provided then syscon API's are used.

If we decide that we still allow register access then please change the
driver so it instantiates its own regmap instance in this case instead
of doing checks all the time.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document

[Raveendra Padasalagi]

> -----Original Message-----
> From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> Sent: 09 February 2016 23:14
> To: Raveendra Padasalagi
> Cc: Arnd Bergmann; Russell King; devicetree@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linux-input@vger.kernel.org; Rob Herring;
Pawel
> Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson; Jon
> Mason; Florian Fainelli; Ray Jui; Scott Branden;
linux-kernel@vger.kernel.org;
> bcm-kernel-feedback-list@broadcom.com
> Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
document
>
> On Tue, Feb 09, 2016 at 10:25:26PM +0530, Raveendra Padasalagi wrote:
> > > -----Original Message-----
> > > From: Arnd Bergmann [mailto:arnd@arndb.de]
> > > Sent: 09 February 2016 16:34
> > > To: Raveendra Padasalagi
> > > Cc: Dmitry Torokhov; Russell King; devicetree@vger.kernel.org;
> > linux-arm-
> > > kernel@lists.infradead.org; linux-input@vger.kernel.org; Rob
> > > Herring;
> > Pawel
> > > Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson;
> > > Jon Mason; Florian Fainelli; Ray Jui; Scott Branden;
> > linux-kernel@vger.kernel.org;
> > > bcm-kernel-feedback-list@broadcom.com
> > > Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
> > document
> > >
> > > On Tuesday 09 February 2016 14:27:55 Raveendra Padasalagi wrote:
> > > > +
> > > > +One of the below 2 property is mandatory.
> > > > +- ts_syscon: handler of syscon node defining physical base
> > > > +address
> > > > +  of the controller and length of memory mapped region.
> > > > +  If this property is selected please make sure MFD_SYSCON config
> > > > +  is enabled in the defconfig file.
> > > > +or
> > > > +- reg: physical base address of the controller and length of
> > > > +memory
> > > > +  mapped region.
> > > > +
> > > >  - clocks:  The clock provided by the SOC to driver the tsc
> > > >  - clock-name:  name for the clock
> > > >  - interrupts: The touchscreen controller's interrupt @@ -54,12
> > > > +62,16 @@ Optional properties:
> > > >  - touchscreen-inverted-y: Y axis is inverted (boolean)
> > > >
> > > >  Example:
> > > > +       ts_adc_syscon: ts_adc_syscon@0x180a6000 {
> > > > +               compatible = "syscon";
> > > > +               reg = <0x180a6000 0xc30>;
> > > > +       };
> > > >
> > > >         touchscreen: tsc@0x180A6000 {
> > > >                 compatible = "brcm,iproc-touchscreen";
> > > >                 #address-cells = <1>;
> > > >                 #size-cells = <1>;
> > > > -               reg = <0x180A6000 0x40>;
> > > > +               syscon = <&ts_adc_syscon>;
> > > >                 clocks = <&adc_clk>;
> > > >                 clock-names = "tsc_clk";
> > > >                 interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
> > >
> > > This doesn't seem right: For one thing, it's an incompatible binding
> > change which
> > > you should try to avoid.
> >
> > > Worse, the syscon has a name indicating that it really has a
> > > particular
> > function,
> > > but the name is completely generic (just "syscon"). Please add a
> > specific
> > > compatible string identifying the device that owns the registers,
> > > and
> > put any
> > > other associated properties (clocks, interrupts, ...) that are for
> > > that
> > device in
> > > general rather than just the touchscreen in there.
> >
> > By mistake "syscon" name is documented in "touchscreen" label above.
> > In the touchscreen driver code and .dtsi file it's named and used as
> > "ts_syscon".
> > Please refer https://lkml.org/lkml/2016/2/9/71 for the changes.
> > Let me know if this ok. I will correct the documentation in the next
> > patch.
> >
> > > Your changelog text fails to describe why you do this change in the
> > first place,
> > > and why you can't do it in a backward-compatible way, so please
> > > rewrite
> > that
> > > text.
> >
> > Yes, I will update the change log in the next patch.
> >
> > In Cygnus SOC, Touch screen controller registers are shared with ADC
> > and flex timer IP's.
> > Using readl/writel could lead to race condition. So this patch is
> > introduced to take care mutually exclusive access of register's in
> > such a case.
> >
> > In the platform's where touchscreen register's are not shared normal
> > readl/writel could be used.
>
> Are there currently platforms where the registers are not shared? If we
have to
> use regmap to correctly update registers (and thus we need
> syscon) then I wonder if we should even attempt to support direct
register
> access by the driver.

Currently there are no platforms without register sharing, but we would
like to keep
the driver generic enough to take care of future SOC's if comes with
dedicated registers
and same touchscreen IP being re-used.
> >
> > Based on the dt  property entry "reg" or "ts_syscon" register accesses
> > are handled.
> > if "reg" is provided normal readl/writel API's are used to access
> > register's else if "ts_syscon" is provided then syscon API's are used.
>
> If we decide that we still allow register access then please change the
driver so it
> instantiates its own regmap instance in this case instead of doing
checks all the
> time.

Using remap will leads to overhead of locking and unlocking for each
register access
in case if a dedicated register's present in the platform. I think current
implementation
of simple check of regs or remap is better over using regmap read/write
calls with locking
overhead. Let me know your opinion.



> Thanks.
>
> --
> Dmitry

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

[Rob Herring]

Your mailer is not handling wrapping correctly. Generally, Outlook does 
not work for maillists.

On Wed, Feb 10, 2016 at 10:24:43AM +0530, Raveendra Padasalagi wrote:
> > -----Original Message-----
> > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > Sent: 09 February 2016 23:14
> > To: Raveendra Padasalagi
> > Cc: Arnd Bergmann; Russell King; devicetree@vger.kernel.org; linux-arm-
> > kernel@lists.infradead.org; linux-input@vger.kernel.org; Rob Herring;
> Pawel
> > Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson; Jon
> > Mason; Florian Fainelli; Ray Jui; Scott Branden;
> linux-kernel@vger.kernel.org;
> > bcm-kernel-feedback-list@broadcom.com
> > Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
> document

And Outlook's reply header is non-standard.

[...]

> > > In Cygnus SOC, Touch screen controller registers are shared with ADC
> > > and flex timer IP's.
> > > Using readl/writel could lead to race condition. So this patch is
> > > introduced to take care mutually exclusive access of register's in
> > > such a case.
> > >
> > > In the platform's where touchscreen register's are not shared normal
> > > readl/writel could be used.
> >
> > Are there currently platforms where the registers are not shared? If we
> have to
> > use regmap to correctly update registers (and thus we need
> > syscon) then I wonder if we should even attempt to support direct
> register
> > access by the driver.
> 
> Currently there are no platforms without register sharing, but we would
> like to keep
> the driver generic enough to take care of future SOC's if comes with
> dedicated registers
> and same touchscreen IP being re-used.

These future SoCs will have a different compatible string and you should 
use that to determine whether reg or syscon is used.

Rob

RE: [PATCH 1/3] input: cygnus-update touchscreen dt node document

[Raveendra Padasalagi]

> -----Original Message-----
> From: Rob Herring [mailto:robh@kernel.org]
> Sent: 12 February 2016 20:35
> To: Raveendra Padasalagi
> Cc: Dmitry Torokhov; Arnd Bergmann; Russell King;
devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; linux-input@vger.kernel.org; Pawel
Moll;
> Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson; Jon Mason;
> Florian Fainelli; Ray Jui; Scott Branden; linux-kernel@vger.kernel.org;
bcm-
> kernel-feedback-list@broadcom.com
> Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
document
>
> Your mailer is not handling wrapping correctly. Generally, Outlook does
not
> work for maillists.
>
> On Wed, Feb 10, 2016 at 10:24:43AM +0530, Raveendra Padasalagi wrote:
> > > -----Original Message-----
> > > From: Dmitry Torokhov [mailto:dmitry.torokhov@gmail.com]
> > > Sent: 09 February 2016 23:14
> > > To: Raveendra Padasalagi
> > > Cc: Arnd Bergmann; Russell King; devicetree@vger.kernel.org;
> > > linux-arm- kernel@lists.infradead.org; linux-input@vger.kernel.org;
> > > Rob Herring;
> > Pawel
> > > Moll; Mark Rutland; Ian Campbell; Kumar Gala; Jonathan Richardson;
> > > Jon Mason; Florian Fainelli; Ray Jui; Scott Branden;
> > linux-kernel@vger.kernel.org;
> > > bcm-kernel-feedback-list@broadcom.com
> > > Subject: Re: [PATCH 1/3] input: cygnus-update touchscreen dt node
> > document
>
> And Outlook's reply header is non-standard.
>
> [...]
>
> > > > In Cygnus SOC, Touch screen controller registers are shared with
> > > > ADC and flex timer IP's.
> > > > Using readl/writel could lead to race condition. So this patch is
> > > > introduced to take care mutually exclusive access of register's in
> > > > such a case.
> > > >
> > > > In the platform's where touchscreen register's are not shared
> > > > normal readl/writel could be used.
> > >
> > > Are there currently platforms where the registers are not shared? If
> > > we
> > have to
> > > use regmap to correctly update registers (and thus we need
> > > syscon) then I wonder if we should even attempt to support direct
> > register
> > > access by the driver.
> >
> > Currently there are no platforms without register sharing, but we
> > would like to keep the driver generic enough to take care of future
> > SOC's if comes with dedicated registers and same touchscreen IP being
> > re-used.
>
> These future SoCs will have a different compatible string and you should
use that
> to determine whether reg or syscon is used.

Thanks Rob. I will address this in the next patch.

> Rob

[PATCH 2/3] input: syscon support in bcm_iproc_tsc driver

[Raveendra Padasalagi]

Touch screen controller registers are shared with ADC and flex timer.
Using readl/writel could lead to race condition. So in such cases
we need to support register access using syscon framework based API's.

In the platform's where touchscreen register's are not shared normal
readl/writel could be used.

Based on the dt entry "reg" or "ts_syscon" register accesses are handled.
if "reg" is provided normal readl/writel API's are used to access
register's else if "ts_syscon" is provided then syscon API's are used.

Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 drivers/input/touchscreen/bcm_iproc_tsc.c | 122 ++++++++++++++++++++++--------
 1 file changed, 89 insertions(+), 33 deletions(-)

diff --git a/drivers/input/touchscreen/bcm_iproc_tsc.c b/drivers/input/touchscreen/bcm_iproc_tsc.c
index ae460a5c..56c7c08 100644
--- a/drivers/input/touchscreen/bcm_iproc_tsc.c
+++ b/drivers/input/touchscreen/bcm_iproc_tsc.c
@@ -23,6 +23,8 @@
 #include <linux/io.h>
 #include <linux/clk.h>
 #include <linux/serio.h>
+#include <linux/mfd/syscon.h>
+#include <linux/regmap.h>
 
 #define IPROC_TS_NAME "iproc-ts"
 
@@ -88,7 +90,7 @@
 #define TS_WIRE_MODE_BIT        BIT(1)
 
 #define dbg_reg(dev, priv, reg) \
-	dev_dbg(dev, "%20s= 0x%08x\n", #reg, readl((priv)->regs + reg))
+	dev_dbg(dev, "%20s= 0x%08x\n", #reg, iproc_reg_read(priv, reg))
 
 struct tsc_param {
 	/* Each step is 1024 us.  Valid 1-256 */
@@ -142,6 +144,7 @@ struct iproc_ts_priv {
 	struct input_dev *idev;
 
 	void __iomem *regs;
+	struct regmap *regmap;
 	struct clk *tsc_clk;
 
 	int  pen_status;
@@ -163,6 +166,41 @@ static const struct tsc_param iproc_default_config = {
 	.max_y            = Y_MAX,
 };
 
+static int iproc_reg_update_bits(struct iproc_ts_priv *priv, u32 reg,
+			   u32 mask, u32 val)
+{
+	int ret = 0;
+	u32 tmp, orig;
+
+	if (priv->regs) {
+		orig = readl(priv->regs);
+		tmp = orig & ~mask;
+		tmp |= val & mask;
+		writel(tmp, priv->regs + reg);
+	} else
+		ret = regmap_update_bits(priv->regmap, reg, mask, val);
+	return ret;
+}
+
+static void iproc_reg_write(struct iproc_ts_priv *priv, u32 reg, u32 val)
+{
+	if (priv->regs)
+		writel(val, priv->regs + reg);
+	else
+		regmap_write(priv->regmap, reg, val);
+}
+
+static u32 iproc_reg_read(struct iproc_ts_priv *priv, u32 reg)
+{
+	u32 val;
+
+	if (priv->regs)
+		val = readl(priv->regs + reg);
+	else
+		regmap_read(priv->regmap, reg, &val);
+	return val;
+}
+
 static void ts_reg_dump(struct iproc_ts_priv *priv)
 {
 	struct device *dev = &priv->pdev->dev;
@@ -196,22 +234,22 @@ static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
 	int i;
 	bool needs_sync = false;
 
-	intr_status = readl(priv->regs + INTERRUPT_STATUS);
-	intr_status &= TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK;
+	intr_status = iproc_reg_read(priv, INTERRUPT_STATUS);
+	intr_status &= (TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK);
 	if (intr_status == 0)
 		return IRQ_NONE;
 
 	/* Clear all interrupt status bits, write-1-clear */
-	writel(intr_status, priv->regs + INTERRUPT_STATUS);
-
+	iproc_reg_write(priv, INTERRUPT_STATUS, intr_status);
 	/* Pen up/down */
 	if (intr_status & TS_PEN_INTR_MASK) {
-		if (readl(priv->regs + CONTROLLER_STATUS) & TS_PEN_DOWN)
+		priv->pen_status = iproc_reg_read(priv, CONTROLLER_STATUS);
+		if (priv->pen_status & TS_PEN_DOWN)
 			priv->pen_status = PEN_DOWN_STATUS;
 		else
 			priv->pen_status = PEN_UP_STATUS;
 
-		input_report_key(priv->idev, BTN_TOUCH, priv->pen_status);
+		input_report_key(priv->idev, BTN_TOUCH,	priv->pen_status);
 		needs_sync = true;
 
 		dev_dbg(&priv->pdev->dev,
@@ -221,7 +259,7 @@ static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
 	/* coordinates in FIFO exceed the theshold */
 	if (intr_status & TS_FIFO_INTR_MASK) {
 		for (i = 0; i < priv->cfg_params.fifo_threshold; i++) {
-			raw_coordinate = readl(priv->regs + FIFO_DATA);
+			raw_coordinate = iproc_reg_read(priv, FIFO_DATA);
 			if (raw_coordinate == INVALID_COORD)
 				continue;
 
@@ -239,7 +277,7 @@ static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
 			x = (x >> 4) & 0x0FFF;
 			y = (y >> 4) & 0x0FFF;
 
-			/* adjust x y according to lcd tsc mount angle */
+			/* Adjust x y according to LCD tsc mount angle. */
 			if (priv->cfg_params.invert_x)
 				x = priv->cfg_params.max_x - x;
 
@@ -262,9 +300,10 @@ static irqreturn_t iproc_touchscreen_interrupt(int irq, void *data)
 
 static int iproc_ts_start(struct input_dev *idev)
 {
-	struct iproc_ts_priv *priv = input_get_drvdata(idev);
 	u32 val;
+	u32 mask;
 	int error;
+	struct iproc_ts_priv *priv = input_get_drvdata(idev);
 
 	/* Enable clock */
 	error = clk_prepare_enable(priv->tsc_clk);
@@ -279,9 +318,10 @@ static int iproc_ts_start(struct input_dev *idev)
 	 *  FIFO reaches the int_th value, and pen event(up/down)
 	 */
 	val = TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK;
-	writel(val, priv->regs + INTERRUPT_MASK);
+	iproc_reg_update_bits(priv, INTERRUPT_MASK, val, val);
 
-	writel(priv->cfg_params.fifo_threshold, priv->regs + INTERRUPT_THRES);
+	val = priv->cfg_params.fifo_threshold;
+	iproc_reg_write(priv, INTERRUPT_THRES, val);
 
 	/* Initialize control reg1 */
 	val = 0;
@@ -289,26 +329,23 @@ static int iproc_ts_start(struct input_dev *idev)
 	val |= priv->cfg_params.debounce_timeout << DEBOUNCE_TIMEOUT_SHIFT;
 	val |= priv->cfg_params.settling_timeout << SETTLING_TIMEOUT_SHIFT;
 	val |= priv->cfg_params.touch_timeout << TOUCH_TIMEOUT_SHIFT;
-	writel(val, priv->regs + REGCTL1);
+	iproc_reg_write(priv, REGCTL1, val);
 
 	/* Try to clear all interrupt status */
-	val = readl(priv->regs + INTERRUPT_STATUS);
-	val |= TS_FIFO_INTR_MASK | TS_PEN_INTR_MASK;
-	writel(val, priv->regs + INTERRUPT_STATUS);
+	val = TS_FIFO_INTR_MASK | TS_PEN_INTR_MASK;
+	iproc_reg_update_bits(priv, INTERRUPT_STATUS, val, val);
 
 	/* Initialize control reg2 */
-	val = readl(priv->regs + REGCTL2);
-	val |= TS_CONTROLLER_EN_BIT | TS_WIRE_MODE_BIT;
-
-	val &= ~TS_CONTROLLER_AVGDATA_MASK;
+	val = TS_CONTROLLER_EN_BIT | TS_WIRE_MODE_BIT;
 	val |= priv->cfg_params.average_data << TS_CONTROLLER_AVGDATA_SHIFT;
 
-	val &= ~(TS_CONTROLLER_PWR_LDO |	/* PWR up LDO */
+	mask = (TS_CONTROLLER_AVGDATA_MASK);
+	mask |= (TS_CONTROLLER_PWR_LDO |	/* PWR up LDO */
 		   TS_CONTROLLER_PWR_ADC |	/* PWR up ADC */
 		   TS_CONTROLLER_PWR_BGP |	/* PWR up BGP */
 		   TS_CONTROLLER_PWR_TS);	/* PWR up TS */
-
-	writel(val, priv->regs + REGCTL2);
+	mask |= val;
+	iproc_reg_update_bits(priv, REGCTL2, mask, val);
 
 	ts_reg_dump(priv);
 
@@ -320,12 +357,17 @@ static void iproc_ts_stop(struct input_dev *dev)
 	u32 val;
 	struct iproc_ts_priv *priv = input_get_drvdata(dev);
 
-	writel(0, priv->regs + INTERRUPT_MASK); /* Disable all interrupts */
+	/*
+	 * Disable FIFO int_th and pen event(up/down)Interrupts only
+	 * as the interrupt mask register is shared between ADC, TS and
+	 * flextimer.
+	 */
+	val = TS_PEN_INTR_MASK | TS_FIFO_INTR_MASK;
+	iproc_reg_update_bits(priv, INTERRUPT_MASK, val, 0);
 
 	/* Only power down touch screen controller */
-	val = readl(priv->regs + REGCTL2);
-	val |= TS_CONTROLLER_PWR_TS;
-	writel(val, priv->regs + REGCTL2);
+	val = TS_CONTROLLER_PWR_TS;
+	iproc_reg_update_bits(priv, REGCTL2, val, val);
 
 	clk_disable(priv->tsc_clk);
 }
@@ -422,13 +464,27 @@ static int iproc_ts_probe(struct platform_device *pdev)
 	if (!priv)
 		return -ENOMEM;
 
-	/* touchscreen controller memory mapped regs */
+	/* touchscreen controller memory mapped regs if provided */
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->regs = devm_ioremap_resource(&pdev->dev, res);
-	if (IS_ERR(priv->regs)) {
-		error = PTR_ERR(priv->regs);
-		dev_err(&pdev->dev, "unable to map I/O memory: %d\n", error);
-		return error;
+	if (res) {
+		priv->regs = devm_ioremap_resource(&pdev->dev, res);
+		if (IS_ERR(priv->regs)) {
+			error = PTR_ERR(priv->regs);
+			dev_err(&pdev->dev, "unable to map I/O memory:%d\n",
+				error);
+			return error;
+		}
+	} else {
+		/* touchscreen controller memory mapped regs */
+		priv->regmap = syscon_regmap_lookup_by_phandle(
+							pdev->dev.of_node,
+							"ts_syscon");
+		if (IS_ERR(priv->regmap)) {
+			error = PTR_ERR(priv->regs);
+			dev_err(&pdev->dev, "unable to map I/O memory:%d\n",
+				error);
+			return error;
+		}
 	}
 
 	priv->tsc_clk = devm_clk_get(&pdev->dev, "tsc_clk");
-- 
1.9.1

[PATCH 3/3] ARM: dts: use syscon in cygnus touchscreen dt node

[Raveendra Padasalagi]

In cygnus touch screen controller registers are shared with ADC
and flex timer. Using readl/writel could lead to race condition.
In such cases we need to support register access using syscon
framework based API's. So Added dt changes to use syscon.

Signed-off-by: Raveendra Padasalagi <raveendra.padasalagi@broadcom.com>
Reviewed-by: Ray Jui <ray.jui@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
---
 arch/arm/boot/dts/bcm-cygnus.dtsi | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm-cygnus.dtsi b/arch/arm/boot/dts/bcm-cygnus.dtsi
index 3878793..9190224 100644
--- a/arch/arm/boot/dts/bcm-cygnus.dtsi
+++ b/arch/arm/boot/dts/bcm-cygnus.dtsi
@@ -351,9 +351,16 @@
 					<&pinctrl 142 10 1>;
 		};
 
+		ts_adc_syscon: ts_adc_syscon@0x180a6000 {
+			compatible = "syscon";
+			reg = <0x180a6000 0xc30>;
+		};
+
 		touchscreen: tsc@180a6000 {
 			compatible = "brcm,iproc-touchscreen";
-			reg = <0x180a6000 0x40>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+			ts_syscon = <&ts_adc_syscon>;
 			clocks = <&asiu_clks BCM_CYGNUS_ASIU_ADC_CLK>;
 			clock-names = "tsc_clk";
 			interrupts = <GIC_SPI 164 IRQ_TYPE_LEVEL_HIGH>;
-- 
1.9.1