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

2018-06-22 Thread Darren Hart
On Wed, Jun 20, 2018 at 03:49:27PM +0200, 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 
> Acked-by: Henrique de Moraes Holschuh 

Thanks Ben and Henrique, queued up.

-- 
Darren Hart
VMware Open Source Technology Center

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

2018-01-05 Thread Darren Hart
On Wed, Jan 03, 2018 at 09:41:04AM +0100, SF Markus Elfring wrote:
> > I understand it can be frustrating to encounter different policies
> > across kernel maintainers.
> 
> The change acceptance is varying for special transformation patterns.
> 
> 
> > You'll even run in to this with maintainers of the same subsystem
> > from time to time.
> 
> Interesting, isn't it?
> 
> 
> > I'm supportive of cleaning up old code in general,
> 
> Nice.
> 
> 
> > and we routinely apply such patches as these developed with cocci.
> 
> Good to know …
> 
> 
> > 1. This is init code )so any space savings is short lived)
> 
> Would you dare to achieve another small improvement there?
> 
> 
> > So it isn't that we place a low value on coding style guidelines,
> > but rather we place higher value on not perturbing code
> 
> I can follow this view in principle.
> 
> 
> > we can't fully test without a demonstrable functional reasons to do so.
> 
> How do you think about to get a bit nicer run time characteristics?

If this was code that affected all systems, the impact would be greater
- and it would be much easier to test. As it applies only to Thinkpad
systems, far fewer total systems are affected, and it is much harder to
test/verify. For something like this, we (Andy and I) will typically
defer to the driver-specific maintainer. Henrique has declined the
patch, and I think the reasoning is defensible.

If you feel that is the wrong call, you will need to present convincing
evidence to Henrique that this is worth the risk. From what I've seen of
the patch series thus far... I don't think the advantages can be argued
to outweigh the risks - or that it would be worth the effort.

Henrique, I'm going to stop there and let you chime in if you feel
differently about any of the above.

-- 
Darren Hart
VMware Open Source Technology Center

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

2018-01-02 Thread Darren Hart
On Sat, Dec 23, 2017 at 08:12:21AM +0100, SF Markus Elfring wrote:
> >> Do you find the Linux allocation failure report insufficient in this case?
> > 
> > Leave those pr_ messages alone, please,
> 
> Have you got special software development concerns?
> 
> 
> > unless they are really causing some sort of issue (which?).
> 
> Can the code be redundant here?
> 
> 
> > Doing it just for "compliance" with a doc isn't nearly good enough reason.
> 
> Do you give only a low value to coding style guidelines in this use case?

Hi Markus,

Thanks for submitting the patch. I understand it can be frustrating to
encounter different policies across kernel maintainers. You'll even run
in to this with maintainers of the same subsystem from time to time.
While we try to be as consistent as possible, and to document policy as
we resolve vague areas, this is unfortunately still part of kernel
development.

I'm supportive of cleaning up old code in general, and we routinely
apply such patches as these developed with cocci. We have also had them
fail in unexpected ways.

Andy and Henrique raised a few reasons why these patches should not be
accepted:

1. This is init code )so any space savings is short lived)
2. There is no functional fix, and the change is to code which supports
   a lot of unique platforms, most of which we have no way to test. We are
   particularly cautious with drivers such as the thinkpad driver for this
   reason.

So it isn't that we place a low value on coding style guidelines, but
rather we place higher value on not perturbing code we can't fully test
without a demonstrable functional reasons to do so.

Thanks,

-- 
Darren Hart
VMware Open Source Technology Center

--
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 v6] battery: Add the battery hooking API

2017-12-20 Thread Darren Hart
On Wed, Dec 20, 2017 at 03:30:18PM +0100, Ognjen Galić wrote:
> On 18/12/2017, Rafael J. Wysocki <raf...@kernel.org> wrote:
> > Also, what if someone attempts to add a hook when the ACPI battery

> > module is not loaded?  Will that still work?
> 
> The module that requests the hooks will fail to load with the following
> 
> > [  149.259127] thinkpad_acpi: Unknown symbol battery_hook_register (err 0)
> > [  149.259158] thinkpad_acpi: Unknown symbol battery_hook_unregister (err 0)

Will a synchronous request_module() address this concern?

-- 
Darren Hart
VMware Open Source Technology Center

--
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] platform/x86: thinkpad_acpi: Fix warning about deprecated hwmon_device_register

2017-08-18 Thread Darren Hart
On Tue, Jun 20, 2017 at 08:45:13PM -0700, Stanislav Fomichev wrote:
> From: Stanislav Fomichev <s...@google.com>
> 

Oi, apologies for the long delay on this.

> 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.
> 
> 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.
> 
> before:
> $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input}
> thinkpad
> 2007
> $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input}
> cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/name: No such file or 
> directory
> cat: /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/fan1_input: No such 
> file or directory
> $ cat /sys/class/hwmon/hwmon1/{name,fan1_input}
> cat: /sys/class/hwmon/hwmon1/name: No such file or directory
> cat: /sys/class/hwmon/hwmon1/fan1_input: No such file or directory
> $ sensors
> thinkpad-isa-
> Adapter: ISA adapter
> fan1:3533 RPM
> 
> after:
> $ cat /sys/devices/platform/thinkpad_hwmon/{name,fan1_input}
> cat: /sys/devices/platform/thinkpad_hwmon/name: No such file or directory
> cat: /sys/devices/platform/thinkpad_hwmon/fan1_input: No such file or 
> directory
> $ cat /sys/devices/platform/thinkpad_hwmon/hwmon/hwmon1/{name,fan1_input}
> thinkpad
> 3478
> $ cat /sys/class/hwmon/hwmon1/{name,fan1_input}
> thinkpad
> 3478
> $ sensors
> thinkpad-isa-
> Adapter: ISA adapter
> fan1:3489 RPM
> 

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.

> $ sensors -v
> sensors version 3.4.0 with libsensors version 3.4.0
> 
> Changes since v1:
> * Bumped TPACPI_SYSFS_VERSION
> * Updated documentation
> 

Please see Documentation/process/submitting-patches.rst section 14
regarding the placement of the Changelog below the --- marker line as it
is not part of the commit message. (for future patches)

> Signed-off-by: Stanislav Fomichev <ker...@fomichev.me>
> ---
>  Documentation/laptops/thinkpad-acpi.txt |  9 ++--
>  drivers/platform/x86/thinkpad_acpi.c| 38 
> ++---
>  2 files changed, 18 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/laptops/thinkpad-acpi.txt 
> b/Documentation/laptops/thinkpad-acpi.txt
> index ba2e7d254842..1dbef8b8c7b1 100644
> --- a/Documentation/laptops/thinkpad-acpi.txt
> +++ b/Documentation/laptops/thinkpad-acpi.txt
> @@ -121,8 +121,9 @@ space, for 2.6.23+ this is 
> /sys/devices/platform/thinkpad_acpi/.
>  Sysfs device attributes for the sensors and fan are on the
>  thinkpad_hwmon device's sysfs attribute space, but you should locate it
>  looking for a hwmon device with the name attribute of "thinkpad", or
> -better yet, through libsensors.
> -
> +better yet, through libsensors. For 4.13+ sysfs attributes were moved to the

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.

-- 
Darren Hart
VMware Open Source Technology Center

--
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 09/11] platform: thinkpad_acpi: convert to use DRIVER_ATTR_RO/RW

2017-06-09 Thread Darren Hart
On Fri, Jun 09, 2017 at 12:02:25PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jun 09, 2017 at 12:35:35PM +0300, Andy Shevchenko wrote:
> > On Fri, Jun 9, 2017 at 12:03 PM, Greg Kroah-Hartman
> > <gre...@linuxfoundation.org> 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().
> > 
> > Which tree is it supposed to go through?
> > We might need an immutable tag / branch.
> 
> I can take it in my tree, sorry for not saying that in a 00/11 email, I
> forgot that for this series.

OK, no dependencies that I see, but it's a simple mechanical change. Happy for
it to go through Greg's tree.

Reviewed-by: Darren Hart (VMware) <dvh...@infradead.org>

-- 
Darren Hart
VMware Open Source Technology Center

