Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Add P53/73 firmware to fan_quirk_table for dual fan control

2021-01-15 Thread Henrique de Moraes Holschuh
On Fri, 15 Jan 2021, jeanniestevenson wrote:
> This commit enables dual fan control for the new Lenovo P53 and P73 laptop 
> models.
> 
> Signed-off-by: Jeannie Stevenson 

It has been tested on which thinkpad model (name and numerical model,
please) ?  It is also nice to have that kind of information in the
commit message.

With that information:
Acked-by: Henrique de Moraes Holschuh 

> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index c404706379d9..69402758b99c 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8782,6 +8782,7 @@ static const struct tpacpi_quirk fan_quirk_table[] 
> __initconst = {
>   TPACPI_Q_LNV3('N', '1', 'T', TPACPI_FAN_2CTL),  /* P71 */
>   TPACPI_Q_LNV3('N', '1', 'U', TPACPI_FAN_2CTL),  /* P51 */
>   TPACPI_Q_LNV3('N', '2', 'C', TPACPI_FAN_2CTL),  /* P52 / P72 */
> + TPACPI_Q_LNV3('N', '2', 'N', TPACPI_FAN_2CTL),  /* P53 / P73 */
>   TPACPI_Q_LNV3('N', '2', 'E', TPACPI_FAN_2CTL),  /* P1 / X1 Extreme (1st 
> gen) */
>   TPACPI_Q_LNV3('N', '2', 'O', TPACPI_FAN_2CTL),  /* P1 / X1 Extreme (2nd 
> gen) */
>   TPACPI_Q_LNV3('N', '2', 'V', TPACPI_FAN_2CTL),  /* P1 / X1 Extreme (3nd 
> gen) */

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: fix: use scnprintf instead of snprintf.

2021-01-15 Thread Henrique de Moraes Holschuh
On Wed, 06 Jan 2021, Joe Perches wrote:
> On Wed, 2021-01-06 at 14:36 +0800, YANG LI wrote:
> > The snprintf() function returns the number of characters which would
> > have been printed if there were enough space, but the scnprintf()
> > returns the number of characters which were actually printed. If the
> > buffer is not large enough, then using snprintf() would result in a
> > read overflow and an information leak. This error was found with the
> > help of coccicheck.
> 
> In all cases, the buffer _is_ large enough.

Thank you for double-checking!

> _show function lengths are OK for all the uses with PAGE_SIZE.
> And it's probably better to use sysfs_emit for all the _show functions

Indeed.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 0/3] Add 3 new keycodes and use them for 3 new hotkeys on new Lenovo Thinkpads

2020-08-01 Thread Henrique de Moraes Holschuh
On Mon, 27 Jul 2020, Andy Shevchenko wrote:
> On Mon, Jul 27, 2020 at 10:49 AM Andy Shevchenko
>  wrote:
> > On Mon, Jul 27, 2020 at 10:45 AM Hans de Goede  wrote:
> > >
> > > Hi,
> > >
> > > On 7/27/20 2:50 AM, Henrique de Moraes Holschuh wrote:
> > > > On Tue, 21 Jul 2020, Dmitry Torokhov wrote:
> > > >> On Sun, Jul 19, 2020 at 07:56:49PM -0300, Henrique de Moraes Holschuh 
> > > >> wrote:
> > > >>> On Fri, 17 Jul 2020, Hans de Goede wrote:
> > > >>>> This is a simple patch-series adding support for 3 new hotkeys found
> > > >>>> on various new Lenovo Thinkpad models.
> > > >>>
> > > >>> For all three patches, pending an ack for the new keycodes by the 
> > > >>> input
> > > >>> maintainers:
> > > >>>
> > > >>> Acked-by: Henrique de Moraes Holschuh 
> > > >>
> > > >> Do you want me to merge all 3 through input tree?
> > > >
> > > > Hans, Daren, Andy, what do you prefer?
> > >
> > > Taking all this upstream through Dmitry's input tree is fine with
> > > me, but this really is up to Andy and/or Daren.
> >
> > Fine with me.
> 
> To be clear, I assume it will go thru input tree.
> If my formal tag needed, use
> Acked-by: Andy Shevchenko 

Also,
Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 0/3] Add 3 new keycodes and use them for 3 new hotkeys on new Lenovo Thinkpads

2020-07-26 Thread Henrique de Moraes Holschuh
On Tue, 21 Jul 2020, Dmitry Torokhov wrote:
> On Sun, Jul 19, 2020 at 07:56:49PM -0300, Henrique de Moraes Holschuh wrote:
> > On Fri, 17 Jul 2020, Hans de Goede wrote:
> > > This is a simple patch-series adding support for 3 new hotkeys found
> > > on various new Lenovo Thinkpad models.
> > 
> > For all three patches, pending an ack for the new keycodes by the input
> > maintainers:
> > 
> > Acked-by: Henrique de Moraes Holschuh 
> 
> Do you want me to merge all 3 through input tree?

Hans, Daren, Andy, what do you prefer?

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 0/3] Add 3 new keycodes and use them for 3 new hotkeys on new Lenovo Thinkpads

2020-07-19 Thread Henrique de Moraes Holschuh
On Fri, 17 Jul 2020, Hans de Goede wrote:
> This is a simple patch-series adding support for 3 new hotkeys found
> on various new Lenovo Thinkpad models.

For all three patches, pending an ack for the new keycodes by the input
maintainers:

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: lap or desk mode interface

2020-06-01 Thread Henrique de Moraes Holschuh
On Mon, 01 Jun 2020, Mark Pearson wrote:
>   Newer Lenovo Thinkpad platforms have support to identify whether the
>   system is on-lap or not using an ACPI DYTC event from the firmware.
> 
>   This patch provides the ability to retrieve the current mode via sysfs
>   entrypoints and will be used by userspace for thermal mode and WWAN
>   functionality
> 
> Co-developed-by: Nitin Joshi 
> Signed-off-by: Nitin Joshi 
> Reviewed-by: Sugumaran 
> Signed-off-by: Mark Pearson 

We need to take this through the kernel platform-driver-x86 ML.

It would be nice to have standard events for "latop on a surface you
don't want to burn ("lap"), and the input people might want to suggest
something, too, so I'd also ask the input maintainer.

Please resend, cc to:
platform-driver-...@vger.kernel.org

While at it there is something I noticed right away:

> +static int dytc_command(int command)
> +{
> + acpi_handle dytc_handle;
> + int output;
> +
> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", &dytc_handle))) {
> + pr_warn("Thinkpad ACPI has no DYTC interface.\n");
> + return -ENODEV;
> + }
> + if (!acpi_evalf(dytc_handle, &output, NULL, "dd", command))
> + return -EIO;
> + return output;
> +}

dytc_command() is called by dytc_lapmode_get():

> +static int dytc_lapmode_get(void)
> +{
> + int output;
> +
> + output = dytc_command(DYTC_CMD_GET);
> + if ((output == -ENODEV) || (output == -EIO))
> + return output;
> +
> + return ((output >> DYTC_GET_LAPMODE_SHIFT) &
> + DYTC_GET_ENABLE_MASK);
> +}

Which is used by dytc_lapmode_init():


> +static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
> +{
> + int res;
> +
> + dytc_available = false;
> + dytc_lapmode = dytc_lapmode_get();
> +
> + if (dytc_lapmode < 0 && dytc_lapmode != -ENODEV)
> + return dytc_lapmode;
> +
> + dytc_available = true;
> +
> + res = sysfs_create_group(&tpacpi_pdev->dev.kobj,
> + &dytc_attr_group);
> +
> + return res;
> +}

Looks like this code flow is going to spam people with pr_warn() on an
older thinkpad laptop that doesn't have DYTC.  Please fix this, it is
not strange for an older thinkpad to not have DYTC (even if it is a
decade's old thinkpad).

There is a thinkpad-acpi driver-level debug facility you can use to send
developer-level debug info (such as the init function of a subdriver did
not find what it wanted), if you want.

Also, if the code flow goes through dytc_init fine and registers the
sub-driver, you likely don't have to optimize the rest of the code flow
for DYTC disappearing from APCI tables ;-)  ENODEV silently would be
fine for something so unlikely to happen.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] T60: Xubuntu 20.04 LTS reduces maximum fan speed

2020-05-18 Thread Henrique de Moraes Holschuh
On Wed, 06 May 2020, Leonhard Klein wrote:
> I just upgraded my T60 from some old Ubuntu from ~2016 to the newest
> release and found out that the fan is really quiet --- and the thing is
> overheating under load (crashing).

Try to remove anything you had installed to "control fan speed", nothing
on Ubuntu should be touching the fans by itself.

But if it *is* doing that, check thermald and friends.  That might be
it.

And you can always use the driver's default of 'fan control disabled' to
ensure nothing can mess with the fan (it will remain in auto mode).

> When I control it manually to 7 it hardly reaches 4000rpm.
> In disengaged it slowly increases until the watchdog restores the value.

Hmm, the EC is likely taking too long to restart the tachometer in
disengaged mode, but still it should not trigger a watchdog restore.

> Is this a bug in this kernel module, or do I need to search somewhere else?

Somewhere else, unfortunately.  The thinkpad-acpi kernel module just
obeys, it won't try to change fan speed by itself, ever.

But something else might be trying to control the fan through
thinkpad-acpi, yes.  Or not applying some thermal control policy that
used to work, and thus your system overheats more easily.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCHv2 5/7] platform/x86: thinkpad_acpi: Use input_device_enabled()

2020-05-15 Thread Henrique de Moraes Holschuh
On Fri, 15 May 2020, Andrzej Pietrasiewicz wrote:
> Use the new helper. Inspecting input device's 'users' member needs to be
> done under device's mutex, so add appropriate invocations.
> 
> Signed-off-by: Andrzej Pietrasiewicz 

Acked-by: Henrique de Moraes Holschuh 

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 0f704484ae1d..8ae11b8c3ebb 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -2671,9 +2671,10 @@ static void hotkey_poll_setup(const bool may_warn)
>   const u32 poll_driver_mask = hotkey_driver_mask & hotkey_source_mask;
>   const u32 poll_user_mask = hotkey_user_mask & hotkey_source_mask;
>  
> + mutex_lock(&tpacpi_inputdev->mutex);
>   if (hotkey_poll_freq > 0 &&
>   (poll_driver_mask ||
> -  (poll_user_mask && tpacpi_inputdev->users > 0))) {
> +  (poll_user_mask && input_device_enabled(tpacpi_inputdev {
>   if (!tpacpi_hotkey_task) {
>   tpacpi_hotkey_task = kthread_run(hotkey_kthread,
>   NULL, TPACPI_NVRAM_KTHREAD_NAME);
> @@ -2690,6 +2691,7 @@ static void hotkey_poll_setup(const bool may_warn)
> poll_user_mask, poll_driver_mask);
>   }
>   }
> + mutex_unlock(&tpacpi_inputdev->mutex);
>  }
>  
>  static void hotkey_poll_setup_safe(const bool may_warn)

-- 
  Henrique Holschuh

  "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


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v4] thinkpad_acpi: Add support for dual fan control on select models

2020-04-30 Thread Henrique de Moraes Holschuh
On Wed, 29 Apr 2020, Stefan Assmann wrote:
> On 29.04.20 02:20, la...@apache.org wrote:
> > Do you have a use case for that behavior?
> > 
> > The previous patch broke the /proc interface, didn't not work with the 
> > current version of thinkfan
> > (but a a version with multi-fan support is in the works), and it had hard 
> > to track internal mutable state.
> > 
> > The proposed change is clean on all these fronts.
> > 
> > I'm not a fan of surprising the user with unnecessarily complex behavior 
> > (but perhaps this can be added as an option in the future.)
> 
> I concur to keep the patch as is. Any additional functionality could be
> added later on, if deemed necessary.

Yes, but let's avoid changing exposed APIs as much as possible...

Anyway, the correct interface *when exposing both fans* is:

1. Use the "hwmon" sysfs interface to expose each fan separately, and
control them separately.

1a. it is quite acceptable to control them in group by default, and have
a module parameter to select grouped, or separate behavior.

2. /proc/acpi/ibm/fan shall control both of them at the same time, and
report data from the first fan.  THIS INTERFACE IS FROZEN, LET'S NOT
MESS WITH IT.

Also, "fail-safe" for this is to have the two fans on automatic, and to
enable both fans.

All of that said, *it is very much acceptable* to merge a first set of
patches that controls both fans simultaneously and exposes the fan group
as if it were just the main fan, and later improve on the patch to
expose the second fan separately (provided safe group behavior is
maintained, see above).

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] thinkpad_acpi: unhandled HKEY event , kernel NULL pointer dereference

2019-12-08 Thread Henrique de Moraes Holschuh
On Mon, 02 Dec 2019, Jenny wrote:
> Whenever I reattach the keyboard cover, the tablet becomes unresponsive
> to inputs shortly after and has to be rebooted forcefully.

...

> [  157.064177] int3403 thermal INT3403:01: Unsupported event [0x91]
> [  157.064181] int3403 thermal INT3403:00: Unsupported event [0x91]
> [  159.581940] thinkpad_acpi: unhandled HKEY event 0x4012
> [  159.581942] thinkpad_acpi: please report the conditions when this
> event happened to ibm-acpi-devel@lists.sourceforge.net
> [  159.583681] int3403 thermal INT3403:01: Unsupported event [0x91]
> [  159.583684] int3403 thermal INT3403:00: Unsupported event [0x91]

The above are Red Herrings, but...

> [  173.851420] usb 1-7: USB disconnect, device number 4
> [  173.897429] hid-rmi 0003:17EF:6085.0009: failed to write hid report
> (-19)
> [  173.897438] hid-rmi 0003:17EF:6085.0009: rmi_set_page: set page
> failed: -19.
> [  173.897444] rmi4_physical rmi4-01: rmi_driver_clear_irq_bits: Failed
> to change enabled interrupts!
> [  173.921419] BUG: kernel NULL pointer dereference, address:
> 0040
> [  173.921432] #PF: supervisor read access in kernel mode
> [  173.921440] #PF: error_code(0x) - not-present page
> [  173.921445] PGD 0 P4D 0 
> [  173.921460] Oops:  [#1] SMP NOPTI
> [  173.921473] CPU: 3 PID: 27 Comm: kworker/3:0 Not tainted 5.3.12-
> 300.fc31.x86_64 #1
> [  173.921480] Hardware name: LENOVO 20GHS1UK00/20GHS1UK00, BIOS
> N1LET86W (1.86 ) 06/26/2019
> [  173.921500] Workqueue: usb_hub_wq hub_event
> [  173.921517] RIP: 0010:__irq_domain_deactivate_irq+0x14/0x40
> [  173.921527] Code: 05 e9 c0 28 ab 00 c3 66 66 2e 0f 1f 84 00 00 00 00
> 00 0f 1f 40 00 0f 1f 44 00 00 48 85 ff 74 2f 53 48 89 fb eb 1e 48 8b 47
> 18 <48> 8b 40 40 48 85 c0 74 08 48 89 de e8 8b 28 ab 00 48 8b 5b 28 48
> [  173.921536] RSP: 0018:bbf6801638b8 EFLAGS: 00010082
> [  173.921545] RAX:  RBX: 9f8251818c28 RCX:
> 0085
> [  173.921552] RDX: 0001 RSI: 0246 RDI:
> 9f8288891b40
> [  173.921559] RBP: 9f8251818ca4 R08:  R09:
> 9f828def3280
> [  173.921564] R10:  R11: bd4554c8 R12:
> 9f8251bf4c00
> [  173.921570] R13: 9f8251818d60 R14: 0246 R15:
> 9f8251818c28
> [  173.921579] FS:  () GS:9f829138()
> knlGS:
> [  173.921587] CS:  0010 DS:  ES:  CR0: 80050033
> [  173.921594] CR2: 0040 CR3: 000407012006 CR4:
> 003606e0
> [  173.921601] DR0:  DR1:  DR2:
> 
> [  173.921607] DR3:  DR6: fffe0ff0 DR7:
> 0400
> [  173.921613] Call Trace:
> [  173.921632]  irq_domain_deactivate_irq+0x1a/0x30
> [  173.921646]  __free_irq+0x242/0x300
> [  173.921658]  free_irq+0x37/0x60
> [  173.921670]  release_nodes+0x239/0x280
> [  173.921691]  device_release_driver_internal+0xe8/0x1b0
> [  173.921703]  bus_remove_device+0xdb/0x140
> [  173.921718]  device_del+0x163/0x360
> [  173.921740]  rmi_unregister_function+0x30/0x70 [rmi_core]
> [  173.921762]  rmi_free_function_list+0x96/0x110 [rmi_core]
> [  173.921784]  rmi_driver_remove+0x48/0x50 [rmi_core]
> [  173.921798]  device_release_driver_internal+0xd8/0x1b0
> [  173.921809]  bus_remove_device+0xdb/0x140
> [  173.921823]  device_del+0x163/0x360
> [  173.921842]  rmi_unregister_transport_device+0x12/0x20 [rmi_core]
> [  173.921857]  rmi_remove+0x41/0x50 [hid_rmi]
> [  173.921874]  hid_device_remove+0x4c/0xb0
> [  173.921887]  device_release_driver_internal+0xd8/0x1b0
> [  173.921898]  bus_remove_device+0xdb/0x140
> [  173.921911]  device_del+0x163/0x360
> [  173.921926]  hid_destroy_device+0x42/0x60
> [  173.921940]  usbhid_disconnect+0x4a/0x60
> [  173.921953]  usb_unbind_interface+0x84/0x250
> [  173.921967]  device_release_driver_internal+0xd8/0x1b0
> [  173.921978]  bus_remove_device+0xdb/0x140
> [  173.921991]  device_del+0x163/0x360
> [  173.922004]  usb_disable_device+0x93/0x240
> [  173.922018]  usb_disconnect+0xba/0x260
> [  173.922032]  hub_event+0xd23/0x1490
> [  173.922048]  process_one_work+0x19d/0x340
> [  173.922057]  worker_thread+0x50/0x3b0
> [  173.922071]  kthread+0xfb/0x130
> [  173.922081]  ? process_one_work+0x340/0x340
> [  173.922093]  ? kthread_park+0x80/0x80
> [  173.922104]  ret_from_fork+0x1f/0x40

This one isn't.  It shows the RMI driver blowing up in flames because of
some bug either in the RMI driver itself, or somewhere else in the USB
stack.

I don't think thinkpad-acpi can help much on this one, it would be
better to file a bug against the RMI driver or ask for help in LKML.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] lde disk-activity trigger on Thinkpad T570/580

