ASoC: amd: fix report accurate hw_ptr during dma
diff mbox

Message ID 20171108181841.29332-1-alexander.deucher@amd.com
State New
Headers show

Commit Message

Alex Deucher Nov. 8, 2017, 6:18 p.m. UTC
From: Guenter Roeck <groeck@chromium.org>

ERROR: "__aeabi_uldivmod" [sound/soc/amd/snd-soc-acp-pcm.ko] undefined!

64-bit divides require special operations to avoid build errors on 32-bit
systems.

fixes: 61add8147942 (ASoC: amd: Report accurate hw_ptr during dma)
Signed-off-by: Guenter Roeck <groeck@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/678919
Reviewed-by: Jason Clinton <jclinton@chromium.org>
(cherry picked from commit 7ca726e80f21abdbaed9a5a70def1c33a26f8533)
Reviewed-on: https://chromium-review.googlesource.com/681618
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
---

v2: resend with the Chromium tags removed and the subject updated.

 sound/soc/amd/acp-pcm-dma.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Mark Brown Nov. 8, 2017, 6:22 p.m. UTC | #1
On Wed, Nov 08, 2017 at 01:18:41PM -0500, Alex Deucher wrote:
> From: Guenter Roeck <groeck@chromium.org>
> 
> ERROR: "__aeabi_uldivmod" [sound/soc/amd/snd-soc-acp-pcm.ko] undefined!
> 
> 64-bit divides require special operations to avoid build errors on 32-bit
> systems.

Like I said in reply to your other mail please don't resubmit already
applied patches.  The current tip of my topic/amd branch appears to be
this very patch, if there's anything needs changing please send an
incremental patch.
Alex Deucher Nov. 8, 2017, 6:40 p.m. UTC | #2
On Wed, Nov 8, 2017 at 1:22 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Nov 08, 2017 at 01:18:41PM -0500, Alex Deucher wrote:
>> From: Guenter Roeck <groeck@chromium.org>
>>
>> ERROR: "__aeabi_uldivmod" [sound/soc/amd/snd-soc-acp-pcm.ko] undefined!
>>
>> 64-bit divides require special operations to avoid build errors on 32-bit
>> systems.
>
> Like I said in reply to your other mail please don't resubmit already
> applied patches.  The current tip of my topic/amd branch appears to be
> this very patch, if there's anything needs changing please send an
> incremental patch.

I'm not seeing this one in your tree either.  This is just a resend of
Guenter's patch from an hour ago with the chromium stuff removed.
Maybe you already applied it in the interim?

Alex
Mark Brown Nov. 8, 2017, 6:47 p.m. UTC | #3
On Wed, Nov 08, 2017 at 01:40:32PM -0500, Alex Deucher wrote:
> On Wed, Nov 8, 2017 at 1:22 PM, Mark Brown <broonie@kernel.org> wrote:

> > Like I said in reply to your other mail please don't resubmit already
> > applied patches.  The current tip of my topic/amd branch appears to be
> > this very patch, if there's anything needs changing please send an
> > incremental patch.

> I'm not seeing this one in your tree either.  This is just a resend of
> Guenter's patch from an hour ago with the chromium stuff removed.
> Maybe you already applied it in the interim?

Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
which was applied at 16:07?
Deucher, Alexander Nov. 8, 2017, 7:18 p.m. UTC | #4
> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Wednesday, November 08, 2017 1:48 PM
> To: Alex Deucher
> Cc: amd-gfx list; alsa-devel@alsa-project.org; Maling list - DRI developers;
> Mukunda, Vijendar; Liam Girdwood; Takashi Iwai; Guenter Roeck; Deucher,
> Alexander
> Subject: Re: [PATCH] ASoC: amd: fix report accurate hw_ptr during dma
> 
> On Wed, Nov 08, 2017 at 01:40:32PM -0500, Alex Deucher wrote:
> > On Wed, Nov 8, 2017 at 1:22 PM, Mark Brown <broonie@kernel.org>
> wrote:
> 
> > > Like I said in reply to your other mail please don't resubmit already
> > > applied patches.  The current tip of my topic/amd branch appears to be
> > > this very patch, if there's anything needs changing please send an
> > > incremental patch.
> 
> > I'm not seeing this one in your tree either.  This is just a resend of
> > Guenter's patch from an hour ago with the chromium stuff removed.
> > Maybe you already applied it in the interim?
> 
> Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
> which was applied at 16:07?

Yes, this is a fix for that patch.  It fixes a 64 bit division that wasn't properly handled.

Alex
Alex Deucher Nov. 8, 2017, 7:35 p.m. UTC | #5
On Wed, Nov 8, 2017 at 2:30 PM, Guenter Roeck <groeck@google.com> wrote:
> On Wed, Nov 8, 2017 at 11:18 AM, Deucher, Alexander
> <Alexander.Deucher@amd.com> wrote:
>>
>> > -----Original Message-----
>> > From: Mark Brown [mailto:broonie@kernel.org]
>> > Sent: Wednesday, November 08, 2017 1:48 PM
>> > To: Alex Deucher
>> > Cc: amd-gfx list; alsa-devel@alsa-project.org; Maling list - DRI
>> > developers;
>> > Mukunda, Vijendar; Liam Girdwood; Takashi Iwai; Guenter Roeck; Deucher,
>> > Alexander
>> > Subject: Re: [PATCH] ASoC: amd: fix report accurate hw_ptr during dma
>> >
>> > On Wed, Nov 08, 2017 at 01:40:32PM -0500, Alex Deucher wrote:
>> > > On Wed, Nov 8, 2017 at 1:22 PM, Mark Brown <broonie@kernel.org>
>> > wrote:
>> >
>> > > > Like I said in reply to your other mail please don't resubmit
>> > > > already
>> > > > applied patches.  The current tip of my topic/amd branch appears to
>> > > > be
>> > > > this very patch, if there's anything needs changing please send an
>> > > > incremental patch.
>> >
>> > > I'm not seeing this one in your tree either.  This is just a resend of
>> > > Guenter's patch from an hour ago with the chromium stuff removed.
>> > > Maybe you already applied it in the interim?
>> >
>> > Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
>> > which was applied at 16:07?
>>
>> Yes, this is a fix for that patch.  It fixes a 64 bit division that wasn't
>> properly handled.
>>
>
> In that case, the subject should reflect the problem fixed, the description
> should describe the problem, and there should be a Fixes: tag pointing to
> the problematic patch.

