| Message ID | 1352860249-26348-4-git-send-email-ricardo.neri@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.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 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.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"?
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 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 > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 | \
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(-)