2019-06-30 Thread Henrique de Moraes Holschuh
On Sun, 30 Jun 2019, Frank Steiner wrote:
> > with the disk-activity trigger not being connected to NVME, and there is
> > nothing thinkpad-acpi can do about it...
> 
> Ok! I was just wondering because NVME disks in our PCs do work with
> disk-activity. Is this connection you mention a hardware problem?
> Or could it be worth to ask for a possible fix on the lkml?

I'd guess it is a kernel-level difference, yes.  Maybe some NVME drivers
work with the triggers while others don't.  I don't know much about how
NVME is currently implemented, myself.

Please do ask in LKML, preferably ask the responsible parties for the
disk-activity trigger or the people responsible for the exact NVME
driver you are using, they're more likely to know.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] lde disk-activity trigger on Thinkpad T570/580

2019-06-29 Thread Henrique de Moraes Holschuh
On Thu, 27 Jun 2019, Frank Steiner wrote:
> This works fine on all our older thinkpad models with SATA hard disks
> works also on our PCs with SSD disks (SATA as well as NVME), but
> it fails on our newer thinkpads, i.e. T570 and T580 with NVME SSDs.
> 
> The LED just stays off all the time no matter what we do with the hard
> disk.

If any other trigger (e.g. heartbeat) works, that means the issue is
with the disk-activity trigger not being connected to NVME, and there is
nothing thinkpad-acpi can do about it...

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


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

2018-12-04 Thread Henrique de Moraes Holschuh
On Mon, 03 Dec 2018, Andy Shevchenko wrote:
> On Fri, Nov 16, 2018 at 3:18 PM Jouke Witteveen  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 

there you have it :-)

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


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

2018-11-27 Thread Henrique de Moraes Holschuh
On Mon, 26 Nov 2018, Takashi Iwai wrote:
> this is patch series I've hacked after useful conversation with
> Pavel.  Basically this adds a new LED trigger audio-mute and
> audio-micmute, and convert the HD-audio driver and the platform
> drivers to use the LED trigger instead of the ugly direct dynamic
> symbol binding.

For the thinkpad-acpi bits, provided that Andy's comments are addressed:
Acked-by: Henrique de Moraes Holschuh 

> I checked briefly on my Dell laptop, and a Thinkpad model.

Thanks :-)

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

2018-10-23 Thread Henrique de Moraes Holschuh
On Tue, 23 Oct 2018, Jouke Witteveen wrote:
> > Apparently it has to be fixed properly, because if I understood the
> > issue correctly, every two-character quirk will trigger on *any*
> > three-character model that starts with those two characters.
> 
> This should be (and hopefully is) incorrect.
> In the tpacpi_check_quirks function, equality between the quirk bios
> and the actual model bios is tested. For two-character models there
> are three nonzero bytes, for two character models there are only two.

Hmm, good.  Need to check the parser that is feeding it, though.

One actually has to parse a DMI string like aaETxxWW (old models) and
aaaET  (need to actually verify how the ending goes on three-char
models).  This for BIOS.  For EC, it used to be HT instead of ET for
H8S-based embedded controllers, maybe it changed for the new ARC-based
embedded controllers.

> That said, I do wonder why the fan quirks redefine all matching macros
> (TPACPI_FAN_Q*) and don't use the generic ones (TPACPI_Q*). This looks
> like an easy thing to clean up.

They used to have some minor differences.  A cleanup patch is likely
fine, as long as it is tested.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

2018-10-22 Thread Henrique de Moraes Holschuh
On Mon, 15 Oct 2018, Dmitry Torokhov wrote:
> On Sat, Oct 13, 2018 at 6:32 AM Jaak Ristioja  wrote:
> > On 27.08.2018 19:22, Jaak Ristioja wrote:
> > > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > > spurious input events, mostly right mouse button click events, but also
> > > cursor jumps.
> > >
> > > I have not attempted to understand whether these events come from the
> > > trackpoint or touchpad or some other driver, but I managed to bisect
> > > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> > >
> > > Apparently the quirk mitigation is applied when the BIOS version begins
> > > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > > which is probably why this commit causes the described problems on the
> > > ThinkPad L570. How exactly? - I don't know.
> > >
> > > The issue did not reproduce when running some stable 4.17 and 4.18
> > > kernels with commit a986c75a7df0 reverted.
> > >
> > > Please fix this for future kernels. Thanks! :)
> >
> > Ping. Do you need any additional information?
> 
> Sounds like we need tighter check for the quirk, maybe based on
> DMI/Board name? Can we revert the offending commit for now?

Apparently it has to be fixed properly, because if I understood the
issue correctly, every two-character quirk will trigger on *any*
three-character model that starts with those two characters.

We could revert the three-character quirk support, but this would cause
regressions as well.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] 2nd Fan quirk for Thinkpad P50 causes spurios touchpad/trackpoint events on ThinkPad L570

2018-10-22 Thread Henrique de Moraes Holschuh
On Sat, 13 Oct 2018, Jaak Ristioja wrote:
> On 27.08.2018 19:22, Jaak Ristioja wrote:
> > Upgrading Linux from 4.16 to 4.17, a ThinkPad L570 started receiving
> > spurious input events, mostly right mouse button click events, but also
> > cursor jumps.
> > 
> > I have not attempted to understand whether these events come from the
> > trackpoint or touchpad or some other driver, but I managed to bisect
> > this issue to commit a986c75a7df0 titled "platform/x86: thinkpad_acpi:
> > Add 2nd Fan Support for Thinkpad P50" by Alexander Kappner.
> > 
> > Apparently the quirk mitigation is applied when the BIOS version begins
> > with N1. The BIOS version on the L570 in question is N1XET57W (1.35 )
> > which is probably why this commit causes the described problems on the
> > ThinkPad L570. How exactly? - I don't know.
> > 
> > The issue did not reproduce when running some stable 4.17 and 4.18
> > kernels with commit a986c75a7df0 reverted.
> > 
> > Please fix this for future kernels. Thanks! :)
> 
> Ping. Do you need any additional information?

It looks like it is failing to detect N1X and N1 as different models,
the bug is likely on this commit:

commit 846a416b4630fc960c10500b4194ad504bc22d4b
platform/x86: thinkpad_acpi: Proper model/release matching

Real life has been **extremely** busy, so I'd appreciate a patch.
Otherwise I will eventually get to it, but it might take a few
weeks(!!).

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: Fix multi-battery bug

2018-09-07 Thread Henrique de Moraes Holschuh
On Fri, 07 Sep 2018, Yves-Alexis Perez wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
> 
> On Fri, 2018-09-07 at 19:59 +0200, Thomas Weißschuh wrote:
> > > was this included in any tree and is it on route to Linus somehow?
> > 
> > This is on track to be released in 4.19.
> 
> Thanks. I've applied the three patches (this one and the two from Jouke) on
> top of 4.18.6 and I confirm I can set the thresholds on BAT1.
> > 
> > > My ThinkPad X250 has two batteries and I can only configure threshold on 
> > > BAT0,
> > > and I guess it's because of this, so it'd be nice to have it fixed (and 
> > > maybe
> > > backported to relevant stable kernels).
> > 
> > As far as I know only changes that fix behaviour that worked before are
> > eligible for stable. As this specific functionality never worked before I
> > figured it would be moot to also send it to stable.
> 
> Well, it does somehow work when you have one battery, but it's really
> frustrating to have it not work for the second one.
> > 
> > (If it is fine to send stuff like this to stable, we could try, though)
> 
> Indeed, not sure if “frustrating” is reason enough for stable :)

FWIW, once given enough testing, yes, I think this should be proposed
for -stable.

-- 
  Henrique Holschuh


___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Extend battery quirk coverage

2018-08-01 Thread Henrique de Moraes Holschuh
On Wed, 01 Aug 2018, Andy Shevchenko wrote:
> On Wed, Aug 1, 2018 at 5:32 PM, Jouke Witteveen  wrote:
> > Based on bug reports and a web search for
> > "Thinkpad_acpi: Error probing battery 2"
> > four more models were found that require the battery quirk:
> > Lenovo B5400, Thinkpad 11e (original and gen 3), Thinkpad 13 gen 3.
> 
> I think you may add Henrique's ACK here since he did for previous "v2".
> But I let him speak for himself.

Yeah, the ack is still valid.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2 2/2] platform/x86: thinkpad_acpi: Support battery quirk

2018-08-01 Thread Henrique de Moraes Holschuh
On Wed, 01 Aug 2018, Jouke Witteveen wrote:
> Some Thinkpads have a single battery, but expose it as BAT1. Use the quirks
> engine to force these machines into always addressing the primary battery.
> Without this, the battery name would resolve to the non-existent secondary
> battery and ACPI calls would fail.
> 
> Signed-off-by: Jouke Witteveen 

Acked-by: Henrique de Moraes Holschuh 

