2014-02-12 15:32:48

by Takashi Iwai

[permalink] [raw]
Subject: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

The mute LED states have to be restored after resume.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70351
Cc: <[email protected]> [v3.13+]
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/platform/x86/thinkpad_acpi.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index defb6afc1409..e2a91c845ac9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -8447,9 +8447,21 @@ static void mute_led_exit(void)
tpacpi_led_set(i, false);
}

+static void mute_led_resume(void)
+{
+ int i;
+
+ for (i = 0; i < TPACPI_LED_MAX; i++) {
+ struct tp_led_table *t = &led_tables[i];
+ if (t->state >= 0)
+ mute_led_on_off(t, t->state);
+ }
+}
+
static struct ibm_struct mute_led_driver_data = {
.name = "mute_led",
.exit = mute_led_exit,
+ .resume = mute_led_resume,
};

/****************************************************************************
--
1.8.5.2


Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

On Wed, Feb 12, 2014, at 13:32, Takashi Iwai wrote:
> The mute LED states have to be restored after resume.
>
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=70351
> Cc: <[email protected]> [v3.13+]
> Signed-off-by: Takashi Iwai <[email protected]>

Acked-by: Henrique de Moraes Holschuh <[email protected]>

--
"One disk to rule them all, One disk to find them. One disk to bring
them all and in the darkness grind them. In the Land of Redmond
where the shadows lie." -- The Silicon Valley Tarot
Henrique Holschuh

2014-02-12 20:36:51

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

On Wed, 2014-02-12 at 16:32 +0100, Takashi Iwai wrote:
> The mute LED states have to be restored after resume.

Hm. Is this something that should be done in the class code rather than
in individual drivers?

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-12 20:37:59

by Matthew Garrett

[permalink] [raw]
Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

On Wed, 2014-02-12 at 16:32 +0100, Takashi Iwai wrote:
> The mute LED states have to be restored after resume.

Oh, never mind, we're not doing this through the LED class, are we?

--
Matthew Garrett <[email protected]>
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2014-02-13 08:35:32

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

At Wed, 12 Feb 2014 20:37:36 +0000,
Matthew Garrett wrote:
>
> On Wed, 2014-02-12 at 16:32 +0100, Takashi Iwai wrote:
> > The mute LED states have to be restored after resume.
>
> Oh, never mind, we're not doing this through the LED class, are we?

Good point. It seems that leds_class has pm_ops but it's enabled only
when a flag is set.

I added the original bug reporter to Cc. (Don't know the name,
Bugzilla shows only the email address.)
Could you check the patch below works instead of the previous fix?


Takashi

---
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index defb6afc1409..16edf75d9c00 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4796,6 +4796,7 @@ static struct tpacpi_led_classdev tpacpi_led_thinklight = {
.name = "tpacpi::thinklight",
.brightness_set = &light_sysfs_set,
.brightness_get = &light_sysfs_get,
+ .flags = LED_CORE_SUSPENDRESUME,
}
};

2014-02-13 18:03:07

by Thomas Mann

[permalink] [raw]
Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

Hi,

this patch doesn't work. The sound is now muted after the resume (light is on).
But the userspace knows nothing about it. (kmix shows the volume as umuted)

Peter


On Thu, 13 Feb 2014 09:35:30 +0100
Takashi Iwai <[email protected]> wrote:

> At Wed, 12 Feb 2014 20:37:36 +0000,
> Matthew Garrett wrote:
> >
> > On Wed, 2014-02-12 at 16:32 +0100, Takashi Iwai wrote:
> > > The mute LED states have to be restored after resume.
> >
> > Oh, never mind, we're not doing this through the LED class, are we?
>
> Good point. It seems that leds_class has pm_ops but it's enabled only
> when a flag is set.
>
> I added the original bug reporter to Cc. (Don't know the name,
> Bugzilla shows only the email address.)
> Could you check the patch below works instead of the previous fix?
>
>
> Takashi
>
> ---
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index defb6afc1409..16edf75d9c00 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -4796,6 +4796,7 @@ static struct tpacpi_led_classdev tpacpi_led_thinklight = {
> .name = "tpacpi::thinklight",
> .brightness_set = &light_sysfs_set,
> .brightness_get = &light_sysfs_get,
> + .flags = LED_CORE_SUSPENDRESUME,
> }
> };
>

2014-02-14 06:37:46

by Takashi Iwai

[permalink] [raw]
Subject: Re: [PATCH] thinkpad_acpi: Fix inconsistent mute LED after resume

At Thu, 13 Feb 2014 19:02:46 +0100,
[email protected] wrote:
>
> Hi,
>
> this patch doesn't work. The sound is now muted after the resume (light is on).
> But the userspace knows nothing about it. (kmix shows the volume as umuted)

OK, scratch the last patch. I thought the mute LED is also a part of
thinkpad_acpi's leds_class, but it's not. So, my initial patch is a
correct fix in the end.

Matthew, could you take it please?


thanks,

Takashi

>
> Peter
>
>
> On Thu, 13 Feb 2014 09:35:30 +0100
> Takashi Iwai <[email protected]> wrote:
>
> > At Wed, 12 Feb 2014 20:37:36 +0000,
> > Matthew Garrett wrote:
> > >
> > > On Wed, 2014-02-12 at 16:32 +0100, Takashi Iwai wrote:
> > > > The mute LED states have to be restored after resume.
> > >
> > > Oh, never mind, we're not doing this through the LED class, are we?
> >
> > Good point. It seems that leds_class has pm_ops but it's enabled only
> > when a flag is set.
> >
> > I added the original bug reporter to Cc. (Don't know the name,
> > Bugzilla shows only the email address.)
> > Could you check the patch below works instead of the previous fix?
> >
> >
> > Takashi
> >
> > ---
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> > index defb6afc1409..16edf75d9c00 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -4796,6 +4796,7 @@ static struct tpacpi_led_classdev tpacpi_led_thinklight = {
> > .name = "tpacpi::thinklight",
> > .brightness_set = &light_sysfs_set,
> > .brightness_get = &light_sysfs_get,
> > + .flags = LED_CORE_SUSPENDRESUME,
> > }
> > };
> >
>