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

2020-06-17 Thread Mark Pearson
  Newer Lenovo Thinkpad platforms have support to identify whether the
  system is on-lap or not using an ACPI DYTC event from the firmware.

  This patch provides the ability to retrieve the current mode via sysfs
  entrypoints and will be used by userspace for thermal mode and WWAN
  functionality

Co-developed-by: Nitin Joshi 
Signed-off-by: Nitin Joshi 
Reviewed-by: Sugumaran 
Signed-off-by: Mark Pearson 
---
Resend:
- Adding Bastien Nocera to cc list as requested
Changes in v2:
- cleaned up initialisation sequence to be cleaner and avoid spamming
  platforms that don't have DYTC with warning message. Tested on P52
- Adding platform-driver-x86 mailing list for review as requested

 drivers/platform/x86/thinkpad_acpi.c | 113 +++
 1 file changed, 113 insertions(+)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 0f704484ae1d..8f51bbba21cd 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -4049,6 +4049,7 @@ static bool hotkey_notify_6xxx(const u32 hkey,
pr_debug("EC reports: Thermal Control Command set completed 
(DYTC)\n");
/* recommended action: do nothing, we don't have
 * Lenovo ATM information */
+   tpacpi_driver_event(hkey);
return true;
case TP_HKEY_EV_THM_TRANSFM_CHANGED:
pr_debug("EC reports: Thermal Transformation changed (GMTS)\n");
@@ -9811,6 +9812,110 @@ static struct ibm_struct lcdshadow_driver_data = {
.write = lcdshadow_write,
 };
 
+/*
+ * DYTC subdriver, for the Lenovo performace mode feature
+ */
+
+#define DYTC_CMD_GET  2 /*To get current IC function and mode*/
+
+#define DYTC_GET_ENABLE_MASK  0x1 /*0 = disabled, 1 = enabled*/
+#define DYTC_GET_LAPMODE_SHIFT 17
+
+static int  dytc_lapmode;
+static void dytc_lapmode_notify_change(void)
+{
+   sysfs_notify(_pdev->dev.kobj, NULL,
+   "dytc_lapmode");
+}
+
+static int dytc_command(int command)
+{
+   acpi_handle dytc_handle;
+   int output;
+
+   if (ACPI_FAILURE(acpi_get_handle(hkey_handle, "DYTC", _handle))) {
+   /*Platform doesn't support DYTC*/
+   return -ENODEV;
+   }
+   if (!acpi_evalf(dytc_handle, , NULL, "dd", command))
+   return -EIO;
+   return output;
+}
+
+static int dytc_lapmode_get(void)
+{
+   int output;
+
+   output = dytc_command(DYTC_CMD_GET);
+   if ((output == -ENODEV) || (output == -EIO))
+   return output;
+
+   return ((output >> DYTC_GET_LAPMODE_SHIFT) &
+   DYTC_GET_ENABLE_MASK);
+}
+
+static void dytc_lapmode_refresh(void)
+{
+   int new_state;
+
+   new_state = dytc_lapmode_get();
+   if ((new_state == -ENODEV) || (new_state == -EIO))
+   return;
+
+   if (dytc_lapmode != new_state) {
+   dytc_lapmode = new_state;
+   dytc_lapmode_notify_change();
+   }
+}
+
+/* sysfs lapmode entry */
+static ssize_t dytc_lapmode_show(struct device *dev,
+   struct device_attribute *attr,
+   char *buf)
+{
+   if (dytc_lapmode < 0)
+   return dytc_lapmode;
+
+   return snprintf(buf, PAGE_SIZE, "%d\n", dytc_lapmode);
+}
+
+static DEVICE_ATTR_RO(dytc_lapmode);
+
+static struct attribute *dytc_attributes[] = {
+   _attr_dytc_lapmode.attr,
+   NULL
+};
+
+static const struct attribute_group dytc_attr_group = {
+   .attrs = dytc_attributes,
+};
+
+static int tpacpi_dytc_init(struct ibm_init_struct *iibm)
+{
+   int res;
+
+   dytc_lapmode = dytc_lapmode_get();
+
+   if (dytc_lapmode < 0 && dytc_lapmode != -ENODEV)
+   return dytc_lapmode;
+
+   res = sysfs_create_group(_pdev->dev.kobj,
+   _attr_group);
+
+   return res;
+}
+
+static void dytc_exit(void)
+{
+   sysfs_remove_group(_pdev->dev.kobj,
+   _attr_group);
+}
+
+static struct ibm_struct dytc_driver_data = {
+   .name = "dytc",
+   .exit = dytc_exit
+};
+
 /
  
  *
@@ -9858,6 +9963,10 @@ static void tpacpi_driver_event(const unsigned int 
hkey_event)
 
mutex_unlock(_mutex);
}
+
+   if (hkey_event == TP_HKEY_EV_THM_CSM_COMPLETED)
+   dytc_lapmode_refresh();
+
 }
 
 static void hotkey_driver_event(const unsigned int scancode)