> Compared to the previous version of this patch, four new models were found
> that need the quirk. It was tested on the Thinkpad 13 (three character ID)
> and the Thinkpad 11e (two character ID).
> 
> The other three IDs were scraped from bug reports where the driver reports
> 
> Thinkpad_acpi: Error probing battery 2
> 
> These bug reports were found by a simple web search.
> 
>  drivers/platform/x86/thinkpad_acpi.c | 27 +--
>  1 file changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 2cd3ca7e..bc9a11b8 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -335,6 +335,7 @@ static struct {
>   u32 second_fan:1;
>   u32 beep_needs_two_args:1;
>   u32 mixer_no_level_control:1;
> + u32 battery_force_primary:1;
>   u32 input_device_registered:1;
>   u32 platform_drv_registered:1;
>   u32 platform_drv_attrs_registered:1;
> @@ -343,7 +344,6 @@ static struct {
>   u32 sensors_pdev_attrs_registered:1;
>   u32 hotkey_poll_active:1;
>   u32 has_adaptive_kbd:1;
> - u32 battery:1;
>  } tp_features;
>  
>  static struct {
> @@ -471,6 +471,12 @@ do { 
> \
> .ec = TPACPI_MATCH_ANY,   \
> .quirks = (__quirk) }
>  
> +#define TPACPI_Q_LNV3(__id1, __id2, __id3, __quirk) \
> + { .vendor = PCI_VENDOR_ID_LENOVO,   \
> +   .bios = TPID3(__id1, __id2, __id3),   \
> +   .ec = TPACPI_MATCH_ANY,   \
> +   .quirks = (__quirk) }
> +
>  #define TPACPI_QEC_LNV(__id1, __id2, __quirk)\
>   { .vendor = PCI_VENDOR_ID_LENOVO,   \
> .bios = TPACPI_MATCH_ANY, \
> @@ -9423,7 +9429,8 @@ static int tpacpi_battery_probe(int battery)
>  static int tpacpi_battery_get_id(const char *battery_name)
>  {
>  
> - if (strcmp(battery_name, "BAT0") == 0)
> + if (strcmp(battery_name, "BAT0") == 0 ||
> + tp_features.battery_force_primary)
>   return BAT_PRIMARY;
>   if (strcmp(battery_name, "BAT1") == 0)
>   return BAT_SECONDARY;
> @@ -9599,8 +9606,24 @@ static struct acpi_battery_hook battery_hook = {
>  
>  /* Subdriver init/exit */
>  
> +static const struct tpacpi_quirk battery_quirk_table[] __initconst = {
> + /*
> +  * Individual addressing is broken on models that expose the
> +  * primary battery as BAT1.
> +  */
> + TPACPI_Q_LNV('J', '7', true),   /* B5400 */
> + TPACPI_Q_LNV('J', 'I', true),   /* 11e */
> + TPACPI_Q_LNV3('R', '0', 'B', true), /* 11e gen 3 */
> + TPACPI_Q_LNV3('R', '0', 'C', true), /* 13 */
> + TPACPI_Q_LNV3('R', '0', 'J', true), /* 13 gen 2 */
> +};
> +
>  static int __init tpacpi_battery_init(struct ibm_init_struct *ibm)
>  {
> + tp_features.battery_force_primary = tpacpi_check_quirks(
> + battery_quirk_table,
> + ARRAY_SIZE(battery_quirk_table));
> +
>   battery_hook_register(&battery_hook);
>   return 0;
>  }

-- 
  Henrique Holschuh

  "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

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2 1/2] platform/x86: thinkpad_acpi: Proper model/release matching

2018-08-01 Thread Henrique de Moraes Holschuh
On Wed, 01 Aug 2018, Jouke Witteveen wrote:
> Modern Thinkpads have three character model designators. Previously, they
> would be accepted, but recorded incompletely. Revision matching extracted
> the wrong bytes from the ID string. This made the use of quirks for modern
> machines impossible.
> 
> Fixes: 1b0eb5bc2413
> Signed-off-by: Jouke Witteveen 

Acked-by: Henrique de Moraes Holschuh 

> This has now been tested on both 3-character models (Thinkpad 13) and two
> character models (Thinkpad 11e).
> The part one of this series is identical to the previously submitted patch.
> 
>  drivers/platform/x86/thinkpad_acpi.c | 98 ++--
>  1 file changed, 48 insertions(+), 50 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index cae9b059..2cd3ca7e 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -358,9 +358,9 @@ struct thinkpad_id_data {
>   char *bios_version_str; /* Something like 1ZET51WW (1.03z) */
>   char *ec_version_str;   /* Something like 1ZHT51WW-1.04a */
>  
> - u16 bios_model; /* 1Y = 0x5931, 0 = unknown */
> - u16 ec_model;
> - u16 bios_release;   /* 1ZETK1WW = 0x314b, 0 = unknown */
> + u32 bios_model; /* 1Y = 0x3159, 0 = unknown */
> + u32 ec_model;
> + u16 bios_release;   /* 1ZETK1WW = 0x4b31, 0 = unknown */
>   u16 ec_release;
>  
>   char *model_str;/* ThinkPad T43 */
> @@ -444,17 +444,20 @@ do {
> \
>  /*
>   * Quirk handling helpers
>   *
> - * ThinkPad IDs and versions seen in the field so far
> - * are two-characters from the set [0-9A-Z], i.e. base 36.
> + * ThinkPad IDs and versions seen in the field so far are
> + * two or three characters from the set [0-9A-Z], i.e. base 36.
>   *
>   * We use values well outside that range as specials.
>   */
>  
> -#define TPACPI_MATCH_ANY 0xU
> +#define TPACPI_MATCH_ANY 0xU
> +#define TPACPI_MATCH_ANY_VERSION 0xU
>  #define TPACPI_MATCH_UNKNOWN 0U
>  
> -/* TPID('1', 'Y') == 0x5931 */
> -#define TPID(__c1, __c2) (((__c2) << 8) | (__c1))
> +/* TPID('1', 'Y') == 0x3159 */
> +#define TPID(__c1, __c2) (((__c1) << 8) | (__c2))
> +#define TPID3(__c1, __c2, __c3)  (((__c1) << 16) | ((__c2) << 8) | 
> (__c3))
> +#define TPVER TPID
>  
>  #define TPACPI_Q_IBM(__id1, __id2, __quirk)  \
>   { .vendor = PCI_VENDOR_ID_IBM,  \
> @@ -476,8 +479,8 @@ do {  
> \
>  
>  struct tpacpi_quirk {
>   unsigned int vendor;
> - u16 bios;
> - u16 ec;
> + u32 bios;
> + u32 ec;
>   unsigned long quirks;
>  };
>  
> @@ -1647,16 +1650,16 @@ static void tpacpi_remove_driver_attributes(struct 
> device_driver *drv)
>   { .vendor   = (__v),\
> .bios = TPID(__id1, __id2),   \
> .ec   = TPACPI_MATCH_ANY, \
> -   .quirks   = TPACPI_MATCH_ANY << 16\
> -   | (__bv1) << 8 | (__bv2) }
> +   .quirks   = TPACPI_MATCH_ANY_VERSION << 16 \
> +   | TPVER(__bv1, __bv2) }
>  
>  #define TPV_Q_X(__v, __bid1, __bid2, __bv1, __bv2,   \
>   __eid, __ev1, __ev2)\
>   { .vendor   = (__v),\
> .bios = TPID(__bid1, __bid2), \
> .ec   = __eid,\
> -   .quirks   = (__ev1) << 24 | (__ev2) << 16 \
> -   | (__bv1) << 8 | (__bv2) }
> +   .quirks   = TPVER(__ev1, __ev2) << 16 \
> +   | TPVER(__bv1, __bv2) }
>  
>  #define TPV_QI0(__id1, __id2, __bv1, __bv2) \
>   TPV_Q(PCI_VENDOR_ID_IBM, __id1, __id2, __bv1, __bv2)
> @@ -1798,7 +1801,7 @@ static void __init tpacpi_check_outdated_fw(void)
>   /* note that unknown versions are set to 0x and we use that */
>   if ((bios_version > thinkpad_id.bios_release) ||
>   (ec_version > thinkpad_id.ec_release &&
> - ec_version != TPACPI_MATCH_ANY)) {
> + ec_version != TPACPI_MATCH_ANY_VERSION)) {
>   /*
>* The changelogs would let us track down the exact
>* reason, but it is just too much of a pain to track
> @@ -9808,36 +9811,37 @@ static int __init ibm_init(struct

Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: don't overwrite supported battery operations

2018-08-01 Thread Henrique de Moraes Holschuh
On Mon, 30 Jul 2018, Thomas Weißschuh wrote:
> On Sat, 2018-06-16T11:39+0200, Thomas Weißschuh wrote:
> > Previously the struct containing the supported operations for all
> > batteries was zeroed every time a battery was probed.
> > This prevented all batteries except the lastly probed one from being
> > configured.
> 
> Is something wrong with this patch?
> If so I would like to remedy it.

Nah, it is fine.  Please resend it to
platform-driver-...@vger.kernel.org and I will ack it there.

However, it will have a trivial conflict with an already accepted patch
that adds quirk handling to tpacpi_battery_init().  The fix is obvious
(place the memset before the quirk handler is called).

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 2/2] platform/x86: thinkpad_acpi: Support battery quirk

2018-07-11 Thread Henrique de Moraes Holschuh
On Wed, 11 Jul 2018, Jouke Witteveen wrote:
> Some Thinkpads have a single battery, but expose it as BAT1. Use the quirks
> engine to force these machines into always addressing the primary battery.
> Without this, the battery name would resolve to the non-existent secondary
> battery and ACPI calls would fail.
> 
> Signed-off-by: Jouke Witteveen 

Needs patch 1/1 to be acked-by first before being accepted, but otherwise:

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/2] platform/x86: thinkpad_acpi: Proper model/release matching

2018-07-11 Thread Henrique de Moraes Holschuh
On Wed, 11 Jul 2018, Jouke Witteveen wrote:
> Modern Thinkpads have three character model designators. Previously, they
> would be accepted, but recorded incompletely. Revision matching extracted
> the wrong bytes from the ID string. This made the use of quirks for modern
> machines impossible.
> 
> Fixes: 1b0eb5bc2413
> Signed-off-by: Jouke Witteveen 

In which thinkpad models did you test this?  It would be interesting
to have that in the commit message, please.  It needs to be tested on a
two-char and a three-char model.

If necessary, I can test on a T43 if you don't have a two-char model
around, but that might take a few days.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Add support for calculator hotkey

2018-06-20 Thread Henrique de Moraes Holschuh
On Tue, 19 Jun 2018, Benjamin Berg wrote:
> > That said, it would be nice to know if other models also have this
> > hotkey, especially if it has a different meaning (in which case we just
> > special-case the keymap in the driver, so it is *not* a problem).
> 
> I believe this is not specific to the P52 but simply the keyboard
> version that includes a numpad. I only have the P52, but looking at
> photos of e.g. the T570 and T580 the keyboard looks identical there.

Hopefully we get a report from an user of these keyboards.  Identical is
good, the only issue would be if they were not identical...

> On a different note, these keyboards also have "=", "(" and ")" keys
> above the numpad. These keys emit the key combinations required for a
> US keyboard layout on the P52 I have. I do believe there is a way to
> set the keyboard layout through ACPI which I suspect changes the
> emitted keycodes to match the expected values for other layouts.
> Unfortunately supporting that seems quite involved.

Yikes.  Hopefully we won't need to mess with this.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Add support for calculator hotkey

2018-06-19 Thread Henrique de Moraes Holschuh
On Tue, 19 Jun 2018, Benjamin Berg wrote:
> The P52 has a keyboard which features a calculator key above the numpad.
> Add support for this the calculator key (0x1313).
> 
> Signed-off-by: Benjamin Berg 
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index cae9b0595692..6c979fe44ea7 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -1928,7 +1928,7 @@ enum {  /* hot key scan codes (derived from ACPI DSDT) 
> */
>   /* first new observed key (star, favorites) is 0x1311 */
>   TP_ACPI_HOTKEYSCAN_STAR = 69,
>   TP_ACPI_HOTKEYSCAN_CLIPPING_TOOL2,
> - TP_ACPI_HOTKEYSCAN_UNK25,
> + TP_ACPI_HOTKEYSCAN_CALCULATOR,
>   TP_ACPI_HOTKEYSCAN_BLUETOOTH,
>   TP_ACPI_HOTKEYSCAN_KEYBOARD,
>  
> @@ -3449,7 +3449,7 @@ static int __init hotkey_init(struct ibm_init_struct 
> *iibm)
>  
>   KEY_FAVORITES,   /* Favorite app, 0x311 */
>   KEY_RESERVED,/* Clipping tool */
> - KEY_RESERVED,
> + KEY_CALC,/* Calculator (above numpad), 0x313 */

Replace 0x313 with (P52) please.

Other than that,
Acked-by: Henrique de Moraes Holschuh 


That said, it would be nice to know if other models also have this
hotkey, especially if it has a different meaning (in which case we just
special-case the keymap in the driver, so it is *not* a problem).

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: use consistent return values

2018-06-16 Thread Henrique de Moraes Holschuh
On Sat, 16 Jun 2018, Thomas Weißschuh wrote:
> The error codes for ACPI failures when setting battery start and stop
> thresholds were gobbling the real error code and also inconsistent.

Well, we do not care for ACPI failure codes (other than the fact it
failed): we care for proper errno return to userspace, which is to be
enforced on *this* function, so, "return rval" is not what we want
here...

> - if (tpacpi_battery_set(THRESHOLD_START, battery, value))
> - return -ENODEV;
> + rval = tpacpi_battery_set(THRESHOLD_START, battery, value);
> + if (rval)
> + return rval;

Before, a side-effect of this was to report that the device isn't there,
I very much think this was done on purpose.

Ognjen, what would be the best interpretation of a failure on
THRESHOLD_START ?  -ENODEV (current code) or -EINVAL ?

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

2018-06-16 Thread Henrique de Moraes Holschuh
On Sat, 16 Jun 2018, Pavel Machek wrote:
> Question is if we want flexibility or security.

For thinkpad-acpi, it is security by default.  Flexibility is allowed as
*compile*-time (Kconfig) option, and only as long as it defaults to
secure *and* the help text is very explicit at instructing distros to
NOT enable this option.

That said, if mic-mute is under HDA mixer control, it will only be
"secure" if ALSA is also blocking userspace access to the relevant bits,
and if that is not possible to do properly, might as well go for
flexibility.  But only in that case.

That would be why I use phisical shutters on embedded webcams unless the
hardware actually airgaps them from the USB bus when the privacy switch
is active.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

2018-06-15 Thread Henrique de Moraes Holschuh
On Fri, 15 Jun 2018, Pali Rohár wrote:
> This means that kernel should not export any led class device. Or when
> exported, then "set" operation should always fail.

"not export" is right.

> > 2. Otherwise implement it in-kernel, so that userspace cannot unmute
> >when the human has activated the "mute" switch, and the LED cannot be
> >controlled by userspace to lie (report mute when it is not mute).
> 
> This looks like a good candidate to use led "trigger" interface. Create
> a mute trigger and attach it to that led device.

Maybe, as long as done in-kernel and not possible to mess with from
userspace.

> means set. What is SHDA doing, I have not figured out. It does not

Look at the HDA mixer state, SHDA is likely to be directly messing with
it.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad T480s & LED_MUTE, LED_MICMUTE

2018-06-15 Thread Henrique de Moraes Holschuh
On Fri, 15 Jun 2018, Pali Rohár wrote:
> Henrique, any idea why there are no exported led classes for mute and
> micmute? And how are suppose to be controlled?

I have to look into the code, it was contributed by someone who had
access to the proper hardware to test it.

But the way *I* would like it to work is this:

1. When implemented in *hardware* or *EC*, let the hardware and EC take
   full control, and never allow the operating system to mess with it.
   So, it becomes much harder for that LED to "lie".

2. Otherwise implement it in-kernel, so that userspace cannot unmute
   when the human has activated the "mute" switch, and the LED cannot be
   controlled by userspace to lie (report mute when it is not mute).

It might, or might not be possible to achieve the above.

> > With thinkpad_acpi.ko unloaded, hardware drives the LEDs, so nothing
> > for us to do...
> 
> So somehow tell thinkpad_acpi.ko to let hardware control those LEDs when
> thinkpad_acpi.ko is loaded?

I.e. look into the DSDT and XSDT, to find out what it is doing.  It will
be there: it is very rare for the thinkpad EC itself to implement these
behavior changes.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad T480s & thinkpad_acpi.ko

2018-06-08 Thread Henrique de Moraes Holschuh
On Fri, 08 Jun 2018, Pali Rohár wrote:
> which has only 4.9 kernel and LEN0268 identifier was added in git commit
> a3c42a467a254a17236ab817d5c7c6bc054e4f84 for 4.10.

You could always request it to be backported to -stable *if* you
manually apply it to a 4.9 kernel and test the heck out of it...

Debian would pick it up from -stable after a while.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 2/3] thinkpad_acpi: add support for force_discharge

2018-05-24 Thread Henrique de Moraes Holschuh
On Mon, 21 May 2018, Kevin Locke wrote:
> as-expected.  The only oddity I noticed is that force_discharge has a
> delay taking effect (<1 sec) transitioning from 0 to 1 (but not 1 to
> 0).

The EC can take its sweet time to obey any such requests ;-)

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge

2018-05-14 Thread Henrique de Moraes Holschuh
On Mon, 14 May 2018, Ognjen Galić wrote:
> On Mon, May 14, 2018 at 08:39:28AM -0300, Henrique de Moraes Holschuh wrote:
> > On Mon, 14 May 2018, Christoph Böhmwalder wrote:
> > > > +   case INHIBIT_CHARGE:
> > > > +   if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, 
> > > > ret, battery))
> > > > +   return -ENODEV;
> > > > +
> > > > +   /* The inhibit charge status is in the first bit */
> > > > +   *ret = *ret & 0x01;
> > > > +   return 0;
> > 
> > Do we know what is in the other bits?  If so, please document the ACPI
> > method using a comment somewhere in the driver code, like you did for
> > SET_INHIBIT.
> 
> I got the specs for the methods in a Lenovo doc I was told not to share
> around, including information in it.

Hmm, ok. Please send to me in private the email address of the person I
should ask for the documentation since you cannot distribute the
documentation itself.  This isn't the first time it happened.

The usual rule I follow is: document what is either already being used
in the driver or which very likely wil have to be used in the driver.
Unrelated stuff (e.g. for some unrelated functionality the driver does
not implement) doesn't have to be documented.

In this case, we need to at least know what bits need to be RMW, or
always-zero when calling SET_* methods...

> So what do you people want me to do? Should I fix the comments or leave
> as-is?

Since the subsystem maintainer said it should be kept as-is, and I don't
much care, please keep it as-is.  It won't matter for my driver
maintainer ack :-)

> > > > +   case INHIBIT_CHARGE:
> > > > +   if (!battery_info.batteries[battery].inhibit_support)
> > > > +   return -ENODEV;
> > > > +   /* The only valid values are 1 and 0 */
> > > > +   if (value != 0 && value != 1)
> > > 
> > > I'm not sure, but maybe `if (value < 2)` is better here?
> > 
> > Indeed... with a comment that says 0 = main battery, 1 = extra/dock
> > battery or something.
> 
> That seems like obfuscation to me, this way its clear that it must be
> either 1 or 0. And inhibiting is set per-battery so 1 is on and not
> battery 1.

Hmm, yes I got that wrong. "battery" should use 0 <= battery <2 for
checking (if needed), but value can only be 0 or 1.

I'd personally have used a bitmask test for "value", ensuring all other
bits are zero, though.  But that's just a matter of taste and I don't
think any of the choices (including the one you used) are "bad taste",
so all of them are fine as far as I am concerned.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 3/3] thinkpad_acpi: document the battery controls

2018-05-14 Thread Henrique de Moraes Holschuh
On Sun, 13 May 2018, Ognjen Galic wrote:
> +Battery force discharging
> +--
> +
> +There is also support for AC overriding. This means that you can force the 
> battery to discharge
> +even when AC is attached. This is also exposed via the generic ACPI driver:
> +
> +/sys/class/power_supply/BATx/force_discharge [int, 1, 0]
> +
> +Setting it to 1 forces the battery to discharge on AC.

This is used to run a battery "fuel gauge" callibration cycle.

One has to set the limits to 0,100 (i.e. disable the anti-wear
limiters), trigger a force_discharge, and wait until it discharges to
zero.  At that point, the EC is supposed to reset the force_discharge
bit by itself, and then you have to wait the battery to charge back to
full.

(and at least on older thinkpads, where you had to use SMAPI to do the
above, it didn't need much help from the operating system.  Once a
couple years you could just set force_discharge on both batteries (with
the thing connected to AC), shutdown the operating system, and go to
sleep.  By morning, it would be fully charged and both gauges
calibrated, still powered off, ready to go :P ).

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/3] thinkpad_acpi: add support for inhibit_charge

2018-05-14 Thread Henrique de Moraes Holschuh
On Mon, 14 May 2018, Christoph Böhmwalder wrote:
> > +   case INHIBIT_CHARGE:
> > +   if ACPI_FAILURE(tpacpi_battery_acpi_eval(GET_INHIBIT, ret, 
> > battery))
> > +   return -ENODEV;
> > +
> > +   /* The inhibit charge status is in the first bit */
> > +   *ret = *ret & 0x01;
> > +   return 0;

Do we know what is in the other bits?  If so, please document the ACPI
method using a comment somewhere in the driver code, like you did for
SET_INHIBIT.

> > default:
> > pr_crit("wrong parameter: %d", what);
> > return -EINVAL;
> > @@ -9343,6 +9357,21 @@ static int tpacpi_battery_set(int what, int battery, 
> > int value)
> > return -ENODEV;
> > }
> > return 0;
> > +   case INHIBIT_CHARGE:
> > +   /* When setting inhbitit charge, we set a default vaulue of
> 
> This comment does not adhere to the Linux coding style

Much on the driver doesn't, because it is _OLD_.  But yeah, it is
preferrable to fix this as we add code, so it would be good to have all
new (and modified) comments switched to modern kernel style.

> > +   case INHIBIT_CHARGE:
> > +   if (!battery_info.batteries[battery].inhibit_support)
> > +   return -ENODEV;
> > +   /* The only valid values are 1 and 0 */
> > +   if (value != 0 && value != 1)
> 
> I'm not sure, but maybe `if (value < 2)` is better here?

Indeed... with a comment that says 0 = main battery, 1 = extra/dock
battery or something.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi: unknown/reserved multi-mode value

2018-04-27 Thread Henrique de Moraes Holschuh
On Thu, 26 Apr 2018, Jordan Glover via ibm-acpi-devel wrote:
> I have to admit that I can't handle booting and checking this in tablet mode
> (without keyboard) so I won't confirm it, sorry.

FWIW, any USB keyboard should allow you to do this easily if there is at
least one USB port still exposed while in tablet mode...

That said, thanks for the help you gave us thus far!

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi: unknown/reserved multi-mode value

2018-04-25 Thread Henrique de Moraes Holschuh
(changed subject and split email thread on purpose).

On Wed, 25 Apr 2018, Jordan Glover wrote:
> thinkpad_acpi: Unknown/reserved multi mode value 0x for type 4,
> please report this to ibm-acpi-devel@lists.sourceforge.net
> 
> After reboot this message is gone. Previously it was printed on every boot, 
> that's
> why I thought it's gone completely. There aren't any messages about unhandled
> keys anymore.

Well, the first problem report (and the patches I sent to this ML) do
not address this specific error message at all, thus it is no wonder it
can still show up.

FWIW, the error "unknown/reserved multi mode value" is about a real
problem, and not related to "unhandled events" at all.  What we'd have
to do is to track down the root cause for this one.

But let's do that in a separate thread, shall we?  It is an unrelated
issue to the ones in this thread, after all...

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [PATCH 1/3] thinkpad-acpi: silence HKEY 0x6032, 0x60f0, 0x6030

2018-04-24 Thread Henrique de Moraes Holschuh
Demote to debug level one existing thermal-control related event, and
also add two new ones that would otherwise trigger unknown event
warnings.  These events are Windows-only for now.

We do report them to userspace in case they become useful in the future.

Signed-off-by: Henrique de Moraes Holschuh 
Reported-by: Jordan Glover 
Tested-by: Jordan Glover 
---
 Documentation/laptops/thinkpad-acpi.txt |  2 ++
 drivers/platform/x86/thinkpad_acpi.c| 19 +--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/laptops/thinkpad-acpi.txt 
b/Documentation/laptops/thinkpad-acpi.txt
index 00b6dfed573c..6cced88de6da 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -540,8 +540,10 @@ Events that are propagated by the driver to userspace:
 0x6021 ALARM: a sensor is too hot
 0x6022 ALARM: a sensor is extremely hot
 0x6030 System thermal table changed
+0x6032 Thermal Control command set completion  (DYTC, Windows)
 0x6040 Nvidia Optimus/AC adapter related (TO BE VERIFIED)
 0x60C0 X1 Yoga 2016, Tablet mode status changed
+0x60F0 Thermal Transformation changed (GMTS, Windows)
 
 Battery nearly empty alarms are a last resort attempt to get the
 operating system to hibernate or shutdown cleanly (0x2313), or shutdown
diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index da1ca4856ea1..cf25f0121d3b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -212,7 +212,12 @@ enum tpacpi_hkey_event_t {
TP_HKEY_EV_ALARM_BAT_XHOT   = 0x6012, /* battery critically hot */
TP_HKEY_EV_ALARM_SENSOR_HOT = 0x6021, /* sensor too hot */
TP_HKEY_EV_ALARM_SENSOR_XHOT= 0x6022, /* sensor critically hot */
-   TP_HKEY_EV_THM_TABLE_CHANGED= 0x6030, /* thermal table changed */
+   TP_HKEY_EV_THM_TABLE_CHANGED= 0x6030, /* windows; thermal table 
changed */
+   TP_HKEY_EV_THM_CSM_COMPLETED= 0x6032, /* windows; thermal control 
set
+  * command completed. Related 
to
+  * AML DYTC */
+   TP_HKEY_EV_THM_TRANSFM_CHANGED  = 0x60F0, /* windows; thermal 
transformation
+  * changed. Related to AML 
GMTS */
 
/* AC-related events */
TP_HKEY_EV_AC_CHANGED   = 0x6040, /* AC status changed */
@@ -4042,7 +4047,17 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 
switch (hkey) {
case TP_HKEY_EV_THM_TABLE_CHANGED:
-   pr_info("EC reports that Thermal Table has changed\n");
+   pr_debug("EC reports: Thermal Table has changed\n");
+   /* recommended action: do nothing, we don't have
+* Lenovo ATM information */
+   return true;
+   case TP_HKEY_EV_THM_CSM_COMPLETED:
+   pr_debug("EC reports: Thermal Control Command set completed 
(DYTC)\n");
+   /* recommended action: do nothing, we don't have
+* Lenovo ATM information */
+   return true;
+   case TP_HKEY_EV_THM_TRANSFM_CHANGED:
+   pr_debug("EC reports: Thermal Transformation changed (GMTS)\n");
/* recommended action: do nothing, we don't have
 * Lenovo ATM information */
return true;
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [PATCH 2/3] thinkpad-acpi: do not report thermal sensor state for tablet mode switch

2018-04-24 Thread Henrique de Moraes Holschuh
We should not do a thermal sensors state dump to the kernel log just
because the laptop is reporting that it changed into or out of tablet
mode.

Signed-off-by: Henrique de Moraes Holschuh 
Tested-by: Jordan Glover 
---
 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 cf25f0121d3b..3d70ef7e8a68 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4098,7 +4098,7 @@ static bool hotkey_notify_6xxx(const u32 hkey,
tpacpi_input_send_tabletsw();
hotkey_tablet_mode_notify_change();
*send_acpi_ev = false;
-   break;
+   return true;
 
case TP_HKEY_EV_PALM_DETECTED:
case TP_HKEY_EV_PALM_UNDETECTED:
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [PATCH 3/3] thinkpad-acpi: silence false-positive-prone pr_warn

2018-04-24 Thread Henrique de Moraes Holschuh
Do not consider unknown HKEY events in the 0x6000 range to be thermal
warnings.  Instead, handle them as a generic unknown HKEY event, which
are reported to the kernel log at priority "notice", and do not trigger
a thermal registers state dump to the log.

Signed-off-by: Henrique de Moraes Holschuh 
Tested-by: Jordan Glover 
---
 drivers/platform/x86/thinkpad_acpi.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 3d70ef7e8a68..a0e9ce0d85b9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4039,8 +4039,6 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 bool *send_acpi_ev,
 bool *ignore_acpi_ev)
 {
-   bool known = true;
-
/* 0x6000-0x6FFF: thermal alarms/notices and keyboard events */
*send_acpi_ev = true;
*ignore_acpi_ev = false;
@@ -4107,13 +4105,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
return true;
 
default:
-   pr_warn("unknown possible thermal alarm or keyboard event 
received\n");
-   known = false;
+   /* report simply as unknown, no sensor dump */
+   return false;
}
 
thermal_dump_all_sensors();
-
-   return known;
+   return true;
 }
 
 static void hotkey_notify(struct ibm_struct *ibm, u32 event)
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi unhandled events

2018-04-24 Thread Henrique de Moraes Holschuh
On Tue, 24 Apr 2018, Jordan Glover via ibm-acpi-devel wrote:
> On April 20, 2018 5:17 PM, Henrique de Moraes Holschuh  
> wrote:
> > I will send the (untested) patches, please test.
> 
> I tested patches on top of Linux 4.17-rc1. The warning message is gone, 
> everything works good.

Thanks for testing!

Andy, Darren, looks like we can add Jordan's Tested-by: and schedule the
three patches for the next merge window...

Do you want me to respin the patches with the tested-by and without the
untested prefix, or can you take them directly?

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi unhandled events

2018-04-20 Thread Henrique de Moraes Holschuh
On Fri, 20 Apr 2018, Jordan Glover wrote:
> On April 9, 2018 4:58 AM, Henrique de Moraes Holschuh  wrote:
> 
> > On Tue, 03 Apr 2018, Henrique de Moraes Holschuh wrote:
> > 
> > > > Would you mind giving some hints to Jordan? Thanks.
> > > 
> > > Not a problem, I will either give hints or write the patch myself soon.
> > > 
> > > Poke me in one week if I did not reply by that timeframe, please.
> > 
> > Patches written (fixed one bug and one annoyance while at it), will send
> > 
> > them later this week after I get some time to test.

I apologise for the delay, had severe issues to deal with this week and
they are not over yet.

I will send the (untested) patches, please test.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [UNTESTED 2/3] thinkpad-acpi: do not report thermal sensor state for tablet mode switch

2018-04-20 Thread Henrique de Moraes Holschuh
We should not do a thermal sensors state dump to the kernel log just
because the laptop is reporting that it changed into or out of tablet
mode.

Signed-off-by: Henrique de Moraes Holschuh 
---
 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 cf25f0121d3b..3d70ef7e8a68 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4098,7 +4098,7 @@ static bool hotkey_notify_6xxx(const u32 hkey,
tpacpi_input_send_tabletsw();
hotkey_tablet_mode_notify_change();
*send_acpi_ev = false;
-   break;
+   return true;
 
case TP_HKEY_EV_PALM_DETECTED:
case TP_HKEY_EV_PALM_UNDETECTED:
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [UNTESTED 3/3] thinkpad-acpi: silence false-positive-prone pr_warn

2018-04-20 Thread Henrique de Moraes Holschuh
Do not consider unknown HKEY events in the 0x6000 range to be thermal
warnings.  Instead, handle them as a generic unknown HKEY event, which
are reported to the kernel log at priority "notice", and do not trigger
a thermal registers state dump to the log.

Signed-off-by: Henrique de Moraes Holschuh 
---
 drivers/platform/x86/thinkpad_acpi.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 3d70ef7e8a68..a0e9ce0d85b9 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4039,8 +4039,6 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 bool *send_acpi_ev,
 bool *ignore_acpi_ev)
 {
-   bool known = true;
-
/* 0x6000-0x6FFF: thermal alarms/notices and keyboard events */
*send_acpi_ev = true;
*ignore_acpi_ev = false;
@@ -4107,13 +4105,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
return true;
 
default:
-   pr_warn("unknown possible thermal alarm or keyboard event 
received\n");
-   known = false;
+   /* report simply as unknown, no sensor dump */
+   return false;
}
 
thermal_dump_all_sensors();
-
-   return known;
+   return true;
 }
 
 static void hotkey_notify(struct ibm_struct *ibm, u32 event)
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


[ibm-acpi-devel] [UNTESTED 1/3] thinkpad-acpi: silence HKEY 0x6032, 0x60f0, 0x6030

2018-04-20 Thread Henrique de Moraes Holschuh
Demote to debug level one existing thermal-control related event, and
also add two new ones that would otherwise trigger unknown event
warnings.  These events are Windows-only for now.

We do report them to userspace in case they become useful in the future.

Signed-off-by: Henrique de Moraes Holschuh 
Reported-by: Jordan Glover 
---
 Documentation/laptops/thinkpad-acpi.txt |  2 ++
 drivers/platform/x86/thinkpad_acpi.c| 19 +--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/Documentation/laptops/thinkpad-acpi.txt 
b/Documentation/laptops/thinkpad-acpi.txt
index 00b6dfed573c..6cced88de6da 100644
--- a/Documentation/laptops/thinkpad-acpi.txt
+++ b/Documentation/laptops/thinkpad-acpi.txt
@@ -540,8 +540,10 @@ Events that are propagated by the driver to userspace:
 0x6021 ALARM: a sensor is too hot
 0x6022 ALARM: a sensor is extremely hot
 0x6030 System thermal table changed
+0x6032 Thermal Control command set completion  (DYTC, Windows)
 0x6040 Nvidia Optimus/AC adapter related (TO BE VERIFIED)
 0x60C0 X1 Yoga 2016, Tablet mode status changed
+0x60F0 Thermal Transformation changed (GMTS, Windows)
 
 Battery nearly empty alarms are a last resort attempt to get the
 operating system to hibernate or shutdown cleanly (0x2313), or shutdown
diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index da1ca4856ea1..cf25f0121d3b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -212,7 +212,12 @@ enum tpacpi_hkey_event_t {
TP_HKEY_EV_ALARM_BAT_XHOT   = 0x6012, /* battery critically hot */
TP_HKEY_EV_ALARM_SENSOR_HOT = 0x6021, /* sensor too hot */
TP_HKEY_EV_ALARM_SENSOR_XHOT= 0x6022, /* sensor critically hot */
-   TP_HKEY_EV_THM_TABLE_CHANGED= 0x6030, /* thermal table changed */
+   TP_HKEY_EV_THM_TABLE_CHANGED= 0x6030, /* windows; thermal table 
changed */
+   TP_HKEY_EV_THM_CSM_COMPLETED= 0x6032, /* windows; thermal control 
set
+  * command completed. Related 
to
+  * AML DYTC */
+   TP_HKEY_EV_THM_TRANSFM_CHANGED  = 0x60F0, /* windows; thermal 
transformation
+  * changed. Related to AML 
GMTS */
 
/* AC-related events */
TP_HKEY_EV_AC_CHANGED   = 0x6040, /* AC status changed */
@@ -4042,7 +4047,17 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 
switch (hkey) {
case TP_HKEY_EV_THM_TABLE_CHANGED:
-   pr_info("EC reports that Thermal Table has changed\n");
+   pr_debug("EC reports: Thermal Table has changed\n");
+   /* recommended action: do nothing, we don't have
+* Lenovo ATM information */
+   return true;
+   case TP_HKEY_EV_THM_CSM_COMPLETED:
+   pr_debug("EC reports: Thermal Control Command set completed 
(DYTC)\n");
+   /* recommended action: do nothing, we don't have
+* Lenovo ATM information */
+   return true;
+   case TP_HKEY_EV_THM_TRANSFM_CHANGED:
+   pr_debug("EC reports: Thermal Transformation changed (GMTS)\n");
/* recommended action: do nothing, we don't have
 * Lenovo ATM information */
return true;
-- 
2.11.0


--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi unhandled events

2018-04-08 Thread Henrique de Moraes Holschuh
On Tue, 03 Apr 2018, Henrique de Moraes Holschuh wrote:
> > Would you mind giving some hints to Jordan? Thanks.
> 
> Not a problem, I will either give hints or write the patch myself soon.
> Poke me in one week if I did not reply by that timeframe, please.

Patches written (fixed one bug and one annoyance while at it), will send
them later this week after I get some time to test.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] Add Second Fan Support for Thinkpad P50

2018-04-05 Thread Henrique de Moraes Holschuh
On Thu, 05 Apr 2018, Sasha Levin wrote:
> This commit has been processed by the -stable helper bot and determined
> to be a high probability candidate for -stable trees. (score: 9.2381)
> 
> The bot has tested the following trees: v4.15.15, v4.14.32, v4.9.92, 
> v4.4.126, 
> 
> v4.15.15: Build OK!
> v4.14.32: Build OK!
> v4.9.92: Build OK!
> v4.4.126: Build OK!
> 
> Please let us know if you'd like to have this patch included in a stable tree.

As far as I am concerned, yes, after a suitable period in mainline.

What would be a suitable period in mainline I'd rather be set by the
subsystem maintainer.  But this is a very low risk patch, if it worked
in mainline, it will work anywhere it compiles.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi unhandled events

2018-04-03 Thread Henrique de Moraes Holschuh
On Tue, 03 Apr 2018, Peter FP1 Zhang wrote:
> @Henrique,
> 
> Would you mind giving some hints to Jordan? Thanks.

Not a problem, I will either give hints or write the patch myself soon.
Poke me in one week if I did not reply by that timeframe, please.

> -Original Message-
> From: Jordan Glover [mailto:golden_mille...@protonmail.ch] 
> Sent: Tuesday, April 3, 2018 8:04 PM
> To: Peter FP1 Zhang 
> Cc: Henrique de Moraes Holschuh ; Andy Shevchenko 
> ; Platform Driver 
> ; Darren Hart ; 
> Christian Kellner ; 
> ibm-acpi-devel@lists.sourceforge.net; bb...@redhat.com
> Subject: RE: ThinkPad Yoga 370 thinkpad_acpi unhandled events
> 
> On February 17, 2018 2:47 PM, Peter FP1 Zhang  wrote:
> 
> > Thank you Henrique!
> > 
> > @Jordan,
> > 
> > Would you like to try the patch Henrique mentioned?
> > 
> > Thanks,
> > 
> > Peter Zhang \ 张福平, PMP
> > 
> > ThinkPad & ThinkStation Linux Solutions
> > 
> > Tel: (+86) 181-1611-8005 | Lenovo Shanghai
> > 
> > Linux for Those Who Do - http://www.lenovo.com/linux
> > 
> > -Original Message-
> > 
> > From: Henrique de Moraes Holschuh [mailto:h...@hmh.eng.br]
> > 
> > Sent: Friday, February 16, 2018 2:21 AM
> > 
> > To: Andy Shevchenko
> > 
> > Cc: Peter FP1 Zhang; Platform Driver; Darren Hart; Jordan Glover; 
> > Christian Kellner; ibm-acpi-devel@lists.sourceforge.net; 
> > bb...@redhat.com
> > 
> > Subject: Re: ThinkPad Yoga 370 thinkpad_acpi unhandled events
> > 
> > On Thu, 15 Feb 2018, Andy Shevchenko wrote:
> > 
> > > (Luckily entire discussion is kept in this mail)
> > > 
> > > On Thu, Feb 1, 2018 at 4:07 AM, Peter FP1 Zhang zhang...@lenovo.com wrote:
> > > 
> > > > Dear all,
> > > > 
> > > > I was told by our BIOS/Thermal team that the two events are designed 
> > > > for Windows OS, for Linux we can just ignore them directly.
> > 
> > The patch to do that is quite trivial, would anyone like to try his/her 
> > hand at it? There are examples already in the thinkpad-acpi code of events 
> > we ignore...
> > 
> > Although it would have been nice to know what these events actually 
> > mean and what they hint the windows drivers to do, since we might have 
> > wanted to leverage them in Linux userspace as well :p
> > 
> > > > 0x60F0 HK_THERMAL_TRANSFORMATION_CHANGED, ASL method is GTMS
> > > > 
> > > > 0x6032 HK_DYNAMIC_THERMAL_CONTROL_SET_COMMAND_COMPLIETION, ASL
> > > > 
> > > > method is DYTC
> > 
> > --
> > 
> > Henrique Holschuh
> 
> 
> Hi, any status update on this?
> 
> BTW: What's the official git repo for thinkpad acpi? One mentioned in 
> MAINTAINERS doesn't seem legit[1]. It would be nice if it can be updated.
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/MAINTAINERS?h=v4.16#n13701

Nowadays, thinkpad-acpi lives in the normal Linux trees.  So, patches
get merged into the platform-x86 subsystem tree, and then go to Linus'
tree.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] Add Second Fan Support for Thinkpad P50

2018-04-02 Thread Henrique de Moraes Holschuh
On Mon, 02 Apr 2018, Alexander Kappner wrote:
> The Thinkpad P50 has 2 fans. Add the 2FAN quirk so the tpacpi driver
> properly reports both fan speeds.
> Because the P50 doesn't report the version of its EC controller, we need to
> identify it by BIOS version (N1).
> 
> Signed-off-by: Alexander Kappner 

I assume you tested this on real hardware?  If so:

Acked-by: Henrique de Moraes Holschuh 

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index d5eaf3b1..ebdc300 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8699,16 +8699,24 @@ static const struct attribute_group fan_attr_group = {
> .ec = TPID(__id1, __id2), \
> .quirks = __quirks }
>  
> +#define TPACPI_FAN_QL_BIOS(__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_QL_BIOS('N', '1', TPACPI_FAN_2FAN) // Thinkpad P50 has 2 fans
>  };
>  
>  #undef TPACPI_FAN_QL
>  #undef TPACPI_FAN_QI
> +#undef TPACPI_FAN_QL_BIOS
>  
>  static int __init fan_init(struct ibm_init_struct *iibm)
>  {

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370 thinkpad_acpi unhandled events

2018-02-15 Thread Henrique de Moraes Holschuh
On Thu, 15 Feb 2018, Andy Shevchenko wrote:
> (Luckily entire discussion is kept in this mail)
> 
> On Thu, Feb 1, 2018 at 4:07 AM, Peter FP1 Zhang  wrote:
> > Dear all,
> >
> > I was told by our BIOS/Thermal team that the two events are designed for 
> > Windows OS, for Linux we can just ignore them directly.

The patch to do that is quite trivial, would anyone like to try his/her
hand at it?  There are examples already in the thinkpad-acpi code of
events we ignore...

Although it would have been nice to know what these events actually mean
and what they hint the windows drivers to do, since we might have wanted
to leverage them in Linux userspace as well :p

> > 0x60F0  HK_THERMAL_TRANSFORMATION_CHANGED,  ASL method is GTMS
> > 0x6032  HK_DYNAMIC_THERMAL_CONTROL_SET_COMMAND_COMPLIETION,  ASL method is 
> > DYTC

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v13 4/4] battery: Add the ThinkPad "Not Charging" quirk

2018-02-09 Thread Henrique de Moraes Holschuh
On Wed, 07 Feb 2018, Ognjen Galic wrote:
> The EC/ACPI firmware on Lenovo ThinkPads used to report a status
> of "Unknown" when the battery is between the charge start and
> charge stop thresholds. On Windows, it reports "Not Charging"
> so the quirk has been added to also report correctly.
> 
> Now the "status" attribute returns "Not Charging" when the
> battery on ThinkPads is not physicaly charging.
> 
> Reviewed-by: Andy Shevchenko 
> Signed-off-by: Ognjen Galic 

AFAIK, This behavior goes back to the initial SBS implementation in the
IBM era ECs of the Thinkpads...  We've always called it "idle" in the
linux-thinkpad community.  The behavior comes from SBS
(http://smartbattery.org/specs/), the EC was reporting its status
(charging/not charging *THIS* battery) in one bit, and the battery's
status (discharging/not discharging *THIS* battery) in a different bit.

It was rather simple to observe the behavior of those bits in a
two-battery system.

Would that apply to these newer Lenovo models?  If so, you might want to
consider using "idle", instead. "not charging" does _not_ imply "neither
charging nor discharging", while "idle" does.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v13 3/4] thinkpad_acpi: Add support for battery thresholds

2018-02-09 Thread Henrique de Moraes Holschuh
Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] Add Thinkpad palm detection acpi codes to thinkpad_acpi

2018-01-03 Thread Henrique de Moraes Holschuh
On Tue, 02 Jan 2018, mth...@mthode.org wrote:
> From: Matthew Thode 
> 
> Signed-off-by: Matthew Thode 

Please, could you add some far more verbose explanations to the
(currently empty) commit log?

Such as: which thinkpad models you know to have this feature, how one is
expected to use it, in which thinkpads you tested the patch, etc...

Also, please send me a copy of the dmidecode output and full ACPI tables
of a thinkpad that has this.  Please remove any UUIDs and serial
numbers, first.

Thanks!

> + case TP_HKEY_EV_PALM_UNDETECT:
> + /* we do nothing here */

But we could, depending on the usecase for the feature?  Should we?

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] platform/x86/thinkpad_acpi: Adjustments for four function implementations

2017-12-22 Thread Henrique de Moraes Holschuh
On Tue, 19 Dec 2017, SF Markus Elfring wrote:
> >>   Delete an error message for a failed memory allocation in three functions
> > 
> > This one is questionable since it prints error messages at ->init() stage.
> > I would rather not touch this.
> 
> Do you find the Linux allocation failure report insufficient in this case?

Leave those pr_ messages alone, please, unless they are really causing
some sort of issue (which?).

> >>   Improve a size determination in tpacpi_new_rfkill()
> > 
> > Doesn't make any sense right now. One style over the other.
> > Nothing gets better or worth at this point.
> 
> Would you like to care for a bit more compliance with information
> from the section “14) Allocating memory” in the document “coding-style.rst”?

No, unless the change is actually fixing something, or gives us a
down-to-earth, *real* advantage of some sort.  In which case, the commit
message better do a rather good job of explaining it.

Doing it just for "compliance" with a doc isn't nearly good enough
reason.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 0/2] platform/x86/thinkpad_acpi: Adjustments for four function implementations

