[alsa-devel] [PATCH RFC 0/3] WM97xx AC97 GPIO support
Here's 3 patches which implement simple AC97 GPIO support to the WM97xx touch drivers and the Alchemy (au1x) AC97 ASoC implementation.
Tested on Au1200 hardware with a WM9712 codec: According to Mark Brown the WM9712 only sets GPIOs when data is sent through slot 12, which prompted me to implement this.
Thanks, Manuel Lauss
Manuel Lauss (3): sound: ac97: Add GPIO manipulation hooks input: wm97xx: set GPIOs with AC97 GPIO busops if implemented sound: asoc: au1x: implement AC97 GPIO access
drivers/input/touchscreen/wm97xx-core.c | 5 +++- include/sound/ac97_codec.h | 2 + sound/soc/au1x/psc-ac97.c | 39 +++++++++++++++++++++++++++++- sound/soc/au1x/psc.h | 2 + 4 files changed, 45 insertions(+), 3 deletions(-)
-- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Add callbacks to struct snd_ac97 to set and retrieve ac97 codec GPIO status.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com --- include/sound/ac97_codec.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 4940045..db3aee1 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -437,6 +437,8 @@ struct snd_ac97_bus_ops { unsigned short (*read) (struct snd_ac97 *ac97, unsigned short reg); void (*wait) (struct snd_ac97 *ac97); void (*init) (struct snd_ac97 *ac97); + void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio); + unsigned short (*getgpio) (struct snd_ac97 *ac97); };
struct snd_ac97_bus {
On Tue, Jun 15, 2010 at 05:55:45PM +0200, Manuel Lauss wrote:
Add callbacks to struct snd_ac97 to set and retrieve ac97 codec GPIO status.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com
CCing in Takashi since this is generic AC'97 stuff.
include/sound/ac97_codec.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 4940045..db3aee1 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -437,6 +437,8 @@ struct snd_ac97_bus_ops { unsigned short (*read) (struct snd_ac97 *ac97, unsigned short reg); void (*wait) (struct snd_ac97 *ac97); void (*init) (struct snd_ac97 *ac97);
- void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio);
- unsigned short (*getgpio) (struct snd_ac97 *ac97);
};
struct snd_ac97_bus {
1.7.1
So, I'd kind of expect some sort of interaction with gpiolib somewhere if we do support GPIOs (I know the WM97xx custom stuff doesn't currently use GPIOs but...). -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 15, 2010 at 6:02 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Tue, Jun 15, 2010 at 05:55:45PM +0200, Manuel Lauss wrote:
Add callbacks to struct snd_ac97 to set and retrieve ac97 codec GPIO status.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com
CCing in Takashi since this is generic AC'97 stuff.
include/sound/ac97_codec.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 4940045..db3aee1 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -437,6 +437,8 @@ struct snd_ac97_bus_ops { unsigned short (*read) (struct snd_ac97 *ac97, unsigned short reg); void (*wait) (struct snd_ac97 *ac97); void (*init) (struct snd_ac97 *ac97);
- void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio);
- unsigned short (*getgpio) (struct snd_ac97 *ac97);
};
struct snd_ac97_bus {
1.7.1
So, I'd kind of expect some sort of interaction with gpiolib somewhere if we do support GPIOs (I know the WM97xx custom stuff doesn't currently use GPIOs but...).
If I ever get access to the testhardware again, I'll implement it in the wm97xx core.
Manuel -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
At Tue, 15 Jun 2010 17:02:53 +0100, Mark Brown wrote:
On Tue, Jun 15, 2010 at 05:55:45PM +0200, Manuel Lauss wrote:
Add callbacks to struct snd_ac97 to set and retrieve ac97 codec GPIO status.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com
CCing in Takashi since this is generic AC'97 stuff.
Thanks.
include/sound/ac97_codec.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 4940045..db3aee1 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -437,6 +437,8 @@ struct snd_ac97_bus_ops { unsigned short (*read) (struct snd_ac97 *ac97, unsigned short reg); void (*wait) (struct snd_ac97 *ac97); void (*init) (struct snd_ac97 *ac97);
- void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio);
- unsigned short (*getgpio) (struct snd_ac97 *ac97);
Can this be handled simply in read/write callbacks in the controller side? In callbacks, you can check whether reg == AC97_GPIO_STATUS and handle differently for such an access.
thanks,
Takashi -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 16, 2010 at 9:55 AM, Takashi Iwai tiwai@suse.de wrote:
include/sound/ac97_codec.h | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/include/sound/ac97_codec.h b/include/sound/ac97_codec.h index 4940045..db3aee1 100644 --- a/include/sound/ac97_codec.h +++ b/include/sound/ac97_codec.h @@ -437,6 +437,8 @@ struct snd_ac97_bus_ops { unsigned short (*read) (struct snd_ac97 *ac97, unsigned short reg); void (*wait) (struct snd_ac97 *ac97); void (*init) (struct snd_ac97 *ac97);
- void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio);
- unsigned short (*getgpio) (struct snd_ac97 *ac97);
Can this be handled simply in read/write callbacks in the controller side? In callbacks, you can check whether reg == AC97_GPIO_STATUS and handle differently for such an access.
Interesting Idea, and as a bonus requires no changes to the wm97xx code.
Thank you! Manuel Lauss -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jun 16, 2010 at 10:00:19AM +0200, Manuel Lauss wrote:
On Wed, Jun 16, 2010 at 9:55 AM, Takashi Iwai tiwai@suse.de wrote:
- void (*setgpio) (struct snd_ac97 *ac97, unsigned short gpio);
- unsigned short (*getgpio) (struct snd_ac97 *ac97);
Can this be handled simply in read/write callbacks in the controller side? In callbacks, you can check whether reg == AC97_GPIO_STATUS and handle differently for such an access.
Interesting Idea, and as a bonus requires no changes to the wm97xx code.
The only thing that worries me there is any non-standard behavior the chip may implement - it may actually want a write to that register for some extended functionality. -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
If the AC97 controller implements the ->setgpio callback, use it. Some codecs (i.e. WM9712) only accept GPIO data through AC97 slot 12.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com --- drivers/input/touchscreen/wm97xx-core.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/drivers/input/touchscreen/wm97xx-core.c b/drivers/input/touchscreen/wm97xx-core.c index cbfef1e..4f7b38c 100644 --- a/drivers/input/touchscreen/wm97xx-core.c +++ b/drivers/input/touchscreen/wm97xx-core.c @@ -206,7 +206,10 @@ void wm97xx_set_gpio(struct wm97xx *wm, u32 gpio, reg &= ~gpio;
if (wm->id == WM9712_ID2 && wm->variant != WM97xx_WM1613) - wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg << 1); + reg <<= 1; + + if (wm->ac97->bus->ops->setgpio) + wm->ac97->bus->ops->setgpio(wm->ac97, reg); else wm97xx_reg_write(wm, AC97_GPIO_STATUS, reg); mutex_unlock(&wm->codec_mutex);
On Tue, Jun 15, 2010 at 05:55:46PM +0200, Manuel Lauss wrote:
If the AC97 controller implements the ->setgpio callback, use it. Some codecs (i.e. WM9712) only accept GPIO data through AC97 slot 12.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com
It seems like it'd be easier to just do this as a generic AC'97 thing rather than adding it to the WM97xx driver - that way any CODEC that has GPIO support can use it, we don't have to add custom per-CODEC GPIO APIs. If you're using the WM97xx-specific code you're already doing something custom so it's not much loss to use a generic thing.
Like I said in reply to the previous mail super bonus fun points for doing it in terms of gpiolib. -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 15, 2010 at 6:08 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Tue, Jun 15, 2010 at 05:55:46PM +0200, Manuel Lauss wrote:
If the AC97 controller implements the ->setgpio callback, use it. Some codecs (i.e. WM9712) only accept GPIO data through AC97 slot 12.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com
It seems like it'd be easier to just do this as a generic AC'97 thing rather than adding it to the WM97xx driver - that way any CODEC that has GPIO support can use it, we don't have to add custom per-CODEC GPIO APIs. If you're using the WM97xx-specific code you're already doing something custom so it's not much loss to use a generic thing.
Oh, now I get it. Hmm, what about setting pin direction? That certainly is per-codec, no?
Manuel -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 15, 2010 at 06:17:18PM +0200, Manuel Lauss wrote:
On Tue, Jun 15, 2010 at 6:08 PM, Mark Brown
It seems like it'd be easier to just do this as a generic AC'97 thing rather than adding it to the WM97xx driver - that way any CODEC that has GPIO support can use it, we don't have to add custom per-CODEC GPIO APIs. If you're using the WM97xx-specific code you're already doing something custom so it's not much loss to use a generic thing.
Oh, now I get it. Hmm, what about setting pin direction? That certainly is per-codec, no?
IIRC, yes - I'd need to check. I'd guess a lot of devices will be fixed function and only support one direction. -- To unsubscribe from this list: send the line "unsubscribe alsa-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
The Alchemy PSC AC97 controller can receive/transmit AC97 Slot 12 data (Codec GPIO) through 2 registers.
Signed-off-by: Manuel Lauss manuel.lauss@googlemail.com --- sound/soc/au1x/psc-ac97.c | 39 +++++++++++++++++++++++++++++++++++++-- sound/soc/au1x/psc.h | 2 ++ 2 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/sound/soc/au1x/psc-ac97.c b/sound/soc/au1x/psc-ac97.c index a61ccd2..1da2949 100644 --- a/sound/soc/au1x/psc-ac97.c +++ b/sound/soc/au1x/psc-ac97.c @@ -56,6 +56,31 @@ /* instance data. There can be only one, MacLeod!!!! */ static struct au1xpsc_audio_data *au1xpsc_ac97_workdata;
+static void au1xpsc_ac97_setgpio(struct snd_ac97 *ac97, unsigned short gpio) +{ + /* FIXME */ + struct au1xpsc_audio_data *pscdata = au1xpsc_ac97_workdata; + au_writel(gpio, AC97_GPO(pscdata)); + au_sync(); +} + +static unsigned short au1xpsc_ac97_getgpio(struct snd_ac97 *ac97) +{ + /* FIXME */ + struct au1xpsc_audio_data *pscdata = au1xpsc_ac97_workdata; + unsigned long data; + + while (!(au_readl(AC97_EVNT(pscdata)) & PSC_AC97EVNT_GR)) + cpu_relax(); + au_writel(PSC_AC97EVNT_GR, AC97_EVNT(pscdata)); + au_sync(); + + data = au_readl(AC97_GPI(pscdata)); + data = (data >> 4) & 0xffff; + + return data; +} + /* AC97 controller reads codec register */ static unsigned short au1xpsc_ac97_read(struct snd_ac97 *ac97, unsigned short reg) @@ -195,6 +220,8 @@ struct snd_ac97_bus_ops soc_ac97_ops = { .write = au1xpsc_ac97_write, .reset = au1xpsc_ac97_cold_reset, .warm_reset = au1xpsc_ac97_warm_reset, + .setgpio = au1xpsc_ac97_setgpio, + .getgpio = au1xpsc_ac97_getgpio, }; EXPORT_SYMBOL_GPL(soc_ac97_ops);
@@ -229,10 +256,12 @@ static int au1xpsc_ac97_hw_params(struct snd_pcm_substream *substream, r &= ~PSC_AC97CFG_TXSLOT_MASK; r |= PSC_AC97CFG_TXSLOT_ENA(3); r |= PSC_AC97CFG_TXSLOT_ENA(4); + r |= PSC_AC97CFG_TXSLOT_ENA(12); /* GPIO */ } else { r &= ~PSC_AC97CFG_RXSLOT_MASK; r |= PSC_AC97CFG_RXSLOT_ENA(3); r |= PSC_AC97CFG_RXSLOT_ENA(4); + r |= PSC_AC97CFG_RXSLOT_ENA(12); /* GPIO */ }
/* do we need to poke the hardware? */ @@ -384,9 +413,15 @@ static int __devinit au1xpsc_ac97_drvprobe(struct platform_device *pdev) if (!wd->mmio) goto out1;
- /* configuration: max dma trigger threshold, enable ac97 */ + /* configuration: max dma trigger threshold, enable ac97, + * enable GPIO regs, enable GPIO slots, 16bit len default + * so it clocks out all GPIO info. + */ wd->cfg = PSC_AC97CFG_RT_FIFO8 | PSC_AC97CFG_TT_FIFO8 | - PSC_AC97CFG_DE_ENABLE; + PSC_AC97CFG_DE_ENABLE | PSC_AC97CFG_GE_ENABLE | + PSC_AC97CFG_TXSLOT_ENA(12) | + PSC_AC97CFG_RXSLOT_ENA(12) | + PSC_AC97CFG_SET_LEN(16);
/* preserve PSC clock source set up by platform */ sel = au_readl(PSC_SEL(wd)) & PSC_SEL_CLK_MASK; diff --git a/sound/soc/au1x/psc.h b/sound/soc/au1x/psc.h index 32d3807..915c3de 100644 --- a/sound/soc/au1x/psc.h +++ b/sound/soc/au1x/psc.h @@ -55,5 +55,7 @@ struct au1xpsc_audio_data { #define AC97_PCR(x) ((unsigned long)((x)->mmio) + PSC_AC97PCR_OFFSET) #define AC97_RST(x) ((unsigned long)((x)->mmio) + PSC_AC97RST_OFFSET) #define AC97_STAT(x) ((unsigned long)((x)->mmio) + PSC_AC97STAT_OFFSET) +#define AC97_GPO(x) ((unsigned long)((x)->mmio) + PSC_AC97GPO_OFFSET) +#define AC97_GPI(x) ((unsigned long)((x)->mmio) + PSC_AC97GPI_OFFSET)
#endif
participants (3)
-
Manuel Lauss -
Mark Brown -
Takashi Iwai