--
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-13 Thread Darren Hart
On Fri, Dec 23, 2016 at 10:55:34PM +0100, Jacek Anaszewski wrote:
> Hi,
> 
> On 12/21/2016 07:49 PM, Pavel Machek wrote:
> > Hi!
> > 
> >>>>> In my eyes trigger approach is neccessary at least for some hardware,
> >>>>> and things it pretty clear: trigger on == LED changes without
> >>>>> userspace involvement. trigger off == userspace controls the LED.
> >>>>
> >>>> It is likely that it would break many existing users.
> >>>
> >>> Can you elaborate on that?
> >>
> >> There might exist users that adjust LED brightness while having
> >> active trigger. The best example is default-on trigger - it sets
> >> brightness only on init, but remains active all the time. Whereas
> >> this could be fixed, there is another case: think of changing blinking
> >> brightness - it would be impossible.
> > 
> > I don't see the breakage. For existing blinking and default-on
> > triggers, existing behaviour would be kept. Difference would be that
> > for hardware that changes triggers itself (like the keyboard light) we
> > would present it as a trigger. And you are right -- there would be
> > user visible changes there. You could not assign blinking, because
> > .. it already has a trigger. But I believe it is reasonable.
> 
> We've already received negative feedback regarding blocking
> application of different trigger when hw brightness change notifications
> are enabled. One solution to that is making the notifications
> orthogonal to the trigger mechanism. The other possibility is to allow
> for having more than one active trigger for a LED.
> 
> We could think of defining a special type of persistent trigger that
> would be always enabled.
> 
> Best regards,
> Jacek Anaszewski
> 
> 
> >>> I just tried with leds on thinkpad
> >>>
> >>> root@duo:/sys/class/leds/tpacpi::power# echo 1 > brightness
> >>> root@duo:/sys/class/leds/tpacpi::power# echo 0 > brightness
> >>> root@duo:/sys/class/leds/tpacpi::power# cat trigger
> >>> [none] bluetooth-power kbd-scrollock kbd-numlock kbd-capslock
> >>> kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock kbd-altlock
> >>> kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock AC-online
> >>> BAT0-charging-or-full BAT0-charging BAT0-full
> >>> BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc
> >>> phy0radio phy0tpt mmc0 timer pattern rfkill1 hci0-power rfkill74
> >>> heartbeat
> >>> root@duo:/sys/class/leds/tpacpi::power#
> >>>
> >>> I can control the LED from userspace, but then there's no way to put
> >>> the LED back to firmware control. That's just broken.
> >>>
> >>> Do you have a proposal how to handle that?
> >>
> >> Isn't it under firmware control all the time?
> > 
> > I'm not 100% sure in the thinkpad case. In the N900 case, we have LED
> > that displays (user_brightness || (user_enabled_indicator && cpu_activity)).
> > 
> > I believe clean way to do that would be a trigger.
> > 
> >>>>> So I'd do the trigger here. It is same way we can handle LEDs on
> >>>>> thinkpad. Yes, it means you won't be able to do oneshot trigger on
> >>>>> backlight. I don't think that's a huge problem.
> >>>>
> >>>> There have been voices in this discussion claiming the opposite. :-)
> >>>
> >>> Well, lets ignore those voices until the voices understand the current
> >>> design :-).
> >>
> >> Sheer user is not interested in design, but in usability.
> > 
> > Well, end user is not expected to touch /sys files directly -- we
> > should create a script for that. /sys should be logical and map well
> > to what we do in kernel. Being "nice to use from shell" is
> > secondary...
> > 
> > Pavel
> > 

This seems to have stalled out. From the driver/platform/x86 perspective, I
believe we're still waiting on the leds subsystem mechanism to be decided on.
Hans, you said you'd send a v6 once that was squared away I believe.

For now, I'm marking these as "changes requested" and archiving the patches.
Will revisit once the LEDs bits are sorted and v6 lands on the list.


If I've missed some context some place, please point me at it.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

--
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 5/5] platform: x86: dell-*: Simplify dell-rbtn integration with dell-laptop [untested]

2017-01-10 Thread Darren Hart
On Thu, Oct 27, 2016 at 02:42:55PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 27-10-16 13:59, Pali Rohár wrote:
> > On Thursday 27 October 2016 13:46:33 Hans de Goede wrote:
> > > Hi,
> > > 
> > > On 27-10-16 12:38, Pali Rohár wrote:
> > > > On Wednesday 26 October 2016 19:41:18 Hans de Goede wrote:
> > > > > Use dell_smbios*notifier for dell-laptop to listen to dell-rbtn slider
> > > > > events, replace dell_rbtn_notifier_register() /
> > > > > dell_rbtn_notifier_unregister() with a single dell_rbtn_has_rfkill() 
> > > > > used
> > > > > by dell-laptop to decide whether or not to use the i8042 filter and 
> > > > > used
> > > > > by dell-rbtn to auto-remove its rfkill interface when called.
> > > > > 
> > > > > This results in a nice cleanup, downside is that the rfkill interface
> > > > > of dell-rbtn is not automatically re-enabled on rmmod dell-laptop, 
> > > > > this
> > > > > now requires rmmod + insmod of dell-rbtn. But people who do not want
> > > > > dell-laptop for some reason will have it blacklisted anyways, so this
> > > > > is not an issue and there is a work-around.
> > > > > 
> > > > > Signed-off-by: Hans de Goede <hdego...@redhat.com>
> > > > > ---
> > > > > Changes in v2:
> > > > > -This is a new patch in v2 of my platform/x86/dell-* notifier set 
> > > > > intended
> > > > > to show how dell_smbios*notifier can be used to improve the dell-rbtn
> > > > > integration too
> > > > > Changes in v3:
> > > > > -Call dell_rbtn_has_rfkill_func instead of dell_rbtn_has_rfkill, so 
> > > > > that the
> > > > > dynamic symbol dance we do to allow loading without dell-rbtn 
> > > > > actually works.
> > > > > ---
> > > > > drivers/platform/x86/Kconfig   |  1 +
> > > > > drivers/platform/x86/dell-laptop.c | 53 
> > > > > +++-
> > > > > drivers/platform/x86/dell-rbtn.c   | 63 
> > > > > +-
> > > > > drivers/platform/x86/dell-rbtn.h   |  5 +--
> > > > > drivers/platform/x86/dell-smbios.h |  1 +
> > > > > 5 files changed, 29 insertions(+), 94 deletions(-)
> > > > 
> > > > Looks like that for preventing sending event that rfkill switch was
> > > > changed by hardware slider we must always drop atk i8042 keycode...
> > > > 
> > > > Needs to check if key is really send by both dell-rbtn and also by atk
> > > > i8042 keyboard driver and if yes then i8042_install_filter() is always
> > > > needed (if rbtn is there or not)...
> > > 
> > > But this is not related to this patch, right? This patch does not change
> > > any behavior. Other then your concerns about where to put the notifier,
> > > do you like the approach of this patch?
> > 
> > It is not related, but if above is truth, then another rewrite (also of
> > those your changes!) is needed. So maybe it would be better to postpone
> > (or drop this patch from your patch series) so we do not invest time for
> > something which will be again rewritten...
> 
> Ok, I'm fine with delaying this cleanup till your questions are answered.
> 
> Maybe we should then also move ahead with the notifier in dell-smbios,
> because without this patch it is only used by dell-wmi and dell-laptop
> both of which already depend on dell-smbios.

This one has been at the bottom of my queue for a while, but as far as I know we
haven't moved forward with the testing and/or clarification we needed. I'm
dropping for now (from the patchwork list), but please feel free to resurrect if
I've missed something, or new information is available.

-- 
Darren Hart
Intel Open Source Technology Center

--
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-12-12 Thread Darren Hart
On Fri, Nov 25, 2016 at 01:31:30PM -0200, Henrique de Moraes Holschuh wrote:
> 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 <tklau...@distanz.ch>
> 
> Acked-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>
> 

Queued to testing, thank you.

-- 
Darren Hart
Intel Open Source Technology Center

--
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: adding new hotkey ID for Lenovo thinkpad

2016-12-12 Thread Darren Hart
On Tue, Nov 08, 2016 at 04:13:23PM +0800, Hui Wang wrote:
> Recently we met an issue on lots of Lenovo thinkpad laptops (those
> 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.
> 
> Signed-off-by: Hui Wang <hui.w...@canonical.com>

With Henrique's ack, I've queued this to testing.

However, in the commit message you mention two deltas:

1) HKEY_HID is LEN0268
2) MHKV is 0x200

This patch appears to address only #1. Is another patch needed for #2?

(I'm digging myself out of a deep backlog, so if it's here already, I'll find
it, but it might expedite things to point it out to me).

Thanks for the patch.

-- 
Darren Hart
Intel Open Source Technology Center

--
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/3] thinkpad_acpi: Move tablet detection into separate function

2016-11-05 Thread Darren Hart
On Mon, Oct 31, 2016 at 07:56:40PM -0400, Lyude wrote:
> Suggested by Daniel Martin <consume.no...@gmail.com>
> 
> 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
review Documentation/SubmittingPatches for the changelog (below ---) and please
be consistent in your placement of "v2" in the subject [PATCH vX N/M] prefix.

Most all of my feedback here is minor, but there were enough little things that
added up and I'd like to see this resubmitted as a series with those addressed
that applies cleanly - largely to make sure I haven't missed context and somehow
merged the wrong bits.

Thanks Lyude,

-- 
Darren Hart
Intel Open Source Technology Center

--
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 3/3 v3] thinkpad_acpi: Add support for X1 Yoga (2016) Tablet Mode

2016-11-05 Thread Darren Hart
On Mon, Oct 31, 2016 at 06:55:34PM -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
> 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.

These go below the ---