2017-12-22 Thread Henrique de Moraes Holschuh
On Tue, 19 Dec 2017, Andy Shevchenko wrote:
> On Mon, Dec 18, 2017 at 11:26 PM, SF Markus Elfring
>  wrote:
> > From: Markus Elfring 
> > Date: Mon, 18 Dec 2017 22:23:45 +0100
> >
> > Two update suggestions were taken into account
> > from static source code analysis.
> >
> > Markus Elfring (2):
> >   Delete an error message for a failed memory allocation in three functions
> 
> This one is questionable since it prints error messages at ->init() stage.
> I would rather not touch this.
> 
> >   Improve a size determination in tpacpi_new_rfkill()
> 
> Doesn't make any sense right now. One style over the other.
> Nothing gets better or worth at this point.
> 
> Sorry, but NAK for both.

Agreed.  NAK from me as well.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] ThinkPad Yoga 370

2017-10-09 Thread Henrique de Moraes Holschuh

On Mon, 09 Oct 2017, Peter wrote:
> This is the exact same as reported months before on this mailing list:
> https://sourceforge.net/p/ibm-acpi/mailman/message/35803112/
> Unfortunately, this went unanswered.

Yeah, sorry about that.  We need to done down the driver messages a
bit...

> The event triggers twice during conversion to tablet mode, and twice
> again during conversion back to laptop mode.

Thanks, that's useful.

> The keyboard mechanically disables itself during this conversion. On
> Windows, the touchpad and trackpoint are disabled as well but that
> happens in software, and since this event is unhandles it obviously
> doesn't (yet) on linux.

thinkpad-acpi can't handle that kind of cross-driver enable/disable
internally most of the time, it needs to be done by userspace.  You need
an udev rule, acpid rule, or systemd-udev rule, for example.

If someone deploys a generic "supress input devices" in-kernel interface
(notifier), I could add the calls, though.  AFAIK, it doesn't exist.

> Here's a demonstration video:
> https://www.youtube.com/watch?v=nBAY0JJ0CLk (mymodel is not exactly
> the same but close enoug).
> 
> This is the only thing that makes the convertible experience
> incomplete. Everything else works perfectly (also thanks to
> gnome-shell and wayland, which support all the touch stuff perfectly).

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] fan inhibited by kernel's acpi: which info to provide?

2017-10-09 Thread Henrique de Moraes Holschuh
On Thu, 05 Oct 2017, Raph wrote:
> > BTW, the DSDT has HFSP in the EC space at the expected offset (0x2f), so
> > you *could* try to enable fan control in thinkpad-acpi and force the fan
> > to level 7.  If that turns on the fan, then the question becomes WTF is
> > turning it out of "auto" mode in the first place.
> 
> booted with fan_control=1.
> /proc/acpi/ibm/fan shows "disable" / 0 / 0

Please boot with fan_control=1 debug=0x8011

Maybe it will give some better information in the logs...

But, so far, it looks like you will need to ask for help in the
linux-acpi ML.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] fan inhibited by kernel's acpi: which info to provide?

2017-09-30 Thread Henrique de Moraes Holschuh
On Sat, 30 Sep 2017, Raph wrote:
> I didn't find XSDT/RSDP tables (but maybe I didn't understand the concept)

Nah, I screwed up.  I need all the SSDT tables, not the XSDT.

BTW, the DSDT has HFSP in the EC space at the expected offset (0x2f), so
you *could* try to enable fan control in thinkpad-acpi and force the fan
to level 7.  If that turns on the fan, then the question becomes WTF is
turning it out of "auto" mode in the first place.

Look for anything that might be messing with HFSP in the SSDTs, and also
FANG and FANW just because their name is too suspect...

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] Lid sensor on Yoga 11e

2017-09-29 Thread Henrique de Moraes Holschuh
On Wed, 27 Sep 2017, Steven Presser wrote:
> I have some new elements to this puzzle that may be useful:
> 
> - I've extracted the ACPI DST table.  This device, like the X1 Yoga
> (https://patchwork.kernel.org/patch/9395641/) has the MHKG method.  But on
> this device it's a no-op!  (just returns zero)
> 
> - I've extracted the same table under Windows, just to make sure this wasn't
> an OS difference.  It's a no-op there too, but the sensor works.
> 
> 
> I'm trying to figure out how to watch ACPI events under Windows to see if
> that gives any clues.  However, is this new info helpful to anyone's
> understanding?  Is anyone able to help me with where to investigate next?

It is likely behind WMI, then...

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] fan inhibited by kernel's acpi: which info to provide?

2017-09-29 Thread Henrique de Moraes Holschuh
On Thu, 28 Sep 2017, Raph wrote:
> On Sun, Sep 24, 2017 at 07:11:47PM -0300, Henrique de Moraes Holschuh wrote:
> => but should a working Kaveri provide a fifth, GPU-specific, cooling device?

No idea. Not necessarily.

> thinkpad_acpi parameters of interest are:
> /sys/module/thinkpad_acpi/parameters/fan_control =  N
> /sys/bus/platform/drivers/thinkpad_hwmon/fan_watchdog = 0
> /sys/devices/platform/thinkpad_hwmon/fan1_input = 0

So, it will *not* touch the EC fan control stuff at all.  It is also
reporting that the fan is stoppend (tachometer reads 0) -- assuming it
can actually read the tachometer in your computer.

> "fan" module is not loaded during boot (!)
> Manually loading creates nothing more than:
> > /sys/module/fan/drivers/platform:acpi-fan
> [and nothing specific inside dmesg]

Hmm, that *is* bad.  And your prime suspect.

> Maybe related to this line in dmesg?
> > [drm] Internal thermal controller without fan control

No, that just means your GPU does not have its own fan, which is
expected since this is a notebook.

> # find /sys -regextype posix-egrep -regex '.*(cool|fan|temp)[^/]*'
> /sys/devices/pci:00/:00:18.3/hwmon/hwmon2/temp1_max
> /sys/devices/pci:00/:00:18.3/hwmon/hwmon2/temp1_crit
> /sys/devices/pci:00/:00:18.3/hwmon/hwmon2/temp1_crit_hyst
> /sys/devices/pci:00/:00:18.3/hwmon/hwmon2/temp1_input
> /sys/devices/pci:00/:00:01.0/hwmon/hwmon0/temp1_crit
> /sys/devices/pci:00/:00:01.0/hwmon/hwmon0/temp1_crit_hyst
> /sys/devices/pci:00/:00:01.0/hwmon/hwmon0/temp1_input

Looks like chipset thermometers... but not cooling devices.

Can you send me the DSDT and XSDTs?  You'll need a recent version of
acpidump to extract them.  Binary format (compressed) is fine, I can run
them through iasl (the ACPI AML disassembler) myself.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] fan inhibited by kernel's acpi: which info to provide?

2017-09-24 Thread Henrique de Moraes Holschuh
On Sun, 24 Sep 2017, Raph wrote:
> On Sat, Sep 16, 2017 at 08:34:08AM -0300, Henrique de Moraes Holschuh wrote:
> > On Wed, 13 Sep 2017, Raf D wrote:
> > > I own a Lenovo E555.
> > > After the switch from Debian Jessie to Stretch fan stopped working
> > > Symptom: overheat then halt circa 90° and fan does not wake up.
> > > 
> > > Booting with acpi=off: fan run automatically when needed as it should.
> > 
> > We need to  know if the issue is caused by thinkpad-acpi, or by the
> > standard ACPI support in the kernel, or by userspace.
> > 
> > > What info may I provide to efficiently debug/solve this problem?
> > > acpidumps, dmidecode, /sys/, dmesg|egrep -i 
> > > 'cool|acpi|thinkpad|fan...' ?
> > 
> > dmesg on boot for starters, I think.  Feel free to remove any UUIDs and
> > serial numbers.

Ok, it is not thinkpad-acpi messing with your fan (at least not
directly).  Please check the cooling devices registered in ACPI, and
ensure the ACPI "fan" module was loaded.

The cooling devices are likely at:
/sys/devices/virtual/thermal

Check what is in there.  Anything that can be switched on or off to cool
the system can be a cooling_device, so both fans (switch on to cool),
and GPU (switch to low power mode) could be "cooling" devices...

My guess, so far, is that you will need to ask for help in the main
kernel mailing list.

Also, we may need to check your ACPI's DSDT and XSDT, but let's try
ensuring the "fan" module/ACPI driver is loaded, first.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] fan inhibited by kernel's acpi: which info to provide?

2017-09-16 Thread Henrique de Moraes Holschuh
On Wed, 13 Sep 2017, Raf D wrote:
> I own a Lenovo E555.
> After the switch from Debian Jessie to Stretch fan stopped working
> Symptom: overheat then halt circa 90° and fan does not wake up.
> 
> Booting with acpi=off: fan run automatically when needed as it should.

We need to  know if the issue is caused by thinkpad-acpi, or by the
standard ACPI support in the kernel, or by userspace.

> What info may I provide to efficiently debug/solve this problem?
> acpidumps, dmidecode, /sys/, dmesg|egrep -i 
> 'cool|acpi|thinkpad|fan...' ?

dmesg on boot for starters, I think.  Feel free to remove any UUIDs and
serial numbers.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Implement tablet mode resolving using GMMS method

2017-09-16 Thread Henrique de Moraes Holschuh
On Fri, 15 Sep 2017, Benjamin Berg wrote:
> Many thinkpad laptops and convertibles provide the GMMS method to
> resolve how far the laptop has been opened and whether it has been
> converted into tablet mode. This allows reporting a more precise tablet
> mode state to userspace.
> 
> The current implementation only reports a summarized tablet mode state
> which is triggered as soon as the input devices become unusable as they
> are folded away from the display.
> 
> This will work on all models where the CMMD method was used previously and
> it may also work in other cases.
> 
> Thanks to Peter Zhang of Lenovo for providing information on how to use the
> GMMS method to query the tablet mode.
> 
> Signed-off-by: Benjamin Berg 
> Cc: Peter FP1 Zhang 
> Cc: Lyude Paul 

Looks good.

Acked-by: Henrique de Moraes Holschuh 

---
  PS: please send me the documentation for CMMD/GMMS off-list.

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 132 
> +++
>  1 file changed, 119 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 2242d6035d9e..91fab1a13a6d 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -310,8 +310,7 @@ static struct {
>   enum {
>   TP_HOTKEY_TABLET_NONE = 0,
>   TP_HOTKEY_TABLET_USES_MHKG,
> - /* X1 Yoga 2016, seen on BIOS N1FET44W */
> - TP_HOTKEY_TABLET_USES_CMMD,
> + TP_HOTKEY_TABLET_USES_GMMS,
>   } hotkey_tablet;
>   u32 kbdlight:1;
>   u32 light:1;
> @@ -2044,8 +2043,28 @@ static void hotkey_poll_setup(const bool may_warn);
>  
>  /* HKEY.MHKG() return bits */
>  #define TP_HOTKEY_TABLET_MASK (1 << 3)
> -/* ThinkPad X1 Yoga (2016) */
> -#define TP_EC_CMMD_TABLET_MODE 0x6
> +enum {
> + TP_ACPI_MULTI_MODE_INVALID  = 0,
> + TP_ACPI_MULTI_MODE_UNKNOWN  = 1 << 0,
> + TP_ACPI_MULTI_MODE_LAPTOP   = 1 << 1,
> + TP_ACPI_MULTI_MODE_TABLET   = 1 << 2,
> + TP_ACPI_MULTI_MODE_FLAT = 1 << 3,
> + TP_ACPI_MULTI_MODE_STAND= 1 << 4,
> + TP_ACPI_MULTI_MODE_TENT = 1 << 5,
> + TP_ACPI_MULTI_MODE_STAND_TENT   = 1 << 6,
> +};
> +
> +enum {
> + /* The following modes are considered tablet mode for the purpose of
> +  * reporting the status to userspace. i.e. in all these modes it makes
> +  * sense to disable the laptop input devices such as touchpad and
> +  * keyboard.
> +  */
> + TP_ACPI_MULTI_MODE_TABLET_LIKE  = TP_ACPI_MULTI_MODE_TABLET |
> +   TP_ACPI_MULTI_MODE_STAND |
> +   TP_ACPI_MULTI_MODE_TENT |
> +   TP_ACPI_MULTI_MODE_STAND_TENT,
> +};
>  
>  static int hotkey_get_wlsw(void)
>  {
> @@ -2066,6 +2085,90 @@ static int hotkey_get_wlsw(void)
>   return (status) ? TPACPI_RFK_RADIO_ON : TPACPI_RFK_RADIO_OFF;
>  }
>  
> +static int hotkey_gmms_get_tablet_mode(int s, int *has_tablet_mode)
> +{
> + int type = (s >> 16) & 0x;
> + int value = s & 0x;
> + int mode = TP_ACPI_MULTI_MODE_INVALID;
> + int valid_modes = 0;
> +
> + if (has_tablet_mode)
> + *has_tablet_mode = 0;
> +
> + switch (type) {
> + case 1:
> + valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> +   TP_ACPI_MULTI_MODE_TABLET |
> +   TP_ACPI_MULTI_MODE_STAND_TENT;
> + break;
> + case 2:
> + valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> +   TP_ACPI_MULTI_MODE_FLAT |
> +   TP_ACPI_MULTI_MODE_TABLET |
> +   TP_ACPI_MULTI_MODE_STAND |
> +   TP_ACPI_MULTI_MODE_TENT;
> + break;
> + case 3:
> + valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> +   TP_ACPI_MULTI_MODE_FLAT;
> + break;
> + case 4:
> + valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> +   TP_ACPI_MULTI_MODE_TABLET |
> +   TP_ACPI_MULTI_MODE_STAND |
> +   TP_ACPI_MULTI_MODE_TENT;
> + break;
> + case 5:
> + valid_modes = TP_ACPI_MULTI_MODE_LAPTOP |
> +   TP_ACPI_MULTI_MODE_FLAT |
> +   TP_ACPI_MULTI_MODE_TABLET |
> +   TP_ACPI_MULTI_MODE_STAND |
> +   TP_ACPI_MULTI_MODE_TENT;
&

Re: [ibm-acpi-devel] [PATCH v2] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register

2017-08-31 Thread Henrique de Moraes Holschuh
On Fri, 18 Aug 2017, Darren Hart wrote:
> > before:
> > $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input}

> > after:
> > $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input}
> > thinkpad
> > 3478
> > $ cat /sys/class/hwmon/hwmon1/{name,fan1_input}
> > thinkpad
> > 3478

I wonder what's the point of retaining the thinkpad_hwmon separate
device [from the thinkpad_acpi platform device] then... but changing
that might break the userspace API even further or cause other
annoyances down the road, so I guess it is the lesser evil.

> > $ sensors
> > thinkpad-isa-
> > Adapter: ISA adapter
> > fan1:3489 RPM

Yeah, that should cover >90% of the usecases since most people just read
these.  It *will* break write accesses using /etc/sysfs.conf and
similar, though (to set fan mode on boot, etc).

It is documented and the userspace ABI is being updated according to the
hwmon subsystem rules *and* the thinkpad-acpi rules...  this is enough
for me, but be warned that people might complain.

> This looks very reasonable to me. The lm-sensors user experience is
> effectively unchanged, and the /sys/* changes move from a specific
> implementation to a generic implementation, taking advantage for the
> subsystem.

Yes, which is why I am not against the ABI change.

> This will be 4.14 because we let it sit too long. I'll correct this.
> 
> I've queued this to testing for 4.14.
> 
> Henrique, please shout if you have any objections here.

No objections.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register

2017-06-19 Thread Henrique de Moraes Holschuh
On Mon, 19 Jun 2017, Stanislav Fomichev wrote:
> Use hwmon_device_register_with_groups instead of deprecated
> hwmon_device_register and fix a dmesg warning.
> 
> This patch however changes the userspace API.
> hwmon_device_register_with_groups takes `hwmon' name as an argument and 
> creates
> a name file in the `hwmon' device, not in the `platform_device'. This
> allows us to remove custom `name' device attribute, but in order to make
> lm-sensors happy we also have to move fans and thermal attributes to the
> `hwmon' device.

Can you clarify that with a ls -l of the "before" and "after"?

Also, does it change anything visible to lmsensors users (such as the
sensor/adapter name or address)? If so, please post a "before" and
"after"...

> Even though this patch changes userspace API, it's still compatible with
> the lm-sensors. Starting with lm-sensors 3.0 (circa 2007), it looks at both
> hwmon and the backing device for the name and other attributes.

I am not sure this is OK or not, it *is* an userspace ABI break.  I am
not really opposed to it, as long as it is done properly.  Let's see
what our subsystem maintainers think of it.

That said: the patch is still incomplete, because this driver has
documentation that also needs to be updated...

Please update Documentation/laptops/thinkpad-acpi.txt  with the changes
to the thinkpad_hwmon sensor interfaces.  Search for "hwmon" on that
text, that should locate everything.

Also, please bump "#define TPACPI_SYSFS_VERSION 0x020700" to 0x020800 in
drivers/platform/x86/thinkpad_acpi.c, and update the interface changelog
table at the end of Documentation/laptops/thinkpad-acpi.txt.

(that assumes the change is not a major ABI break, please reply with
that "ls -l" I asked above: we might need to bump that
TPACPI_SYSFS_VERSION to 0x03, instead...)

> Signed-off-by: Stanislav Fomichev 
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 36 
> ++--
>  1 file changed, 10 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 7b6cb0c69b02..0e0a1616f273 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6401,7 +6401,7 @@ static int __init thermal_init(struct ibm_init_struct 
> *iibm)
>  
>   switch (thermal_read_mode) {
>   case TPACPI_THERMAL_TPEC_16:
> - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj,
> + res = sysfs_create_group(&tpacpi_hwmon->kobj,
>   &thermal_temp_input16_group);
>   if (res)
>   return res;
> @@ -6409,7 +6409,7 @@ static int __init thermal_init(struct ibm_init_struct 
> *iibm)
>   case TPACPI_THERMAL_TPEC_8:
>   case TPACPI_THERMAL_ACPI_TMP07:
>   case TPACPI_THERMAL_ACPI_UPDT:
> - res = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj,
> + res = sysfs_create_group(&tpacpi_hwmon->kobj,
>   &thermal_temp_input8_group);
>   if (res)
>   return res;
> @@ -6426,13 +6426,13 @@ static void thermal_exit(void)
>  {
>   switch (thermal_read_mode) {
>   case TPACPI_THERMAL_TPEC_16:
> - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj,
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
>  &thermal_temp_input16_group);
>   break;
>   case TPACPI_THERMAL_TPEC_8:
>   case TPACPI_THERMAL_ACPI_TMP07:
>   case TPACPI_THERMAL_ACPI_UPDT:
> - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj,
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
>  &thermal_temp_input8_group);
>   break;
>   case TPACPI_THERMAL_NONE:
> @@ -8773,7 +8773,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
>   fan_attributes[ARRAY_SIZE(fan_attributes)-2] =
>   &dev_attr_fan2_input.attr;
>   }
> - rc = sysfs_create_group(&tpacpi_sensors_pdev->dev.kobj,
> + rc = sysfs_create_group(&tpacpi_hwmon->kobj,
>&fan_attr_group);
>   if (rc < 0)
>   return rc;
> @@ -8781,7 +8781,7 @@ static int __init fan_init(struct ibm_init_struct *iibm)
>   rc = driver_create_file(&tpacpi_hwmon_pdriver.driver,
>   &driver_attr_fan_watchdog);
>   if (rc < 0) {
> - sysfs_remove_group(&tpacpi_sensors_pdev->dev.kobj,
> + sysfs_remove_group(&tpacpi_hwmon->kobj,
>   &fan_attr_group);
>   return rc;
>   }
> @@ -8796,7 +8796,7 @@ static void fan_exit(void)
>   "cancelling any pending fan watchdog tasks\n");
>  
>   /* FIXME: can we really do this unconditionally? */

Re: [ibm-acpi-devel] [PATCH 09/11] platform: thinkpad_acpi: convert to use DRIVER_ATTR_RO/RW

2017-06-09 Thread Henrique de Moraes Holschuh
On Fri, 09 Jun 2017, Greg Kroah-Hartman wrote:
> We are trying to get rid of DRIVER_ATTR(), and the thinkpad_acpi
> driver's attributes can be trivially changed to use DRIVER_ATTR_RO() and
> DRIVER_ATTR_RW().
> 
> Cc: Henrique de Moraes Holschuh 
> Cc: Darren Hart 
> Cc: Andy Shevchenko 
> Cc: 
> Cc: 
> Signed-off-by: Greg Kroah-Hartman 

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] x270 CPU temp / throttling and unhandled HKEY event when I close the lid

