[29/46] Input: touchscreen: use wrapper for pxa2xx ac97 registers
diff mbox series

Message ID 20191018154201.1276638-29-arnd@arndb.de
State New
Headers show
Series
  • ARM: pxa: towards multiplatform support
Related show

Commit Message

Arnd Bergmann Oct. 18, 2019, 3:41 p.m. UTC
To avoid a dependency on the pxa platform header files with
hardcoded registers, change the driver to call a wrapper
in the pxa2xx-ac97-lib that encapsulates all the other
ac97 stuff.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: linux-input@vger.kernel.org
Cc: alsa-devel@alsa-project.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/input/touchscreen/Kconfig            |  2 ++
 drivers/input/touchscreen/mainstone-wm97xx.c | 16 ++++++++--------
 drivers/input/touchscreen/zylonite-wm97xx.c  | 12 ++++++------
 include/sound/pxa2xx-lib.h                   |  4 ++++
 sound/arm/pxa2xx-ac97-lib.c                  | 12 ++++++++++++
 5 files changed, 32 insertions(+), 14 deletions(-)

Comments

Dmitry Torokhov Oct. 18, 2019, 6:48 p.m. UTC | #1
On Fri, Oct 18, 2019 at 05:41:44PM +0200, Arnd Bergmann wrote:
> To avoid a dependency on the pxa platform header files with
> hardcoded registers, change the driver to call a wrapper
> in the pxa2xx-ac97-lib that encapsulates all the other
> ac97 stuff.

Not supper happy about adding module dependencies. Can we include
mach/regs-ac97.h from include/sound/pxa2xx-lib.h and use static inlines?
Someone needs to include mach/regs-ac97.h in the end...

Or there is something later in the series that needs it?

Thanks.
Arnd Bergmann Oct. 18, 2019, 7:39 p.m. UTC | #2
On Fri, Oct 18, 2019 at 8:48 PM Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
>
> On Fri, Oct 18, 2019 at 05:41:44PM +0200, Arnd Bergmann wrote:
> > To avoid a dependency on the pxa platform header files with
> > hardcoded registers, change the driver to call a wrapper
> > in the pxa2xx-ac97-lib that encapsulates all the other
> > ac97 stuff.
>
> Not supper happy about adding module dependencies. Can we include
> mach/regs-ac97.h from include/sound/pxa2xx-lib.h and use static inlines?
> Someone needs to include mach/regs-ac97.h in the end...
>
> Or there is something later in the series that needs it?

