Re: [alsa-devel] [PATCH] asoc tlv320aic3x: add GPIO support
On Fri, Apr 18, 2008 at 11:26:28PM +0300, Jarkko Nikula wrote:
No any show stopper either but I just started to think about gpiolib if AIC3x GPIOs are used as a GPIO.
I agree in general and thought about alternatives as well, but in this case, it turns out to be a thing the aic3x driver is responsible to as the registers in questions are purely in its scope. So the idea is to set up the GPIO's functions via alsa controls and handle its events in some board-related code as this is really not abstractable. One could, however, do this setup in machine-related code within the asoc environment.
My changes don't cast any problems on the existing implementation either, so I would suggest to just take this patch until someone comes up with a more suitable solution :)
Best regards, Daniel
On Sat, Apr 19, 2008 at 02:41:03AM +0200, Daniel Mack wrote:
the registers in questions are purely in its scope. So the idea is to set up the GPIO's functions via alsa controls and handle its events in some board-related code as this is really not abstractable. One could, however, do this setup in machine-related code within the asoc environment.
The usual approach is to have multi-function pins on CODECs are controlled by the ASoC codec driver based on configuration supplied by the machine driver. Their use tends to be heavily constrained by the board design so it usually makes more sense to let the machine driver set things up and deal with exposing controls for any configurability that user space can exercise.
Most of the time when GPIO lines on CODECs are used they tend to be controlling other bits of the audio subsystem anyway (eg, power for simple amplifiers) - most SoCs have plenty of GPIOs on-board and they're usually much easier to work with due to being memory mapped on the CPU.
Hi Mark,
On Sat, Apr 19, 2008 at 12:24:26PM +0100, Mark Brown wrote:
the registers in questions are purely in its scope. So the idea is to set up the GPIO's functions via alsa controls and handle its events in some board-related code as this is really not abstractable. One could, however, do this setup in machine-related code within the asoc environment.
The usual approach is to have multi-function pins on CODECs are controlled by the ASoC codec driver based on configuration supplied by the machine driver.
I was looking for a mechanism to pass such information around but didn't manage to find any. Which way would you suggest? The approach in my patch causes as less overhead as possible at it simply reuses the asoc's widgets, I though.
Their use tends to be heavily constrained by the board design so it usually makes more sense to let the machine driver set things up and deal with exposing controls for any configurability that user space can exercise.
Ok, but why not let the codec driver expose all the functionality it can offer for setting up a certain GPIO and use this alsa control either from user space or from the asoc machine part? I think that's the most flexible way at all, no?
And when it comes to setting or getting a GPIO's state I see no way to access such special purpose functions of the codec driver. At least, 'struct snd_soc_codec' doesn't give me any hints how to achive that.
Most of the time when GPIO lines on CODECs are used they tend to be controlling other bits of the audio subsystem anyway (eg, power for simple amplifiers) - most SoCs have plenty of GPIOs on-board and they're usually much easier to work with due to being memory mapped on the CPU.
Well, I didn't implement that just because it wasn't done yet ;) In the setup I'm working on, one GPIO is needed to control peripherals next to it in the board layout where routing constraints become part of the game. And the other one is connected to one of the processor's GPIO to catch an interrupt from the AIC3x when a headset is plugged in. Thus, we really need to support them both. Others may have different use for them, just have a look at the possible functions of those GPIOs.
Best regards, Daniel
On Sat, Apr 19, 2008 at 02:18:38PM +0200, Daniel Mack wrote:
On Sat, Apr 19, 2008 at 12:24:26PM +0100, Mark Brown wrote:
The usual approach is to have multi-function pins on CODECs are controlled by the ASoC codec driver based on configuration supplied by the machine driver.
I was looking for a mechanism to pass such information around but didn't manage to find any. Which way would you suggest? The approach in my patch causes as less overhead as possible at it simply reuses the asoc's widgets, I though.
The usual thing would be to have your CODEC driver provide its own API which users can call directly.
Their use tends to be heavily constrained by the board design so it usually makes more sense to let the machine driver set things up and deal with exposing controls for any configurability that user space can exercise.
Ok, but why not let the codec driver expose all the functionality it can offer for setting up a certain GPIO and use this alsa control either from user space or from the asoc machine part? I think that's the most flexible way at all, no?
The CODEC driver can't safely give user space control of multi-function pins in general, including GPIOs, since their usage depends very strongly on how they have been connected to the system. At best much of the functionality won't be useful on a given board due to a lack of appropriate connections. At worst the consequences of misusing them can include things like causing one or more chips in the system to fall over, perhaps even the entire system. For example, if you've got two chips each trying to drive a signal to different voltages then it is possible that one or both of them will do something like attempt to consume too much power and cause supplies to collapse.
It's OK for the kernel to be able to misconfigure things and control of these features is very useful so it's good to provide it. The issue is letting user space get involved unless the machine driver has said that it's safe to do so.
And when it comes to setting or getting a GPIO's state I see no way to access such special purpose functions of the codec driver. At least, 'struct snd_soc_codec' doesn't give me any hints how to achive that.
Like I say, export a CODEC specific API for it. The machine driver already needs to know which CODEC and SoC it is working with and how they are connected in a given system in order to set up the audio connections. In some cases the API calls can be avoided - for example, if there is an I2S port on multi-function pins then configuring that I2S port could implicitly configure the multi-function pins to enable that port.
Most of the time when GPIO lines on CODECs are used they tend to be controlling other bits of the audio subsystem anyway (eg, power for simple amplifiers) - most SoCs have plenty of GPIOs on-board and they're usually much easier to work with due to being memory mapped on the CPU.
Well, I didn't implement that just because it wasn't done yet ;) In the setup I'm working on, one GPIO is needed to control peripherals next to it in the board layout where routing constraints become part of the game. And the other one is connected to one of the processor's GPIO to catch an interrupt from the AIC3x when a headset is plugged in. Thus, we really need to support them both. Others may have different use for them, just have a look at the possible functions of those GPIOs.
Those sound like exactly the sort of audio subsystem usages that I was talking about - things that have fixed functions on a given board and which the machine driver should be able to know how to set up in a way that makes sense for the system.
In the case of jack detect I'm actually currently working on a generalisation of that since it's such a common feature. I hope to have something in the next week or two but it depends on my other work.
On Sat, Apr 19, 2008 at 03:50:06PM +0100, Mark Brown wrote:
Ok, but why not let the codec driver expose all the functionality it can offer for setting up a certain GPIO and use this alsa control either from user space or from the asoc machine part? I think that's the most flexible way at all, no?
The CODEC driver can't safely give user space control of multi-function pins in general, including GPIOs, since their usage depends very strongly on how they have been connected to the system. At best much of the functionality won't be useful on a given board due to a lack of appropriate connections. At worst the consequences of misusing them can include things like causing one or more chips in the system to fall over, perhaps even the entire system. For example, if you've got two chips each trying to drive a signal to different voltages then it is possible that one or both of them will do something like attempt to consume too much power and cause supplies to collapse.
Ok, agreed to this point. I wanted to implement this feature as generic as possbile, but you're right, that might cause some trouble.
In the case of jack detect I'm actually currently working on a generalisation of that since it's such a common feature. I hope to have something in the next week or two but it depends on my other work.
I attached a patch for a proprietary interface - is that what you were talking about? Would that approach be mergeable with your generic jack detect code?
Thanks and best regards, Daniel
On Sat, Apr 19, 2008 at 06:24:18PM +0200, Daniel Mack wrote:
On Sat, Apr 19, 2008 at 03:50:06PM +0100, Mark Brown wrote:
In the case of jack detect I'm actually currently working on a generalisation of that since it's such a common feature. I hope to have something in the next week or two but it depends on my other work.
I attached a patch for a proprietary interface - is that what you were talking about? Would that approach be mergeable with your generic jack detect code?
Yes, that was the sort of thing I was thinking about - looks good from an API perspective, though I've not looked at the code properly yet. It should fit in with the jack detect code, the idea is more to let drivers push notifications of jack detect status - it doesn't really care how the driver detects the jack status.
On Sat, Apr 19, 2008 at 9:51 PM, Mark Brown < broonie@opensource.wolfsonmicro.com> wrote:
On Sat, Apr 19, 2008 at 06:24:18PM +0200, Daniel Mack wrote:
On Sat, Apr 19, 2008 at 03:50:06PM +0100, Mark Brown wrote:
In the case of jack detect I'm actually currently working on a generalisation of that since it's such a common feature. I hope to
have
something in the next week or two but it depends on my other work.
I attached a patch for a proprietary interface - is that what you were talking about? Would that approach be mergeable with your generic jack detect code?
Yes, that was the sort of thing I was thinking about - looks good from an API perspective, though I've not looked at the code properly yet. It should fit in with the jack detect code, the idea is more to let drivers push notifications of jack detect status - it doesn't really care how the driver detects the jack status.
This latest version looks ok to me. IMO we can use this as a base for further development.
Jarkko
On Sat, Apr 19, 2008 at 06:24:18PM +0200, Daniel Mack wrote:
I attached a patch for a proprietary interface - is that what you were talking about? Would that approach be mergeable with your generic jack detect code?
The patch looks good but you've not signed off on it - is that just an oversight or do you think more updates are required?
On Tue, Apr 22, 2008 at 10:52:04AM +0100, Mark Brown wrote:
On Sat, Apr 19, 2008 at 06:24:18PM +0200, Daniel Mack wrote:
I attached a patch for a proprietary interface - is that what you were talking about? Would that approach be mergeable with your generic jack detect code?
The patch looks good but you've not signed off on it - is that just an oversight or do you think more updates are required?
No, that's actually final from my side. Here we go.
---
Subject: [PATCH] asoc tlv320aic3x: add GPIO support
This patch adds support for GPIO pins on TI's TLV320AIC3x via a proprietary API. I2C read was also implemented as the actual state of GPIOs must be read from hardware and can not be served from the register cache.
Signed-off-by: Daniel Mack daniel@caiaq.de
On Tue, Apr 22, 2008 at 12:04:30PM +0200, Daniel Mack wrote:
No, that's actually final from my side. Here we go.
Looks like you've generated this against a kernel with the clocking stuff applied:
Applying asoc tlv320aic3x: add GPIO support error: patch failed: sound/soc/codecs/tlv320aic3x.h:108 error: sound/soc/codecs/tlv320aic3x.h: patch does not apply Patch failed at 0001.
On Sat, Apr 19, 2008 at 4:50 PM, Mark Brown broonie@opensource.wolfsonmicro.com wrote:
On Sat, Apr 19, 2008 at 02:18:38PM +0200, Daniel Mack wrote:
On Sat, Apr 19, 2008 at 12:24:26PM +0100, Mark Brown wrote:
The usual approach is to have multi-function pins on CODECs are controlled by the ASoC codec driver based on configuration supplied by the machine driver.
I was looking for a mechanism to pass such information around but didn't manage to find any. Which way would you suggest? The approach in my patch causes as less overhead as possible at it simply reuses the asoc's widgets, I though.
The usual thing would be to have your CODEC driver provide its own API which users can call directly.
Their use tends to be heavily constrained by the board design so it usually makes more sense to let the machine driver set things up and deal with exposing controls for any configurability that user space can exercise.
Ok, but why not let the codec driver expose all the functionality it can offer for setting up a certain GPIO and use this alsa control either from user space or from the asoc machine part? I think that's the most flexible way at all, no?
The CODEC driver can't safely give user space control of multi-function pins in general, including GPIOs, since their usage depends very strongly on how they have been connected to the system. At best much of the functionality won't be useful on a given board due to a lack of appropriate connections. At worst the consequences of misusing them can include things like causing one or more chips in the system to fall over, perhaps even the entire system. For example, if you've got two chips each trying to drive a signal to different voltages then it is possible that one or both of them will do something like attempt to consume too much power and cause supplies to collapse.
It's OK for the kernel to be able to misconfigure things and control of these features is very useful so it's good to provide it. The issue is letting user space get involved unless the machine driver has said that it's safe to do so.
And when it comes to setting or getting a GPIO's state I see no way to access such special purpose functions of the codec driver. At least, 'struct snd_soc_codec' doesn't give me any hints how to achive that.
Like I say, export a CODEC specific API for it. The machine driver already needs to know which CODEC and SoC it is working with and how they are connected in a given system in order to set up the audio connections. In some cases the API calls can be avoided - for example, if there is an I2S port on multi-function pins then configuring that I2S port could implicitly configure the multi-function pins to enable that port.
Most of the time when GPIO lines on CODECs are used they tend to be controlling other bits of the audio subsystem anyway (eg, power for simple amplifiers) - most SoCs have plenty of GPIOs on-board and they're usually much easier to work with due to being memory mapped on the CPU.
Well, I didn't implement that just because it wasn't done yet ;) In the setup I'm working on, one GPIO is needed to control peripherals next to it in the board layout where routing constraints become part of the game. And the other one is connected to one of the processor's GPIO to catch an interrupt from the AIC3x when a headset is plugged in. Thus, we really need to support them both. Others may have different use for them, just have a look at the possible functions of those GPIOs.
Those sound like exactly the sort of audio subsystem usages that I was talking about - things that have fixed functions on a given board and which the machine driver should be able to know how to set up in a way that makes sense for the system.
In the case of jack detect I'm actually currently working on a generalisation of that since it's such a common feature. I hope to have something in the next week or two but it depends on my other work.
This is interesting, what is the generic jack detect feature supposed to do exactly? I ask because I just export the jack detect interrupt to userspace via input subsystem (SW_HEADPHONE_INSERT). Will it replace this switch in functionality?
regards Philipp
On Mon, Apr 21, 2008 at 04:38:58PM +0200, pHilipp Zabel wrote:
This is interesting, what is the generic jack detect feature supposed to do exactly? I ask because I just export the jack detect interrupt to userspace via input subsystem (SW_HEADPHONE_INSERT). Will it replace this switch in functionality?
As far as user space is concerned I'd expect it to turn out as pretty much exactly that, wrapping it up so that sound drivers don't need to reproduce the code to register an input device and so on. Devices that can detect the type of inserted will need some additional switch types adding.
participants (4)
-
Daniel Mack -
Jarkko Nikula -
Mark Brown -
pHilipp Zabel