2017-05-21 Thread Henrique de Moraes Holschuh
On Sun, 21 May 2017, neil k wrote:
> attachment.)  If I boot up the laptop with the lid closed, it won't start
> thermal throttling until after I trigger the event for the first time.  So

Some people broke the LID handling hideously for dubious reasons on
later kernel versions, and I don't know exactly when.  And the breakage
is exactly that they screwed up LID state at power up if the LID is
closed.  I won't go into the details because I am very likely to insult
people if I do.

So, in order to avoid a possible Linux bug, it is best to test it by
closing the LID *later*, so that Linux can see the LID getting closed.

The LID mess is apparently being handled upstream right now.

> that makes me think the throttling is 1. caused by the firmware and 2.
> definitely related to this event somehow.

Looks so.

> Considering that 60 degrees is nowhere near the critical temperature, and
> the fact that I can bypass the throttling by closing the lid on A/C then
> unplugging, booting up with the lid closed, etc. It seems to me like this
> is either a bug in the firmware or a poorly implemented "feature."  Is that

It likely has a few Linux bugs at play on how easily you can bypass the
"feature", on top of whatever bugs/corner cases Lenovo left unhandled.

However, it is very very clear that your laptop *requires* a much more
conservative thermal envelope while the LID is closed to safely operate.

> what it seems like to you too?  If there was something dangerous about
> exceeding 60 degrees with the lid closed, I'd expect it to throttle to that
> temperature regardless of whether it's plugged in at the time the lid is
> closed...

Yeah, it should be doing that throttling *always*.  The fact that it
isn't is a bug (likely caused by more than one underlying bug).

> Is there anything else I can do on Windows to help investigate what the lid
> helper event means?  I have it dual booting now, so I can test things in
> Windows or Linux.

Can you somehow test the GPU performance state?

It clearly uses standard ACPI tricks to limit the processor (which are
not exactly working right, but still...), so why would they have added
that extra private event?  It must be doing something else as well...

> neil@totbox ~ $ sudo acpi_listen
> processor LNXCPU:00 0080 0001
> processor LNXCPU:01 0080 0001
> processor LNXCPU:02 0080 0001
> processor LNXCPU:03 0080 0001
> ibm/hotkey LEN0268:00 0080 6032
> button/lid LID close
> ibm/hotkey LEN0268:00 0080 6032
> button/lid LID open
> jack/lineout LINEOUT unplug
> jack/videoout VIDEOOUT unplug
> jack/lineout LINEOUT plug
> jack/videoout VIDEOOUT plug
> battery PNP0C0A:01 0080 0001
> ac_adapter ACPI0003:00 0080 0001
> ibm/hotkey LEN0268:00 0080 6030

This 6030 seems related to the 6032...

> thermal_zone LNXTHERM:00 0081 
> battery PNP0C0A:01 0080 0001
> button/lid LID close
> ibm/hotkey LEN0268:00 0080 6032
> button/lid LID open

can you annotate this with your actions, e.g.:

powered up LID closed on A/C:

opened LID:

etc...

?

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] Thinkpad X1 Carbon 5th gen,

2017-05-15 Thread Henrique de Moraes Holschuh
On Sat, 13 May 2017, Hrvoje Stojic wrote:
> Are there any solutions for these issues?

A newer kernel should support the 0x13xx hotkeys, I think.  It is also
possible to patch support into 4.9.  But mostly you can just ignore the
reports in dmesg.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] x270 CPU temp / throttling and unhandled HKEY event when I close the lid

2017-05-14 Thread Henrique de Moraes Holschuh
On Sun, 14 May 2017, neil k wrote:
> Thanks for the response Henrique.  I do have the laptop setup not to
> suspend when I close the lid.

Ok.

> So 60C / ~2.0GHz sounds right to you if I have it set to "balanced" for
> battery power, as opposed to the 80C / ~2.9GHz when I run on battery with
> the lid open?  I do have a couple options available through the power

Well, to know what it should be doing, you really need to run it under
Windows *with* the Lenovo drivers ("thinkvantage suite") loaded.

But, since it was cooking itself with the lid closed, and now it is not
cooking itself anymore, I'd say it is at least *safer* to keep it set to
"balanced" for now.

> I haven't noticed anything weird with the fan controls.  But I can be

Ok.

> so I'll give that a try and see if anything changes.  In any case,
> whatever's in control of this appears to be blowing off my settings in the
> BIOS, because the clock speeds and temps don't seem to change even if I set
> everything to Max Performance.

Hmm, check if you have "thermald" running.  If you do, try without it
(but keep a close look at temperatures the entire time!).  If you don't,
try installing it.

thermald is prone to do very idiotic things on Xeon processors, but for
laptops with mobile processors, it is often a good way to work around
misbehavior.

> If I wanted to change the code of thinkpad-acpi.c to "handle" this event,
> should I treat it the same way as the normal lid open/close events in the
> hotkey_notify_usrevent section?  Is this something that could be put in the
> code for later versions?

LID events are actually handled by the acpi "lid" driver, which is sane
on 4.9, but currently insanely broken on 4.11 (and maybe 4.10) -- the
breakage is in the process of being reverted.

Lenovo woudln't add a *new* "LID changed" helper event (like the one you
reported) if its drivers did not have to handle it *differently* from
what is already in there.  I think we actually need to know what happens
in Windows to correctly handle this one.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

2017-05-12 Thread Henrique de Moraes Holschuh
On Fri, 12 May 2017, Pavel Machek wrote:
> On Sun 2017-05-07 20:49:03, Henrique de Moraes Holschuh wrote:
> > On Sun, 07 May 2017, Pavel Machek wrote:
> > > On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> > > > This allows the control of the red status LED, which is the dot of the 
> > > > "i"
> > > > in the word "ThinkPad" on the outside cover of newer models.
> > > > 
> > > > In the manual, both this LED and the power LED are referred to as
> > > > the "system-status indicators" without distinction between the two, so
> > > > I chose "status" as the LED name.
> > > 
> > > Could we name it something like external_status or something? "status" is 
> > > way too generic.
> > 
> > "thinkdot" ;-)
> 
> Sounds good to me ;-).

Ok.  Adam, care to respin this when you have the time?

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] Detecting tablet mode on Yoga 11e?

2017-05-10 Thread Henrique de Moraes Holschuh
On Wed, 10 May 2017, Lars Kellogg-Stedman wrote:
> I'm looking for some help getting Linux to successfully detect the switch
> between tablet/notebook mode on a Yoga 11e.  With v4.11.0, it doesn't look
> like the kernel sees any events generated by switching mode: acpi_listen
> doesn't show anything, and there are no kernel log messages generated in
> response to the change.
> 
> Booting with acpi.debug_layer=0x2 and acpi.debug_level=0x ("debug
> all hardware messages") doesn't show anything, either.
> 
> I'd appreciate your suggestions on how to further diagnose this. Thanks!

If Windows knows it, then the sensor is typically exported through ACPI,
or WMI.  If Windows does not do anything different when you change
modes, there might not be a sensor at all.

Anyway, if it is not in ACPI, check WMI, the keyboard controller stream,
or any suspicious built-in USB HID devices...

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] Multimedia keypresses failing in Ubuntu 17.04 on T470

2017-05-10 Thread Henrique de Moraes Holschuh
On Thu, 04 May 2017, John Marc Imbrescia wrote:
> These 3 key presses are F10-F12 without the Fn key pressed.  They're
> labeled "Bluetooth" "Keyboard" "Star"

Thanks for the report!

> Other multimedia keys work as expected.
> 
> On Thu, May 4, 2017 at 11:08 AM, John Marc Imbrescia 
> wrote:
> 
> > Reporting as per system.log request.
> >
> > [40674.594159] thinkpad_acpi: Unhandled adaptive keyboard key: 0x314
> > [40674.594163] thinkpad_acpi: unhandled HKEY event 0x1314
> > [40674.594165] thinkpad_acpi: please report the conditions when this event
> > happened to ibm-acpi-devel@lists.sourceforge.net
> > [40674.882394] thinkpad_acpi: Unhandled adaptive keyboard key: 0x315
> > [40674.882398] thinkpad_acpi: unhandled HKEY event 0x1315
> > [40674.882400] thinkpad_acpi: please report the conditions when this event
> > happened to ibm-acpi-devel@lists.sourceforge.net
> > [40675.142227] thinkpad_acpi: Unhandled adaptive keyboard key: 0x311
> > [40675.142231] thinkpad_acpi: unhandled HKEY event 0x1311
> > [40675.142232] thinkpad_acpi: please report the conditions when this event
> > happened to ibm-acpi-devel@lists.sourceforge.net

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] x270 CPU temp / throttling and unhandled HKEY event when I close the lid

2017-05-10 Thread Henrique de Moraes Holschuh
On Sat, 22 Apr 2017, neil k wrote:
> I spent some more time testing this today. It seems like it only happens
> when I close the lid while *running on battery*. If I close the lid with

Yeah, that event is some sort of hint to the Lenovo Windows drivers to
change their behavior, or to reprogram something in other windows
drivers.

I assume you are closing the lid **without** suspending, right?

Probably, the Lenovo windows drivers kick everything to idle-mode
forcibly (or maybe kicks the GPU into low-power mode, disables the
discrete GPU, whatever).

thinkpad-acpi is not doing much other than warning you of an event we
don't know about, blacklisting it won't help, unless it is actually
being used to mess with fan control.  Do note thinkpad-acpi fan-control
is opt-in, someone must have forced it to be enabled using a module
parameter, if it is active.

> the laptop plugged in, THEN unplug it, I don't see these temperature
> messages happen during idling / light use. Also notable is that unhandled
> HKEY event 0x6032 doesn't show up if I close the lid on A/C power. This

Yeah, firmware implementation did not cover all possibilities :p

> issue still happens if I blacklist the thinkpad-acpi module, but I'm
> wondering if it's something Lenovo specific that isn't being handled and
> triggers the issue.

Exactly.

> Thermal
> AC – max performance
> Battery – Balanced

Check if it is not messing with the fan when you close the lid.  That
might be it...

> I also tested to see how the CPU behaved under load and found something
> else interesting. If I close the lid with the laptop unplugged (the
> condition that causes this issue), the processor runs at a much lower clock
> speed (~2.0 GHz) and much lower temperature (60C) when under load. This

Yeah, that's the UEFI balanced mode properly working.

> makes me think something about the lid being closed on battery power causes
> an issue with Intel Speedstep / p states / c states. I don't really know
> much about how that works though, unfortunately.

Either that, or some stupid crap is touching knobs it shouldn't.  Usual
suspects are the power management stuff from pm-tools, systemd, or
laptop-mode.

> converting video with lid closed plugged in will hover at 96-98C / 3.48 GHz
> <-- 3.5GHz is the maximum turbo boost for this processor

Yeah, this is too much if the fan slows down or the box does non-trivial
dissipation through the keyboard or palm-rest, and it will overheat.

Do ensure you keep the firmware of that box up-to-date, please.   You
should get a new UEFI and EC update from Lenovo every so often on such a
new machine.  If you didn't check their site this week yet, please do so
;-)

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v1 2/3] platform/x86: thinkpad_acpi: Join string literals back

2017-05-10 Thread Henrique de Moraes Holschuh
On Wed, 10 May 2017, Andy Shevchenko wrote:
> Okay, what I'm going to do is:
> 1) drop patch 1 for now;
> 2) split patch 2 into two patches (and append your Ack on both);
> 3) push to our testing branch (I can send v2 if we need one more round
> of review).
> 
> Tell me if there is any objection.

Eh, I already went through the patch and acked it, so you can send it
as-is if you want to.  That said, I am also fine if you break it into
two and add my ack to both.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v1 2/3] platform/x86: thinkpad_acpi: Join string literals back

2017-05-09 Thread Henrique de Moraes Holschuh
On Tue, May 9, 2017, at 14:33, Andy Shevchenko wrote:
> On Tue, 2017-05-09 at 14:10 -0300, Henrique de Moraes Holschuh wrote:
> > > While here, print negative error without changing a sign as it is a
> > > common pattern in the kernel.
> > 
> > A separate patch for this would be better: it would be easier to
> > actually check that no functional changes crept in by mistake.
> 
> It doesn't make sense to me. It would touch same lines of code I do
> already here and it's only one place, see below.

I had to go line-by-line looking for the darn thing, instead of just
compiling before-and-after and checking for an unchanged  object file.

> > >   rc = fan_set_enable();
> > >   if (rc < 0) {
> > > - pr_err("fan watchdog: error %d while enabling fan,
> > > "
> > > -    "will try again later...\n", -rc);
> > > + pr_err("fan watchdog: error %d while enabling fan,
> > > will try again later...\n",
> > > +    rc);

Yeah. This one.  I don't have a problem with this change at all (I acked
it), but it took some effort to find the nail in the hailstack.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v1 2/3] platform/x86: thinkpad_acpi: Join string literals back

2017-05-09 Thread Henrique de Moraes Holschuh
On Tue, 09 May 2017, Andy Shevchenko wrote:
> There is no point to keep string literal split. It even makes slightly
> harder to maintain and debug.

Ooold code, from a time when random people would annoy others over
80-column instead of doing useful reviews...

> Join string literals back to be oneliners.

Thanks.  I agree.

> While here, print negative error without changing a sign as it is a
> common pattern in the kernel.

A separate patch for this would be better: it would be easier to
actually check that no functional changes crept in by mistake.

> Other than above there were no functional changes intended.
> 
> Signed-off-by: Andy Shevchenko 

