ibm-acpi-devel — ibm-acpi Linux driver development

Re: [ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Jacek A. <jac...@gm...>]

Hi Pavel,

On 4/26/19 2:35 PM, Pavel Machek wrote:
> Hi!
> 
> Something is wrong with blinking on thinkpad
> 
> pavel@amd:~/bt/blee$ cat /sys/class/leds/tpacpi\:\:power/trigger
> [none] bluetooth-power rfkill-any rfkill-none kbd-scrolllock
> kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock
> kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock
> kbd-ctrlrlock AC-online BAT0-charging-or-full BAT0-charging BAT0-full
> BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc
> phy0radio phy0tpt mmc0 timer heartbeat audio-mute audio-micmute
> rfkill1 hci0-power rfkill5
> pavel@amd:~/bt/blee$ echo 1 >
> /sys/class/leds/tpacpi\:\:power/brightness
> pavel@amd:~/bt/blee$ echo 0 >
> /sys/class/leds/tpacpi\:\:power/brightness
> pavel@amd:~/bt/blee$
> 
> Trigger indicates "none" but it keeps blinking, until I echo 0 >
> brightness to stop it. I have python script to trigger this
> behaviour... it seems to work from shell.
> 
> It behaves strangely. If I ask for blinking twice, it stops blinking
> on second request.
> 
> pavel@amd:~/bt/blee$ echo timer >
> /sys/class/leds/tpacpi\:\:power/trigger
> pavel@amd:~/bt/blee$ echo timer >
> /sys/class/leds/tpacpi\:\:power/trigger
> 
> echoing none fixes it.
> 
> Kernel 5.1.0-rc1 + some unrelated bits.

Maybe this is manifestation of the possible problem we still
have in the LED core.

I once proposed a patch [0]. You could try if it changes
something in your case.

[0] https://marc.info/?l=linux-kernel&m=151622365715313&w=2

-- 
Best regards,
Jacek Anaszewski

[ibm-acpi-devel] Thinkpad ACPI led -- it keeps blinking

[Pavel M. <pa...@uc...>]

Hi!

Something is wrong with blinking on thinkpad

pavel@amd:~/bt/blee$ cat /sys/class/leds/tpacpi\:\:power/trigger
[none] bluetooth-power rfkill-any rfkill-none kbd-scrolllock
kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock
kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock
kbd-ctrlrlock AC-online BAT0-charging-or-full BAT0-charging BAT0-full
BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc
phy0radio phy0tpt mmc0 timer heartbeat audio-mute audio-micmute
rfkill1 hci0-power rfkill5
pavel@amd:~/bt/blee$ echo 1 >
/sys/class/leds/tpacpi\:\:power/brightness
pavel@amd:~/bt/blee$ echo 0 >
/sys/class/leds/tpacpi\:\:power/brightness
pavel@amd:~/bt/blee$

Trigger indicates "none" but it keeps blinking, until I echo 0 >
brightness to stop it. I have python script to trigger this
behaviour... it seems to work from shell.

It behaves strangely. If I ask for blinking twice, it stops blinking
on second request.

pavel@amd:~/bt/blee$ echo timer >
/sys/class/leds/tpacpi\:\:power/trigger
pavel@amd:~/bt/blee$ echo timer >
/sys/class/leds/tpacpi\:\:power/trigger

echoing none fixes it. 

Kernel 5.1.0-rc1 + some unrelated bits.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Read EC information on newer models

[Andy S. <and...@gm...>]

On Fri, Mar 8, 2019 at 3:15 PM Jiaxun Yang <jia...@fl...> wrote:
>
> Newer ThinkPads have a totally different EC program information DMI
> table. And thermal subdriver can't work without correct EC version.
>
> Read from this entry if the old method failed to get EC information.
>

Thank you for the patch. See my comments below.

> Signed-off-by: Jiaxun Yang <jia...@fl...>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 62 ++++++++++++++++++++++------
>  1 file changed, 49 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 2e24ee42a3c6..17fd2065000b 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9959,6 +9959,37 @@ static char __init tpacpi_parse_fw_id(const char * const s,
>         return '\0';
>  }
>
> +static void find_new_ec_fwstr(const struct dmi_header *dm, void *private)
> +{

Today it's a new one, tomorrow something else might come. Care to use
versioning instead?
Something line find_ec_fwstr_vXYZ().

> +}

> -                       tp->ec_version_str = kstrdup(ec_fw_string, GFP_KERNEL);
> -                       if (!tp->ec_version_str)
> -                               return -ENOMEM;

>
> -                       t = tpacpi_parse_fw_id(ec_fw_string,
> -                                              &tp->ec_model, &tp->ec_release);
> -                       if (t != 'H') {
> -                               pr_notice("ThinkPad firmware release %s doesn't match the known patterns\n",
> -                                         ec_fw_string);
> -                               pr_notice("please report this to %s\n",
> -                                         TPACPI_MAIL);
> -                       }

Seems to me that this is the same to below. Can you leave indentation
in place and fix it in a separate (following) patch?

> -                       break;
> +       if (ec_fw_string[0]) {
> +               tp->ec_version_str = kstrdup(ec_fw_string, GFP_KERNEL);
> +               if (!tp->ec_version_str)
> +                       return -ENOMEM;
> +
> +               t = tpacpi_parse_fw_id(ec_fw_string,
> +                        &tp->ec_model, &tp->ec_release);
> +               if (t != 'H') {
> +                       pr_notice("ThinkPad firmware release %s doesn't match the known patterns\n",
> +                                 ec_fw_string);
> +                       pr_notice("please report this to %s\n", TPACPI_MAIL);
>                 }
>         }


-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] Question about thinkpad_acpi

[Andy S. <and...@gm...>]

On Thu, Mar 21, 2019 at 4:59 PM Sebastian Dörner <bas...@gm...> wrote:
>
> I totally forgot to mention:
> There are not only warnings in journal, it simply does not work.
> /proc/acpi/ibm/fan always shows "status: disabled", level and speed are
> 0, and commands via echo do not work...

Thank you for the report. I Cc'ed this to public mailing lists and
maintainers of the driver and subsystem.