@@ -10296,6 +10405,10 @@ static struct ibm_init_struct ibms_init[] __initdata = 
{
.init = tpacpi_lcdshadow_init,
.data = _driver_data,
},
+   {
+   .init = tpacpi_dytc_init,
+   .data = _driver_data,
+   },
 };

Re: [ibm-acpi-devel] [PATCH v2] Input: document inhibiting

2020-06-17 Thread Randy Dunlap
On 6/17/20 3:18 AM, Andrzej Pietrasiewicz wrote:
> Document inhibiting input devices and its relation to being
> a wakeup source.
> 
> Signed-off-by: Andrzej Pietrasiewicz 
> ---
> v1..v2:
> 
> - Addressed editorial comments from Randy
> - Added a paragraph by Hans
> 
>  Documentation/input/input-programming.rst | 40 +++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/Documentation/input/input-programming.rst 
> b/Documentation/input/input-programming.rst
> index 45a4c6e05e39..7432315cc829 100644
> --- a/Documentation/input/input-programming.rst
> +++ b/Documentation/input/input-programming.rst
> @@ -164,6 +164,46 @@ disconnects. Calls to both callbacks are serialized.
>  The open() callback should return a 0 in case of success or any nonzero value
>  in case of failure. The close() callback (which is void) must always succeed.
>  
> +Inhibiting input devices
> +
> +
> +Inhibiting a device means ignoring input events from it. As such it is about 
> maintaining
> +relationships with input handlers - either already existing relationships, 
> or relationships
> +to be established while the device is in inhibited state.
> +
> +If a device is inhibited, no input handler will receive events from it.
> +
> +The fact that nobody wants events from the device is exploited further, by 
> calling device's
> +close() (if there are users) and open() (if there are users) on inhibit and 
> uninhibit
> +operations, respectively. Indeed, the meaning of close() is to stop 
> providing events
> +to the input core and that of open() is to start providing events to the 
> input core.
> +
> +Calling the device's close() method on inhibit (if there are users) allows 
> the driver
> +to save power. Either by directly powering down the device or by releasing 
> the
> +runtime-pm reference it got in open() when the driver is using runtime-pm.
> +
> +Inhibiting and uninhibiting are orthogonal to opening and closing the device 
> by input
> +handlers. Userspace might want to inhibit a device in anticipation before 
> any handler is
> +positively matched against it.
> +
> +Inhibiting and uninhibiting are orthogonal to device's being a wakeup 
> source, too. Being a
> +wakeup source plays a role when the system is sleeping, not when the system 
> is operating.
> +How drivers should program their interaction between inhibiting, sleeping 
> and being a wakeup
> +source is driver-specific.
> +
> +Taking the analogy with the network devices - bringing a network interface 
> down doesn't mean
> +that it should be impossible be wake the system up on LAN through this 
> interface. So, there
> +may be input drivers which should be considered wakeup sources even when 
> inhibited. Actually,
> +in many I2C input devices their interrupt is declared a wakeup interrupt and 
> its handling
> +happens in driver's core, which is not aware of input-specific inhibit (nor 
> should it be).
> +Composite devices containing several interfaces can be inhibited on a 
> per-interface basis and
> +e.g. inhibiting one interface shouldn't affect the device's capability of 
> being a wakeup source.
> +
> +If a device is to be considered a wakeup source while inhibited, special 
> care must be taken when
> +programming its suspend(), as it might need to call device's open(). 
> Depending on what close()
> +means for the device in question, not opening() it before going to sleep 
> might make it
> +impossible to provide any wakeup events. The device is going to sleep anyway.
> +
>  Basic event types
>  ~
>  
> 

Reviewed-by: Randy Dunlap 

Thanks.

-- 
~Randy


___
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] Input: document inhibiting

2020-06-17 Thread Hans de Goede

Hi,

On 6/17/20 12:18 PM, Andrzej Pietrasiewicz wrote:

Document inhibiting input devices and its relation to being
a wakeup source.

Signed-off-by: Andrzej Pietrasiewicz 
---
v1..v2:

- Addressed editorial comments from Randy
- Added a paragraph by Hans


Thank you.

v2 looks good to me:

Reviewed-by: Hans de Goede 

Regards,

Hans





  Documentation/input/input-programming.rst | 40 +++
  1 file changed, 40 insertions(+)

diff --git a/Documentation/input/input-programming.rst 
b/Documentation/input/input-programming.rst
index 45a4c6e05e39..7432315cc829 100644
--- a/Documentation/input/input-programming.rst
+++ b/Documentation/input/input-programming.rst
@@ -164,6 +164,46 @@ disconnects. Calls to both callbacks are serialized.
  The open() callback should return a 0 in case of success or any nonzero value
  in case of failure. The close() callback (which is void) must always succeed.
  
+Inhibiting input devices

