| Message ID | 20191018154201.1276638-29-arnd@arndb.de |
|---|---|
| State | Accepted |
| Commit | e217b085a1ac1c4108fe632bdaa08ea5b0ecd145 |
| Headers | show |
| Series | None | expand |
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. -- Dmitry
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
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. -- Dmitry
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
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");
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(-) -- 2.20.0