One of the goals of the series is to completely remove all mach/*.h headers
and place them somewhere else, ideally inaccessible to device drivers.

In case of mach/regs-ac97.h, the later patch "ASoC: pxa: ac97: use normal
MMIO accessors" passes the physical register base address as a platform
device resource that gets ioremapped in the ac97 driver, rather than
hardcoding the virtual address in a global header.

I agree that the exported function is still ugly, but I hope it's enough of
an improvement over the previous state that we can do it anyway.

If you have any better ideas though, I can give that a try, too.
One possibility would be a higher-level interface exported on top
of 'struct snd_ac97', but I could not figure out how to do this.

     Arnd
Dmitry Torokhov Oct. 18, 2019, 8:39 p.m. UTC | #3
On Fri, Oct 18, 2019 at 09:39:31PM +0200, Arnd Bergmann wrote:
> On Fri, Oct 18, 2019 at 8:48 PM Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> >
> > On Fri, Oct 18, 2019 at 05:41:44PM +0200, Arnd Bergmann wrote:
> > > To avoid a dependency on the pxa platform header files with
> > > hardcoded registers, change the driver to call a wrapper
> > > in the pxa2xx-ac97-lib that encapsulates all the other
> > > ac97 stuff.
> >
> > Not supper happy about adding module dependencies. Can we include
> > mach/regs-ac97.h from include/sound/pxa2xx-lib.h and use static inlines?
> > Someone needs to include mach/regs-ac97.h in the end...
> >
> > Or there is something later in the series that needs it?
> 
> One of the goals of the series is to completely remove all mach/*.h headers
> and place them somewhere else, ideally inaccessible to device drivers.
> 
> In case of mach/regs-ac97.h, the later patch "ASoC: pxa: ac97: use normal
> MMIO accessors" passes the physical register base address as a platform
> device resource that gets ioremapped in the ac97 driver, rather than
> hardcoding the virtual address in a global header.
> 
> I agree that the exported function is still ugly, but I hope it's enough of
> an improvement over the previous state that we can do it anyway.
> 
> If you have any better ideas though, I can give that a try, too.
> One possibility would be a higher-level interface exported on top
> of 'struct snd_ac97', but I could not figure out how to do this.

No, I do mot really have better ideas given your stated goals and I
guess there is not really much benefit on spending too much effort
polishing essentially one driver.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Thanks.
Robert Jarzmik Oct. 30, 2019, 8:33 p.m. UTC | #4
Arnd Bergmann <arnd@arndb.de> writes:

> To avoid a dependency on the pxa platform header files with
> hardcoded registers, change the driver to call a wrapper
> in the pxa2xx-ac97-lib that encapsulates all the other
> ac97 stuff.
>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: alsa-devel@alsa-project.org
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Robert Jarzmik <robert.jarzmik@free.fr>

Cheers.

--
Robert

Patch
diff mbox series

diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 46ad9090493b..c60199550d89 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -848,6 +848,7 @@  config TOUCHSCREEN_WM9713
 config TOUCHSCREEN_WM97XX_MAINSTONE
 	tristate "WM97xx Mainstone/Palm accelerated touch"
 	depends on TOUCHSCREEN_WM97XX && ARCH_PXA
+	depends on SND_PXA2XX_LIB_AC97
 	help
 	  Say Y here for support for streaming mode with WM97xx touchscreens
 	  on Mainstone, Palm Tungsten T5, TX and LifeDrive systems.
@@ -860,6 +861,7 @@  config TOUCHSCREEN_WM97XX_MAINSTONE
 config TOUCHSCREEN_WM97XX_ZYLONITE
 	tristate "Zylonite accelerated touch"
 	depends on TOUCHSCREEN_WM97XX && MACH_ZYLONITE
+	depends on SND_PXA2XX_LIB_AC97
 	select TOUCHSCREEN_WM9713
 	help
 	  Say Y here for support for streaming mode with the touchscreen
diff --git a/drivers/input/touchscreen/mainstone-wm97xx.c b/drivers/input/touchscreen/mainstone-wm97xx.c
index 940d3c92b1f8..8f6fe68f1f99 100644
--- a/drivers/input/touchscreen/mainstone-wm97xx.c
+++ b/drivers/input/touchscreen/mainstone-wm97xx.c
@@ -28,7 +28,7 @@ 
 #include <linux/soc/pxa/cpu.h>
 #include <linux/wm97xx.h>
 
-#include <mach/regs-ac97.h>
+#include <sound/pxa2xx-lib.h>
 
 #include <asm/mach-types.h>
 
@@ -104,11 +104,11 @@  static void wm97xx_acc_pen_up(struct wm97xx *wm)
 	msleep(1);
 
 	if (cpu_is_pxa27x()) {
-		while (MISR & (1 << 2))
-			MODR;
+		while (pxa2xx_ac97_read_misr() & (1 << 2))
+			pxa2xx_ac97_read_modr();
 	} else if (cpu_is_pxa3xx()) {
 		for (count = 0; count < 16; count++)
-			MODR;
+			pxa2xx_ac97_read_modr();
 	}
 }
 
@@ -130,7 +130,7 @@  static int wm97xx_acc_pen_down(struct wm97xx *wm)
 		return RC_PENUP;
 	}
 
-	x = MODR;
+	x = pxa2xx_ac97_read_modr();
 	if (x == last) {
 		tries++;
 		return RC_AGAIN;
@@ -138,10 +138,10 @@  static int wm97xx_acc_pen_down(struct wm97xx *wm)
 	last = x;
 	do {
 		if (reads)
-			x = MODR;
-		y = MODR;
+			x = pxa2xx_ac97_read_modr();
+		y = pxa2xx_ac97_read_modr();
 		if (pressure)
-			p = MODR;
+			p = pxa2xx_ac97_read_modr();
 
 		dev_dbg(wm->dev, "Raw coordinates: x=%x, y=%x, p=%x\n",
 			x, y, p);
diff --git a/drivers/input/touchscreen/zylonite-wm97xx.c b/drivers/input/touchscreen/zylonite-wm97xx.c
index cabdd6e3c6f8..ed7eae638713 100644
--- a/drivers/input/touchscreen/zylonite-wm97xx.c
+++ b/drivers/input/touchscreen/zylonite-wm97xx.c
@@ -24,7 +24,7 @@ 
 #include <linux/soc/pxa/cpu.h>
 #include <linux/wm97xx.h>
 
-#include <mach/regs-ac97.h>
+#include <sound/pxa2xx-lib.h>
 
 struct continuous {
 	u16 id;    /* codec id */