> On 21.03.19 15:53, Sebastian Dörner wrote:
> > Hi Mr. Shevchenko,
> >
> > I'm writing to you because I noticed that you were one of the most
> > active contributors to the thinkpad_acpi linux driver on github last
> > year.
> > I've got a question about this driver and would be happy if you've got
> > time for advice.
> > My problem is, that my new laptop's fan, a Thinkpad X380, is
> > annoyingly loud, even if the machine is idle.
> > So I figured to manage the fan speed with thinkfan, which uses
> > thinkpad_acpi's /proc/acpi/ibm/fan to control the fan.
> > But each time I try to cat from or echo to /proc/acpi/ibm/fan I get
> > several kernel ACPI warnings:
> > "kernel: ACPI: \_SB.PCI0.LPCB.EC.ECRD: 1 arguments were passed to a
> > non-method ACPI object (RegionField) (20170831/nsarguments-243)"
> > I tried this with the latest kernel (5.0.3) and activated fan_control
> > in the thinkpad_acpi module.
> > I guess this means that some ACPI stuff between kernel and machine
> > went wrong?! don't know much about this.
> > So my question is:
> > Is there a way I can fix this, don't know, by figuring out the correct
> > fan control registers for my machine and then patch it on my own. Or
> > am I screwed with this machine and have to wait for Lenovo to patch this?
> > You see this really bugs me :-) and I'm looking forward to your council.
> >
> > Greetings,
> > Sebastian Dörner
> >
>


-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] fix fan-quirk macro for Lenovo P50

[Andy S. <and...@gm...>]

On Wed, Mar 27, 2019 at 9:25 PM Matthias Hensler <mat...@ws...> wrote:
>
> Commit 846a416b4630fc960c10500b4194ad504bc22d4b changed the
> quirk-matching for modern thinkpads to use three characters instead of
> two. Unfortunately that commit did break the fan-quirk matching for the
> P50. With commit 599eefffcf6b9cff3e8cc2d96fd8eebfadab339c there were
> some cleanups in the quirk-macros, but still did not fix the wrong
> matching for the P50.
>
> Note: I guess the quirk-list needs to be expanded for the P70 and
>       newer models.

You need to put your Signed-off-by tag as well.
This is according to the process:
https://www.kernel.org/doc/Documentation/process/submitting-patches.rst

In any case, don't send updated version right now. We still have time
in this release cycle and I would like to hear Henrique and others
opinions.

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index 726341f2b638..575a1ea161f0 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8721,7 +8721,7 @@ static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
>         TPACPI_QEC_IBM('7', '6', TPACPI_FAN_Q1),
>         TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
>         TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
> -       TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
> +       TPACPI_Q_LNV3('N', '1', 'E', TPACPI_FAN_2FAN),
>  };
>
>  static int __init fan_init(struct ibm_init_struct *iibm)
> --
> 2.20.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] Bugfix thinkpad_acpi.c / proper fanquirk-matching for P50 series

[Andy S. <and...@li...>]

On Wed, Mar 27, 2019 at 12:32:50PM +0100, Matthias Hensler wrote:
> Hi Andy,
> 
> as far as I see you are responsible for pushing bugfixes to the
> thinkpad_acpi-module. If not please point me in the correct direction,
> thanks a lot.

Thanks for the report and fix.

The best way to submit this is to use kernel source tree, where you can run

scripts/get_maintainer.pl -f drivers/platform/x86/thinkpad_acpi.c

to get people responsible for the driver in upstream, followed by

git format-patch ... (you need to commit your fix first)
git send-email...

commands.

For your convenience I Cc'ed this mail to all stakeholders, though it doesn't
abrogate the need to follow the process described above.

> Commit 846a416b4630fc960c10500b4194ad504bc22d4b changed the
> quirk-matching for modern thinkpads to use three characters instead of
> two. Unfortunately that commit did break the fan-quirk matching for my
> P50. With commit 599eefffcf6b9cff3e8cc2d96fd8eebfadab339c there were
> some cleanups in the quirk-macros, but still did not fix the wrong
> matching for the P50.
> 
> Please find attached a patch to fix the quirk-macro to properly detect
> the second fan on the P50 (Modell N1Exxx). I guess there should be some
> additional definitions for the P70 and newer P-series, but I do not know
> the exact model-numbers unfortunately.
> 
> 
> Additional what I would also like to see, is a possibility to also
> control the second fan. I use a modified version of a patch that was
> posted some years ago to the mailinglist for that, but it is really
> ugly. If you want I can try to reimplement that in a cleaner way, but
> for now at least the wrong quirk-matching should be fixed. Thanks again.
> 
> 
> Regards,
> Matthias

> --- thinkpad_acpi.c	2019-03-27 12:26:11.977106074 +0100
> +++ thinkpad_acpi.c.fixed	2019-03-27 12:27:44.090340050 +0100
> @@ -8721,7 +8721,7 @@
>  	TPACPI_QEC_IBM('7', '6', TPACPI_FAN_Q1),
>  	TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
>  	TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
> -	TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
> +	TPACPI_Q_LNV3('N', '1', 'E', TPACPI_FAN_2FAN),
>  };
>  
>  static int __init fan_init(struct ibm_init_struct *iibm)




-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Read EC information on newer models

[Greg KH <gr...@li...>]

On Fri, Mar 08, 2019 at 09:14:26PM +0800, Jiaxun Yang wrote:
> Newer ThinkPads have a totally different EC program information DMI
> table. And thermal subdriver can't work without correct EC version.
> 
> Read from this entry if the old method failed to get EC information.
> 
> Signed-off-by: Jiaxun Yang <jia...@fl...>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 62 ++++++++++++++++++++++------
>  1 file changed, 49 insertions(+), 13 deletions(-)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: disable bluetooth for some machines

[Andy S. <and...@gm...>]

On Thu, Mar 7, 2019 at 2:30 PM Jiaxun Yang <jia...@fl...> wrote:
> > On Thu, Mar 07, 2019 at 04:08:20PM +0800, Jiaxun Yang wrote:
> >> Some AMD based ThinkPads have a firmware bug that calling
> >> "GBDC" will cause bluetooth on Intel wireless cards blocked.
> >>
> >> Probe these models by DMI match and disable bluetooth subdriver
> >> if specified Intel wireless card exist.

> Thanks for remind.
>
> Should I resend this patch in correct way?

It still needs a bit of work, and as I told you, we are now during an
opened merge window.

-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: disable bluetooth for some machines

[Greg KH <gr...@li...>]

On Thu, Mar 07, 2019 at 04:08:20PM +0800, Jiaxun Yang wrote:
> Some AMD based ThinkPads have a firmware bug that calling
> "GBDC" will cause bluetooth on Intel wireless cards blocked.
> 
> Probe these models by DMI match and disable bluetooth subdriver
> if specified Intel wireless card exist.
> 
> Signed-off-by: Jiaxun Yang <jia...@fl...>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 70 ++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: disable bluetooth for some machines

