The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
kernel, declare a gpio chip.
Signed-off-by: Robert Jarzmik <[email protected]>
---
sound/soc/codecs/wm9713.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++
sound/soc/codecs/wm9713.h | 1 +
2 files changed, 124 insertions(+)
diff --git a/sound/soc/codecs/wm9713.c b/sound/soc/codecs/wm9713.c
index 79e143625ac3..904fe4fc5bf1 100644
--- a/sound/soc/codecs/wm9713.c
+++ b/sound/soc/codecs/wm9713.c
@@ -19,6 +19,7 @@
#include <linux/slab.h>
#include <linux/module.h>
#include <linux/device.h>
+#include <linux/gpio/driver.h>
#include <linux/regmap.h>
#include <sound/core.h>
#include <sound/pcm.h>
@@ -33,11 +34,21 @@
#define WM9713_VENDOR_ID 0x574d4c13
#define WM9713_VENDOR_ID_MASK 0xffffffff
+#define AC97_GPIO_PUSH_PULL 0x58
+
+struct wm9713_gpio_priv {
+#ifdef CONFIG_GPIOLIB
+ struct gpio_chip gpio_chip;
+#endif
+ struct snd_soc_codec *codec;
+};
+
struct wm9713_priv {
struct snd_ac97 *ac97;
u32 pll_in; /* PLL input frequency */
unsigned int hp_mixer[2];
struct mutex lock;
+ struct wm9713_gpio_priv *gpio_priv;
};
#define HPL_MIXER 0
@@ -1202,10 +1213,116 @@ static int wm9713_soc_resume(struct snd_soc_codec *codec)
return ret;
}
+#ifdef CONFIG_GPIOLIB
+static inline struct snd_soc_codec *gpio_to_codec(struct gpio_chip *chip)
+{
+ struct wm9713_gpio_priv *gpio_priv =
+ container_of(chip, struct wm9713_gpio_priv, gpio_chip);
+
+ return gpio_priv->codec;
+}
+
+static int wm9713_gpio_request(struct gpio_chip *chip, unsigned offset)
+{
+ if (offset >= WM9713_NUM_GPIOS)
+ return -EINVAL;
+
+ return 0;
+}
+
+static int wm9713_gpio_direction_in(struct gpio_chip *chip, unsigned offset)
+{
+ struct snd_soc_codec *codec = gpio_to_codec(chip);
+
+ return snd_soc_update_bits(codec, AC97_GPIO_CFG,
+ 1 << (offset + 1), 1 << (offset + 1));
+}
+
+static int wm9713_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct snd_soc_codec *codec = gpio_to_codec(chip);
+ int ret;
+
+ ret = snd_soc_read(codec, AC97_GPIO_STATUS);
+
+ return ret < 0 ? ret : ret & (1 << (offset + 1));
+}
+
+static void wm9713_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
+{
+ struct snd_soc_codec *codec = gpio_to_codec(chip);
+
+ snd_soc_update_bits(codec, AC97_GPIO_PUSH_PULL,
+ 1 << offset | (1 << (offset + 8)),
+ 1 << (offset + 8 * !!value));
+}
+
+static int wm9713_gpio_direction_out(struct gpio_chip *chip,
+ unsigned offset, int value)
+{
+ struct snd_soc_codec *codec = gpio_to_codec(chip);
+
+ wm9713_gpio_set(chip, offset, value);
+
+ return snd_soc_update_bits(codec, AC97_GPIO_CFG, 1 << (offset + 1), 0);
+}
+
+static struct gpio_chip wm9713_gpio_chip = {
+ .label = "wm9713",
+ .owner = THIS_MODULE,
+ .request = wm9713_gpio_request,
+ .direction_input = wm9713_gpio_direction_in,
+ .get = wm9713_gpio_get,
+ .direction_output = wm9713_gpio_direction_out,
+ .set = wm9713_gpio_set,
+ .can_sleep = 1,
+};
+
+static int wm9713_init_gpio(struct snd_soc_codec *codec)
+{
+ struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
+ struct wm9713_gpio_priv *gpio_priv;
+ int ret;
+
+ gpio_priv = devm_kzalloc(codec->dev, sizeof(*wm9713->gpio_priv),
+ GFP_KERNEL);
+ if (!gpio_priv)
+ return -ENOMEM;
+ wm9713->gpio_priv = gpio_priv;
+ gpio_priv->codec = codec;
+ gpio_priv->gpio_chip = wm9713_gpio_chip;
+ gpio_priv->gpio_chip.ngpio = WM9713_NUM_GPIOS;
+ gpio_priv->gpio_chip.dev = codec->dev;
+ gpio_priv->gpio_chip.base = -1;
+
+ ret = gpiochip_add(&gpio_priv->gpio_chip);
+ if (ret != 0)
+ dev_err(codec->dev, "Failed to add GPIOs: %d\n", ret);
+ return ret;
+}
+
+static void wm9713_free_gpio(struct snd_soc_codec *codec)
+{
+ struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
+
+ gpiochip_remove(&wm9713->gpio_priv->gpio_chip);
+}
+#else
+static int wm9713_init_gpio(struct snd_soc_codec *codec)
+{
+ return 0;
+}
+
+static void wm9713_free_gpio(struct snd_soc_codec *codec)
+{
+}
+#endif
+
static int wm9713_soc_probe(struct snd_soc_codec *codec)
{
struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
struct regmap *regmap;
+ int ret;
wm9713->ac97 = snd_soc_new_ac97_codec(codec, WM9713_VENDOR_ID,
WM9713_VENDOR_ID_MASK);
@@ -1223,6 +1340,11 @@ static int wm9713_soc_probe(struct snd_soc_codec *codec)
/* unmute the adc - move to kcontrol */
snd_soc_update_bits(codec, AC97_CD, 0x7fff, 0x0000);
+ ret = wm9713_init_gpio(codec);
+ if (ret) {
+ snd_soc_free_ac97_codec(wm9713->ac97);
+ return ret;
+ }
return 0;
}
@@ -1230,6 +1352,7 @@ static int wm9713_soc_remove(struct snd_soc_codec *codec)
{
struct wm9713_priv *wm9713 = snd_soc_codec_get_drvdata(codec);
+ wm9713_free_gpio(codec);
snd_soc_codec_exit_regmap(codec);
snd_soc_free_ac97_codec(wm9713->ac97);
return 0;
diff --git a/sound/soc/codecs/wm9713.h b/sound/soc/codecs/wm9713.h
index 53df11b1f727..d72f96e653d1 100644
--- a/sound/soc/codecs/wm9713.h
+++ b/sound/soc/codecs/wm9713.h
@@ -45,4 +45,5 @@
#define WM9713_DAI_AC97_AUX 1
#define WM9713_DAI_PCM_VOICE 2
+#define WM9713_NUM_GPIOS 8
#endif
--
2.1.4
On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
> kernel, declare a gpio chip.
>
> Signed-off-by: Robert Jarzmik <[email protected]>
> ---
You should probably make a seperate driver within GPIO for this
and then tie the two together, using an MFD. I appreciate that is
more work but it is likely a nicer solution overall.
Thanks,
Charles
Charles Keepax <[email protected]> writes:
> On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
>> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
>> kernel, declare a gpio chip.
>>
>> Signed-off-by: Robert Jarzmik <[email protected]>
>> ---
>
> You should probably make a seperate driver within GPIO for this
> and then tie the two together, using an MFD. I appreciate that is
> more work but it is likely a nicer solution overall.
I'd like to first have a confirmation from :
- Mark (Brown)
- and Lee (Jones)
The confirmation I'm looking for states that :
- the wm9713 should have a part in the mfd tree
- the gpio part should be in drivers/gpio
- the sound soc codecs will remain as is
- if the future driver/mfd/wm9713.c is technically sound, it will be accepted
I remember at least one example where the MFD approach was rejected from mfd
tree for pxa gpios, so I won't work unless I have a confirmation from both
maintainers.
Thanks.
--
Robert
On Wed, Nov 04, 2015 at 08:35:18PM +0100, Robert Jarzmik wrote:
> Charles Keepax <[email protected]> writes:
>
> > On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
> >> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
> >> kernel, declare a gpio chip.
> >>
> >> Signed-off-by: Robert Jarzmik <[email protected]>
> >> ---
> >
> > You should probably make a seperate driver within GPIO for this
> > and then tie the two together, using an MFD. I appreciate that is
> > more work but it is likely a nicer solution overall.
>
> I'd like to first have a confirmation from :
> - Mark (Brown)
> - and Lee (Jones)
That seems like a good idea to me as well :-)
Thanks,
Charles
On Wed, Nov 04, 2015 at 06:33:22PM +0000, Charles Keepax wrote:
> You should probably make a seperate driver within GPIO for this
> and then tie the two together, using an MFD. I appreciate that is
> more work but it is likely a nicer solution overall.
GPIO chips are small and simple enough that we've not worried about it
historically. However my question here is why this is wm9713 specific -
the GPIO functionality is part of the AC'97 spec and present on many
AC'97 chips so it seems sensible to have something at the AC'97 level.
Unfortunately a quick check seems not to show anything indicating this
in the AC'97 capability information so we might want to make it opt in.
On Fri, Nov 06, 2015 at 09:29:13AM +0000, Lee Jones wrote:
> On Wed, 04 Nov 2015, Robert Jarzmik wrote:
>
> > Charles Keepax <[email protected]> writes:
> >
> > > On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
> > >> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
> > >> kernel, declare a gpio chip.
> > >>
> > >> Signed-off-by: Robert Jarzmik <[email protected]>
> > >> ---
> > >
> > > You should probably make a seperate driver within GPIO for this
> > > and then tie the two together, using an MFD. I appreciate that is
> > > more work but it is likely a nicer solution overall.
> >
> > I'd like to first have a confirmation from :
> > - Mark (Brown)
> > - and Lee (Jones)
> >
> > The confirmation I'm looking for states that :
> > - the wm9713 should have a part in the mfd tree
> > - the gpio part should be in drivers/gpio
> > - the sound soc codecs will remain as is
> > - if the future driver/mfd/wm9713.c is technically sound, it will be accepted
> >
> > I remember at least one example where the MFD approach was rejected from mfd
> > tree for pxa gpios, so I won't work unless I have a confirmation from both
> > maintainers.
>
> I have no idea what you're talking about. Context please?
Apologies Lee, we are discussing a patch that adds a GPIO driver
into an AC97 CODEC. I had suggested that perhaps we should put
the GPIO driver as a seperate driver under GPIO and link the two
with an MFD. But Mark has already replied in the thread to say
that he doesn't think that will be necessary. Although he did
raise some concerns that perhaps it could be done more generally
as it should apply to other AC97 CODECs as well.
So I think you can probably safely ignore this for now, sorry
for the noise.
Thanks,
Charles
On Wed, 04 Nov 2015, Robert Jarzmik wrote:
> Charles Keepax <[email protected]> writes:
>
> > On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
> >> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
> >> kernel, declare a gpio chip.
> >>
> >> Signed-off-by: Robert Jarzmik <[email protected]>
> >> ---
> >
> > You should probably make a seperate driver within GPIO for this
> > and then tie the two together, using an MFD. I appreciate that is
> > more work but it is likely a nicer solution overall.
>
> I'd like to first have a confirmation from :
> - Mark (Brown)
> - and Lee (Jones)
>
> The confirmation I'm looking for states that :
> - the wm9713 should have a part in the mfd tree
> - the gpio part should be in drivers/gpio
> - the sound soc codecs will remain as is
> - if the future driver/mfd/wm9713.c is technically sound, it will be accepted
>
> I remember at least one example where the MFD approach was rejected from mfd
> tree for pxa gpios, so I won't work unless I have a confirmation from both
> maintainers.
I have no idea what you're talking about. Context please?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Fri, 06 Nov 2015, Charles Keepax wrote:
> On Fri, Nov 06, 2015 at 09:29:13AM +0000, Lee Jones wrote:
> > On Wed, 04 Nov 2015, Robert Jarzmik wrote:
> >
> > > Charles Keepax <[email protected]> writes:
> > >
> > > > On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
> > > >> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
> > > >> kernel, declare a gpio chip.
> > > >>
> > > >> Signed-off-by: Robert Jarzmik <[email protected]>
> > > >> ---
> > > >
> > > > You should probably make a seperate driver within GPIO for this
> > > > and then tie the two together, using an MFD. I appreciate that is
> > > > more work but it is likely a nicer solution overall.
> > >
> > > I'd like to first have a confirmation from :
> > > - Mark (Brown)
> > > - and Lee (Jones)
> > >
> > > The confirmation I'm looking for states that :
> > > - the wm9713 should have a part in the mfd tree
> > > - the gpio part should be in drivers/gpio
> > > - the sound soc codecs will remain as is
> > > - if the future driver/mfd/wm9713.c is technically sound, it will be accepted
> > >
> > > I remember at least one example where the MFD approach was rejected from mfd
> > > tree for pxa gpios, so I won't work unless I have a confirmation from both
> > > maintainers.
> >
> > I have no idea what you're talking about. Context please?
>
> Apologies Lee, we are discussing a patch that adds a GPIO driver
> into an AC97 CODEC. I had suggested that perhaps we should put
> the GPIO driver as a seperate driver under GPIO and link the two
> with an MFD. But Mark has already replied in the thread to say
> that he doesn't think that will be necessary. Although he did
> raise some concerns that perhaps it could be done more generally
> as it should apply to other AC97 CODECs as well.
>
> So I think you can probably safely ignore this for now, sorry
> for the noise.
Roger that. Thanks for the explanation.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Lee Jones <[email protected]> writes:
> On Fri, 06 Nov 2015, Charles Keepax wrote:
>
>> On Fri, Nov 06, 2015 at 09:29:13AM +0000, Lee Jones wrote:
>> > On Wed, 04 Nov 2015, Robert Jarzmik wrote:
>> >
>> > > Charles Keepax <[email protected]> writes:
>> > >
>> > > > On Wed, Nov 04, 2015 at 06:12:44PM +0100, Robert Jarzmik wrote:
>> > > >> The Wolfson WM9713 provides 8 GPIOs. If the gpiolib is compiled in the
>> > > >> kernel, declare a gpio chip.
>> > > >>
>> > > >> Signed-off-by: Robert Jarzmik <[email protected]>
>>
>> Apologies Lee, we are discussing a patch that adds a GPIO driver
>> into an AC97 CODEC. I had suggested that perhaps we should put
>> the GPIO driver as a seperate driver under GPIO and link the two
>> with an MFD. But Mark has already replied in the thread to say
>> that he doesn't think that will be necessary. Although he did
>> raise some concerns that perhaps it could be done more generally
>> as it should apply to other AC97 CODECs as well.
>>
>> So I think you can probably safely ignore this for now, sorry
>> for the noise.
Ok, so where I should target this code at ? Should this land in
sound/soc/soc-ac97.c ? Or somewhere else ? I'd like to see where you think the
init_gpio() and free_gpio() should be put.
Cheers.
--
Robert
On Fri, Nov 06, 2015 at 09:47:35PM +0100, Robert Jarzmik wrote:
> Ok, so where I should target this code at ? Should this land in
> sound/soc/soc-ac97.c ? Or somewhere else ? I'd like to see where you think the
> init_gpio() and free_gpio() should be put.
Sounds like a reasonable place, or possibly even the ALSA level AC'97
stuff (though I'd question if anyone would ever use it outside of an
ASoC system).