@@ -79,7 +79,7 @@  static void wm97xx_acc_pen_up(struct wm97xx *wm)
 	msleep(1);
 
 	for (i = 0; i < 16; i++)
-		MODR;
+		pxa2xx_ac97_read_modr();
 }
 
 static int wm97xx_acc_pen_down(struct wm97xx *wm)
@@ -100,7 +100,7 @@  static int wm97xx_acc_pen_down(struct wm97xx *wm)
 		return RC_PENUP;
 	}
 
-	x = MODR;
+	x = pxa2xx_ac97_read_modr();
 	if (x == last) {
 		tries++;
 		return RC_AGAIN;
@@ -108,10 +108,10 @@  static int wm97xx_acc_pen_down(struct wm97xx *wm)
 	last = x;
 	do {
 		if (reads)
-			x = MODR;
-		y = MODR;
+			x = pxa2xx_ac97_read_modr();
+		y = pxa2xx_ac97_read_modr();
 		if (pressure)
-			p = MODR;
+			p = pxa2xx_ac97_read_modr();
 
 		dev_dbg(wm->dev, "Raw coordinates: x=%x, y=%x, p=%x\n",
 			x, y, p);
diff --git a/include/sound/pxa2xx-lib.h b/include/sound/pxa2xx-lib.h
index 6758fc12fa84..79c32a8f4c91 100644
--- a/include/sound/pxa2xx-lib.h
+++ b/include/sound/pxa2xx-lib.h
@@ -41,4 +41,8 @@  extern int pxa2xx_ac97_hw_resume(void);
 extern int pxa2xx_ac97_hw_probe(struct platform_device *dev);
 extern void pxa2xx_ac97_hw_remove(struct platform_device *dev);
 
+/* modem registers, used by touchscreen driver */
+u32 pxa2xx_ac97_read_modr(void);
+u32 pxa2xx_ac97_read_misr(void);
+
 #endif
diff --git a/sound/arm/pxa2xx-ac97-lib.c b/sound/arm/pxa2xx-ac97-lib.c
index 8c79d224f03b..572b73d73762 100644
--- a/sound/arm/pxa2xx-ac97-lib.c
+++ b/sound/arm/pxa2xx-ac97-lib.c
@@ -428,6 +428,18 @@  void pxa2xx_ac97_hw_remove(struct platform_device *dev)
 }
 EXPORT_SYMBOL_GPL(pxa2xx_ac97_hw_remove);
 
+u32 pxa2xx_ac97_read_modr(void)
+{
+	return MODR;
+}
+EXPORT_SYMBOL_GPL(pxa2xx_ac97_read_modr);
+
+u32 pxa2xx_ac97_read_misr(void)
+{
+	return MISR;
+}
+EXPORT_SYMBOL_GPL(pxa2xx_ac97_read_misr);
+
 MODULE_AUTHOR("Nicolas Pitre");
 MODULE_DESCRIPTION("Intel/Marvell PXA sound library");
 MODULE_LICENSE("GPL");