[Andy S. <and...@gm...>]

On Thu, Mar 7, 2019 at 10:09 AM Jiaxun Yang <jia...@fl...> wrote:
>
> Some AMD based ThinkPads have a firmware bug that calling
> "GBDC" will cause bluetooth on Intel wireless cards blocked.
>
> Probe these models by DMI match and disable bluetooth subdriver
> if specified Intel wireless card exist.

Thanks for a patch. My comments below.

(and take your time, there is merge window going on, so, your fix is
probably material for v5.1-rc2)

> @@ -79,6 +79,7 @@
>  #include <linux/jiffies.h>
>  #include <linux/workqueue.h>
>  #include <linux/acpi.h>

> +#include <linux/pci.h>
>  #include <linux/pci_ids.h>

The new include has included the old one with IDs.

>  #include <linux/power_supply.h>
>  #include <sound/core.h>
> @@ -4501,11 +4502,80 @@ static void bluetooth_exit(void)
>         bluetooth_shutdown();
>  }

> +static int __init have_bt_fwbug(void)
> +{
> +       /* Some AMD based ThinkPads have a firmware bug that calling
> +        * "GBDC" will cause bluetooth on Intel wireless cards blocked
> +        */

/*
 * Comments in multi-line style example.
 * Should look like this one.
 */

> +       if (dmi_check_system(bt_fwbug_list)) {

> +               if (pci_get_device(PCI_VENDOR_ID_INTEL, 0x24F3, NULL) || \
> +                   pci_get_device(PCI_VENDOR_ID_INTEL, 0x24FD, NULL) || \
> +                   pci_get_device(PCI_VENDOR_ID_INTEL, 0x2526, NULL))

You can do a table and use pci_match_device().
On top of it

if (x) {
 if (y)
  ...
}

equivalent to

if (x && y) {
 ...
}

> +                       return 1;
> +               else
> +                       return 0;
> +       } else {
> +                       return 0;
> +       }
> +}

> +       if (have_bt_fwbug()) {
> +               vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
> +                       FW_BUG "disable bluetooth subdriver for Intel cards\n");
> +               return 1;
> +       }
>         vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_RFKILL,
>                         "initializing bluetooth subdriver\n");

Below I see the following:
        /* bluetooth not supported on 570, 600e/x, 770e, 770x, A21e, A2xm/p,
          G4x, R30, R31, R40e, R50e, T20-22, X20-21 */
       tp_features.bluetooth = hkey_handle &&
           acpi_evalf(hkey_handle, &status, "GBDC", "qd");

Is it possible to integrate your stuff to this one?
Or can you do similar, like tp_features.bluetooth = !have_bt_fwbug(); ?

-- 
With Best Regards,
Andy Shevchenko

[ibm-acpi-devel] Mic/Volume Mute leds on Thinkpad X1 Yoga 3rd-gen

[Giacomo C. <in...@gi...>]

Hi all,
I've recently upgraded my kernel from 4.17.19 to 4.19.9 and volume/mic 
mute leds stopped working. If I enable volume_control=1 and 
volume_capabilities=1 kernel parameters of thinkpad_acpi module, using 
'mute' command on /proc/acpi/ibm/volume interface, leds switch on/off 
correctly, but if I press mute switches, while the muting happens at 
soundcard level, leds remain switched off.

I've reverted to old kernel, but I'm available for any testing that can 
help solve the problem.

-- 
Giacomo Cariello

Mobile/Telegram/WhatsApp: +39.348.3043150
Email: in...@gi...

Re: [ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: Cleanup quirks macros

[Andy S. <and...@gm...>]

On Wed, Dec 5, 2018 at 1:18 PM Jouke Witteveen <j.w...@gm...> wrote:
>
> - Use generic quirks macros for fan quirks
> The fan-specific quirks macros were duplicates of the generic ones.
>
> - Remove useless #undef lines
> The referenced macros are not defined anywhere.
>

Patch had been pushed, thanks!

> Signed-off-by: Jouke Witteveen <j.w...@gm...>
> Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
> ---
>
> This time, with a more detailed commit message.
>
>  drivers/platform/x86/thinkpad_acpi.c | 45 ++++++++--------------------
>  1 file changed, 13 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index fde08a997..e1e51bfb3 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -478,6 +478,12 @@ do {                                                                       \
>           .ec = TPACPI_MATCH_ANY,               \
>           .quirks = (__quirk) }
>
> +#define TPACPI_QEC_IBM(__id1, __id2, __quirk)  \
> +       { .vendor = PCI_VENDOR_ID_IBM,          \
> +         .bios = TPACPI_MATCH_ANY,             \
> +         .ec = TPID(__id1, __id2),             \
> +         .quirks = (__quirk) }
> +
>  #define TPACPI_QEC_LNV(__id1, __id2, __quirk)  \
>         { .vendor = PCI_VENDOR_ID_LENOVO,       \
>           .bios = TPACPI_MATCH_ANY,             \
> @@ -5973,9 +5979,6 @@ static const struct tpacpi_quirk led_useful_qtable[] __initconst = {
>         },
>  };
>
> -#undef TPACPI_LEDQ_IBM
> -#undef TPACPI_LEDQ_LNV
> -
>  static enum led_access_mode __init led_init_detect_mode(void)
>  {
>         acpi_status status;
> @@ -8710,40 +8713,18 @@ static const struct attribute_group fan_attr_group = {
>         .attrs = fan_attributes,
>  };
>
> -#define        TPACPI_FAN_Q1   0x0001          /* Unitialized HFSP */
> +#define TPACPI_FAN_Q1  0x0001          /* Unitialized HFSP */
>  #define TPACPI_FAN_2FAN        0x0002          /* EC 0x31 bit 0 selects fan2 */
>
> -#define TPACPI_FAN_QI(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_IBM,          \
> -         .bios = TPACPI_MATCH_ANY,             \
> -         .ec = TPID(__id1, __id2),             \
> -         .quirks = __quirks }
> -
> -#define TPACPI_FAN_QL(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_LENOVO,       \
> -         .bios = TPACPI_MATCH_ANY,             \
> -         .ec = TPID(__id1, __id2),             \
> -         .quirks = __quirks }
> -
> -#define TPACPI_FAN_QB(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_LENOVO,       \
> -         .bios = TPID(__id1, __id2),           \
> -         .ec = TPACPI_MATCH_ANY,               \
> -         .quirks = __quirks }
> -
>  static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> -       TPACPI_FAN_QI('1', 'Y', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '8', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '6', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '0', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QL('7', 'M', TPACPI_FAN_2FAN),
> -       TPACPI_FAN_QB('N', '1', TPACPI_FAN_2FAN),
> +       TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '8', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '6', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
> +       TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
> +       TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
>  };
>
> -#undef TPACPI_FAN_QL
> -#undef TPACPI_FAN_QI
> -#undef TPACPI_FAN_QB
> -
>  static int __init fan_init(struct ibm_init_struct *iibm)
>  {
>         int rc;
> --
> 2.19.2
>


-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] Well-known LED names was Re: [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Jacek A. <jac...@gm...>]