I updated the patch subject and added a fixes tag when I resent it
after removing the chrome tags.  The subject could have been bit
better, but I wasn't sure how far to take it since it wasn't my patch
originally.

Alex

>
> Sorry, I was not aware that the problematic patch is already pending
> upstream, or I would have submitted a proper patch upstream myself.
>
> Guenter
>
>>
>> Alex
>>
>
Mark Brown Nov. 8, 2017, 7:49 p.m. UTC | #6
On Wed, Nov 08, 2017 at 11:13:50AM -0800, Guenter Roeck wrote:
> On Wed, Nov 8, 2017 at 10:47 AM, Mark Brown <broonie@kernel.org> wrote:

> > Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
> > which was applied at 16:07?

> I suspect we may be getting hit by chromeos-isms again. In Chrome OS, we
> don't use Fixes: tags but "FIXUP: <original description>" in the subject
> (not that I like it, but it is what it is).

So it probably is the same patch that was just resent with a slightly
modified subject line?  That's what it looked like initially.
Alex Deucher Nov. 8, 2017, 8:01 p.m. UTC | #7
On Wed, Nov 8, 2017 at 2:49 PM, Mark Brown <broonie@kernel.org> wrote:
> On Wed, Nov 08, 2017 at 11:13:50AM -0800, Guenter Roeck wrote:
>> On Wed, Nov 8, 2017 at 10:47 AM, Mark Brown <broonie@kernel.org> wrote:
>
>> > Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
>> > which was applied at 16:07?
>
>> I suspect we may be getting hit by chromeos-isms again. In Chrome OS, we
>> don't use Fixes: tags but "FIXUP: <original description>" in the subject
>> (not that I like it, but it is what it is).
>
> So it probably is the same patch that was just resent with a slightly
> modified subject line?  That's what it looked like initially.

The original patch which was applied to your tree already:
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git/commit/?h=for-next&id=61add8147942d23519b91f0edc30980d7c14482c
That patch as a 64 bit divide in it which was fixed by this patch from Guenter:
https://lists.freedesktop.org/archives/dri-devel/2017-November/157063.html
which contained a bunch of Chrome tags.  I removed the tags and
updated the subject and added a fixes line and resent it as this patch
(subject of this thread):
https://lists.freedesktop.org/archives/dri-devel/2017-November/157076.html
Please apply.  Sorry for the confusion.

Alex
Mark Brown Nov. 8, 2017, 8:47 p.m. UTC | #8
On Wed, Nov 08, 2017 at 02:35:05PM -0500, Alex Deucher wrote:
> On Wed, Nov 8, 2017 at 2:30 PM, Guenter Roeck <groeck@google.com> wrote:
> > On Wed, Nov 8, 2017 at 11:18 AM, Deucher, Alexander

> >> > Is this different to "ASoC: amd: Report accurate hw_ptr during dma"
> >> > which was applied at 16:07?

> >> Yes, this is a fix for that patch.  It fixes a 64 bit division that wasn't
> >> properly handled.

Ugh.  Two problems here.  One is obviously that there's a patch that was
sent out only a couple of days ago with a whole bunch of tags about
internal testing and review and now you've found issues with it.  That's
worrying.

The other is that you really, really need to get more on top of upstream
process.  There have been constant problems with this driver and they
feel like they're getting worse not better, with the original send of
this the noise in the subject lines and body of the e-mail means that
what you're sending looks like a case of git send-email CCing people in
accidentally rather than a deliberate upstream submission.

The constant problems mean that even as I open the e-mail I'm expecting
to have to push back on something which isn't great and affects the way
I review things.

> > In that case, the subject should reflect the problem fixed, the description
> > should describe the problem, and there should be a Fixes: tag pointing to
> > the problematic patch.

> I updated the patch subject and added a fixes tag when I resent it
> after removing the chrome tags.  The subject could have been bit
> better, but I wasn't sure how far to take it since it wasn't my patch
> originally.

If you're concerned about rewriting commit logs just add a note saying
that you did so in the commit log.  Please resend normally.

Patch
diff mbox

diff --git a/sound/soc/amd/acp-pcm-dma.c b/sound/soc/amd/acp-pcm-dma.c
index 13d040a4d26f..ef7e98ad960c 100644
--- a/sound/soc/amd/acp-pcm-dma.c
+++ b/sound/soc/amd/acp-pcm-dma.c
@@ -856,12 +856,11 @@  static snd_pcm_uframes_t acp_dma_pointer(struct snd_pcm_substream *substream)
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		if (bytescount > rtd->renderbytescount)
 			bytescount = bytescount - rtd->renderbytescount;
-		pos =  bytescount % buffersize;
 	} else {
 		if (bytescount > rtd->capturebytescount)
 			bytescount = bytescount - rtd->capturebytescount;
-		pos = bytescount % buffersize;
 	}
+	pos = do_div(bytescount, buffersize);
 	return bytes_to_frames(runtime, pos);
 }