Acked-by: Henrique de Moraes Holschuh 

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 182 
> +--
>  1 file changed, 65 insertions(+), 117 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 7740b5e1b998..e6fbb2579dd9 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -590,8 +590,8 @@ static int acpi_evalf(acpi_handle handle,
>   break;
>   /* add more types as needed */
>   default:
> - pr_err("acpi_evalf() called "
> -"with invalid format character '%c'\n", c);
> + pr_err("acpi_evalf() called with invalid format 
> character '%c'\n",
> +c);
>   va_end(ap);
>   return 0;
>   }
> @@ -619,8 +619,8 @@ static int acpi_evalf(acpi_handle handle,
>   break;
>   /* add more types as needed */
>   default:
> - pr_err("acpi_evalf() called "
> -"with invalid format character '%c'\n", res_type);
> + pr_err("acpi_evalf() called with invalid format character 
> '%c'\n",
> +res_type);
>   return 0;
>   }
>  
> @@ -790,8 +790,8 @@ static int __init setup_acpi_notify(struct ibm_struct 
> *ibm)
>   ibm->acpi->type, dispatch_acpi_notify, ibm);
>   if (ACPI_FAILURE(status)) {
>   if (status == AE_ALREADY_EXISTS) {
> - pr_notice("another device driver is already "
> -   "handling %s events\n", ibm->name);
> + pr_notice("another device driver is already handling %s 
> events\n",
> +   ibm->name);
>   } else {
>   pr_err("acpi_install_notify_handler(%s) failed: %s\n",
>  ibm->name, acpi_format_exception(status));
> @@ -1095,8 +1095,7 @@ static void printk_deprecated_attribute(const char * 
> const what,
>   const char * const details)
>  {
>   tpacpi_log_usertask("deprecated sysfs attribute");
> - pr_warn("WARNING: sysfs attribute %s is deprecated and "
> - "will be removed. %s\n",
> + pr_warn("WARNING: sysfs attribute %s is deprecated and will be removed. 
> %s\n",
>   what, details);
>  }
>  
> @@ -1828,8 +1827,7 @@ static void __init tpacpi_check_outdated_fw(void)
>* best if the user upgrades the firmware anyway.
>*/
>   pr_warn("WARNING: Outdated ThinkPad BIOS/EC firmware\n");
> - pr_warn("WARNING: This firmware may be missing critical bug "
> - "fixes and/or important features\n");
> + pr_warn("WARNING: This firmware may be missing critical bug 
> fixes and/or important features\n");
>   }
>  }
>  
> @@ -2198,8 +2196,7 @@ static int hotkey_mask_set(u32 mask)
>* a given event.
>*/
>   if (!hotkey_mask_get() && !rc && (fwmask & ~hotkey_acpi_mask)) {
> - pr_notice("asked for hotkey mask 0x%08x, but "
> -   "firmware forced it to 0x%08x\n",
> + pr_notice("asked for hotkey mask 0x%08x, but firmware forced it 
> to 0x%08x\n",
> fwmask, hotkey_acpi_mask);
>   }
>  
> @@ -2224,11 +2221,9 @@ static int hotkey_user_mask_set(const u32 mask)
>   (mask == 0x || mask == 0xff ||
>mask == 0x)) {
>   tp_warned.hotkey_mask_ff = 1;
> - pr_notice("setting the hotkey mask to 0x%08x

Re: [ibm-acpi-devel] [PATCH v1 3/3] platform/x86: thinkpad_acpi: Add a comment about 0 in module_param_call()

2017-05-09 Thread Henrique de Moraes Holschuh
On Tue, 09 May 2017, Andy Shevchenko wrote:
> As per discussion [1] there are only few users of module_param_call() in
> kernel which prevent to read module parameters back.
> 
> It thinkpad_acpi driver there is even no method do so. Thus, for now,
> add just a comment to explain why 0 is used as permissions in
> module_param_call().
> 
> [1]: https://patchwork.ozlabs.org/patch/713245/
> 
> Cc: Richard Weinberger 
> Signed-off-by: Andy Shevchenko 

Acked-by: Henrique de Moraes Holschuh 

> ---
>  drivers/platform/x86/thinkpad_acpi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index e6fbb2579dd9..f5bc888b2ef4 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9663,6 +9663,7 @@ module_param_named(enable, alsa_enable, bool, 0444);
>  MODULE_PARM_DESC(enable, "Enable the ALSA interface for the ACPI EC Mixer");
>  #endif /* CONFIG_THINKPAD_ACPI_ALSA_SUPPORT */
>  
> +/* The module parameter can't be read back, that's why 0 is used here */
>  #define TPACPI_PARAM(feature) \
>   module_param_call(feature, set_ibm_param, NULL, NULL, 0); \
>   MODULE_PARM_DESC(feature, "Simulates thinkpad-acpi procfs command at 
> module load, see documentation")

-- 
  Henrique Holschuh

  "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

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v1 1/3] platform/x86: thinkpad_acpi: Make logic straight in hotkey_exit()

2017-05-09 Thread Henrique de Moraes Holschuh
On Tue, 09 May 2017, Andy Shevchenko wrote:
> The commit 4be73005e4dc
> 
>   ("thinkpad-acpi: remove uneeded tp_features.hotkey tests in 
> hotkey_exit")
> 
> adds a complex logic behind hotkey status check in a way
> it started mixing logical operations with bitwise ones.
> 
> Refactor the code to make it straight and slightly clearer.

Eh, I find this actually less clear, given the comment that was in the
old code, which you deleted.

Please keep the important part of the comment at the very least.

> Signed-off-by: Andy Shevchenko 
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 9 -
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index 7b6cb0c69b02..7740b5e1b998 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -3090,6 +3090,8 @@ static void tpacpi_send_radiosw_update(void)
>  
>  static void hotkey_exit(void)
>  {
> + int res;
> +
>  #ifdef CONFIG_THINKPAD_ACPI_HOTKEY_POLL
>   mutex_lock(&hotkey_mutex);
>   hotkey_poll_stop_sync();
> @@ -3101,11 +3103,8 @@ static void hotkey_exit(void)
>  
>   dbg_printk(TPACPI_DBG_EXIT | TPACPI_DBG_HKEY,
>  "restoring original HKEY status and mask\n");
> - /* yes, there is a bitwise or below, we want the
> -  * functions to be called even if one of them fail */
> - if (((tp_features.hotkey_mask &&
> -   hotkey_mask_set(hotkey_orig_mask)) |
> -  hotkey_status_set(false)) != 0)
> + res = tp_features.hotkey_mask ? hotkey_mask_set(hotkey_orig_mask) : 0;
> + if (hotkey_status_set(false) || res)
>   pr_err("failed to restore hot key mask "
>  "to BIOS defaults\n");
>  }

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

2017-05-07 Thread Henrique de Moraes Holschuh
On Sun, 07 May 2017, Pavel Machek wrote:
> On Thu 2017-01-19 12:21:32, Adam Goode wrote:
> > This allows the control of the red status LED, which is the dot of the "i"
> > in the word "ThinkPad" on the outside cover of newer models.
> > 
> > In the manual, both this LED and the power LED are referred to as
> > the "system-status indicators" without distinction between the two, so
> > I chose "status" as the LED name.
> 
> Could we name it something like external_status or something? "status" is way 
> too generic.

"thinkdot" ;-)

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] Lid status always set to open on X1 Carbon 2nd edition

2017-05-04 Thread Henrique de Moraes Holschuh
On Wed, 03 May 2017, Marco Marzetti wrote:
> For some reason /proc/acpi/button/lid/LID/state is always open even if
> the lid is closed.
> I am quite annoyed by the issue, but i ran out of options pretty fast
> 
> Hardware is Lenovo X1 Carbon 2nd edition.
> Below you can find:
>  - dmesg
>  - dmidecode
>  - /proc/acpi/wakeup

Hmm, next time *please* remove serial numbers and UUIDs :-(

> Please note that everything works as expected (buttons, microphone,
> speakers, brightness, etc.. ).
> if i run acpi_listen it spots every button, but no event is reported
> for lid open/close.
> 
> Please note that the same issue happens with 4.9.0-2
> Do you have any ideas why?

Not at first glance, no.  This stuff is handled by the standard ACPI
layer, it is probably best that you open a bug report in the kernel
bugzilla, assign it to the ACPI subcomponent, and then send an email to
linux-a...@vger.kernel.org about it (with a link to the bugzilla
report).  It could be the lid ACPI driver, or the EC driver, or ACPICA
itself.

I am sure ACPI upstream will ask you to test the latest *mainline* (not
the Debian) 4.9 kernel.  That would be the 4.9 kernel you build from the
git repository:

https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable.git/

(use branch linux-4.9.y).   You should be able to reuse the Debian
kernel config with few changes (it will be in /boot/config-*).  "make
oldconfig" in the kernel tree is your friend (after you copied the
Debian kernel config to ".config" in the checked out kernel source
tree).

You can use 'make-kpkg' from the Debian kernel-package package to build
and package your test kernel, if you want.  But it is slow for this kind
of build :-(

linux-acpi is likely to ask you to try git bissecting to locate the
commit that introduced the problem.  If you are not familiar with it, is
explained here:

https://wiki.gentoo.org/wiki/Kernel_git-bisect

https://www.kernel.org/pub/software/scm/git/docs/git-bisect-lk2009.html
http://www.reactivated.net/weblog/archives/2006/01/using-git-bisect-to-find-buggy-kernel-patches/


This clearly requires that you be somewhat confortable building your own
kernels :-(

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2 2/2] platform/x86: thinkpad_acpi: add mapping for new hotkeys

2017-02-28 Thread Henrique de Moraes Holschuh
On Tue, 28 Feb 2017, Christian Kellner wrote:
> From: Christian Kellner 
> 
> The T470, X270 emits new hkey events in the 0x1311 - 0x1315 range.
> According to the user manual they should launch a user selected
> favorite application (star icon, 0x1311), snipping tool (0x1312,
> currently ignored), enable/disable bluetooth (0x1314) and open they
> keyboard settings (0x1315).
> 
> The third nibble (0xf00) is used to differentiate between the original
> hotkeys, the adaptive keyboard codes and the new, additional ones.
> 
> Signed-off-by: Christian Kellner 
> Reviewed-by: Hans de Goede 

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2 1/2] platform/x86: thinkpad_acpi: guard generic hotkey case

2017-02-28 Thread Henrique de Moraes Holschuh
On Tue, 28 Feb 2017, Christian Kellner wrote:
> From: Christian Kellner 
> 
> Currently when dispatching hotkeys we check if the scancode is in
> the range of 0 and TPACPI_HOTKEY_MAP_LEN, although the bottom 20
> entries in the hotkey keymap are already adaptive keycodes.
> Therefore we introduce a TP_ACPI_HOTKEYSCAN_ADAPTIVE_START and
> ensure that we are in the range 0 and ADAPTIVE_START for the generic
> keycode case.
> 
> Signed-off-by: Christian Kellner 
> Reviewed-by: Hans de Goede 

Acked-by: Henrique de Moraes Holschuh 


-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 2/2] platform/x86: thinkpad_acpi: add mapping for new hotkeys on T470

2017-02-26 Thread Henrique de Moraes Holschuh
On Sat, 25 Feb 2017, Christian Kellner wrote:
> The T470 emits new hkey events in the 0x1311 - 0x1315 range. According
> to the user manual they should launch a user selected favorite
> application (star icon, 0x1311), snipping tool (0x1312, currently
> ignored), enable/disable bluetooth (0x1314) and open they keyboard
> settings (0x1315).
> 
> The third nibble (0xf00) is used to differentiate between the original
> hotkeys, the adaptive keyboard codes and the new, additional ones.

Looks good.

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/2] platform/x86: thinkpad_acpi: guard generic hotkey case

2017-02-26 Thread Henrique de Moraes Holschuh
On Sat, 25 Feb 2017, Christian Kellner wrote:
> @@ -1923,7 +1923,8 @@ enum {  /* hot key scan codes (derived from ACPI DSDT) 
> */
>   TP_ACPI_HOTKEYSCAN_UNK7,
>   TP_ACPI_HOTKEYSCAN_UNK8,
>  
> - TP_ACPI_HOTKEYSCAN_MUTE2,
> + TP_ACPI_HOTKEYSCAN_ADAPTIVE_START = 32,
> + TP_ACPI_HOTKEYSCAN_MUTE2 = 32,
>   TP_ACPI_HOTKEYSCAN_BRIGHTNESS_ZERO,
>   TP_ACPI_HOTKEYSCAN_CLIPPING_TOOL,
>   TP_ACPI_HOTKEYSCAN_CLOUD,

This works, but...

enum {
TP_ACPI_HOTKEYSCAN_ADAPTIVE_START,
TP_ACPI_HOTKEYSCAN_MUTE2 = TP_ACPI_HOTKEYSCAN_ADAPTIVE_START,
TP_ACPI_HOTKEYSCAN_BRIGHTNESS_ZERO,
...
} 

or something to that effect might be better.  Either that or an
compile-time assert that the block boundaries are where we expect them
to be.

Feel free to use formatting tricks with whitespace to make it create
visual blocks in the enum {}  definition, or use comments as spacers...
;-)

> @@ -3657,7 +3658,6 @@ static const int adaptive_keyboard_modes[] = {
>  #define DFR_CHANGE_ROW   0x101
>  #define DFR_SHOW_QUICKVIEW_ROW   0x102
>  #define FIRST_ADAPTIVE_KEY   0x103
> -#define ADAPTIVE_KEY_OFFSET  0x020
>  
>  /* press Fn key a while second, it will switch to Function Mode. Then
>   * release Fn key, previous mode be restored.
> @@ -3748,12 +3748,13 @@ static bool 
> adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
>   default:
>   if (scancode < FIRST_ADAPTIVE_KEY ||
>   scancode >= FIRST_ADAPTIVE_KEY + TPACPI_HOTKEY_MAP_LEN -
> - ADAPTIVE_KEY_OFFSET) {
> + TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
>   pr_info("Unhandled adaptive keyboard key: 0x%x\n",
>   scancode);
>   return false;
>   }
> - keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY + 
> ADAPTIVE_KEY_OFFSET];
> + keycode = hotkey_keycode_map[scancode - FIRST_ADAPTIVE_KEY +
> +  TP_ACPI_HOTKEYSCAN_ADAPTIVE_START];
>   if (keycode != KEY_RESERVED) {
>   mutex_lock(&tpacpi_inputdev_send_mutex);
>  
> @@ -3779,7 +3780,7 @@ static bool hotkey_notify_hotkey(const u32 hkey,
>   *ignore_acpi_ev = false;
>  
>   /* HKEY event 0x1001 is scancode 0x00 */
> - if (scancode > 0 && scancode <= TPACPI_HOTKEY_MAP_LEN) {
> + if (scancode > 0 && scancode <= TP_ACPI_HOTKEYSCAN_ADAPTIVE_START) {
>   scancode--;
>   if (!(hotkey_source_mask & (1 << scancode))) {
>   tpacpi_input_send_key_masked(scancode);

Other than that, I like the idea.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 1/1] thinkpad_acpi: Add support for status (external cover) LED

2017-02-07 Thread Henrique de Moraes Holschuh
Hello Adam,

I apologise for the delay on answering you.

On Tue, 31 Jan 2017, Adam Goode wrote:
> On Thu, Jan 19, 2017 at 12:21 PM, Adam Goode  wrote:
> > This allows the control of the red status LED, which is the dot of the "i"
> > in the word "ThinkPad" on the outside cover of newer models.
> >
> > In the manual, both this LED and the power LED are referred to as
> > the "system-status indicators" without distinction between the two, so
> > I chose "status" as the LED name.

I seem to recall this LED had an ACPI interface that was specific for
it, and allowed it to on/off/sine-wave?

> > Signed-off-by: Adam Goode 
> >
> > diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/
> > thinkpad_acpi.c
> > index cacb43fb1df7..6577bf8e5635 100644
> > --- a/drivers/platform/x86/thinkpad_acpi.c
> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> > @@ -5611,11 +5611,11 @@ static const char * const
> > tpacpi_led_names[TPACPI_LED_NUMLEDS] = {
> > "tpacpi::standby",
> > "tpacpi::dock_status1",
> > "tpacpi::dock_status2",
> > -   "tpacpi::unknown_led2",
> > +   "tpacpi::status",
> > "tpacpi::unknown_led3",
> > "tpacpi::thinkvantage",
> >  };
> > -#define TPACPI_SAFE_LEDS   0x1081U
> > +#define TPACPI_SAFE_LEDS   0x1481U

What happens on older Lenovo models (x00, x10, x20 series?)?  I think
the T410 already had it...

Also, please add code to not export it to userspace (as a led class) on
IBM.

-- 
  Henrique Holschuh

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v5 2/6] leds: triggers: Add a keyboard backlight trigger

2017-01-15 Thread Henrique de Moraes Holschuh
On Sun, 15 Jan 2017, Hans de Goede wrote:
> Do you want me to also send out a new version of the platform patches when
> I send the next shot at the LED side of things, or shall I keep those
> in my personal tree until the LED api is finalized ?

If you don't sent the entire patch set here, please keep me CC'd.  You
guys seem to have some misconceptions about how the ThinkPad LEDs work.

For one, they're LIFO, as in: whomever set their state last, wins.  And
you have several agents trying to command them: EC itself, SMBIOS (in
SMM mode), ACPI DSDT, thinkpad-acpi.   Most of the time, changes are
event-driven, so you _could_ try to have the last word on those which
all such events are visible...

Anyway, the fact is that you can never be really ensure they're at the
state you want.  It is a best-effort kind of thing.   Also, they're
controlled by EC firmware, not GPIO, so you don't know their physical
state (on/off/brightness level): you just know what high-level state the
EC is set to (on/off/blink/ramp) and that information is not always
available (the LEDs can be write-only, or worse, turn-on-only or
turn-off-only at the ACPI DSDT interface level, which is what we
typically use in thinkpad-acpi :p).

I.e: some thinkpad LEDs are better behaved than others, and more
controllable than others.  And this changes with firmware model/version.

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Fix old style declaration GCC warning

2016-11-25 Thread Henrique de Moraes Holschuh
On Fri, 25 Nov 2016, Tobias Klauser wrote:
> Fix an [-Wold-style-declaration] GCC warning by moving the inline
> keyword before the return type.
> 
> Signed-off-by: Tobias Klauser 

Acked-by: Henrique de Moraes Holschuh 

> ---
>  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 b65ce7519411..c24ce1adc577 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -7716,7 +7716,7 @@ static struct ibm_struct volume_driver_data = {
>  
>  #define alsa_card NULL
>  
> -static void inline volume_alsa_notify_change(void)
> +static inline void volume_alsa_notify_change(void)
>  {
>  }
>  

-- 
  Henrique Holschuh

--
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: adding new hotkey ID for Lenovo thinkpad

2016-11-08 Thread Henrique de Moraes Holschuh
On Tue, 08 Nov 2016, Hui Wang wrote:

> laptops are not released to market yet), the issue is that the
> thinkpad_acpi.ko can't be automatically loaded as before.
> 
> Through debugging, we found the HKEY_HID is LEN0268 instead of
> LEN0068 on those machines, and the MHKV is 0x200 instead of
> 0x100. So adding the new ID into the driver.

This usually means Lenovo expects to have different windows drivers.
Does thinkpad-acpi work properly on these new thinkpads?

If it does, you have my
Acked-by: Henrique de Moraes Holschuh 

> Signed-off-by: Hui Wang 
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index b65ce75..dbd2e27 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -128,6 +128,7 @@ enum {
>  /* ACPI HIDs */
>  #define TPACPI_ACPI_IBM_HKEY_HID "IBM0068"
>  #define TPACPI_ACPI_LENOVO_HKEY_HID  "LEN0068"
> +#define TPACPI_ACPI_LENOVO_HKEY_V2_HID   "LEN0268"
>  #define TPACPI_ACPI_EC_HID   "PNP0C09"
>  
>  /* Input IDs */
> @@ -4143,6 +4144,7 @@ static int hotkey_write(char *buf)
>  static const struct acpi_device_id ibm_htk_device_ids[] = {
>   {TPACPI_ACPI_IBM_HKEY_HID, 0},
>   {TPACPI_ACPI_LENOVO_HKEY_HID, 0},
> + {TPACPI_ACPI_LENOVO_HKEY_V2_HID, 0},
>   {"", 0},
>  };

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 2/3] thinkpad_acpi: Don't repeat ourselves in hotkey_init_tablet_mode()

2016-11-08 Thread Henrique de Moraes Holschuh
On Mon, 07 Nov 2016, Lyude wrote:
> There's no need to have multiple copies of the logic we use for checking
> whether or not we're in tablet mode, so just use
> hotkey_get_tablet_mode() when checking the initial state in
> hotkey_init_tablet_mode().

...

> @@ -3130,13 +3130,16 @@ hotkey_init_tablet_mode(void)
>   /* For X41t, X60t, X61t Tablets... */
>   if (acpi_evalf(hkey_handle, &res, "MHKG", "qd")) {
>   tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_MHKG;
> - in_tablet_mode = !!(res & TP_HOTKEY_TABLET_MASK);
>   type = "MHKG";
>   }
>  
>   if (!tp_features.hotkey_tablet)
>   return 0;
>  
> + res = hotkey_get_tablet_mode(&in_tablet_mode);
> + if (res)
> + return res;
> +

Won't this way of doing things cause the ACPI methods to be called
twice in a row?

The hotkey_init_tablet_mode() code does mode detection, and thus
hotkey_get_tablet_mode() must be kept in sync with it *anyway* (and not
the opposite), so it doesn't look like a very relevant maintenance
burden...

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v4 3/3] thinkpad_acpi: Add support for X1 Yoga (2016) Tablet Mode

2016-11-08 Thread Henrique de Moraes Holschuh
On Mon, 07 Nov 2016, Lyude wrote:
> For whatever reason, the X1 Yoga doesn't support the normal method of
> querying for tablet mode. Instead of providing the MHKG method under the
> hotkey handle, we're instead given the CMMD method under the EC handle.
> Values on this handle are either 0x1, laptop mode, or 0x6, tablet mode.
> 
> Cc: Daniel Martin 
> Signed-off-by: Lyude 
> ---
> Changes since v1:
> - Clarify kernel output when finding the tablet mode switch
> Changes since v2:
> - Rebase on top of previous patch
> - Use an enum for hotkey_tablet. This does make a bit more sense then
>   just adding another flag.
> - Call hotkey_tablet_mode_notify_change() when getting TABLET_CHANGED
>   event.
> Changes since v3:
> - Move changelog below ---
> 
>  drivers/platform/x86/thinkpad_acpi.c | 37 
> 
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index ad93c41..c60701e 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -190,6 +190,9 @@ enum tpacpi_hkey_event_t {
>   TP_HKEY_EV_LID_OPEN = 0x5002, /* laptop lid opened */
>   TP_HKEY_EV_TABLET_TABLET= 0x5009, /* tablet swivel up */
>   TP_HKEY_EV_TABLET_NOTEBOOK  = 0x500a, /* tablet swivel down */
> + TP_HKEY_EV_TABLET_CHANGED   = 0x60c0, /* X1 Yoga (2016):
> +* enter/leave tablet mode
> +*/

I was not going to nitpick this, but since a new respin will be needed
for the first patch anyway, can you add the BIOS and EC model numbers of
the X1 Yoga (2016) in a comment somewhere in your changes (e.g. in a
comment next to the new tablet mode code you added) ?

These model numbers are the product componet codes used in the
thinkpad-apci-style hardware black/white lists.  You should easily find
them in the dmidecode output, thinkpad-acpi logging, or in the BIOS and
EC firmware update web pages for that specific thinkpad.

Also, please send me off-list a copy of the binary ACPI tables (DSDT and
all XSDTs), and dmidecode output (with serial numbers and UUIDs
-out) for the X1 Yoga.

Finally, while this is not a requirement at this time, if you could
update the driver documentation with the newer events and any visible
changes to the tablet mode stuff, I'd be grateful.

It is in Documentation/laptops/thinkpad-acpi.txt

I apologise for not asking for these earlier.

Thanks,

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v3 1/3] thinkpad_acpi: Move tablet detection into separate function

2016-11-08 Thread Henrique de Moraes Holschuh
On Mon, 07 Nov 2016, Lyude wrote:
> @@ -3117,6 +3120,34 @@ static const struct tpacpi_quirk 
> tpacpi_hotkey_qtable[] __initconst = {
>  typedef u16 tpacpi_keymap_entry_t;
>  typedef tpacpi_keymap_entry_t tpacpi_keymap_t[TPACPI_HOTKEY_MAP_LEN];
>  
> +static int
> +hotkey_init_tablet_mode(void)
> +{

Function definiton all in one line, please.

Provided that you fix that minor nit,

Acked-by: Henrique de Moraes Holschuh 

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] v4.8-rc1: thinkpad x60: running at low frequency even during kernel build

2016-11-05 Thread Henrique de Moraes Holschuh
On Sat, 05 Nov 2016, Pavel Machek wrote:
> On Sat 2016-11-05 15:46:12, Henrique de Moraes Holschuh wrote:
> > On Sat, 05 Nov 2016, Pavel Machek wrote:
> > > Hmm, thanks for the pointer. But it seems like I'll have to build my
> > > own, as /proc/acpi/ibm does not follow the usual infrastructure...
> > 
> > /proc/acpi/ibm has been deprecated for years.  99% of the functionality
> > is available through more modern, standard interfaces.
> 
> Right, I see sensors can do it these days. Would it be good to expose
> them as /sys/class/thermal/thermal_zone*, too?

I don't like the idea of touching vendor-screwup-land like thermal
zones, especially when thinkpads *already* have thermal zones and they
must come from the very same sensors...

This would need a lot of careful studying and planning.

> Is it known what various fields in /proc/acpi/ibm/thermal measure?

It varies with each model.  You can map it out with better precision by
using a cold spray while doing an "open-heart" surgery on the thinkpad.
I kid you not, that's how they were mapped for some models.

Look for information about this in thinkwiki:
http://www.thinkwiki.org/wiki/Thermal_Sensors#ThinkPad_X60

The battery-pack-related sensor that is stuck at 50°C is something the
Lenovo Yamato labs guys wouldn't be clear about.  They told me to look
at what the Windows drivers do, but that would require (1) Windows in
the first place, and (2) clean room reverse engineering protocols.

> Basically... 100C is okay for semiconductors, but I'd prefer not to
> kill the hard drive

Well, that first sensor getting to 100°C is your CPU for sure.

> > thinkpad-acpi is supposed to export standard hwmon temperature sensors
> > as well.  Try them instead, please.
> 
> Heh, I just finished python to work with /proc/acpi/ibm. Oh well.

Hey, I *did* properly document that driver, and the documentation is
even mostly up-to-date...  Please refer to
Documentation/laptops/thinkpad-acpi.txt in the kernel tree.

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2 1/3] thinkpad_acpi: Move tablet detection into separate function

2016-11-05 Thread Henrique de Moraes Holschuh
On Sat, 05 Nov 2016, Darren Hart wrote:
> On Mon, Oct 31, 2016 at 07:56:40PM -0400, Lyude wrote:
> > Suggested by Daniel Martin 
> > 
> > Lenovo's having a bit of fun randomly changing what hotkey events and
> > acpi handles they use for reporting tablet mode, so unfortunately this
> > means we're definitely going to need to probe for multiple types of
> > tablet mode support. Since the hotkey_init() is already a lot larger
> > then it should be, let's split up this detection into it's own function
> > to make things a little easier to read.
> > 
> > As well, since we're going to have multiple types of tablet modes, make
> > hotkey_tablet into an enum so we can also use it to indicate the type of
> > tablet mode reporting the machine supports.
> > 
> > Changes since v1:
> > - Don't use bool for in_tablet_mode (fixes complaints from kbuild test
> >   robot)
> > 
> 
> This series doesn't apply cleanly now (simple fuzz).
> 
> Once we hear back from Henrique on his enum preference and thoughts on the
> refactoring (which looks reasonable to me), please resubmit this series and

I don't mind the enum usage, as long as it is correct.  As you said, the
driver is not consistent there.  I also don't mind the refactoring.

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2] thinkpad_acpi: Add support for X1 Yoga (2016) Tablet Mode

