[alsa-devel] [PATCH 0/3] Updated names for ASoC OMAP HDMI drivers
Hi Mark, Liam,
Now that the OMAPDSS HDMI driver will create the platform device for HDMI audio [1], the ASoC drivers need to be updated as well. Also, I took the opportunity to give the drivers more meaningful and descriptive names.
The last patch in the series makes the CPU DAI driver look cleaner by obtaining its resources from the platform device and not from hard-coded value inside the driver.
Also, I still have pending a couple of patches to update the platform device creation. I wanted to submit these patches first in case there are comments about the naming I propose.
BR,
Ricardo
[1]. http://www.mail-archive.com/linux-omap@vger.kernel.org/msg79795.html
Ricardo Neri (3): ASoC: OMAP: HDMI: Update machine driver name ASoC: OMAP: HDMI: Update CPU DAI driver name ASoC: OMAP: HDMI: Obtain DMA port from resources
sound/soc/omap/omap-hdmi-card.c | 4 ++-- sound/soc/omap/omap-hdmi.c | 5 ++--- sound/soc/omap/omap-hdmi.h | 2 -- 3 files changed, 4 insertions(+), 7 deletions(-)
Being the name of a machine driver, it aims to describe the connection between the HDMI IP of the processor and the companion chip it uses to connect to the outside world. This name tries to follow the same naming convention as in the OMAP-ABE-TWL6040 machine driver.
TPD12S015 is an HDMI companion chip for DC-DC step-up, I2C level shifter and low-capacitance ESD protection. This chip is used on all OMAP4 Pandaboards an SDPs as well as in OMAP5 EVMs.
Signed-off-by: Ricardo Neri ricardo.neri@ti.com --- sound/soc/omap/omap-hdmi-card.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/omap/omap-hdmi-card.c b/sound/soc/omap/omap-hdmi-card.c index eaa2ea0..184180e 100644 --- a/sound/soc/omap/omap-hdmi-card.c +++ b/sound/soc/omap/omap-hdmi-card.c @@ -27,7 +27,7 @@ #include <asm/mach-types.h> #include <video/omapdss.h>
-#define DRV_NAME "omap-hdmi-audio" +#define DRV_NAME "omap-hdmi-tpd12s015-audio"
static struct snd_soc_dai_link omap_hdmi_dai = { .name = "HDMI",
On 14/11/12 02:30, Ricardo Neri wrote:
Being the name of a machine driver, it aims to describe the connection between the HDMI IP of the processor and the companion chip it uses to connect to the outside world. This name tries to follow the same naming convention as in the OMAP-ABE-TWL6040 machine driver.
TPD12S015 is an HDMI companion chip for DC-DC step-up, I2C level shifter and low-capacitance ESD protection. This chip is used on all OMAP4 Pandaboards an SDPs as well as in OMAP5 EVMs.
Is the TPD12S015 passive or actively controlled by the CPU in this case ?
If it's passive then it's not really worth changing the driver name since other OMAP4/5 devices may use other HDMI companion chips (an we can reuse this driver without changes).
Liam
Hi Liam,
Thanks for reviewing!
On 11/14/2012 04:16 AM, Liam Girdwood wrote:
On 14/11/12 02:30, Ricardo Neri wrote:
Being the name of a machine driver, it aims to describe the connection between the HDMI IP of the processor and the companion chip it uses to connect to the outside world. This name tries to follow the same naming convention as in the OMAP-ABE-TWL6040 machine driver.
TPD12S015 is an HDMI companion chip for DC-DC step-up, I2C level shifter and low-capacitance ESD protection. This chip is used on all OMAP4 Pandaboards an SDPs as well as in OMAP5 EVMs.
Is the TPD12S015 passive or actively controlled by the CPU in this case ?
Yes the CPU controls the TPD12S015 actively through two GPIOs. One GPIO is used to enable data transfer. The second GPIO is enable the TPD to detect cable connection. However, this abstracted by the OMAPDSS HDMI driver and the ASoC driver does not know about it.
If it's passive then it's not really worth changing the driver name since other OMAP4/5 devices may use other HDMI companion chips (an we can reuse this driver without changes).
I believe it would be good to change the name of the driver to describe better the machine. omap-hdmi-audio makes reference only to the processor and sounds good to reserve it for the CPU DAI driver. Maybe something more generic could be used if the ASoC driver will not control the companion chip. How about omap-hdmi-board-audio or omap-hdmi-machine-audio? omap-hdmi-[companion]-audio was the best-looking to me.
BR,
Ricardo
Liam
Update the driver name to use the platform device created by the OMAPDSS HDMI driver for interested audio drivers to use.
This platform device represents the audio functionality of the HDMI IP in the applicable OMAP processors. It does not make explicit reference to ASoC concepts such as DAI.
Signed-off-by: Ricardo Neri ricardo.neri@ti.com --- sound/soc/omap/omap-hdmi-card.c | 2 +- sound/soc/omap/omap-hdmi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/omap/omap-hdmi-card.c b/sound/soc/omap/omap-hdmi-card.c index 184180e..d6f4dba 100644 --- a/sound/soc/omap/omap-hdmi-card.c +++ b/sound/soc/omap/omap-hdmi-card.c @@ -32,7 +32,7 @@ static struct snd_soc_dai_link omap_hdmi_dai = { .name = "HDMI", .stream_name = "HDMI", - .cpu_dai_name = "omap-hdmi-audio-dai", + .cpu_dai_name = "omap_hdmi_audio", .platform_name = "omap-pcm-audio", .codec_name = "hdmi-audio-codec", .codec_dai_name = "omap-hdmi-hifi", diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c index f59c69f..6592d6f 100644 --- a/sound/soc/omap/omap-hdmi.c +++ b/sound/soc/omap/omap-hdmi.c @@ -37,7 +37,7 @@ #include "omap-pcm.h" #include "omap-hdmi.h"
-#define DRV_NAME "omap-hdmi-audio-dai" +#define DRV_NAME "omap_hdmi_audio"
struct hdmi_priv { struct omap_pcm_dma_data dma_params;
Instead of defining the address offset of the DMA port for transfers of audio samples, obtain this information from the resources of the device. This device and its resources are created by the OMAPDSS HDMI driver.
Signed-off-by: Ricardo Neri ricardo.neri@ti.com --- sound/soc/omap/omap-hdmi.c | 3 +-- sound/soc/omap/omap-hdmi.h | 2 -- 2 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/sound/soc/omap/omap-hdmi.c b/sound/soc/omap/omap-hdmi.c index 6592d6f..306fbfb 100644 --- a/sound/soc/omap/omap-hdmi.c +++ b/sound/soc/omap/omap-hdmi.c @@ -281,8 +281,7 @@ static __devinit int omap_hdmi_probe(struct platform_device *pdev) return -ENODEV; }
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start - + OMAP_HDMI_AUDIO_DMA_PORT; + hdmi_data->dma_params.port_addr = hdmi_rsrc->start;
hdmi_rsrc = platform_get_resource(pdev, IORESOURCE_DMA, 0); if (!hdmi_rsrc) { diff --git a/sound/soc/omap/omap-hdmi.h b/sound/soc/omap/omap-hdmi.h index 6ad2bf4..33d7a93 100644 --- a/sound/soc/omap/omap-hdmi.h +++ b/sound/soc/omap/omap-hdmi.h @@ -25,8 +25,6 @@ #ifndef __OMAP_HDMI_H__ #define __OMAP_HDMI_H__
-#define OMAP_HDMI_AUDIO_DMA_PORT 0x8c - #define OMAP_HDMI_RATES (SNDRV_PCM_RATE_32000 | \ SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 | \ SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 | \
On Tue, Nov 13, 2012 at 08:30:49PM -0600, Ricardo Neri wrote:
Instead of defining the address offset of the DMA port for transfers of audio samples, obtain this information from the resources of the device. This device and its resources are created by the OMAPDSS HDMI driver.
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start
+ OMAP_HDMI_AUDIO_DMA_PORT;
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start;
...both before and after versions use the resource.
Hi Mark,
Thanks for reviewing!
On 11/13/2012 09:27 PM, Mark Brown wrote:
On Tue, Nov 13, 2012 at 08:30:49PM -0600, Ricardo Neri wrote:
Instead of defining the address offset of the DMA port for transfers of audio samples, obtain this information from the resources of the device. This device and its resources are created by the OMAPDSS HDMI driver.
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start
+ OMAP_HDMI_AUDIO_DMA_PORT;
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start;
...both before and after versions use the resource.
This change should be present in K3.8.
BR,
Ricardo
[1].http://www.mail-archive.com/linux-omap@vger.kernel.org/msg79801.html
On Wed, Nov 14, 2012 at 11:07:09AM -0600, Ricardo Neri wrote:
On 11/13/2012 09:27 PM, Mark Brown wrote:
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the parts of the changes together. Otherwise there's a risk that one of them won't get merged and even if they do both get merged you'll break bisection - it's clear that nobody can be testing this on Tomi's branch.
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start
+ OMAP_HDMI_AUDIO_DMA_PORT;
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start;
...both before and after versions use the resource.
This change should be present in K3.8.
What is "K3.8"?
Hi Mark,
On 11/14/2012 05:08 PM, Mark Brown wrote:
On Wed, Nov 14, 2012 at 11:07:09AM -0600, Ricardo Neri wrote:
On 11/13/2012 09:27 PM, Mark Brown wrote:
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the parts of the changes together. Otherwise there's a risk that one of them won't get merged and even if they do both get merged you'll break bisection - it's clear that nobody can be testing this on Tomi's branch.
but the changes will hit linux-next at some point and they could be tested there, right?
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start
+ OMAP_HDMI_AUDIO_DMA_PORT;
- hdmi_data->dma_params.port_addr = hdmi_rsrc->start;
...both before and after versions use the resource.
This change should be present in K3.8.
What is "K3.8"?
I meant that Tomi took the DSS changes in [1] on his master branch and he should push them upstream when the merge window for kernel v3.8 opens.
BR,
Ricardo
[1].http://www.mail-archive.com/linux-omap@vger.kernel.org/msg79795.html
On Wed, Nov 14, 2012 at 08:33:11PM -0600, Ricardo Neri wrote:
On 11/14/2012 05:08 PM, Mark Brown wrote:
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the
but the changes will hit linux-next at some point and they could be tested there, right?
This isn't much fun for anyone doing git bisect, it means the device will be unusuable for large periods of history.
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
You need to make sure everyone involved knows what's going on - review should really turn up the sort of issue I just highlighted.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
Yes, we're going to need to do that. Ideally all the changes would be squashed into a single commit.
This change should be present in K3.8.
What is "K3.8"?
I meant that Tomi took the DSS changes in [1] on his master branch and he should push them upstream when the merge window for kernel v3.8 opens.
Do you mean that "K3.8" is "v3.8"?
Hi Mark,
On 11/14/2012 10:10 PM, Mark Brown wrote:
On Wed, Nov 14, 2012 at 08:33:11PM -0600, Ricardo Neri wrote:
On 11/14/2012 05:08 PM, Mark Brown wrote:
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the
but the changes will hit linux-next at some point and they could be tested there, right?
This isn't much fun for anyone doing git bisect, it means the device will be unusuable for large periods of history.
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
You need to make sure everyone involved knows what's going on - review should really turn up the sort of issue I just highlighted.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
Yes, we're going to need to do that. Ideally all the changes would be squashed into a single commit.
OK. Tomi has offered to revert the patch that create the platform device. I will resubmit a new series with all the required changes squashing all the platform devices into a single one.
This change should be present in K3.8.
What is "K3.8"?
I meant that Tomi took the DSS changes in [1] on his master branch and he should push them upstream when the merge window for kernel v3.8 opens.
Do you mean that "K3.8" is "v3.8"?
Yes, that is what I mean.
BR,
Ricardo
On 2012-11-15 04:33, Ricardo Neri wrote:
Hi Mark,
On 11/14/2012 05:08 PM, Mark Brown wrote:
On Wed, Nov 14, 2012 at 11:07:09AM -0600, Ricardo Neri wrote:
On 11/13/2012 09:27 PM, Mark Brown wrote:
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the parts of the changes together. Otherwise there's a risk that one of them won't get merged and even if they do both get merged you'll break bisection - it's clear that nobody can be testing this on Tomi's branch.
but the changes will hit linux-next at some point and they could be tested there, right?
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
Hmm, ok, I'm a bit confused here.
I though the omap_hdmi_audio device was a new thing, and thus it was ok to add to omapdss. But now I see omap_hdmi_audio is already registered in arch/arm/mach-omap2/devices.c, meaning the same platform device is registered twice now. Well, with the exception of the device id, which is -1 for the one from devices.c, and 0 for the one in omapdss.
Mark is right, this is not right. The kernel should work fine after each patch, which means that sometimes a patch will touch multiple different areas of kernel.
omapdss master branch is a stable branch, so I don't want to rebase it. But I guess I can "reset" it by merging a mainline using some merge strategy. I haven't done that before, but I guess it'll work.
Can you look at all the HDMI patches related to this hdmi-device change, and rewrite them so that they'll keep the kernel working after each patch?
Tomi
On 2012-11-15 11:45, Tomi Valkeinen wrote:
On 2012-11-15 04:33, Ricardo Neri wrote:
Hi Mark,
On 11/14/2012 05:08 PM, Mark Brown wrote:
On Wed, Nov 14, 2012 at 11:07:09AM -0600, Ricardo Neri wrote:
On 11/13/2012 09:27 PM, Mark Brown wrote:
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the parts of the changes together. Otherwise there's a risk that one of them won't get merged and even if they do both get merged you'll break bisection - it's clear that nobody can be testing this on Tomi's branch.
but the changes will hit linux-next at some point and they could be tested there, right?
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
Hmm, ok, I'm a bit confused here.
I though the omap_hdmi_audio device was a new thing, and thus it was ok to add to omapdss. But now I see omap_hdmi_audio is already registered in arch/arm/mach-omap2/devices.c, meaning the same platform device is registered twice now. Well, with the exception of the device id, which is -1 for the one from devices.c, and 0 for the one in omapdss.
Mark is right, this is not right. The kernel should work fine after each patch, which means that sometimes a patch will touch multiple different areas of kernel.
omapdss master branch is a stable branch, so I don't want to rebase it. But I guess I can "reset" it by merging a mainline using some merge strategy. I haven't done that before, but I guess it'll work.
Can you look at all the HDMI patches related to this hdmi-device change, and rewrite them so that they'll keep the kernel working after each patch?
After some testing I think resetting my master branch with merge is not a very good option.
However, the HDMI audio platform device patch is almost on top of the branch, so reverting it would only create a few steps where the HDMI audio may be broken.
So I can revert the patch and apply a new series with the patches organized so that things will work.
Tomi
Hi Tomi,
On 11/15/2012 06:36 AM, Tomi Valkeinen wrote:
On 2012-11-15 11:45, Tomi Valkeinen wrote:
On 2012-11-15 04:33, Ricardo Neri wrote:
Hi Mark,
On 11/14/2012 05:08 PM, Mark Brown wrote:
On Wed, Nov 14, 2012 at 11:07:09AM -0600, Ricardo Neri wrote:
On 11/13/2012 09:27 PM, Mark Brown wrote:
Presumably this needs some other corresponding change in the resource setup to go in simultaneously...
Yes, the change in the resources setup has been submitted to the OMAPDSS HDMI driver and accepted by Tomi [1].
Don't do this. With a change like this which must be made at the same time over multiple subsystems it is very important that you send all the parts of the changes together. Otherwise there's a risk that one of them won't get merged and even if they do both get merged you'll break bisection - it's clear that nobody can be testing this on Tomi's branch.
but the changes will hit linux-next at some point and they could be tested there, right?
Sorry, as changes for the HDMI drivers go to several maintainers, I was trying to send the relevant patches to the respective maintainers and avoid potential merge/rebase issues.
Tomi has already taken the DSS changes. Perhaps, if you and Tomi agree, Tomi could also take the ASoC and the arch/arm/mach-omap2 changes. This way all changes come from the same tree.
Hmm, ok, I'm a bit confused here.
I though the omap_hdmi_audio device was a new thing, and thus it was ok to add to omapdss. But now I see omap_hdmi_audio is already registered in arch/arm/mach-omap2/devices.c, meaning the same platform device is registered twice now. Well, with the exception of the device id, which is -1 for the one from devices.c, and 0 for the one in omapdss.
Mark is right, this is not right. The kernel should work fine after each patch, which means that sometimes a patch will touch multiple different areas of kernel.
omapdss master branch is a stable branch, so I don't want to rebase it. But I guess I can "reset" it by merging a mainline using some merge strategy. I haven't done that before, but I guess it'll work.
Can you look at all the HDMI patches related to this hdmi-device change, and rewrite them so that they'll keep the kernel working after each patch?
After some testing I think resetting my master branch with merge is not a very good option.
However, the HDMI audio platform device patch is almost on top of the branch, so reverting it would only create a few steps where the HDMI audio may be broken.
So I can revert the patch and apply a new series with the patches organized so that things will work.
Thanks! I just resubmitted a new series to complement the creation of the audio device for HDMI with code to remove it from arch/arm/mach-omap2/devices.c. HDMI audio works correctly in every patch of that series.
Thanks Mark and Tomi for pointing out these issues and I apologize for the inconvenience.
BR,
Ricardo
Tomi
participants (4)
-
Liam Girdwood -
Mark Brown -
Ricardo Neri -
Tomi Valkeinen