Hi Pavel,

On 12/1/18 3:41 PM, Pavel Machek wrote:
> Hi!
> 
>>>>> If external USB keyboard is identified as "input7" device, then
>>>>> "input7::mute" is a good name for mute key. But "sys::mute" does not say
>>>>> anything to which device or hardware it belongs nor does not solve
>>>>> problem that which device/driver/subsystem should have privilege to take
>>>>> this "sys" name.
>>>>
>>>> How about just "platform" for the LEDs being part of the device
>>>> on which the system is running?
>>>
>>> "platform" works for me.
>>>
>>> Are we in agreement that this name will be used for all similar LEDs,
>>> as long as they are on the "main box" of the device, no matter if they
>>> are connected using acpi, gpio, i2c, ...?
>>
>> One doubt: say we have hdd activity LED on the "main box" - it
>> would be named "platform::disk".
>>
>> Now, we add external USB disk. Previously we were considering
>> the naming for disk LEDs in the form e.g. "sdb::disk".
>>
>> If so, there would be discrepancy between internal and external disk
>> LED names. Similarly in case of eth/adsl/wlan/camera LEDs.
> 
> Well, it really depends if the "plaform::disk" is for all the disks,
> or if it is for sda. In the second case, it might be better to name it
> sda::disk...
> 
> Anyway... I believe we should start documenting good and bad LEDs, so
> that patch authors know what is there, and can try to be consistent,
> and so that userland knows what names to probe for.

The idea looks reasonable. In addition to that, I'd change one
thing in the LED naming pattern - replace "devicename " with
"affiliation". AFAICS that would fit best for both "platform"
and e.g. "input*".

Also in the DT we would need related "affiliation" property.
In most cases it would be "platform" probably.
Possible is also "camera" - but we would have to state it clear
how to proceed in case of video devices - shouldn't v4l2
drivers create LEDs similarly to how network drivers do that.
Then it would be possible to provide videoN name.

> What about following? Any other LEDs worth mentioning?
disk: sdX
network devices: ethN, adslN

These "affiliations" would have to be provided by the
network drivers creating LED class devices.

> commit c56708addf9c312cefd760bca218a0545258b217
> Author: Pavel <pa...@uc...>
> Date:   Sat Dec 1 15:32:13 2018 +0100
> 
> leds: Add list of well-known LED names
>      
> Signed-off-by: Pavel Machek <pa...@uc...>
> 
> diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt
> new file mode 100644
> index 0000000..9a262db
> --- /dev/null
> +++ b/Documentation/leds/well-known-leds.txt
> @@ -0,0 +1,42 @@
> +-*- org -*-
> +
> +It is somehow important to provide consistent interface to the
> +userland. LED devices have one problem there, and that is naming of
> +directories in /sys/class/leds. It would be nice if userland would
> +just know right "name" for given LED function, but situation got more
> +complex.
> +
> +Anyway, if backwards compatibility is not an issue, new code should
> +use one of the "good" names from this list, and you should extend the
> +list where applicable.
> +
> +Bad names are listed, too, in case you are writing application that
> +wants to use particular feature, you should probe for good name first
> +but then try the bad ones, too".
> +
> +* Keyboards
> +
> +Good: "input*:*:capslock"
> +Good: "input*:*:scrolllock"
> +Good: "input*:*:numlock"
> +
> +Set of common keyboard LEDs, going back to PC AT or so.
> +
> +Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads)
> +Bad: "lp5523:kb{1,2,3,4,5,6,7}" (Nokia N900)
> +
> +Keyboard frontlight/backlight.

Isn't example missing here?

> +
> +* Sound subsystem
> +
> +Good: "platform:*:mute"
> +Good: "platform:*:micmute"
> +
> +LEDs on notebook body, indicating that sound input / output is muted.
> +
> +* System notification
> +
> +Good: "status-led:{red,green,blue}" (Motorola Droid 4)

Two problems here:
- "status-led" is in place of "devicename" instead of "function"
- "-led" is obvious and non-generic - we will have "status"
   in the "functions.h"

> +Bad: "lp5523:{r,g,b}" (Nokia N900)
> +
> +Phones usually have multi-color status LED.
> 
> 
>

-- 
Best regards,
Jacek Anaszewski

[ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: Cleanup quirks macros

[Jouke W. <j.w...@gm...>]

- Use generic quirks macros for fan quirks
The fan-specific quirks macros were duplicates of the generic ones.

- Remove useless #undef lines
The referenced macros are not defined anywhere.

Signed-off-by: Jouke Witteveen <j.w...@gm...>
Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
---

This time, with a more detailed commit message.

 drivers/platform/x86/thinkpad_acpi.c | 45 ++++++++--------------------
 1 file changed, 13 insertions(+), 32 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index fde08a997..e1e51bfb3 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -478,6 +478,12 @@ do {									\
 	  .ec = TPACPI_MATCH_ANY,		\
 	  .quirks = (__quirk) }
 
+#define TPACPI_QEC_IBM(__id1, __id2, __quirk)	\
+	{ .vendor = PCI_VENDOR_ID_IBM,		\
+	  .bios = TPACPI_MATCH_ANY,		\
+	  .ec = TPID(__id1, __id2),		\
+	  .quirks = (__quirk) }
+
 #define TPACPI_QEC_LNV(__id1, __id2, __quirk)	\
 	{ .vendor = PCI_VENDOR_ID_LENOVO,	\
 	  .bios = TPACPI_MATCH_ANY,		\
@@ -5973,9 +5979,6 @@ static const struct tpacpi_quirk led_useful_qtable[] __initconst = {
 	},
 };
 
-#undef TPACPI_LEDQ_IBM
-#undef TPACPI_LEDQ_LNV
-
 static enum led_access_mode __init led_init_detect_mode(void)
 {
 	acpi_status status;
@@ -8710,40 +8713,18 @@ static const struct attribute_group fan_attr_group = {
 	.attrs = fan_attributes,
 };
 
-#define	TPACPI_FAN_Q1	0x0001		/* Unitialized HFSP */
+#define TPACPI_FAN_Q1	0x0001		/* Unitialized HFSP */
 #define TPACPI_FAN_2FAN	0x0002		/* EC 0x31 bit 0 selects fan2 */
 
-#define TPACPI_FAN_QI(__id1, __id2, __quirks)	\
-	{ .vendor = PCI_VENDOR_ID_IBM,		\
-	  .bios = TPACPI_MATCH_ANY,		\
-	  .ec = TPID(__id1, __id2),		\
-	  .quirks = __quirks }
-
-#define TPACPI_FAN_QL(__id1, __id2, __quirks)	\
-	{ .vendor = PCI_VENDOR_ID_LENOVO,	\
-	  .bios = TPACPI_MATCH_ANY,		\
-	  .ec = TPID(__id1, __id2),		\
-	  .quirks = __quirks }
-
-#define TPACPI_FAN_QB(__id1, __id2, __quirks)	\
-	{ .vendor = PCI_VENDOR_ID_LENOVO,	\
-	  .bios = TPID(__id1, __id2),		\
-	  .ec = TPACPI_MATCH_ANY,		\
-	  .quirks = __quirks }
-
 static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
-	TPACPI_FAN_QI('1', 'Y', TPACPI_FAN_Q1),
-	TPACPI_FAN_QI('7', '8', TPACPI_FAN_Q1),
-	TPACPI_FAN_QI('7', '6', TPACPI_FAN_Q1),
-	TPACPI_FAN_QI('7', '0', TPACPI_FAN_Q1),
-	TPACPI_FAN_QL('7', 'M', TPACPI_FAN_2FAN),
-	TPACPI_FAN_QB('N', '1', TPACPI_FAN_2FAN),
+	TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
+	TPACPI_QEC_IBM('7', '8', TPACPI_FAN_Q1),
+	TPACPI_QEC_IBM('7', '6', TPACPI_FAN_Q1),
+	TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
+	TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
+	TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
 };
 
-#undef TPACPI_FAN_QL
-#undef TPACPI_FAN_QI
-#undef TPACPI_FAN_QB
-
 static int __init fan_init(struct ibm_init_struct *iibm)
 {
 	int rc;
-- 
2.19.2

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Cleanup quirks macros

[Andy S. <and...@gm...>]

On Tue, Dec 4, 2018 at 1:20 PM Henrique de Moraes Holschuh
<hm...@hm...> wrote:
>
> On Mon, 03 Dec 2018, Andy Shevchenko wrote:
> > On Fri, Nov 16, 2018 at 3:18 PM Jouke Witteveen <j.w...@gm...> wrote:
> >
> > Please, submit a new version with changelog.
> >
> > Though, before doing this, I would hear if Henrique is OK with the
> > change (for me it looks good)
>
> Acked-by: Henrique de Moraes Holschuh <hm...@hm...>

Thank you!

Jouke, so, I would wait for v2 with changelog and don't forget to
append Henrique's tag as well.

-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Cleanup quirks macros

[Henrique de M. H. <hm...@hm...>]

On Mon, 03 Dec 2018, Andy Shevchenko wrote:
> On Fri, Nov 16, 2018 at 3:18 PM Jouke Witteveen <j.w...@gm...> wrote:
> 
> Please, submit a new version with changelog.
> 
> Though, before doing this, I would hear if Henrique is OK with the
> change (for me it looks good)

Acked-by: Henrique de Moraes Holschuh <hm...@hm...>

there you have it :-)

-- 
  Henrique Holschuh

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Cleanup quirks macros

[Andy S. <and...@gm...>]

On Fri, Nov 16, 2018 at 3:18 PM Jouke Witteveen <j.w...@gm...> wrote:
>

Please, submit a new version with changelog.

Though, before doing this, I would hear if Henrique is OK with the
change (for me it looks good)

> Signed-off-by: Jouke Witteveen <j.w...@gm...>
> ---
> The only difference in the generated code is that the quirk values are
> now within parentheses.
>
>  drivers/platform/x86/thinkpad_acpi.c | 45 ++++++++--------------------
>  1 file changed, 13 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
> index fde08a997..e1e51bfb3 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -478,6 +478,12 @@ do {                                                                       \
>           .ec = TPACPI_MATCH_ANY,               \
>           .quirks = (__quirk) }
>
> +#define TPACPI_QEC_IBM(__id1, __id2, __quirk)  \
> +       { .vendor = PCI_VENDOR_ID_IBM,          \
> +         .bios = TPACPI_MATCH_ANY,             \
> +         .ec = TPID(__id1, __id2),             \
> +         .quirks = (__quirk) }
> +
>  #define TPACPI_QEC_LNV(__id1, __id2, __quirk)  \
>         { .vendor = PCI_VENDOR_ID_LENOVO,       \
>           .bios = TPACPI_MATCH_ANY,             \
> @@ -5973,9 +5979,6 @@ static const struct tpacpi_quirk led_useful_qtable[] __initconst = {
>         },
>  };
>
> -#undef TPACPI_LEDQ_IBM
> -#undef TPACPI_LEDQ_LNV
> -
>  static enum led_access_mode __init led_init_detect_mode(void)
>  {
>         acpi_status status;
> @@ -8710,40 +8713,18 @@ static const struct attribute_group fan_attr_group = {
>         .attrs = fan_attributes,
>  };
>
> -#define        TPACPI_FAN_Q1   0x0001          /* Unitialized HFSP */
> +#define TPACPI_FAN_Q1  0x0001          /* Unitialized HFSP */
>  #define TPACPI_FAN_2FAN        0x0002          /* EC 0x31 bit 0 selects fan2 */
>
> -#define TPACPI_FAN_QI(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_IBM,          \
> -         .bios = TPACPI_MATCH_ANY,             \
> -         .ec = TPID(__id1, __id2),             \
> -         .quirks = __quirks }
> -
> -#define TPACPI_FAN_QL(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_LENOVO,       \
> -         .bios = TPACPI_MATCH_ANY,             \
> -         .ec = TPID(__id1, __id2),             \
> -         .quirks = __quirks }
> -
> -#define TPACPI_FAN_QB(__id1, __id2, __quirks)  \
> -       { .vendor = PCI_VENDOR_ID_LENOVO,       \
> -         .bios = TPID(__id1, __id2),           \
> -         .ec = TPACPI_MATCH_ANY,               \
> -         .quirks = __quirks }
> -
>  static const struct tpacpi_quirk fan_quirk_table[] __initconst = {
> -       TPACPI_FAN_QI('1', 'Y', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '8', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '6', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QI('7', '0', TPACPI_FAN_Q1),
> -       TPACPI_FAN_QL('7', 'M', TPACPI_FAN_2FAN),
> -       TPACPI_FAN_QB('N', '1', TPACPI_FAN_2FAN),
> +       TPACPI_QEC_IBM('1', 'Y', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '8', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '6', TPACPI_FAN_Q1),
> +       TPACPI_QEC_IBM('7', '0', TPACPI_FAN_Q1),
> +       TPACPI_QEC_LNV('7', 'M', TPACPI_FAN_2FAN),
> +       TPACPI_Q_LNV('N', '1', TPACPI_FAN_2FAN),
>  };
>
> -#undef TPACPI_FAN_QL
> -#undef TPACPI_FAN_QI
> -#undef TPACPI_FAN_QB
> -
>  static int __init fan_init(struct ibm_init_struct *iibm)
>  {
>         int rc;
> --
> 2.19.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: add unsafe_leds parameter

[Andy S. <and...@gm...>]

On Tue, Nov 13, 2018 at 6:41 PM Milan Hauth <mi...@gm...> wrote:

> thanks for your replies.

Thanks for an update, though made in wrong format:
- no top post
- should be send as a separate patch in a separate thread
- should have increased version

But the main one is...

> > On Sat, 27 Oct 2018, Milan Hauth wrote:
> > > move CONFIG_THINKPAD_ACPI_UNSAFE_LEDS compile config
> > > to thinkpad_acpi.unsafe_leds module parameter
> > >
> > > so there is no more need to re-compile
> > > to control important LEDs, which is unsafe
> > >
> > > Signed-off-by: Milan Hauth <mi...@gm...>
> >
> > I am not confortable signing-off on this one.

...I'm not familiar with this interface and I rely on Henrique's
opinion, so, I can't take it.
Sorry.

-- 
With Best Regards,
Andy Shevchenko

[ibm-acpi-devel] Well-known LED names was Re: [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Pavel M. <pa...@uc...>]

Hi!

> >>> If external USB keyboard is identified as "input7" device, then
> >>> "input7::mute" is a good name for mute key. But "sys::mute" does not say
> >>> anything to which device or hardware it belongs nor does not solve
> >>> problem that which device/driver/subsystem should have privilege to take
> >>> this "sys" name.
> >>
> >> How about just "platform" for the LEDs being part of the device
> >> on which the system is running?
> > 
> > "platform" works for me.
> > 
> > Are we in agreement that this name will be used for all similar LEDs,
> > as long as they are on the "main box" of the device, no matter if they
> > are connected using acpi, gpio, i2c, ...?
> 
> One doubt: say we have hdd activity LED on the "main box" - it
> would be named "platform::disk".
> 
> Now, we add external USB disk. Previously we were considering
> the naming for disk LEDs in the form e.g. "sdb::disk".
> 
> If so, there would be discrepancy between internal and external disk
> LED names. Similarly in case of eth/adsl/wlan/camera LEDs.

Well, it really depends if the "plaform::disk" is for all the disks,
or if it is for sda. In the second case, it might be better to name it
sda::disk...

Anyway... I believe we should start documenting good and bad LEDs, so
that patch authors know what is there, and can try to be consistent,
and so that userland knows what names to probe for.

What about following? Any other LEDs worth mentioning?

commit c56708addf9c312cefd760bca218a0545258b217
Author: Pavel <pa...@uc...>
Date:   Sat Dec 1 15:32:13 2018 +0100

leds: Add list of well-known LED names
    
Signed-off-by: Pavel Machek <pa...@uc...>

diff --git a/Documentation/leds/well-known-leds.txt b/Documentation/leds/well-known-leds.txt
new file mode 100644
index 0000000..9a262db
--- /dev/null
+++ b/Documentation/leds/well-known-leds.txt
@@ -0,0 +1,42 @@
+-*- org -*-
+
+It is somehow important to provide consistent interface to the
+userland. LED devices have one problem there, and that is naming of
+directories in /sys/class/leds. It would be nice if userland would
+just know right "name" for given LED function, but situation got more
+complex.
+
+Anyway, if backwards compatibility is not an issue, new code should
+use one of the "good" names from this list, and you should extend the
+list where applicable.
+
+Bad names are listed, too, in case you are writing application that
+wants to use particular feature, you should probe for good name first
+but then try the bad ones, too".
+
+* Keyboards
+
+Good: "input*:*:capslock"
+Good: "input*:*:scrolllock"
+Good: "input*:*:numlock"
+
+Set of common keyboard LEDs, going back to PC AT or so.
+
+Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads)
+Bad: "lp5523:kb{1,2,3,4,5,6,7}" (Nokia N900)
+
+Keyboard frontlight/backlight.
+
+* Sound subsystem
+
+Good: "platform:*:mute"
+Good: "platform:*:micmute"
+
+LEDs on notebook body, indicating that sound input / output is muted.
+
+* System notification
+
+Good: "status-led:{red,green,blue}" (Motorola Droid 4)
+Bad: "lp5523:{r,g,b}" (Nokia N900)
+
+Phones usually have multi-color status LED.



-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Andy S. <and...@li...>]