2016-11-05 Thread Henrique de Moraes Holschuh
On Sat, 05 Nov 2016, Darren Hart wrote:
> On Thu, Oct 27, 2016 at 03:46:44PM -0400, Lyude wrote:
> > For whatever reason, the X1 Yoga doesn't support the normal method of
> > querying for tablet mode. Instead of providing the MHKG method under the
> > hotkey handle, we're instead given the CMMD method under the EC handle.
> > Values on this handle are either 0x1, laptop mode, or 0x6, tablet mode.
> > 
> > Changes since v1:
> > - Clarify kernel output when finding the tablet mode switch
> 
> These two lines go below the --- below (they aren't meant to be part of the
> permanent commit message). Please see Documentation/SubmittingPatches.
> 
> I've queued this to testing as it addresses Henrique's change request. 
> Henrique,
> I'll add your Reviewed-by if you send it along, or drop it if you have more
> reservations.

No reservations, since it was tested to not regress previous thinkpads.

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] v4.8-rc1: thinkpad x60: running at low frequency even during kernel build

2016-11-05 Thread Henrique de Moraes Holschuh
On Sat, 05 Nov 2016, Pavel Machek wrote:
> [  825.759661] thinkpad_acpi: THERMAL EMERGENCY: a sensor reports something 
> is extremely hot!
> [  825.761935] thinkpad_acpi: temperatures (Celsius): 101 49 N/A 78 33 N/A 33 
> N/A 47 50 N/A N/A N/A N/A N/A N/A

Oh boy, that must be the second time in a decade that I see that
codepath triggering.  It is the second-level alert that the ThinkPad is
about to catch fire.

It should have logged a "is too hot!" first-level alert earlier, but
this depends on the EC and not the driver.  Maybe the temperature raised
too fast.

In Windows, the system would attempt to hibernate or shutdown.  I would
be quite happy to have thinkpad-acpi trigger such behavior as well,
patches (or guidance) are welcome ;-)

Anyway, if that temperature goes about 1~2°C higher, the EC should cut
power to your motherboard.  Apparently, the built-in thermal protection
clock modulation on the Intel processor is somehow saving your box from
that forced power-off.

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] v4.8-rc1: thinkpad x60: running at low frequency even during kernel build

2016-11-05 Thread Henrique de Moraes Holschuh
On Sat, 05 Nov 2016, Pavel Machek wrote:
> Hmm, thanks for the pointer. But it seems like I'll have to build my
> own, as /proc/acpi/ibm does not follow the usual infrastructure...

/proc/acpi/ibm has been deprecated for years.  99% of the functionality
is available through more modern, standard interfaces.

thinkpad-acpi is supposed to export standard hwmon temperature sensors
as well.  Try them instead, please.

Also, thinkpad-acpi will report the EC thermal sensors.  They don't need
to (and most often won't) match whatever you read from the processor
core(s).

-- 
  Henrique Holschuh

--
Developer Access Program for Intel Xeon Phi Processors
Access to Intel Xeon Phi processor-based developer platforms.
With one year of Intel Parallel Studio XE.
Training and support from Colfax.
Order your platform today. http://sdm.link/xeonphi
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] thinkpad_acpi: Add support for X1 Yoga (2016) Tablet Mode

2016-10-27 Thread Henrique de Moraes Holschuh
On Thu, 27 Oct 2016, Lyude Paul wrote:
> as well, can someone confirm this patch made it to the ibm-acpi-devel
> list? When I originally sent this I realized I wasn't subscribed to the
> list, so I'm guessing I might need to resend.

I received it.  I just didn't have the time to go over it in detail.

At first look, it seems correct...

> > +   /* For X1 Yoga (2016) */
> > +   if (!res && acpi_evalf(ec_handle, &status, "CMMD", "qd")) {
> > +   tp_features.hotkey_tablet = 1;
> > +   tp_features.hotkey_tablet_cmmd = 1;
> > +   tabletsw_state = (status == TP_EC_CMMD_TABLET_MODE);
> > +
> > +   pr_info("Possible tablet mode switch found; ThinkPad
> > in %s mode\n",
> > +   (tabletsw_state) ? "tablet" : "laptop");
> > +   res = add_to_attr_set(hotkey_dev_attributes,
> > +     &dev_attr_hotkey_tablet_mode.a
> > ttr);
> > +   }

Ehh... the code doesn't act as if it were a "possible" switch, but
rather that it _did_ find a x1_yoga-style switch, so the debug message
should be assertive to match the code:

pr_info("Tablet mode switch found (X1 Yoga style), Thinkpad is in...")

or something to that effect.

If the code is wrong, and it has false possitives, we enhance the
detection (possibly by using a quirk table if we must).

Care to respin with that change?

-- 
  Henrique Holschuh

--
The Command Line: Reinvented for Modern Developers
Did the resurgence of CLI tooling catch you by surprise?
Reconnect with the command line and become more productive. 
Learn the new .NET and ASP.NET CLI. Get your free copy!
http://sdm.link/telerik
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH] platform/x86: Use ACPI_FAILURE at appropriate places

2016-09-19 Thread Henrique de Moraes Holschuh
On Mon, 19 Sep 2016, Axel Lin wrote:
> Use ACPI_FAILURE() to replace !ACPI_SUCCESS(), this avoid !! operations.

Surely no compiler is _that_ idiotic for it to make any difference to
generated code?

Anyway, it is arguably more readable, so I certainly have nothing
against the change on that grounds.


for the thinkpad-acpi bits:

Acked-by: Henrique de Moraes Holschuh 

> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index b65ce75..31fb979 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -9018,7 +9018,7 @@ static int mute_led_on_off(struct tp_led_table *t, bool 
> state)
>   acpi_handle temp;
>   int output;
>  
> - if (!ACPI_SUCCESS(acpi_get_handle(hkey_handle, t->name, &temp))) {
> + if (ACPI_FAILURE(acpi_get_handle(hkey_handle, t->name, &temp))) {
>   pr_warn("Thinkpad ACPI has no %s interface.\n", t->name);
>   return -EIO;
>   }

-- 
  Henrique Holschuh

--
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH 0857/1285] Replace numeric parameter like 0444 with macro

2016-08-02 Thread Henrique de Moraes Holschuh
(cc list trimmed)

On Tue, 02 Aug 2016, Baole Ni wrote:
> I find that the developers often just specified the numeric value
> when calling a macro which is defined with a parameter for access permission.
> As we know, these numeric value for access permission have had the 
> corresponding macro,
> and that using macro can improve the robustness and readability of the code,
> thus, I suggest replacing the numeric parameter with the macro.
> 
> Signed-off-by: Chuansheng Liu 
> Signed-off-by: Baole Ni 

NACK.

IMO, the proposed change reduces readiability for no good reason.  Most
people touching kernel code have 0444, 0644, 0755, etc. already
hardwired into their pattern recognition neural network, while the POSIX
S_* crap is actually bug food.

PS: no more ill-managed ultra-large patch bombs, *please*.

-- 
  "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

--
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


Re: [ibm-acpi-devel] [PATCH v2] thinkpad_acpi: Add support for HKEY version 0x200

2016-06-09 Thread Henrique de Moraes Holschuh
On Thu, 09 Jun 2016, Darren Hart wrote:
> On Thu, Jun 09, 2016 at 01:05:12AM -0400, Marco Trevisan (Treviño) wrote:
> > 2016-06-08 10:54 GMT-04:00 Lyude :
> > > From: Dennis Wassenberg 
> > >
> > > Lenovo Thinkpad devices T460, T460s, T460p, T560, X260 use
> > > HKEY version 0x200 without adaptive keyboard.
> > >
> > > HKEY version 0x200 has method MHKA with one parameter value.
> > > Passing parameter value 1 will get hotkey_all_mask (the same like
> > > HKEY version 0x100 without parameter). Passing parameter value 2 to
> > > MHKA method will retrieve hotkey_all_adaptive_mask. If 0 is returned in
> > > that case there is no adaptive keyboard available.
> > >
> > > Signed-off-by: Dennis Wassenberg 
> > > Signed-off-by: Lyude 
> > > Tested-by: Lyude 
> > > Tested-by: Marco Trevisan 
> > > Acked-by: Henrique de Moraes Holschuh 
> > 
> > Can you also Cc: sta...@vger.kernel.org so that it can be imported
> > there once merged?
> 
> It doesn't meet stable kernel rules unfortunately, with 143 lines diff.
> 
> Henrique, do you want to weigh in on whether this goes back to stable?

It should be in a bunch of -rc kernel releases before we even consider
it.

> One thing I could do is include this in my 4.7-rc fixes branch instead of
> waiting for 4.8.

If Linus will take it, I don't see a problem with that.

-- 
  "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

--
What NetFlow Analyzer can do for you? Monitors network bandwidth and traffic
patterns at an interface-level. Reveals which users, apps, and protocols are 
consuming the most bandwidth. Provides multi-vendor support for NetFlow, 
J-Flow, sFlow and other flows. Make informed decisions using capacity 
planning reports. https://ad.doubleclick.net/ddm/clk/305295220;132659582;e
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel


  1   2   3   4   5   6   7   8   9   10   >