+
+
+Inhibiting a device means ignoring input events from it. As such it is about 
maintaining
+relationships with input handlers - either already existing relationships, or 
relationships
+to be established while the device is in inhibited state.
+
+If a device is inhibited, no input handler will receive events from it.
+
+The fact that nobody wants events from the device is exploited further, by 
calling device's
+close() (if there are users) and open() (if there are users) on inhibit and 
uninhibit
+operations, respectively. Indeed, the meaning of close() is to stop providing 
events
+to the input core and that of open() is to start providing events to the input 
core.
+
+Calling the device's close() method on inhibit (if there are users) allows the 
driver
+to save power. Either by directly powering down the device or by releasing the
+runtime-pm reference it got in open() when the driver is using runtime-pm.
+
+Inhibiting and uninhibiting are orthogonal to opening and closing the device 
by input
+handlers. Userspace might want to inhibit a device in anticipation before any 
handler is
+positively matched against it.
+
+Inhibiting and uninhibiting are orthogonal to device's being a wakeup source, 
too. Being a
+wakeup source plays a role when the system is sleeping, not when the system is 
operating.
+How drivers should program their interaction between inhibiting, sleeping and 
being a wakeup
+source is driver-specific.
+
+Taking the analogy with the network devices - bringing a network interface 
down doesn't mean
+that it should be impossible be wake the system up on LAN through this 
interface. So, there
+may be input drivers which should be considered wakeup sources even when 
inhibited. Actually,
+in many I2C input devices their interrupt is declared a wakeup interrupt and 
its handling
+happens in driver's core, which is not aware of input-specific inhibit (nor 
should it be).
+Composite devices containing several interfaces can be inhibited on a 
per-interface basis and
+e.g. inhibiting one interface shouldn't affect the device's capability of 
being a wakeup source.
+
+If a device is to be considered a wakeup source while inhibited, special care 
must be taken when
+programming its suspend(), as it might need to call device's open(). Depending 
on what close()
+means for the device in question, not opening() it before going to sleep might 
make it
+impossible to provide any wakeup events. The device is going to sleep anyway.
+
  Basic event types
  ~
  





___
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] Input: document inhibiting

2020-06-17 Thread Hans de Goede

Hi,

On 6/16/20 7:29 PM, Andrzej Pietrasiewicz wrote:

Document inhibiting input devices and its relation to being
a wakeup source.

Signed-off-by: Andrzej Pietrasiewicz 
---

@Hans, @Dmitry,

My fist attempt at documenting inhibiting. Kindly look at it to see if I 
haven't got anything
wrong.

Andrzej

  Documentation/input/input-programming.rst | 36 +++
  1 file changed, 36 insertions(+)

diff --git a/Documentation/input/input-programming.rst 
b/Documentation/input/input-programming.rst
index 45a4c6e05e39..0cd1ad4504fb 100644
--- a/Documentation/input/input-programming.rst
+++ b/Documentation/input/input-programming.rst
@@ -164,6 +164,42 @@ disconnects. Calls to both callbacks are serialized.
  The open() callback should return a 0 in case of success or any nonzero value
  in case of failure. The close() callback (which is void) must always succeed.
  
+Inhibiting input devices

+
+
+Inhibiting a device means ignoring input events from it. As such it is about 
maintaining
+relationships with input handlers - either an already existing relationships, 
or
+relationships to be established while the device is in inhibited state.
+
+If a device is inhibited, no input handler will receive events from it.
+
+The fact that nobody wants events from the device is exploited further, by 
calling device's
+close() (if there are users) and open() (if there are users) on inhibit and 
uninhibit
+operations, respectively. Indeed, the meaning of close() is to stop providing 
events
+to the input core and that of open() is to start providing events to the input 
core.


Maybe add the following here? :

Calling the device's close() method on inhibit (if there are users) allows the 
driver
to save power. Either by directly powering down the device or by releasing the
runtime-pm reference it got in open() when the driver is using runtime-pm.

Otherwise this looks good to me. Thank you for doing this, we (including myself)
really need to get better at doucmenting all sorts of kernel things. Often we 
have
these long discussions about something on the mailinglist and then everyone is
expected to just know what was decided from the on, which really doesn't work 
all
that well.


+
+Inhibiting and uninhibiting is orthogonal to opening and closing the device by 
input
+handlers. Userspace might want to inhibit a device in anticipation before any 
handler is
+positively matched against it.
+
+Inhibiting and uninhibiting is orthogonal to device's being a wakeup source, 
too. Being a
+wakeup source plays a role when the system is sleeping, not when the system is 
operating.
+How drivers should program their interaction between inhibiting, sleeping and 
being a wakeup
+source is driver-specific.
+
+Taking the analogy with the network devices - bringing a network interface 
down doesn't mean
+that it should be impossible to be wake the system up on LAN through this 
interface. So, there
+may be input drivers which should be considered wakeup sources even when 
inhibited. Actually,
+in many i2c input devices their interrupt is declared a wakeup interrupt and 
its handling
+happens in driver's core, which is not aware of input-specific inhibit (nor 
should it be).
+Composite devices containing several interfaces can be inhibited on a 
per-interface basis and
+e.g. inhibiting one interface shouldn't affect the device's capability of 
being a wakeup source.
+
+If a device is to be considered a wakeup source while inhibited, special care 
must be taken when
+programming its suspend(), as it might need to call device's open(). Depending 
on what close()
+means for the device in question not opening() it before going to sleep might 
make it impossible
+to provide any wakeup events. The device is going to sleep anyway.
+
  Basic event types
  ~
  




Regards,

Hans



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