On Thu, Nov 29, 2018 at 10:23:31PM +0100, Jacek Anaszewski wrote:

> Regarding sound devices - how would we name micmute LED for USB
> microphone? I suppose that it is possible to discover some unique
> identifier in runtime - this is a question to ALSA people.

USB path (e.g. 2-1.1.2.4.3) + '::' + name ?

-- 
With Best Regards,
Andy Shevchenko

Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Takashi I. <ti...@su...>]

On Thu, 29 Nov 2018 22:23:31 +0100,
Jacek Anaszewski wrote:
> 
> On 11/28/2018 10:22 PM, Pavel Machek wrote:
> > Hi!
> > 
> >>> If external USB keyboard is identified as "input7" device, then
> >>> "input7::mute" is a good name for mute key. But "sys::mute" does not say
> >>> anything to which device or hardware it belongs nor does not solve
> >>> problem that which device/driver/subsystem should have privilege to take
> >>> this "sys" name.
> >>
> >> How about just "platform" for the LEDs being part of the device
> >> on which the system is running?
> > 
> > "platform" works for me.
> > 
> > Are we in agreement that this name will be used for all similar LEDs,
> > as long as they are on the "main box" of the device, no matter if they
> > are connected using acpi, gpio, i2c, ...?
> 
> One doubt: say we have hdd activity LED on the "main box" - it
> would be named "platform::disk".
> 
> Now, we add external USB disk. Previously we were considering
> the naming for disk LEDs in the form e.g. "sdb::disk".
> 
> If so, there would be discrepancy between internal and external disk
> LED names. Similarly in case of eth/adsl/wlan/camera LEDs.
> 
> Regarding sound devices - how would we name micmute LED for USB
> microphone? I suppose that it is possible to discover some unique
> identifier in runtime - this is a question to ALSA people.

