| Message ID | 4CDBC26E.7010103@canonical.com |
|---|---|
| State | Rejected |
| Delegated to: | Brad Figg |
| Headers | show |
On Thu, 2010-11-11 at 11:16 +0100, David Henningsson wrote: > BugLink: https://launchpad.net/bugs/617647 > > SRU Justification: > > Impact: A regression in Maverick caused audio playback to stop working > on Acer aspire 7736. > > Fix: Require some new low-impact quirk infrastructure plus the actual quirk. > > Testcase: Try playing back through internal speakers and headphones on > the machine. > > Both patches has been accepted upstream (as commits 90622917 and > c3d226ab) but it does not make sense to send to stable@kernel.org since > 2.6.35 isn't maintained anymore. Looks reasonable and there is positive test confirmation in the bug report. Only minor nit picks would be that I'd also add the BugLink to Patch 1/2 and I'd also add references to the upstream commits in both patches. But I'll leave it to Brad/Steve to decide if they want to fix that up by hand when applying. Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>
On 11/11/2010 07:19 PM, Leann Ogasawara wrote: > On Thu, 2010-11-11 at 11:16 +0100, David Henningsson wrote: >> BugLink: https://launchpad.net/bugs/617647 >> >> SRU Justification: >> >> Impact: A regression in Maverick caused audio playback to stop working >> on Acer aspire 7736. >> >> Fix: Require some new low-impact quirk infrastructure plus the actual quirk. >> >> Testcase: Try playing back through internal speakers and headphones on >> the machine. >> >> Both patches has been accepted upstream (as commits 90622917 and >> c3d226ab) but it does not make sense to send to stable@kernel.org since >> 2.6.35 isn't maintained anymore. > > Looks reasonable and there is positive test confirmation in the bug > report. Only minor nit picks would be that I'd also add the BugLink to > Patch 1/2 and I'd also add references to the upstream commits in both > patches. But I'll leave it to Brad/Steve to decide if they want to fix > that up by hand when applying. > > Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > > As Leann said, the BugLink and upstream references. Also another nitpick, just because .35 is your target and is not supported anymore, it may be nice as a courtesy to send them there for .36. ;-) Acked-by: Stefan Bader <stefan.bader@canonical.com>
On 2010-11-11 19:19, Leann Ogasawara wrote: > On Thu, 2010-11-11 at 11:16 +0100, David Henningsson wrote: >> BugLink: https://launchpad.net/bugs/617647 >> >> SRU Justification: >> >> Impact: A regression in Maverick caused audio playback to stop working >> on Acer aspire 7736. >> >> Fix: Require some new low-impact quirk infrastructure plus the actual quirk. >> >> Testcase: Try playing back through internal speakers and headphones on >> the machine. >> >> Both patches has been accepted upstream (as commits 90622917 and >> c3d226ab) but it does not make sense to send to stable@kernel.org since >> 2.6.35 isn't maintained anymore. > > Looks reasonable and there is positive test confirmation in the bug > report. Only minor nit picks would be that I'd also add the BugLink to > Patch 1/2 and I'd also add references to the upstream commits in both > patches. But I'll leave it to Brad/Steve to decide if they want to fix > that up by hand when applying. > > Acked-by: Leann Ogasawara<leann.ogasawara@canonical.com> > How about then I'll instead ask you to cherrypick commits 90622917 and c3d226ab from linux-2.6? That way you'll get upstream references automatically, I assume. As for the buglink, I don't agree - you have the buglink in the second one only (both in c3d226ab and in the patch posted), which should be correct. That way you don't risk the bug appear to be fixed by only applying the first one by mistake. (And for the sake of argument, the second one won't even apply, or at least not build, without the first one, so if you do the mistake the other way, you'll find it.)
On 11/12/2010 10:12 AM, David Henningsson wrote: > On 2010-11-11 19:19, Leann Ogasawara wrote: >> On Thu, 2010-11-11 at 11:16 +0100, David Henningsson wrote: >>> BugLink: https://launchpad.net/bugs/617647 >>> >>> SRU Justification: >>> >>> Impact: A regression in Maverick caused audio playback to stop working >>> on Acer aspire 7736. >>> >>> Fix: Require some new low-impact quirk infrastructure plus the actual quirk. >>> >>> Testcase: Try playing back through internal speakers and headphones on >>> the machine. >>> >>> Both patches has been accepted upstream (as commits 90622917 and >>> c3d226ab) but it does not make sense to send to stable@kernel.org since >>> 2.6.35 isn't maintained anymore. >> >> Looks reasonable and there is positive test confirmation in the bug >> report. Only minor nit picks would be that I'd also add the BugLink to >> Patch 1/2 and I'd also add references to the upstream commits in both >> patches. But I'll leave it to Brad/Steve to decide if they want to fix >> that up by hand when applying. >> >> Acked-by: Leann Ogasawara<leann.ogasawara@canonical.com> >> > > How about then I'll instead ask you to cherrypick commits 90622917 and > c3d226ab from linux-2.6? That way you'll get upstream references > automatically, I assume. > But still would then miss your s-o-b for the sru (to document who submitted it for that). It is quite similar to what you would do for submitting stable patches to Greg. He also wants a "commit <sha1> upstream" added to the patch. For our srus you alternatively could extend the s-o-b area by (cherry-picked from <sha1> upstream) Signed-of-by: <you> > As for the buglink, I don't agree - you have the buglink in the second > one only (both in c3d226ab and in the patch posted), which should be > correct. That way you don't risk the bug appear to be fixed by only > applying the first one by mistake. (And for the sake of argument, the > second one won't even apply, or at least not build, without the first > one, so if you do the mistake the other way, you'll find it.) > No it isn't. All patches added for SRU need to reference the bug for which they are needed. Duplicates are allowed and ok. But the SRU team will usually reject updates when they contain patches without reference. And they are not keen on reading patches to find out about any dependencies. -Stefan
On 11/11/2010 03:16 AM, David Henningsson wrote: > BugLink: https://launchpad.net/bugs/617647 > > SRU Justification: > > Impact: A regression in Maverick caused audio playback to stop working > on Acer aspire 7736. > > Fix: Require some new low-impact quirk infrastructure plus the actual > quirk. > > Testcase: Try playing back through internal speakers and headphones on > the machine. > > Both patches has been accepted upstream (as commits 90622917 and > c3d226ab) but it does not make sense to send to stable@kernel.org since > 2.6.35 isn't maintained anymore. > pulled and pushed to master-next with minor commit log edits.
From cebe004745a54b5dec1cdfa2f8ce9708c301941a Mon Sep 17 00:00:00 2001 From: David Henningsson <david.henningsson@canonical.com> Date: Thu, 14 Oct 2010 15:42:08 +0200 Subject: [PATCH 2/2] ALSA: HDA: Apply SKU override for Acer aspire 7736z BugLink: http://launchpad.net/bugs/617647 The current SKU value disables playback, so ignore the SKU value. Signed-off-by: David Henningsson <david.henningsson@canonical.com> --- sound/pci/hda/patch_realtek.c | 5 +++++ 1 files changed, 5 insertions(+), 0 deletions(-) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index f826bad..1c82c59 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -10627,6 +10627,7 @@ static struct alc_config_preset alc882_presets[] = { enum { PINFIX_ABIT_AW9D_MAX, PINFIX_PB_M5210, + PINFIX_ACER_ASPIRE_7736, }; static const struct alc_fixup alc882_fixups[] = { @@ -10644,11 +10645,15 @@ static const struct alc_fixup alc882_fixups[] = { {} } }, + [PINFIX_ACER_ASPIRE_7736] = { + .sku = ALC_FIXUP_SKU_IGNORE, + }, }; static struct snd_pci_quirk alc882_fixup_tbl[] = { SND_PCI_QUIRK(0x1025, 0x0155, "Packard-Bell M5120", PINFIX_PB_M5210), SND_PCI_QUIRK(0x147b, 0x107a, "Abit AW9D-MAX", PINFIX_ABIT_AW9D_MAX), + SND_PCI_QUIRK(0x1025, 0x0296, "Acer Aspire 7736z", PINFIX_ACER_ASPIRE_7736), {} }; -- 1.7.1