> 
> Cc: Daniel Martin <consume.no...@gmail.com>
> Signed-off-by: Lyude <ly...@redhat.com>
> ---
>  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 92e8986..4df4153 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
> +*/
>   TP_HKEY_EV_PEN_INSERTED = 0x500b, /* tablet pen inserted */
>   TP_HKEY_EV_PEN_REMOVED  = 0x500c, /* tablet pen removed */
>   TP_HKEY_EV_BRGHT_CHANGED= 0x5010, /* backlight control event */
> @@ -305,6 +308,7 @@ static struct {
>   enum {
>   TP_HOTKEY_TABLET_NONE = 0,
>   TP_HOTKEY_TABLET_USES_MHKG = 1,
> + TP_HOTKEY_TABLET_USES_CMMD = 2,

1 and 2 will follow automatically, no need to specify.

Pending Henrique's preference on inline enum or external definition.

>   } hotkey_tablet;
>   u32 kbdlight:1;
>   u32 light:1;
> @@ -2062,6 +2066,8 @@ 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
>  
>  static int hotkey_get_wlsw(void)
>  {
> @@ -2086,10 +2092,23 @@ static int hotkey_get_tablet_mode(int *status)
>  {
>   int s;
>  
> - if (!acpi_evalf(hkey_handle, , "MHKG", "d"))
> - return -EIO;
> + switch (tp_features.hotkey_tablet) {
> + case TP_HOTKEY_TABLET_USES_MHKG:
> + if (!acpi_evalf(hkey_handle, , "MHKG", "d"))
> + return -EIO;
> +
> + *status = ((s & TP_HOTKEY_TABLET_MASK) != 0);
> + break;
> + case TP_HOTKEY_TABLET_USES_CMMD:
> + if (!acpi_evalf(ec_handle, , "CMMD", "d"))
> + return -EIO;
> +
> + *status = (s == TP_EC_CMMD_TABLET_MODE);
> + break;
> + default:
> + break;
> + }
>  
> - *status = ((s & TP_HOTKEY_TABLET_MASK) != 0);
>   return 0;
>  }
>  
> @@ -3127,10 +3146,14 @@ hotkey_init_tablet_mode(void)
>   char *type;
>   bool in_tablet_mode;
>  
> - /* For X41t, X60t, X61t Tablets... */
>   if (acpi_evalf(hkey_handle, , "MHKG", "qd")) {
> + /* For X41t, X60t, X61t Tablets... */
>   tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_MHKG;
>   type = "MHKG";
> + } else if (acpi_evalf(ec_handle, , "CMMD", "qd")) {
> + /* For X1 Yoga (2016) */
> + tp_features.hotkey_tablet = TP_HOTKEY_TABLET_USES_CMMD;
> + type = "CMMD";
>   }
>  
>   if (!tp_features.hotkey_tablet)
> @@ -3928,6 +3951,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
>   *ignore_acpi_ev = true;
>   return true;
>  
> + case TP_HKEY_EV_TABLET_CHANGED:
> + tpacpi_input_send_tabletsw();
> + hotkey_tablet_mode_notify_change();
> + *send_acpi_ev = false;
> + break;
> +
>   default:
>   pr_warn("unknown possible thermal alarm or keyboard event 
> received\n");
>   known = false;
> -- 
> 2.7.4
> 
> 

-- 
Darren Hart
Intel Open Source Techno

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

2016-11-05 Thread Darren Hart
On Mon, Oct 31, 2016 at 06:55:32PM -0400, Lyude wrote:
> Suggested by Daniel Martin <consume.no...@gmail.com>

Down below with the other -by: tags please.

> 
> Lenovo's having a bit of fun randomly changing what hotkey events and

Please avoid contractions in documentation in general.

> 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

s/then/than/ (nit, but since I'm commenting anyway...)
s/it's/its/

> 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.
> 
> Signed-off-by: Lyude <ly...@redhat.com>
> Cc: Daniel Martin <consume.no...@gmail.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 50 
> +++-
>  1 file changed, 38 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> b/drivers/platform/x86/thinkpad_acpi.c
> index b65ce75..369b483 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -302,7 +302,10 @@ static struct {
>   u32 hotkey:1;
>   u32 hotkey_mask:1;
>   u32 hotkey_wlsw:1;
> - u32 hotkey_tablet:1;
> + enum {
> + TP_HOTKEY_TABLET_NONE = 0,
> + TP_HOTKEY_TABLET_USES_MHKG = 1,

1 will follow automatically, no need to specify explicitly.

enum usage in this driver is not very consistent, but I don't particularly care
to have it defined within the struct, but I'll leave that for Henrique to
decide.

> + } hotkey_tablet;
>   u32 kbdlight:1;
>   u32 light:1;
>   u32 light_status:1;
> @@ -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)
> +{
> + int res;
> + char *type;
> + bool in_tablet_mode;

Declare variables in reverse line length order please:

bool in_tablet_mode;
char *type;
int res;

> +
> + /* For X41t, X60t, X61t Tablets... */
> + if (acpi_evalf(hkey_handle, , "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;
> +
> + pr_info("Tablet mode switch found (type: %s), currently in %s mode\n",
> + type, in_tablet_mode ? "tablet" : "laptop");
> +
> + res = add_to_attr_set(hotkey_dev_attributes,
> +   _attr_hotkey_tablet_mode.attr);
> + if (res)
> + return -1;
> +
> + return in_tablet_mode;
> +}
> +
>  static int __init hotkey_init(struct ibm_init_struct *iibm)
>  {
>   /* Requirements for changing the default keymaps:
> @@ -3464,17 +3495,6 @@ static int __init hotkey_init(struct ibm_init_struct 
> *iibm)
>   res = add_to_attr_set(hotkey_dev_attributes,
>   _attr_hotkey_radio_sw.attr);
>  
> - /* For X41t, X60t, X61t Tablets... */
> - if (!res && acpi_evalf(hkey_handle, , "MHKG", "qd")) {
> - tp_features.hotkey_tablet = 1;
> - tabletsw_state = !!(status & TP_HOTKEY_TABLET_MASK);
> - pr_info("possible tablet mode switch found; "
> - "ThinkPad in %s mode\n",
> - (tabletsw_state) ? "tablet" : "laptop");
> - res = add_to_attr_set(hotkey_dev_attributes,
> - _attr_hotkey_tablet_mode.attr);
> - }
> -
>   if (!res)
>   res = register_attr_set_with_sysfs(
>   hotkey_dev_attributes,
> @@ -3482,6 +3502,12 @@ static int __init hotkey_init(struct ibm_init_struct 
> *iibm)
>   if (res)
>   goto err_exit;
>  
> + res = hotkey_init_tablet_mode();
> + if (res < 0)
> + goto err_exit;
> +
> + tabletsw_state = res;
> +
>   /* Set up key map */
>   hotkey_keycode_map = kmalloc(TPACPI_HOTKEY_MAP_SIZE,
>   GFP_KERNEL);
> -- 
>

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

2016-11-05 Thread Darren Hart
On Sat, Nov 05, 2016 at 11:45:22AM -0700, 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.

Scratch that, I see there is a new series with this patch in it. Will respond
there.

-- 
Darren Hart
Intel Open Source Technology Center

--
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 Darren Hart
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.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

--
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 0857/1285] Replace numeric parameter like 0444 with macro

2016-08-05 Thread Darren Hart
On Tue, Aug 02, 2016 at 02:34:07PM -0300, Henrique de Moraes Holschuh wrote:
> (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 <chuansheng@intel.com>
> > Signed-off-by: Baole Ni <baolex...@intel.com>
> 
> 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.

While I'm generally in favor of using macros where they exist, I do agree with
Henrique that this is actually less legible.

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

Indeed. 1285 patches with the same subject line is "not ideal". Prefixing with
the subsystem at the very least would have been an improvement. An RFC on the
concept, cc'ing the subsystem maintainers to get consensus and direction on how
to manage the large change would have been advisable.

I'm dropping these for pdx86 unless a compelling argument arises for including
them (like - the only subsystem not taking these is pdx86...)

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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 Darren Hart
On Thu, Jun 09, 2016 at 06:04:52PM -0300, Henrique de Moraes Holschuh wrote:
> 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 <cp...@redhat.com>:
> > > > From: Dennis Wassenberg <dennis.wassenb...@secunet.com>
> > > >
> > > > 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 <dennis.wassenb...@secunet.com>
> > > > Signed-off-by: Lyude <cp...@redhat.com>
> > > > Tested-by: Lyude <cp...@redhat.com>
> > > > Tested-by: Marco Trevisan <ma...@ubuntu.com>
> > > > Acked-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>
> > > 
> > > 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.

I'll do that.

-- 
Darren Hart
Intel Open Source Technology Center

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


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

2016-06-09 Thread Darren Hart
On Thu, Jun 09, 2016 at 01:05:12AM -0400, Marco Trevisan (Treviño) wrote:
> 2016-06-08 10:54 GMT-04:00 Lyude <cp...@redhat.com>:
> > From: Dennis Wassenberg <dennis.wassenb...@secunet.com>
> >
> > 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 <dennis.wassenb...@secunet.com>
> > Signed-off-by: Lyude <cp...@redhat.com>
> > Tested-by: Lyude <cp...@redhat.com>
> > Tested-by: Marco Trevisan <ma...@ubuntu.com>
> > Acked-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>
> 
> 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?

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

-- 
Darren Hart
Intel Open Source Technology Center

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


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

2016-06-08 Thread Darren Hart
> 8) {
> + case 1:
>   /*
>* MHKV 0x100 in A31, R40, R40e,
>* T4x, X31, and later
>*/
> - vdbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_HKEY,
> - "firmware HKEY interface version: 0x%x\n",
> - hkeyv);
>  
>   /* Paranoia check AND init hotkey_all_mask */
>   if (!acpi_evalf(hkey_handle, _all_mask,
> @@ -3381,6 +3378,50 @@ static int __init hotkey_init(struct ibm_init_struct 
> *iibm)
>   } else {
>   tp_features.hotkey_mask = 1;
>   }
> + break;
> +
> + case 2:
> + /*
> +  * MHKV 0x200 in X1, T460s, X260, T560, X1 Tablet (2016)
> +  */
> +
> + /* Paranoia check AND init hotkey_all_mask */
> + if (!acpi_evalf(hkey_handle, _all_mask,
> + "MHKA", "dd", 1)) {
> + pr_err("missing MHKA handler, "
> +"please report this to %s\n",

Strings should be kept on the same line so they can be searched for in the
sources. I've corrected this here as well as the existing duplicate message in 
case 1.

Otherwise, queued for 4.8. Thanks.

-- 
Darren Hart
Intel Open Source Technology Center

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


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

2016-06-07 Thread Darren Hart
On Tue, Jun 07, 2016 at 05:10:51PM -0400, Lyude Paul wrote:
> Managed to find someone in the office with one of these laptops and it looks
> like the adaptive keyboard works perfectly with this patch, so I can give my 
> t-
> b:
> 
>   Tested-by: Lyude Paul <cp...@redhat.com>
> 
> Cheers,
>   Lyude
> 
> On Tue, 2016-06-07 at 13:02 -0400, Lyude Paul wrote:
> > Since nothing's really happened with this patch for a while I figured I'd 
> > take
> > over trying to get this upstream.
> > 
> > Regarding testing: This seems to work fine on the 60 series laptops, and 
> > works
> > fine on previous generations. The one thing I haven't been able to test is 
> > an
> > X1
> > carbon with an adaptive keyboard since I don't seem to have one readily
> > available here. I'm doing a search around the office to try to find someone
> > who
> > didn't throw theirs away yet so hopefully I should be able to get back to 
> > you
> > on
> > that soon.
> > 
> > To Dennis: I took the liberty of doing a review of your patch and some
> > testing.
> > There's a few things that need changing, I've outlined them below: (for the
> > future, it's recommended to send patches for the kernel inline in emails to
> > make
> > them easier to review).

If someone would please update the patch with any pending changes from review
and so it follows the rules in Documentation/SubmittingPatches, including the
complete list of recipients on Cc, we can move this forward.

Regards,

-- 
Darren Hart
Intel Open Source Technology Center

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


Re: [ibm-acpi-devel] [PATCH] Remove ambiguous logging for "Unsupported brightness interface"

2016-02-07 Thread Darren Hart
On Sat, Jan 30, 2016 at 04:55:59PM +, Eric Curtin wrote:
> On 30 January 2016 at 12:20, Henrique de Moraes Holschuh <h...@hmh.eng.br> 
> wrote:
> > On Wed, 27 Jan 2016, Joe Perches wrote:
> >> On Wed, 2016-01-27 at 22:14 +, Eric Curtin wrote:
> >> > Message gets logged on machines that are well supported.
> >> >
> >> > Signed-off-by: Eric Curtin <ericcurti...@gmail.com>
> >> > ---
> >> >  drivers/platform/x86/thinkpad_acpi.c | 1 -
> >> >  1 file changed, 1 deletion(-)
> >> >
> >> > diff --git a/drivers/platform/x86/thinkpad_acpi.c
> >> > b/drivers/platform/x86/thinkpad_acpi.c
> >> > index a268a7a..4eb41aa 100644
> >> > --- a/drivers/platform/x86/thinkpad_acpi.c
> >> > +++ b/drivers/platform/x86/thinkpad_acpi.c
> >> > @@ -6661,7 +6661,6 @@ static void __init
> >> > tpacpi_detect_brightness_capabilities(void)
> >> > pr_info("detected a 8-level brightness capable
> >> > ThinkPad\n");
> >> > break;
> >> > default:
> >> > -   pr_info("Unsupported brightness interface\n");
> >> > tp_features.bright_unkfw = 1;
> >> > bright_maxlvl = b - 1;
> >> > }
> >>
> >> Perhaps this should be something like this instead:
> >> ---
> >>  drivers/platform/x86/thinkpad_acpi.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/platform/x86/thinkpad_acpi.c 
> >> b/drivers/platform/x86/thinkpad_acpi.c
> >> index a268a7a..bd12c71 100644
> >> --- a/drivers/platform/x86/thinkpad_acpi.c
> >> +++ b/drivers/platform/x86/thinkpad_acpi.c
> >> @@ -6653,18 +6653,16 @@ static void __init 
> >> tpacpi_detect_brightness_capabilities(void)
> >>   switch (b) {
> >>   case 16:
> >>   bright_maxlvl = 15;
> >> - pr_info("detected a 16-level brightness capable ThinkPad\n");
> >>   break;
> >>   case 8:
> >>   case 0:
> >>   bright_maxlvl = 7;
> >> - pr_info("detected a 8-level brightness capable ThinkPad\n");
> >>   break;
> >>   default:
> >> - pr_info("Unsupported brightness interface\n");
> >>   tp_features.bright_unkfw = 1;
> >>   bright_maxlvl = b - 1;
> >>   }
> >> + pr_info("detected %u brightness levels\n", bright_maxlvl + 1);
> >>  }
> >
> > This can be made pr_debug, since we're touching it...
> >
> > --
> >   "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
> 
> "Unsupported brightness interface" message gets logged on
>  machines that are well supported.
> 
> Signed-off-by: Eric Curtin <ericcurti...@gmail.com>
> ---
>  drivers/platform/x86/thinkpad_acpi.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index a268a7a..e305ab5 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -6653,18 +6653,16 @@ static void __init
> tpacpi_detect_brightness_capabilities(void)
> switch (b) {
> case 16:
> bright_maxlvl = 15;
> -   pr_info("detected a 16-level brightness capable ThinkPad\n");
> break;
> case 8:
> case 0:
> bright_maxlvl = 7;
> -   pr_info("detected a 8-level brightness capable ThinkPad\n");
> break;
> default:
> -   pr_info("Unsupported brightness interface\n");
> tp_features.bright_unkfw = 1;
> bright_maxlvl = b - 1;
> }
> +   pr_debug("detected %u brightness levels\n", bright_maxlvl + 1);

This patch is malformed, has whitespace issues, and doesn't apply. Please apply
all patches before sending them to the list.

Given it's trivial, I made the change manually and have queued this up to the
testing branch.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=272487151=/4140
___
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 4/4] Revert "thinkpad_acpi: Use acpi_video_handles_brightness_key_presses()"

2016-01-14 Thread Darren Hart
On Thu, Jan 14, 2016 at 09:41:48AM +0100, Hans de Goede wrote:
> acpi_video_handles_brightness_key_presses()'s may return false if the i915
> driver is not loaded yet when thinkpad_acpi loads, and then return true
> after the i915 driver has loaded. This means that thinkpad_acpi cannot
> use it as is since thinkpad_acpi caches the return value.
> 
> This reverts commit 7714687a2b2d ("thinkpad_acpi: Use
> acpi_video_handles_brightness_key_presses()").
> 

Rafael, I presume this would go through your tree?

No objection from me. Henrique?

Acked-by: Darren Hart <dvh...@linux.intel.com>

> Signed-off-by: Hans de Goede <hdego...@redhat.com>
> ---
>  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 f453d5d..0bed473 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -3488,7 +3488,7 @@ static int __init hotkey_init(struct ibm_init_struct 
> *iibm)
>   /* Do not issue duplicate brightness change events to
>* userspace. tpacpi_detect_brightness_capabilities() must have
>* been called before this point  */
> - if (acpi_video_handles_brightness_key_presses()) {
> + if (acpi_video_get_backlight_type() != acpi_backlight_vendor) {
>   pr_info("This ThinkPad has standard ACPI backlight "
>   "brightness control, supported by the ACPI "
>   "video driver\n");
> -- 
> 2.5.0
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140
___
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 keyboard backlight

2016-01-12 Thread Darren Hart
On Mon, Jan 11, 2016 at 05:28:00PM -0200, Henrique de Moraes Holschuh wrote:
 > 
> > Henrique, so are you taking back your Ack from 10 minutes prior?
> 
> Hmm? No, the ACK stands.
> 
> Pavel was talking about another feature altogether, apparently: older
> thinkpads did not have "keyboard backlight" (as in light from below the
> keys).  They had a "ThinkLight", which is an overhead light that shines
> down on the keyboard.
> 
> The two features are not the same (and are handled differently by the
> firmware, for whatever reason), although they do serve the same purpose.
>  I don't think a thinkpad will ever have both features at the same time,
> so I have no idea why they changed the firmware interface.
> 
> The patch adds support to the "keyboard backlight" feature, which was
> previously NOT supported.
> 
> > I've dropped this patch. Please let me know if I should pick it back up.
> 
> Please pick it back up.
> 

Gah, these two threads landed next to each other in my Inbox and I didn't pick
up the split. Apologies.

This is queued in testing (again).

-- 
Darren Hart
Intel Open Source Technology Center

--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140
___
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 keyboard backlight

2016-01-11 Thread Darren Hart
On Sat, Jan 09, 2016 at 03:46:41PM -0200, Henrique de Moraes Holschuh wrote:
> On Sat, Jan 9, 2016, at 15:39, Henrique de Moraes Holschuh wrote:
> > On Mon, Jan 4, 2016, at 18:12, Pavel Machek wrote:
> > > > This patch adds support for controlling keyboard backlight via standard
> > > > linux led class interface (::kbd_backlight). It uses ACPI HKEY device 
> > > > with
> > > > MLCG and MLCS methods.
> > > > 
> > > > Signed-off-by: Pali Rohár <pali.ro...@gmail.com>
> > > > Tested-by: Fabio D'Urso <fabiodu...@hotmail.it>
> > > 
> > > On my thinkpad, keyboard light is controlled by
> > > 
> > > /sys/class/leds/tpacpi\:\:thinklight/brightness
> > > 
> > > (that's a bad name).
> > 
> > That's because the driver was not updated to support your laptop, AND I
> > don't recall if someone mapped the full behavior of the ACPI thinklight
> > interface in your thinkpad :-(
> 
> Argh.  If by "keyboard light" you mean the LED above the *screen* that
> shines down on the whole keyboard, please disregard my previous reply...
> 
> As for the naming, the idea of a LED up there shining in the keyboard
> is:
>  1. patented by IBM
>  2. named "ThinkLight" by IBM, one of the "Think Technologies" in the
>  "ThinkPad" (add TM after everything :p)  and every
>  old-timer thinkpad user knew it by that name.
>  4. called "thinklight" by the driver since before the LED sysfs class
>  even existed :p
> 
> ibm-acpi, since then renamed thinkpad-acpi *predates* most generic
> interfaces.  Heck, it predates sysfs.
> 
> So, this is ABI set in stone.  If there is a way to add an "alias" of
> kbd_backlight that won't drive userspace crazy, we might do that though.
>  But it looks quite risky to me...

Henrique, so are you taking back your Ack from 10 minutes prior?

I've dropped this patch. Please let me know if I should pick it back up.

-- 
Darren Hart
Intel Open Source Technology Center

--
Site24x7 APM Insight: Get Deep Visibility into Application Performance
APM + Mobile APM + RUM: Monitor 3 App instances at just $35/Month
Monitor end-to-end web transactions and take corrective actions now
Troubleshoot faster and improve end-user experience. Signup Now!
http://pubads.g.doubleclick.net/gampad/clk?id=267308311=/4140
___
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 keyboard backlight

2016-01-04 Thread Darren Hart
On Mon, Jan 04, 2016 at 09:26:19PM +0100, Pali Rohár wrote:
> On Monday 04 January 2016 21:04:25 Darren Hart wrote:
> > On Wed, Dec 30, 2015 at 11:27:41PM +0100, Pali Rohár wrote:
> > > This patch adds support for controlling keyboard backlight via
> > > standard linux led class interface (::kbd_backlight). It uses ACPI
> > > HKEY device with MLCG and MLCS methods.
> > 
> > Which laptops is this intended to support?
> 
> Thinkpad ??30 series and new which have backlight keyboard.

Thanks, we should include that in the commit message as well as the comments
surrounding the driver section.

Henrique, are your concerns surrounding suspend/resume resolved?

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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 keyboard backlight

2016-01-04 Thread Darren Hart
On Wed, Dec 30, 2015 at 11:27:41PM +0100, Pali Rohár wrote:
> This patch adds support for controlling keyboard backlight via standard
> linux led class interface (::kbd_backlight). It uses ACPI HKEY device with
> MLCG and MLCS methods.
> 

Which laptops is this intended to support?

Henrique, I'm holding off a bit more to give you time to respond given the
holiday season.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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 keyboard backlight

2016-01-04 Thread Darren Hart
On Mon, Jan 04, 2016 at 09:51:23PM +0100, Pali Rohár wrote:
> On Monday 04 January 2016 21:40:20 Darren Hart wrote:
> > On Mon, Jan 04, 2016 at 09:26:19PM +0100, Pali Rohár wrote:
> > > On Monday 04 January 2016 21:04:25 Darren Hart wrote:
> > > > On Wed, Dec 30, 2015 at 11:27:41PM +0100, Pali Rohár wrote:
> > > > > This patch adds support for controlling keyboard backlight via
> > > > > standard linux led class interface (::kbd_backlight). It uses
> > > > > ACPI HKEY device with MLCG and MLCS methods.
> > > > 
> > > > Which laptops is this intended to support?
> > > 
> > > Thinkpad ??30 series and new which have backlight keyboard.
> > 
> > Thanks, we should include that in the commit message as well as the
> > comments surrounding the driver section.
> 
> ??30 is probably not good characteristic, but I mean all those Thinkpad 
> laptops like T430, x230, E430, X1 (1st) and their successors (T440, 
> T450, X1 3rd, ...) All those which are from Ivy Bridge processor 
> generation (and new).
> 
> But basically it cover all Thinkpad laptops which have backlight 
> keyboard. Older Thinkpad laptops had only light in bezel.
> 
> So if you have better idea for commit message, feel free to change it.

Ah, I see. New feature that didn't exist previously. OK, no need for changes on
that score then.



-- 
Darren Hart
Intel Open Source Technology Center

--
___
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/5] acpi-video and platform/x86 driver fixes

2015-12-22 Thread Darren Hart
On Tue, Dec 22, 2015 at 07:09:47PM +0100, Hans de Goede wrote:
> Hi All,
> 
> This patch-set is the result of the discussion surrounding the
> backlight issues on the Dell Vostro V131. The first 3 patches
> cleanup how some platform/x86 drivers detect if acpi-video
> is handling brightness key presses and that they should
> not report duplicate events. This is a cleanup which I wanted
> to do for a while already and the Vostro V131 fix actually
> benefits from this.
> 
> The last 2 patches together actually fix the issues on the
> Vostro V131. Since the 2 platform patches in this set depend
> on an acpi patch, I believe that it would be best if this
> entire set gets merged via the acpi tree with acks
> from the platform driver maintainer.

Agreed on the path. For this situation, the addition of the module parameter
will facilitate identifying similarly broken paltforms to the Vostro V131, and
provide users with a temporary solution until the DMI match can be added. Very
practical.

I have no objection to the changes in platform-drivers-x86.

Reviewed-by: Darren Hart <dvh...@linux.intel.com>

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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: don't yell on unsupported brightness interfaces

2015-10-21 Thread Darren Hart
On Wed, Oct 21, 2015 at 12:33:52PM -0200, Henrique de Moraes Holschuh wrote:
> On Wed, Oct 21, 2015, at 08:46, David Herrmann wrote:
> > The thinkpad_acpi driver currently emits error messages on unsupported
> > brightness interfaces, giving the impression that someone will implement
> > those. However, this error is spit out on nearly every thinkpad in
> > production since 2 years now. Furthermore, the backlight interfaces on
> > those devices are supported by the i915 driver just fine.
> > 
> > Downgrade the error message to a normal pr_info() and stop telling people
> > to report it to IBM.
> 
> IBM?  Those reports go directly to me.
> 
> > Signed-off-by: David Herrmann <dh.herrm...@gmail.com>
> 
> Acked-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>

Thanks, Queued.

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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: Lenovo ThinkPad Yoga 12 side lock button (0x6020)

2015-10-11 Thread Darren Hart
On Sun, Oct 11, 2015 at 09:12:54AM -0300, Henrique de Moraes Holschuh wrote:
> On Sat, 10 Oct 2015, Darren Hart wrote:
> > The Lenovo ThinkPad Yogo 12 has a button on the side by the power and volume
> > keys with a padlock icon on it, I presume intended to be used as Screenlock.
> > 
> > It emits 0x6020 which means it falls into the hotkey_notify_6xxx() block. It
> > seems this should be handled similarly to the 0x1xxx events instead (as a
> > hotkey).
> > 
> > Is this a keymap change candidate? If so, How do I map 0x6020 to a scancode?
> 
> We'd have to extend thinkpad-acpi to have sparse keymaps.  It is not a good
> idea to intrude on the 0x1xxx range, as Lenovo can, and did, extend it at
> least once already (adaptive keyboard).
> 
> We could, as a first step, map 0x6020 to a static keycode, as the padlock
> symbol is pretty specific.

Do we have any examples of such keys currently? I poured over the driver for a
while, but didn't find one. I still need to understand which bits of the driver
are relevant to the current hardware and which are legacy support.

> 
> Extending thinkpad-acpi for sparse keymaps needs two steps:
> 
> 1. Design the keymap, it will need a "vendor range", and a "driver range".
> We should reserve 0x to 0x as vendor range, and something higher for
> driver and future use.

Can you explain? What distinguishes the vendor range and the driver range - or
even what is the semantic difference? What makes a key a vendor key and what
makes it a driver key?

> 
> I think we could reserve bits 28-31 of the keymap "address" to select
> "bank", and have 00 = vendor, 01 = driver, and the others as reserved.
> 
> Also, thinkpad-acpi may need to issue events that are not EV_KBD.  Do we
> have a map that can do that sort of mapping?  Should we leave those out, as
> special cases?
> 

Are you referring to the thermal, dock, battery, etc. events?

> 2. Implement the keymap, and decide what we do when someone maps an event
> that is currently special-cased in thinkpad-acpi, or not something that
> should be mapped to an external event in the first place.
> 
> Reject the attempt?  Override driver behavior?  Keep driver behavior _and_
> issue the event?
> 

When you say "when someone maps an event", I presume you are referring to a
userspace initiated mapping to modify behavior at runtime, not someone sending a
patch to the driver?

I don't know what the mechanisms are like there, but yes, given the experience
you've cited with this driver, and the little bit I've observed over the last
year with things like the hw audio mixer, we want to avoid any unintentional
userpace commitments.

> E.g. allowing 0x6020 is a no-brainer: we should.  But what about 0x6030
> (thermal table changed), or the battery alarms?  I'd say they should never
> become keypresses: if anyone is doing that, they're abusing the input layer
> for sure, and this _always_ comes back to bite us later.
> 
> I've had some bad experiences with this.  People creating feedback loops
> into the driver, etc.  Some of this braindamage, introduced in userspace a
> decade ago, is still limiting or discouraging feature implementation today.

Right, those warnings are loud and clear in the driver comments (which is a good
thing) :-)

-- 
Darren Hart
Intel Open Source Technology Center

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


[ibm-acpi-devel] thinkpad_acpi: Lenovo ThinkPad Yoga 12 side lock button (0x6020)

2015-10-10 Thread Darren Hart
The Lenovo ThinkPad Yogo 12 has a button on the side by the power and volume
keys with a padlock icon on it, I presume intended to be used as Screenlock.

It emits 0x6020 which means it falls into the hotkey_notify_6xxx() block. It
seems this should be handled similarly to the 0x1xxx events instead (as a
hotkey).

Is this a keymap change candidate? If so, How do I map 0x6020 to a scancode?

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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 TP_HKEY_EV_SCREEN_FLIP (0x60c0) for Lenovo ThinkPad Yoga 12

2015-10-10 Thread Darren Hart
On Sat, Oct 10, 2015 at 06:40:34PM -0300, Henrique de Moraes Holschuh wrote:
> On Sat, 10 Oct 2015, Darren Hart wrote:
> > On Sat, Oct 10, 2015 at 11:41:47AM -0700, Darren Hart wrote:
> > > From: Darren Hart <dvh...@infradead.org>
> > > 
> > > thinkpad_acpi reports "unhandled HKEY event 0x60c0" when switching
> > > between laptop and tent/tablet mode on the ThinkPad Yoga 12. This key is
> > > reported by atkbd serio0 as e058 and e059.
> > > 
> > > Add TP_HKEY_EV_SCREEN_FLIP to the list of ignored events and silence the
> > > warning.
> > > 
> > > Cleanup whitespace on preceeding line for TP_HKEY_EV_KEY_FN_ESC.
> > > 
> > > Signed-off-by: Darren Hart <dvh...@infradead.org>
> > > Cc: Henrique de Moraes Holschuh <ibm-a...@hmh.eng.br>
> > > Cc: ibm-acpi-devel@lists.sourceforge.net
> 
> Acked-by: Henrique de Moraes Holschuh <h...@hmh.eng.br>

Thanks Henrique,

Now, as I'm digging into this I'm finding maybe there is more to this. Should I
be attempting to tie this in to the tablet switch mechanism for the older x61t?

Or is this something we now want to strictly hand off to a userspace helper via
the atkbd event?

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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: Remove side effects from vdbg_printk - no_printk macro

2015-08-28 Thread Darren Hart
On Thu, Aug 27, 2015 at 02:33:06PM -0300, Henrique de Moraes Holschuh wrote:
 On Wed, 26 Aug 2015, Joe Perches wrote:
  vdbg_printk when not using CONFIG_THINKPAD_ACPI_DEBUG uses
  no_printk which produces no logging output but always
  evaluates arguments.
  
  Change the macro to surround the no_printk call with
  do { if (0) no_printk(...); } while (0)
  to avoid the unnecessary argument evaluations.
  
  $ size drivers/platform/x86/thinkpad_acpi.o*
 textdata bss dec hex filename
609186184 824   67926   10956 
  drivers/platform/x86/thinkpad_acpi.o.new
609276184 824   67935   1095f 
  drivers/platform/x86/thinkpad_acpi.o.old
  
  Signed-off-by: Joe Perches j...@perches.com
 
 Acked-by: Henrique de Moraes Holschuh h...@hmh.eng.br

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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 00/32] acpi-video: Rewrite backlight interface selection logic

2015-06-16 Thread Darren Hart
On Tue, Jun 16, 2015 at 01:22:40AM +0200, Rafael Wysocki wrote:
 On Friday, June 12, 2015 01:23:19 PM Hans de Goede wrote:
  Hi All,
  
  Here is v2 of my rewrite / cleanup of the acpi-video (and platform/x86)
  backlight interface selection logic.
  
  The major change in v2 of this set are 2 changes to the patch titled
  acpi-video-detect: video: Make video_detect code part of the video module:
  
  1) The __setup call for the acpi_backlight= handling is moved to
 acpi/util.c as __setup may only be used by code which is always builtin
  2) video.c is renamed to acpi_video.c so that it can be combined with
 video_detect.c into video.ko
  
  These 2 changes result in the backlight kernel commandline options after 
  this
  patch-set being 100% compatible with the old options, with the exception of
  the removal of the video.use_native_backlight option as that now is folded
  into acpi_backlight=[video|vendor|native|none] rather then having 2 options
  which influence each other in interesting ways.
  
  Besides that there is a small change to the dell-laptop port to the new API
  removing an unnecessary #ifdef ACPI.
 
 I tried to queue this series up for 4.2, but patch [06/32] didn't apply for 
 me,
 so I decided to apply another patch I've got recently instead and which is now
 in my linux-next branch.
 
 Can you please rebase the series on top of linux-pm.git/linux-next and resend 
 it?

Rafael, do you intent to carry this entire series and submit via linux-pm?

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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: Reduce screen brightness user visible noise.

2015-06-15 Thread Darren Hart
On Mon, Jun 15, 2015 at 12:29:14PM -0300, Ramiro Morales wrote:
 Implement change in logging during screen brightness capabilities
 detection as suggested in
 https://www.mail-archive.com/ibm-acpi-devel%40lists.sourceforge.net/msg03484.html
 

Summarize the rationale here, the change log needs to stand alone. You can
reference a link, but the justification cannot depend on it.

 Signed-off-by: Ramiro Morales cra...@gmail.com
 ---
  drivers/platform/x86/thinkpad_acpi.c | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index 28f3281..b48efc9 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -6450,19 +6450,16 @@ static void __init 
 tpacpi_detect_brightness_capabilities(void)
   switch (b) {
   case 16:
   bright_maxlvl = 15;
 - pr_info(detected a 16-level brightness capable ThinkPad\n);
   break;
   case 8:
   case 0:
   bright_maxlvl = 7;
 - pr_info(detected a 8-level brightness capable ThinkPad\n);
   break;
   default:
 - pr_err(Unsupported brightness interface, 
 -please contact %s\n, TPACPI_MAIL);
   tp_features.bright_unkfw = 1;
   bright_maxlvl = b - 1;
   }
 + pr_debug(firmware reports %d brightness levels\n, bright_maxlvl);
  }
  
  static int __init brightness_init(struct ibm_init_struct *iibm)
 @@ -6482,6 +6479,9 @@ static int __init brightness_init(struct 
 ibm_init_struct *iibm)
  
   /* if it is unknown, we don't handle it: it wouldn't be safe */
   if (tp_features.bright_unkfw)
 + dbg_printk(TPACPI_DBG_INIT | TPACPI_DBG_BRGHT,
 +Unsupported brightness mode, use acpi video 
 +or gpu drivers);

Please run checkpatch on your patch (don't break quoted strings - it makes it
hard to find them with grep).

   return 1;
  
   if (!brightness_enable) {
 -- 
 1.9.1
 
 

-- 
Darren Hart
Intel Open Source Technology Center

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


Re: [ibm-acpi-devel] [RFC 07/24] x86/thinkpad_acpi: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_write_byte()

2015-06-02 Thread Darren Hart
On Tue, Jun 02, 2015 at 07:09:28AM -0300, Henrique de Moraes Holschuh wrote:
 Test results were sent to me privately, and they are correct, so...
 

Finn, unless there is some compelling reason not to - like they are MBs worth of
data, please submit these to the list in the future so we have them for
reference.

 Acked-by: Henrique de Moraes Holschuh h...@hmh.eng.br

I'm fine with the changes, but they need to be submitted with the other changes
as this one change cannot compile independently in my tree.

Finn, please work with whomever is pulling the series to include this in their
pull request.

Reviewed-by: Darren Hart dvh...@linux.intel.com

-- 
Darren Hart
Intel Open Source Technology Center

--
___
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: revert unintentional device attribute renaming

2015-05-20 Thread Darren Hart
On Tue, May 19, 2015 at 02:55:17PM -0300, Henrique de Moraes Holschuh wrote:
 On Tue, May 19, 2015, at 14:45, Bjørn Mork wrote:
  The conversion to DEVICE_ATTR_* macros failed to fixup a few cases where
  the old attribute names didn't match the show/store function names.
  Instead of renaming the functions, the attributes were renamed. This
  caused an unintentional API change.  The hwmon required 'name' attribute
  were among the renamed attribute, causing libsensors to fail to detect
  the hwmon device at all.
  
  Fix by using the DEVICE_ATTR macro for these attributes, allowing the
  show/store functions to keep their system specific prefixes.
  
  Fixes: b4dd04ac6ef8 (thinkpad_acpi: use DEVICE_ATTR_* macros)
  Cc: Bastien Nocera had...@hadess.net
  Cc: Henrique de Moraes Holschuh h...@hmh.eng.br
  Signed-off-by: Bjørn Mork bj...@mork.no
  ---
  v2: kept the original function names, using the DEVICE_ATTR instead
 
 Acked-by: Henrique de Moraes Holschuh h...@hmh.eng.br

Thanks for the catch Bjørn, much appreciated. Queued for 4.1 fixes.

-- 
Darren Hart
Intel Open Source Technology Center

--
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
___
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 59/86] x86/thinkpad_acpi: use uapi/linux/pci_ids.h directly

2015-04-01 Thread Darren Hart
On Sun, Mar 29, 2015 at 03:41:54PM +0200, Michael S. Tsirkin wrote:
 Header moved from linux/pci_ids.h to uapi/linux/pci_ids.h,
 use the new header directly so we can drop
 the wrapper in include/linux/pci_ids.h.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com

This isn't in mainline yet, so I presume whoever is pushing the move to next
will pick this up as well and keep it all together.

Acked-by: Darren Hart dvh...@linux.intel.com

 ---
  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 3b8ceee..f1ac982 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -77,7 +77,7 @@
  #include linux/jiffies.h
  #include linux/workqueue.h
  #include linux/acpi.h
 -#include linux/pci_ids.h
 +#include uapi/linux/pci_ids.h
  #include linux/thinkpad_acpi.h
  #include sound/core.h
  #include sound/control.h
 -- 
 MST
 
 

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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] thinkpad_acpi: signedness bugs getting current_mode

2015-03-18 Thread Darren Hart
On Sun, Mar 15, 2015 at 01:48:13PM +1100, Stephen Rothwell wrote:
 Hi Darren,
 
 On Sun, 15 Mar 2015 13:46:12 +1100 Stephen Rothwell s...@canb.auug.org.au 
 wrote:
 
  The people you will inconvenience more by rebasing are the developers
  who write patches that are based on your tree.  If you rebase under
  them, they may have to rebase and fix up the patches they have already
  tested and had reviewed before they can then submit them to you
  (hopefully before you rebase again).
 
 This, of course, only applied to published trees (which includes
 anything in linux-next) - what you do in your own (logically private)
 development tree is your own business.

Thanks for the guidance Stephen. Unless a more compelling argument comes up,
I'll stick to this going forward.


-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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] thinkpad_acpi: signedness bugs getting current_mode

2015-03-14 Thread Darren Hart
On Wed, Mar 11, 2015 at 12:34:50PM +0300, Dan Carpenter wrote:
 This needs to be signed for the error handling to work.  Valid modes are
 small positive integers.
 
 Fixes: b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 

Question for HPA, Rafael, and Stephen,

I recall discussion at Kernel Summit 2014 about not rebasing or merging patches
when sending to Linus, that he'd prefer to see the history. I recall Stephen
mentioning something similar for linux-next.

That said, I've seen varying behavior among maintainers with respect to fixes
like this one from Dan. This patch fixes a patch that currently only exists in
my for-next and Stephen's linux-next trees.

What is the preference. Do I just queue it up to for-next as is (this is what
I've done for now), or do I roll it into the referred patch causing the error
and credit Dan with the fixup?

Left to my own devices I would prefer not to introduce bugs into the kernel
history if I can help it. That said, I don't want to make extra work for Stephen
or Linus.

What's the prefered best practice here?

 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index 56eaddc..024861d 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -2938,7 +2938,7 @@ static ssize_t adaptive_kbd_mode_show(struct device 
 *dev,
  struct device_attribute *attr,
  char *buf)
  {
 - u32 current_mode;
 + int current_mode;
  
   current_mode = adaptive_keyboard_get_mode();
   if (current_mode  0)
 @@ -3621,7 +3621,7 @@ static int adaptive_keyboard_get_next_mode(int mode)
  
  static bool adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
  {
 - u32 current_mode = 0;
 + int current_mode = 0;
   int new_mode = 0;
   int keycode;
  
 

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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] thinkpad_acpi: signedness bugs getting current_mode

2015-03-14 Thread Darren Hart
On Wed, Mar 11, 2015 at 12:34:50PM +0300, Dan Carpenter wrote:
 This needs to be signed for the error handling to work.  Valid modes are
 small positive integers.
 
 Fixes: b790ceeb0fd9 ('thinkpad_acpi: Add adaptive_kbd_mode sysfs attr')
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
 

Queued, thanks Dan!

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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] thinkpad_acpi: off by one in adaptive_keyboard_hotkey_notify_hotkey()

2015-03-14 Thread Darren Hart
On Wed, Mar 11, 2015 at 12:36:07PM +0300, Dan Carpenter wrote:
 This should be = instead of  because otherwise we read one element
 past the end of the hotkey_keycode_map[] array.
 
 The hotkey_keycode_map[] array has TPACPI_HOTKEY_MAP_LEN elements.
 
 Fixes: 6a68d8557084 ('thinkpad_acpi: Add support for more adaptive kbd 
 buttons')
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Queued, thanks.

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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/7] thinkpad_acpi: Remember adaptive kbd presence

2015-03-06 Thread Darren Hart
On Tue, Mar 03, 2015 at 03:39:54PM -0300, Henrique de Moraes Holschuh wrote:
 On Tue, Mar 3, 2015, at 13:52, Darren Hart wrote:
  Henrique, I believe I may have overstepped with thinkpad-acpi and dealt
  with it like the other drivers in platform/drivers/x86, when instead I 
  should
  have been leaving it to you. My apologies, it was not intentional.
 
 No harm done!  And I did appreciate the extra help :-)
 
  Do you typically send pull-requests for the thinkpad-acpi driver directly 
  to Linus?
 
 thinkpad-acpi is part of the platform-x86 subsystem, so IMHO it should
 continue to be merged through your tree if you are okay with that.
 
 I'd really appreciate if the typical patch approval flux would go
 through me first.  However, I certainly don't mind if you merge patches
 directly should I be unresponsible for some reason, or when they are
 part of the usual tree-wide spot fixes for some pattern/anti-pattern,
 etc.
 
  I would be more than happy to basically ignore anything to thinkpad-acpi 
  until
  after you have provided a review. I can also roll up pull requests from
  you if you prefer to integrate into the platform-drivers-x86 tree as the 
  path to
  Linus.
 
 Well, if it is fine with you, I'd suggest we continue with the process
 used in this patchset, where I will reply to patches in this ML with a
 reviewed-by or acked-by before you merge them.
 
 If you want to change that at some point, just drop me a note and I will
 start pushing patches to you either through this ML or through signed
 pull requests, whichever you prefer.

Great. Things are documented correctly in MAINTAINERS then and we can continue
as before, only I will be more diligent making sure you are Cc'd and have
responded to any thinkpad-acpi.c changes. I assume two weeks is sufficient, but
may nag you after a week.

Thanks!

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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 4/4] thinkpad_acpi: Add support for more adaptive kbd buttons

2015-03-03 Thread Darren Hart
On Mon, Mar 02, 2015 at 02:17:01PM -0300, Henrique de Moraes Holschuh wrote:
 On Mon, Mar 2, 2015, at 10:45, Bastien Nocera wrote:
  This commit adds new elements to the ThinkPad keymaps, and
  will send key events for keys for which an input.h declaration
  exists.
  
  Signed-off-by: Bastien Nocera had...@hadess.net
 
 Reviewed-by: Henrique de Moraes Holschuh h...@hmh.eng.br
 Acked-by: Henrique de Moraes Holschuh h...@hmh.eng.br

As I understand it, and how I interpret these tags, Reviewed-by is a superset of
Acked-by. Acked implies acceptance of the general idea, Reviewed implies a
careful code review. No need for both.

I'm queueing these for testing in the pdx86 tree because we've been discussing
that approach. However, I've asked Henrique to let me know how he prefers to
handle this driver.

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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/7] thinkpad_acpi: Remember adaptive kbd presence

2015-02-25 Thread Darren Hart
On Fri, Feb 20, 2015 at 03:44:10PM +0100, Bastien Nocera wrote:
 Rather than checking on each suspend and resume whether the laptop
 has an adaptive keyboard, check when the driver is initialised.

Bastien, am I awaiting another version of this from you to address comments from
Henrique?

Henrique, when you're satisfied, please provide a Reviewed-by for the series.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

--
Dive into the World of Parallel Programming The Go Parallel Website, sponsored
by Intel and developed in partnership with Slashdot Media, is your hub for all
things parallel software development, from weekly thought leadership blogs to
news, videos, case studies, tutorials and more. Take a look and join the 
conversation now. http://goparallel.sourceforge.net/
___
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/6] thinkpad_acpi: Factor out get/set adaptive kbd mode

2015-02-19 Thread Darren Hart
On Wed, Feb 18, 2015 at 09:53:35PM +0100, Bastien Nocera wrote:

Please provide a commit message. There is always something to say beyond what is
in the subject. In this case, I suggest the motivation and justification for the
change.

While I appreciate the abstraction, it makes the code at the call site easier to
read, note that you added more code than you removed.

So, please provide a justificaiton.

Under no circumstances will I accept a patch without a commit message body.

 Signed-off-by: Bastien Nocera had...@hadess.net
 ---
  drivers/platform/x86/thinkpad_acpi.c | 61 
 ++--
  1 file changed, 38 insertions(+), 23 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index 80db3ce..a6dd017 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -3480,6 +3480,32 @@ const int adaptive_keyboard_modes[] = {
  static bool adaptive_keyboard_mode_is_saved;
  static int adaptive_keyboard_prev_mode;
  
 +static int adaptive_keyboard_get_mode(void)
 +{
 + u32 mode = 0;

acpi_evalf second argument takes an int and this function returns int. Is
there a reason to use u32 for mode?

...

 @@ -3509,39 +3535,28 @@ static bool 
 adaptive_keyboard_hotkey_notify_hotkey(unsigned int scancode)
   new_mode = adaptive_keyboard_prev_mode;
   adaptive_keyboard_mode_is_saved = false;
   } else {
 - if (!acpi_evalf(
 - hkey_handle, current_mode,
 - GTRW, dd, 0)) {
 - pr_err(Cannot read adaptive keyboard mode\n);
 + current_mode = adaptive_keyboard_get_mode();
 + if (current_mode  0)
   return false;
 - } else {
 - new_mode = adaptive_keyboard_get_next_mode(
 - current_mode);
 - }
 + new_mode = adaptive_keyboard_get_next_mode(
 + current_mode);

This now fits on one line I believe.

...

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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/6] thinkpad_acpi: Add adaptive_kbd_mode sysfs attr

2015-02-19 Thread Darren Hart
On Wed, Feb 18, 2015 at 09:53:44PM +0100, Bastien Nocera wrote:

Commit message please.

 Signed-off-by: Bastien Nocera had...@hadess.net
 ---
  drivers/platform/x86/thinkpad_acpi.c | 71 
 +++-
  1 file changed, 62 insertions(+), 9 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index a6dd017..562d958 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c

...

 +static struct device_attribute dev_attr_adaptive_kbd_mode =
 + __ATTR(adaptive_kbd_mode, S_IWUSR | S_IRUGO,
 + adaptive_kbd_mode_show, adaptive_kbd_mode_store);
 +

Please use DEVICE_ATTR_RW() macros for new sysfs files.

I'd very much like to see a cleanup of the driver to use these as well.

Henrique, your thoughts / preference?

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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: unhandled hkey event

2015-02-17 Thread Darren Hart
On Mon, Feb 09, 2015 at 08:44:26PM +0100, Xavier Naveira wrote:
 On 2015-02-07 05:22, Darren Hart wrote:
 On Sat, Jan 31, 2015 at 07:52:03PM +0100, Xavier Naveira wrote:
 Pressing Fn+Esc in a Lenovo Thinkpad x240 to lock the Fn keys generates
 an unhandled hkey event
 
 Signed-off-by: Xavier Naveira xnave...@gmail.com
 ---
   drivers/platform/x86/thinkpad_acpi.c | 7 +++
   1 file changed, 7 insertions(+)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index c3d11fa..e61c43b 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -196,6 +196,7 @@ enum tpacpi_hkey_event_t {
 /* Key-related user-interface events */
 TP_HKEY_EV_KEY_NUMLOCK  = 0x6000, /* NumLock key pressed */
 TP_HKEY_EV_KEY_FN   = 0x6005, /* Fn key pressed? E420 */
 +   TP_HKEY_EV_KEY_FN_ESC   = 0x6060, /* Fn+Esc key pressed X240 */
 
 /* Thermal events */
 TP_HKEY_EV_ALARM_BAT_HOT= 0x6011, /* battery too hot */
 @@ -3717,6 +3718,12 @@ static bool hotkey_notify_6xxx(const u32 hkey,
 *send_acpi_ev = false;
 *ignore_acpi_ev = true;
 return true;
 +   case TP_HKEY_EV_KEY_FN_ESC:
 +   /* key press events, we just ignore them as long as the EC
 +* is still reporting them in the normal keyboard stream */
 +   *send_acpi_ev = false;
 +   *ignore_acpi_ev = true;
 +   return true;
 
 No need to duplicate the logic here, just add TP_HKEY_EV_KEY_FN_ESC to the 
 list
 of fallthrough keys (right after TP_HKEY_EV_KEY_FN).
 
 I don't see the list that you are referring to?

Just before where you add the new case, there is:

/* fallthrough */

case TP_HKEY_EV_KEY_NUMLOCK:
case TP_HKEY_EV_KEY_FN:
/* key press events, we just ignore them as long as the EC
 * is still reporting them in the normal keyboard stream */
*send_acpi_ev = false;
*ignore_acpi_ev = true;
return true;

You should just be able to do:

  case TP_HKEY_EV_KEY_FN:
+ case TP_HKEY_EV_KEY_FN_ESC:

Rather than duplicating the same logic below a new case block.

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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: support new BIOS version string pattern

2015-02-17 Thread Darren Hart
On Wed, Feb 04, 2015 at 05:00:19PM +0800, Adam Lee wrote:
 Latest ThinkPad models use a new string pattern of BIOS version,
 thinkpad_acpi won't be loaded automatically without this fix.
 

Hi Adam,

 Signed-off-by: Adam Lee adam@canonical.com
 ---
  drivers/platform/x86/thinkpad_acpi.c | 25 +++--
  1 file changed, 23 insertions(+), 2 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index c3d11fa..39a1017 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -8883,17 +8883,38 @@ static bool __pure __init tpacpi_is_fw_digit(const 
 char c)
   return (c = '0'  c = '9') || (c = 'A'  c = 'Z');
  }
  
 -/* Most models: xxyTkkWW (#.##c); Ancient 570/600 and -SL lacks (#.##c) */
  static bool __pure __init tpacpi_is_valid_fw_id(const char * const s,
   const char t)
  {
 - return s  strlen(s) = 8 
 + bool is_most = 0;
 + bool is_new = 0;

The new variables aren't really necessary, you certainly do not need two of
them.

 +
 + /*
 +  * Most models: xxyTkkWW (#.##c)
 +  * Ancient 570/600 and -SL lacks (#.##c)
 +  */
 + is_most = s  strlen(s) = 8 

Try:

if (s  strlen(s) =8 
...
)
return true;


   tpacpi_is_fw_digit(s[0]) 
   tpacpi_is_fw_digit(s[1]) 
   s[2] == t 
   (s[3] == 'T' || s[3] == 'N') 
   tpacpi_is_fw_digit(s[4]) 
   tpacpi_is_fw_digit(s[5]);
 +
 + if (is_most)
 + return is_most;
 +
 + /* New models: xxxyTkkW (#.##c); T550 and some others */

Is the second W intentionally left off in xxxyTkkW ? We don't test for it, so I
wasn't sure.

 + is_new = s  strlen(s) = 8 
 + tpacpi_is_fw_digit(s[0]) 
 + tpacpi_is_fw_digit(s[1]) 
 + tpacpi_is_fw_digit(s[2]) 

As far as I can tell, the only significant difference here (compared to above)
is an additional leading digit and the subsequent string indices?

Seems to me this could be done fairly easily in the same block with only a minor
adjustment at the beginning and the use of an incrementing index. Should be
cleaner than duplicating 90% of the block?

 + s[3] == t 
 + (s[4] == 'T' || s[4] == 'N') 
 + tpacpi_is_fw_digit(s[5]) 
 + tpacpi_is_fw_digit(s[6]);
 +
 + return is_new;
  }
  
  /* returns 0 - probe ok, or  0 - probe error.
 -- 
 2.1.4
 
 

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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] x86: thinkpad_acpi.c: fix sparse warning

2015-02-17 Thread Darren Hart
On Thu, Feb 05, 2015 at 02:45:38PM +, Lad Prabhakar wrote:
 From: Lad, Prabhakar prabhakar.cse...@gmail.com
 
 this patch fixes following sparse warning:

Please use the imperative, command form, in the future:

Fix the following sparse warning:

For the subject, just the module name is sufficient, no need to preface with
x86:. Always capitalize the first word after the filename:

thinkpad_acpi: Fix sparse warning

Corrected and applied, thanks!

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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: support new BIOS version string pattern

2015-02-17 Thread Darren Hart
On Wed, Feb 11, 2015 at 01:43:10PM +0800, Adam Lee wrote:
 Latest ThinkPad models use a new string pattern of BIOS version,
 thinkpad_acpi won't be loaded automatically without this fix.
 
 Signed-off-by: Adam Lee adam@canonical.com
 ---

When sending updated versions of a patch or series, please include a changelog
after the ---, see Documentation/SubmittingPatches.

  drivers/platform/x86/thinkpad_acpi.c | 26 --
  1 file changed, 20 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/platform/x86/thinkpad_acpi.c 
 b/drivers/platform/x86/thinkpad_acpi.c
 index c3d11fa..18b7594 100644
 --- a/drivers/platform/x86/thinkpad_acpi.c
 +++ b/drivers/platform/x86/thinkpad_acpi.c
 @@ -8883,17 +8883,31 @@ static bool __pure __init tpacpi_is_fw_digit(const 
 char c)
   return (c = '0'  c = '9') || (c = 'A'  c = 'Z');
  }
  
 -/* Most models: xxyTkkWW (#.##c); Ancient 570/600 and -SL lacks (#.##c) */
  static bool __pure __init tpacpi_is_valid_fw_id(const char * const s,
   const char t)
  {
 - return s  strlen(s) = 8 
 + /*
 +  * Most models: xxyTkkWW (#.##c)
 +  * Ancient 570/600 and -SL lacks (#.##c)
 +  */
 + if (s  strlen(s) = 8 
 + tpacpi_is_fw_digit(s[0]) 
 + tpacpi_is_fw_digit(s[1]) 
 + s[2] == t 
 + (s[3] == 'T' || s[3] == 'N') 
 + tpacpi_is_fw_digit(s[4]) 
 + tpacpi_is_fw_digit(s[5]))

I'm not wild about these big boolean statements, but I also don't see something
that is obviously better. Typically we want to see indentation with tabs and
alignment with the initial line's arguments with spaces... but here that just
looks ugly as the too seemingly similar blocks end up with different
indentations due to the if and return... anyway, subjective and all that. So I'm
leaving it mostly alone, but I dropped one set of tabs from the above lines and
queued it.

Thanks Adam.

 + return true;
 +
 + /* New models: xxxyTkkW (#.##c); T550 and some others */
 + return  s  strlen(s) = 8 
   tpacpi_is_fw_digit(s[0]) 
   tpacpi_is_fw_digit(s[1]) 
 - s[2] == t 
 - (s[3] == 'T' || s[3] == 'N') 
 - tpacpi_is_fw_digit(s[4]) 
 - tpacpi_is_fw_digit(s[5]);
 + tpacpi_is_fw_digit(s[2]) 
 + s[3] == t 
 + (s[4] == 'T' || s[4] == 'N') 
 + tpacpi_is_fw_digit(s[5]) 
 + tpacpi_is_fw_digit(s[6]);



  }
  
  /* returns 0 - probe ok, or  0 - probe error.
 -- 
 2.1.4
 
 

-- 
Darren Hart
Intel Open Source Technology Center

--
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration  more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631iu=/4140/ostg.clktrk
___
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 17/22] thinkpad_acpi: Replace strnicmp with strncasecmp

2014-09-20 Thread Darren Hart
On Tue, Sep 16, 2014 at 10:51:31PM +0200, Rasmus Villemoes wrote:
 The kernel used to contain two functions for length-delimited,
 case-insensitive string comparison, strnicmp with correct semantics
 and a slightly buggy strncasecmp. The latter is the POSIX name, so
 strnicmp was renamed to strncasecmp, and strnicmp made into a wrapper
 for the new strncasecmp to avoid breaking existing users.

When was this done? As of this morning I still see them as independent functions
in lib/string.c.

-- 
Darren Hart
Intel Open Source Technology Center

--
Slashdot TV.  Video for Nerds.  Stuff that Matters.
http://pubads.g.doubleclick.net/gampad/clk?id=160591471iu=/4140/ostg.clktrk
___
ibm-acpi-devel mailing list
ibm-acpi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ibm-acpi-devel