There are little USB-audio devices supporting the LED control, so we
haven't had any chance for LED class for USB-audio.

At most, we may create LED class devices for some HD-audio (usually
driven via exotic verbs or GPIO over HD-audio), but I wouldn't go in
that direction; it'll need too make things more complex than the
direct hook as of now.


thanks,

Takashi

Re: [ibm-acpi-devel] [PATCH 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Jacek A. <jac...@gm...>]

On 11/28/2018 10:22 PM, Pavel Machek wrote:
> Hi!
> 
>>> If external USB keyboard is identified as "input7" device, then
>>> "input7::mute" is a good name for mute key. But "sys::mute" does not say
>>> anything to which device or hardware it belongs nor does not solve
>>> problem that which device/driver/subsystem should have privilege to take
>>> this "sys" name.
>>
>> How about just "platform" for the LEDs being part of the device
>> on which the system is running?
> 
> "platform" works for me.
> 
> Are we in agreement that this name will be used for all similar LEDs,
> as long as they are on the "main box" of the device, no matter if they
> are connected using acpi, gpio, i2c, ...?

One doubt: say we have hdd activity LED on the "main box" - it
would be named "platform::disk".

Now, we add external USB disk. Previously we were considering
the naming for disk LEDs in the form e.g. "sdb::disk".

If so, there would be discrepancy between internal and external disk
LED names. Similarly in case of eth/adsl/wlan/camera LEDs.

Regarding sound devices - how would we name micmute LED for USB
microphone? I suppose that it is possible to discover some unique
identifier in runtime - this is a question to ALSA people.

-- 
Best regards,
Jacek Anaszewski

Re: [ibm-acpi-devel] [PATCH v2 0/6] Introduce audio-mute LED trigger (and conversions to it)

[Takashi I. <ti...@su...>]

On Thu, 29 Nov 2018 08:31:25 +0100,
Takashi Iwai wrote:
> 
> Hi,
> 
> this is revised patch series for adding a new LED trigger audio-mute
> and audio-micmute, and converting the HD-audio driver and the platform 
> drivers to use the LED trigger instead of the ugly direct dynamic
> symbol binding.
> 
> v2 series addressed a couple of issues Andy suggested, and changed the
> led class dev names to "platform::xxx" as discussed in the previous
> thread.
> 
> The latest version of patches are found in topic/leds-trigger branch
> in my sound git tree.
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git
> 
> As these are cross-tree patches, the branch above is based cleanly on
> v4.20-rc3.  From now on, it'll stay as an immutable branch, so feel
> free to merge into other trees.

FYI, I merged to for-next branch now.


Takashi

[ibm-acpi-devel] [PATCH v2 6/6] platform/x86: thinkpad_acpi: Drop superfluous exported function

[Takashi I. <ti...@su...>]

Since we've switched to the LED trigger for binding with HD-audio,
we can drop the exported function as well as the whole
linux/thinkpad_acpi.h.

The own TPACPI_LED_MUTE and TPACPI_LED_MICMUTE definitions are
replaced with the identical ones for LEDS, i.e. LED_AUDIO_MUTE and
LED_AUDIO_MICMUTE, respectively.  They are no longer needed as
referred only locally.

Acked-by: Jacek Anaszewski <jac...@gm...>
Acked-by: Pavel Machek <pa...@uc...>
Acked-by: Andy Shevchenko <and...@gm...>
Acked-by: Henrique de Moraes Holschuh <hm...@hm...>
Acked-by: Pali Rohár <pal...@gm...>
Signed-off-by: Takashi Iwai <ti...@su...>
---
 drivers/platform/x86/thinkpad_acpi.c | 30 ++++++++++------------------
 include/linux/thinkpad_acpi.h        | 16 ---------------
 2 files changed, 11 insertions(+), 35 deletions(-)
 delete mode 100644 include/linux/thinkpad_acpi.h

diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 3d2c1f5f22e2..21ffb961585a 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -81,7 +81,6 @@
 #include <linux/acpi.h>
 #include <linux/pci_ids.h>
 #include <linux/power_supply.h>
-#include <linux/thinkpad_acpi.h>
 #include <sound/core.h>
 #include <sound/control.h>
 #include <sound/initval.h>
@@ -9150,6 +9149,7 @@ static struct ibm_struct fan_driver_data = {
  * Mute LED subdriver
  */
 
+#define TPACPI_LED_MAX		2
 
 struct tp_led_table {
 	acpi_string name;
@@ -9158,13 +9158,13 @@ struct tp_led_table {
 	int state;
 };
 
-static struct tp_led_table led_tables[] = {
-	[TPACPI_LED_MUTE] = {
+static struct tp_led_table led_tables[TPACPI_LED_MAX] = {
+	[LED_AUDIO_MUTE] = {
 		.name = "SSMS",
 		.on_value = 1,
 		.off_value = 0,
 	},
-	[TPACPI_LED_MICMUTE] = {
+	[LED_AUDIO_MICMUTE] = {
 		.name = "MMTS",
 		.on_value = 2,
 		.off_value = 0,
@@ -9189,40 +9189,36 @@ static int mute_led_on_off(struct tp_led_table *t, bool state)
 	return state;
 }
 
-int tpacpi_led_set(int whichled, bool on)
+static int tpacpi_led_set(int whichled, bool on)
 {
 	struct tp_led_table *t;
 
-	if (whichled < 0 || whichled >= TPACPI_LED_MAX)
-		return -EINVAL;
-
 	t = &led_tables[whichled];
 	if (t->state < 0 || t->state == on)
 		return t->state;
 	return mute_led_on_off(t, on);
 }
-EXPORT_SYMBOL_GPL(tpacpi_led_set);
 
 static int tpacpi_led_mute_set(struct led_classdev *led_cdev,
 			       enum led_brightness brightness)
 {
-	return tpacpi_led_set(TPACPI_LED_MUTE, brightness != LED_OFF);
+	return tpacpi_led_set(LED_AUDIO_MUTE, brightness != LED_OFF);
 }
 
 static int tpacpi_led_micmute_set(struct led_classdev *led_cdev,
 				  enum led_brightness brightness)
 {
-	return tpacpi_led_set(TPACPI_LED_MICMUTE, brightness != LED_OFF);
+	return tpacpi_led_set(LED_AUDIO_MICMUTE, brightness != LED_OFF);
 }
 
-static struct led_classdev mute_led_cdev[] = {
-	[TPACPI_LED_MUTE] = {
+static struct led_classdev mute_led_cdev[TPACPI_LED_MAX] = {
+	[LED_AUDIO_MUTE] = {
 		.name		= "platform::mute",
 		.max_brightness = 1,
 		.brightness_set_blocking = tpacpi_led_mute_set,
 		.default_trigger = "audio-mute",
 	},
-	[TPACPI_LED_MICMUTE] = {
+	[LED_AUDIO_MICMUTE] = {
 		.name		= "platform::micmute",
 		.max_brightness = 1,
 		.brightness_set_blocking = tpacpi_led_micmute_set,
@@ -9232,10 +9228,6 @@ static struct led_classdev mute_led_cdev[] = {
 
 static int mute_led_init(struct ibm_init_struct *iibm)
 {
-	static enum led_audio types[] = {
-		[TPACPI_LED_MUTE] = LED_AUDIO_MUTE,
-		[TPACPI_LED_MICMUTE] = LED_AUDIO_MICMUTE,
-	};
 	acpi_handle temp;
 	int i, err;
 
@@ -9246,7 +9238,7 @@ static int mute_led_init(struct ibm_init_struct *iibm)
 			continue;
 		}
 
-		mute_led_cdev[i].brightness = ledtrig_audio_get(types[i]);
+		mute_led_cdev[i].brightness = ledtrig_audio_get(i);
 		err = led_classdev_register(&tpacpi_pdev->dev, &mute_led_cdev[i]);
 		if (err < 0) {
 			while (i--) {
diff --git a/include/linux/thinkpad_acpi.h b/include/linux/thinkpad_acpi.h
deleted file mode 100644
index 9fb317970c01..000000000000
--- a/include/linux/thinkpad_acpi.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __THINKPAD_ACPI_H__
-#define __THINKPAD_ACPI_H__
-
-/* These two functions return 0 if success, or negative error code
-   (e g -ENODEV if no led present) */
-
-enum {
-	TPACPI_LED_MUTE,
-	TPACPI_LED_MICMUTE,
-	TPACPI_LED_MAX,
-};
-
-int tpacpi_led_set(int whichled, bool on);
-
-#endif
-- 
2.19.1

[ibm-acpi-devel] [PATCH v2 5/6] platform/x86: dell-laptop: Drop superfluous exported function

[Takashi I. <ti...@su...>]

Since we've switched to the LED trigger for binding with HD-audio,
we can drop the exported function as well as the whole
linux/dell-led.h.

Acked-by: Jacek Anaszewski <jac...@gm...>
Acked-by: Pavel Machek <pa...@uc...>
Acked-by: Andy Shevchenko <and...@gm...>
Acked-by: Pali Rohár <pal...@gm...>
Signed-off-by: Takashi Iwai <ti...@su...>
---
 drivers/platform/x86/dell-laptop.c | 22 +++++-----------------
 include/linux/dell-led.h           |  7 -------
 2 files changed, 5 insertions(+), 24 deletions(-)
 delete mode 100644 include/linux/dell-led.h

diff --git a/drivers/platform/x86/dell-laptop.c b/drivers/platform/x86/dell-laptop.c
index 0db2dbf7b0d1..fb071e6a5058 100644
--- a/drivers/platform/x86/dell-laptop.c
+++ b/drivers/platform/x86/dell-laptop.c
@@ -29,7 +29,6 @@
 #include <linux/mm.h>
 #include <linux/i8042.h>
 #include <linux/debugfs.h>
-#include <linux/dell-led.h>
 #include <linux/seq_file.h>
 #include <acpi/video.h>
 #include "dell-rbtn.h"
@@ -2109,17 +2108,17 @@ static struct notifier_block dell_laptop_notifier = {
 	.notifier_call = dell_laptop_notifier_call,
 };
 
-int dell_micmute_led_set(int state)
+static int micmute_led_set(struct led_classdev *led_cdev,
+			   enum led_brightness brightness)
 {
 	struct calling_interface_buffer buffer;
 	struct calling_interface_token *token;
+	int state = brightness != LED_OFF;
 
 	if (state == 0)
 		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_DISABLE);
-	else if (state == 1)
-		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
 	else
-		return -EINVAL;
+		token = dell_smbios_find_token(GLOBAL_MIC_MUTE_ENABLE);
 
 	if (!token)
 		return -ENODEV;
@@ -2127,18 +2126,7 @@ int dell_micmute_led_set(int state)
 	dell_fill_request(&buffer, token->location, token->value, 0, 0);
 	dell_send_request(&buffer, CLASS_TOKEN_WRITE, SELECT_TOKEN_STD);
 
-	return state;
-}
-EXPORT_SYMBOL_GPL(dell_micmute_led_set);
-
-static int micmute_led_set(struct led_classdev *led_cdev,
-			   enum led_brightness brightness)
-{
-	int state = brightness != LED_OFF;
-	int err;
-
-	err = dell_micmute_led_set(state);
-	return err < 0 ? err : 0;
+	return 0;
 }
 
 static struct led_classdev micmute_led_cdev = {
diff --git a/include/linux/dell-led.h b/include/linux/dell-led.h
deleted file mode 100644
index 92521471517f..000000000000
--- a/include/linux/dell-led.h
+++ /dev/null
@@ -1,7 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __DELL_LED_H__
-#define __DELL_LED_H__
-
-int dell_micmute_led_set(int on);
-
-#endif
-- 
2.19.1