[PATCH] net: called rtnl_unlock() before runpm resumes devices

[AceLan Kao]

From: "Chia-Lin Kao (AceLan)" 

The rtnl_lock() has been called in rtnetlink_rcv_msg(), and then in
__dev_open() it calls pm_runtime_resume() to resume devices, and in
some devices' resume function(igb_resum,igc_resume) they calls rtnl_lock()
again. That leads to a recursive lock.

It should leave the devices' resume function to decide if they need to
call rtnl_lock()/rtnl_unlock(), so call rtnl_unlock() before calling
pm_runtime_resume() and then call rtnl_lock() after it in __dev_open().

[  967.723577] INFO: task ip:6024 blocked for more than 120 seconds.
[  967.723588]   Not tainted 5.12.0-rc3+ #1
[  967.723592] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this 
message.
[  967.723594] task:ip  state:D stack:0 pid: 6024 ppid:  5957 
flags:0x4000
[  967.723603] Call Trace:
[  967.723610]  __schedule+0x2de/0x890
[  967.723623]  schedule+0x4f/0xc0
[  967.723629]  schedule_preempt_disabled+0xe/0x10
[  967.723636]  __mutex_lock.isra.0+0x190/0x510
[  967.723644]  __mutex_lock_slowpath+0x13/0x20
[  967.723651]  mutex_lock+0x32/0x40
[  967.723657]  rtnl_lock+0x15/0x20
[  967.723665]  igb_resume+0xee/0x1d0 [igb]
[  967.723687]  ? pci_pm_default_resume+0x30/0x30
[  967.723696]  igb_runtime_resume+0xe/0x10 [igb]
[  967.723713]  pci_pm_runtime_resume+0x74/0x90
[  967.723718]  __rpm_callback+0x53/0x1c0
[  967.723725]  rpm_callback+0x57/0x80
[  967.723730]  ? pci_pm_default_resume+0x30/0x30
[  967.723735]  rpm_resume+0x547/0x760
[  967.723740]  __pm_runtime_resume+0x52/0x80
[  967.723745]  __dev_open+0x56/0x160
[  967.723753]  ? _raw_spin_unlock_bh+0x1e/0x20
[  967.723758]  __dev_change_flags+0x188/0x1e0
[  967.723766]  dev_change_flags+0x26/0x60
[  967.723773]  do_setlink+0x723/0x10b0
[  967.723782]  ? __nla_validate_parse+0x5b/0xb80
[  967.723792]  __rtnl_newlink+0x594/0xa00
[  967.723800]  ? nla_put_ifalias+0x38/0xa0
[  967.723807]  ? __nla_reserve+0x41/0x50
[  967.723813]  ? __nla_reserve+0x41/0x50
[  967.723818]  ? __kmalloc_node_track_caller+0x49b/0x4d0
[  967.723824]  ? pskb_expand_head+0x75/0x310
[  967.723830]  ? nla_reserve+0x28/0x30
[  967.723835]  ? skb_free_head+0x25/0x30
[  967.723843]  ? security_sock_rcv_skb+0x2f/0x50
[  967.723850]  ? netlink_deliver_tap+0x3d/0x210
[  967.723859]  ? sk_filter_trim_cap+0xc1/0x230
[  967.723863]  ? skb_queue_tail+0x43/0x50
[  967.723870]  ? sock_def_readable+0x4b/0x80
[  967.723876]  ? __netlink_sendskb+0x42/0x50
[  967.723888]  ? security_capable+0x3d/0x60
[  967.723894]  ? __cond_resched+0x19/0x30
[  967.723900]  ? kmem_cache_alloc_trace+0x390/0x440
[  967.723906]  rtnl_newlink+0x49/0x70
[  967.723913]  rtnetlink_rcv_msg+0x13c/0x370
[  967.723920]  ? _copy_to_iter+0xa0/0x460
[  967.723927]  ? rtnl_calcit.isra.0+0x130/0x130
[  967.723934]  netlink_rcv_skb+0x55/0x100
[  967.723939]  rtnetlink_rcv+0x15/0x20
[  967.723944]  netlink_unicast+0x1a8/0x250
[  967.723949]  netlink_sendmsg+0x233/0x460
[  967.723954]  sock_sendmsg+0x65/0x70
[  967.723958]  sys_sendmsg+0x218/0x290
[  967.723961]  ? copy_msghdr_from_user+0x5c/0x90
[  967.723966]  ? lru_cache_add_inactive_or_unevictable+0x27/0xb0
[  967.723974]  ___sys_sendmsg+0x81/0xc0
[  967.723980]  ? __mod_memcg_lruvec_state+0x22/0xe0
[  967.723987]  ? kmem_cache_free+0x244/0x420
[  967.723991]  ? dentry_free+0x37/0x70
[  967.723996]  ? mntput_no_expire+0x4c/0x260
[  967.724001]  ? __cond_resched+0x19/0x30
[  967.724007]  ? security_file_free+0x54/0x60
[  967.724013]  ? call_rcu+0xa4/0x250
[  967.724021]  __sys_sendmsg+0x62/0xb0
[  967.724026]  ? exit_to_user_mode_prepare+0x3d/0x1a0
[  967.724032]  __x64_sys_sendmsg+0x1f/0x30
[  967.724037]  do_syscall_64+0x38/0x90
[  967.724044]  entry_SYSCALL_64_after_hwframe+0x44/0xae

Signed-off-by: Chia-Lin Kao (AceLan) 
---
 net/core/dev.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 1f79b9aa9a3f..427cbc80d1e5 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1537,8 +1537,11 @@ static int __dev_open(struct net_device *dev, struct 
netlink_ext_ack *extack)
 
if (!netif_device_present(dev)) {
/* may be detached because parent is runtime-suspended */
-   if (dev->dev.parent)
+   if (dev->dev.parent) {
+   rtnl_unlock();
pm_runtime_resume(dev->dev.parent);
+   rtnl_lock();
+   }
if (!netif_device_present(dev))
return -ENODEV;
}
-- 
2.25.1

[PATCH] Input: i8042 - add dmi quirk

[AceLan Kao]

From: "Chia-Lin Kao (AceLan)" 

On some platforms, the EC doesn't support the register reading sequence
for sentelic[1], and then make the EC can't respond commands for a while
when probing. It leads to the keyboard non-responsive for around 10
seconds while waking up from s2idle.

[   44.304488] i8042: [9804] d4 -> i8042 (command)
[   44.304634] i8042: [9804] f3 -> i8042 (parameter)
[   44.304787] i8042: [9804] fa <- i8042 (interrupt, 1, 12)
[   44.304855] i8042: [9804] d4 -> i8042 (command)
[   44.304938] i8042: [9804] 66 -> i8042 (parameter)
[   44.337698] i8042: [9813] d4 -> i8042 (command)
[   44.905695] i8042: [9942] 88 -> i8042 (parameter)
[   45.497478] i8042: [10102] d4 -> i8042 (command)
[   46.098041] i8042: [10253] f3 -> i8042 (parameter)
[   46.098070] i8042: [10253] fe <- i8042 (interrupt, 1, 12)
[   46.718154] i8042: [10386] d4 -> i8042 (command)
[   47.309915] i8042: [10386] f4 -> i8042 (parameter)
[   47.918961] i8042: [10556] d4 -> i8042 (command)
[   48.402624] i8042: [10556] f6 -> i8042 (parameter)

A DMI quirk to mark this platform doesn't have aux device could avoid those
commands to be sent. And the system could still using i2c interface to
communicate with the touchpad.

1. 
https://www.kernel.org/doc/html/v5.11/input/devices/sentelic.html#programming-sequence-for-register-reading-writing

Signed-off-by: Chia-Lin Kao (AceLan) 
---
 drivers/input/serio/i8042.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index dbe1810e6242..3d778c9616dc 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -174,6 +175,24 @@ static irqreturn_t i8042_interrupt(int irq, void *dev_id);
 static bool (*i8042_platform_filter)(unsigned char data, unsigned char str,
 struct serio *serio);
 
+static int __init i8042_set_noaux(const struct dmi_system_id *dmi)
+{
+   i8042_noaux = true;
+   return 1;
+}
+
+static const struct dmi_system_id i8042_quirks[] __initconst = {
+   {
+   .callback = i8042_set_noaux,
+   .ident = "Dell laptop",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Precision 5550"),
+   },
+   },
+   {},
+};
+
 void i8042_lock_chip(void)
 {
mutex_lock(_mutex);
@@ -1559,6 +1578,8 @@ static int __init i8042_probe(struct platform_device *dev)
i8042_dritek_enable();
 #endif
 
+   dmi_check_system(i8042_quirks);
+
if (!i8042_noaux) {
error = i8042_setup_aux();
if (error && error != -ENODEV && error != -EBUSY)
-- 
2.25.1

Re: [PATCH v6 4/6] mfd: ahc1ec0: Add support for Advantech embedded controller

[AceLan Kao]

Campion Kang  於 2021年1月18日 週一 下午8:37寫道：
>
> AHC1EC0 is the embedded controller driver for Advantech industrial
> products. This provides sub-devices such as hwmon and watchdog, and also
> expose functions for sub-devices to read/write the value to embedded
> controller.
>
> Changed since V5:
> - Kconfig: add "AHC1EC0" string to clearly define the EC name
> - fix the code according to reviewer's suggestion
> - remove unnecessary header files
> - change the structure name to lower case, align with others
> naming
>
> Signed-off-by: Campion Kang 
> ---
>  drivers/mfd/Kconfig |  10 +
>  drivers/mfd/Makefile|   2 +
>  drivers/mfd/ahc1ec0.c   | 808 
>  include/linux/mfd/ahc1ec0.h | 276 
>  4 files changed, 1096 insertions(+)
>  create mode 100644 drivers/mfd/ahc1ec0.c
>  create mode 100644 include/linux/mfd/ahc1ec0.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index bdfce7b15621..7d5fb5c17d9a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -2154,5 +2154,15 @@ config MFD_INTEL_M10_BMC
>   additional drivers must be enabled in order to use the functionality
>   of the device.
>
> +config MFD_AHC1EC0
> +   tristate "Advantech AHC1EC0 Embedded Controller Module"
> +   depends on X86
> +   select MFD_CORE
> +   help
> + This is the core function that for Advantech EC drivers. It
> + includes the sub-devices such as hwmon, watchdog, etc. And also
> + provides expose functions for sub-devices to read/write the value
> + to embedded controller.
> +
>  endmenu
>  endif
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index 14fdb188af02..a6af9d8825f4 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -268,3 +268,5 @@ obj-$(CONFIG_MFD_KHADAS_MCU)+= khadas-mcu.o
>  obj-$(CONFIG_SGI_MFD_IOC3) += ioc3.o
>  obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)   += simple-mfd-i2c.o
>  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> +
> +obj-$(CONFIG_MFD_AHC1EC0)  += ahc1ec0.o
> diff --git a/drivers/mfd/ahc1ec0.c b/drivers/mfd/ahc1ec0.c
> new file mode 100644
> index ..015f4307a54e
> --- /dev/null
> +++ b/drivers/mfd/ahc1ec0.c
> @@ -0,0 +1,808 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Advantech embedded controller core driver AHC1EC0
> + *
> + * Copyright 2020 Advantech IIoT Group
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_NAME  "ahc1ec0"
> +
> +enum {
> +   ADVEC_SUBDEV_BRIGHTNESS = 0,
> +   ADVEC_SUBDEV_EEPROM,
> +   ADVEC_SUBDEV_GPIO,
> +   ADVEC_SUBDEV_HWMON,
> +   ADVEC_SUBDEV_LED,
> +   ADVEC_SUBDEV_WDT,
> +   ADVEC_SUBDEV_MAX,
> +};
> +
> +/* Wait IBF (Input Buffer Full) clear */
> +static int ec_wait_write(void)
> +{
> +   int i;
> +
> +   for (i = 0; i < EC_MAX_TIMEOUT_COUNT; i++) {
> +   if ((inb(EC_COMMAND_PORT) & EC_COMMAND_BIT_IBF) == 0)
> +   return 0;
> +
> +   udelay(EC_RETRY_UDELAY);
> +   }
> +
> +   return -ETIMEDOUT;
> +}
> +
> +/* Wait OBF (Output Buffer Full) data ready */
> +static int ec_wait_read(void)
> +{
> +   int i;
> +
> +   for (i = 0; i < EC_MAX_TIMEOUT_COUNT; i++) {
> +   if ((inb(EC_COMMAND_PORT) & EC_COMMAND_BIT_OBF) != 0)
> +   return 0;
> +
> +   udelay(EC_RETRY_UDELAY);
> +   }
> +
> +   return -ETIMEDOUT;
> +}
> +
> +/* Read data from EC HW RAM, the process is the following:
> + * Step 0. Wait IBF clear to send command
> + * Step 1. Send read command to EC command port
> + * Step 2. Wait IBF clear that means command is got by EC
> + * Step 3. Send read address to EC data port
> + * Step 4. Wait OBF data ready
> + * Step 5. Get data from EC data port
> + */
> +int read_hw_ram(struct adv_ec_platform_data *adv_ec_data, unsigned char 
> addr, unsigned char *data)
> +{
> +   int ret;
> +
> +   mutex_lock(_ec_data->lock);
> +
> +   ret = ec_wait_write();
> +   if (ret)
> +   goto error;
> +   outb(EC_HW_RAM_READ, EC_COMMAND_PORT);
> +
> +   ret = ec_wait_write();
> +   if (ret)
> +   goto error;
> +   outb(addr, EC_STATUS_PORT);
> +
> +   ret = ec_wait_read();
> +   if (ret)
> +   goto error;
> +   *data = inb(EC_STATUS_PORT);
> +
> +   mutex_unlock(_ec_data->lock);
> +
> +   return ret;
> +
> +error:
> +   mutex_unlock(_ec_data->lock);
> +   dev_err(adv_ec_data->dev, "%s: Wait for IBF or OBF too long. line: 
> %d\n", __func__,
> +  __LINE__);
> +
> +   return ret;
> +}
> +
> +/* Write data to EC HW RAM
> + * Step 0. Wait IBF clear to send command
> + * Step 1. Send write command to EC command port
> + * Step 2. Wait IBF clear that means command is got 

Re: [PATCH v6 5/6] hwmon: ahc1ec0-hwmon: Add sub-device hwmon for Advantech embedded controller

[AceLan Kao]

Campion Kang  於 2021年1月18日 週一 下午8:37寫道：
>
> This is one of sub-device driver for Advantech embedded controller
> AHC1EC0. This driver provides sysfs ABI for Advantech related
> applications to monitor the system status.
>
> Changed since V5:
> - remove unnecessary header files
> - Using [devm_]hwmon_device_register_with_info() to register
> hwmon driver based on reviewer's suggestion
>
> Signed-off-by: Campion Kang 
> ---
>  drivers/hwmon/Kconfig |  10 +
>  drivers/hwmon/Makefile|   1 +
>  drivers/hwmon/ahc1ec0-hwmon.c | 660 ++
>  3 files changed, 671 insertions(+)
>  create mode 100644 drivers/hwmon/ahc1ec0-hwmon.c
>
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 1ecf697d8d99..bfa007026679 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -2139,6 +2139,16 @@ config SENSORS_INTEL_M10_BMC_HWMON
>   sensors monitor various telemetry data of different components on 
> the
>   card, e.g. board temperature, FPGA core temperature/voltage/current.
>
> +config SENSORS_AHC1EC0_HWMON
> +   tristate "Advantech AHC1EC0 Hardware Monitor Function"
> +   depends on MFD_AHC1EC0
> +   help
> + This driver provide support for the hardware monitoring 
> functionality
> + for Advantech AHC1EC0 embedded controller on the board.
> +
> + This driver provides the sysfs attributes for applications to 
> monitor
> + the system status, including system temperatures, voltages, current.
> +
>  if ACPI
>
>  comment "ACPI drivers"
> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
> index 09a86c5e1d29..0c37747e8c4f 100644
> --- a/drivers/hwmon/Makefile
> +++ b/drivers/hwmon/Makefile
> @@ -45,6 +45,7 @@ obj-$(CONFIG_SENSORS_ADT7411) += adt7411.o
>  obj-$(CONFIG_SENSORS_ADT7462)  += adt7462.o
>  obj-$(CONFIG_SENSORS_ADT7470)  += adt7470.o
>  obj-$(CONFIG_SENSORS_ADT7475)  += adt7475.o
> +obj-$(CONFIG_SENSORS_AHC1EC0_HWMON)+= ahc1ec0-hwmon.o
>  obj-$(CONFIG_SENSORS_AMD_ENERGY) += amd_energy.o
>  obj-$(CONFIG_SENSORS_APPLESMC) += applesmc.o
>  obj-$(CONFIG_SENSORS_ARM_SCMI) += scmi-hwmon.o
> diff --git a/drivers/hwmon/ahc1ec0-hwmon.c b/drivers/hwmon/ahc1ec0-hwmon.c
> new file mode 100644
> index ..688f07e6a6e0
> --- /dev/null
> +++ b/drivers/hwmon/ahc1ec0-hwmon.c
> @@ -0,0 +1,660 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * HWMON Driver for Advantech Embedded Controller chip AHC1EC0
> + *
> + * Copyright 2020, Advantech IIoT Group
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct ec_hwmon_attrs {
> +   const char  *name;
> +   umode_t mode;
> +   int (*read)(struct device *dev, long *val);
> +};
> +
> +struct adv_hwmon_profile {
> +   int offset;
> +   unsigned long resolution, resolution_vin, resolution_sys, 
> resolution_curr, resolution_power;
> +   unsigned long r1, r1_vin, r1_sys, r1_curr, r1_power;
> +   unsigned long r2, r2_vin, r2_sys, r2_curr, r2_power;
> +   int hwmon_in_list_cnt;
> +   int temp_list_cnt;
> +   int *hwmon_in_list;
> +   int *temp_list;
> +};
> +
> +struct ec_hwmon_data {
> +   struct device *dev;
> +   struct device *hwmon_dev;
> +   struct adv_ec_platform_data *adv_ec_data;
> +   unsigned long temperature[3];
> +   unsigned long ec_current[5];
> +   unsigned long power[5];
> +   unsigned long voltage[7];
> +
> +   struct ec_hw_pin_table pin_tbl;
> +   struct ec_smbuso_em0 ec_smboem0;
> +   struct adv_hwmon_profile *profile;
> +};
> +
> +static int get_ec_in_vbat_input(struct device *dev, long *val);
> +static int get_ec_in_v5_input(struct device *dev, long *val);
> +static int get_ec_in_v12_input(struct device *dev, long *val);
> +static int get_ec_in_vcore_input(struct device *dev, long *val);
> +static int get_ec_current1_input(struct device *dev, long *val);
> +static int get_ec_cpu_temp(struct device *dev, long *val);
> +static int get_ec_sys_temp(struct device *dev, long *val);
> +
> +const struct ec_hwmon_attrs ec_hwmon_in_attr_template[] = {
> +   {"VBAT",0444, get_ec_in_vbat_input},// in1
> +   {"5VSB",0444, get_ec_in_v5_input},  // in2
> +   {"Vin", 0444, get_ec_in_v12_input}, // in3 (== in8)
> +   {"VCORE",   0444, get_ec_in_vcore_input},   // in4
> +   {"Vin1",0444, NULL},// in5
> +   {"Vin2",0444, NULL},// in6
> +   {"System Voltage", 0444, NULL}, // in7
> +   {"Current", 0444, get_ec_current1_input},
> +};
> +
> +const struct ec_hwmon_attrs ec_temp_attrs_template[] = {
> +   {"CPU Temp",0444, get_ec_cpu_temp},
> +   {"System Temp", 0444, get_ec_sys_temp},
> +};
> +
> +enum ec_hwmon_in_type {
> +   EC_HWMON_IN_VBAT,
> +   EC_HWMON_IN_5VSB,
> +   

Re: [PATCH v6 6/6] watchdog: ahc1ec0-wdt: Add sub-device watchdog for Advantech embedded controller

[AceLan Kao]

Campion Kang  於 2021年1月18日 週一 下午8:39寫道：
>
> This is one of sub-device driver for Advantech embedded controller
> AHC1EC0. This driver provide watchdog functionality for Advantech
> related applications to restart the system.
>
> Changed since V5:
> - remove unnecessary header files
> - bug fixed: reboot halt if watchdog enabled
> - Kconfig: add "AHC1EC0" string to clearly define the EC name
>
> Signed-off-by: Campion Kang 
> ---
>  drivers/watchdog/Kconfig   |  11 ++
>  drivers/watchdog/Makefile  |   1 +
>  drivers/watchdog/ahc1ec0-wdt.c | 261 +
>  3 files changed, 273 insertions(+)
>  create mode 100644 drivers/watchdog/ahc1ec0-wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 7ff941e71b79..1a27836883ac 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -1636,6 +1636,17 @@ config NIC7018_WDT
>   To compile this driver as a module, choose M here: the module will 
> be
>   called nic7018_wdt.
>
> +config AHC1EC0_WDT
> +   tristate "Advantech AHC1EC0 Watchdog Function"
> +   depends on MFD_AHC1EC0
> +   help
> + This is sub-device for Advantech AHC1EC0 embedded controller.
> +
> + This driver provide watchdog functionality for Advantech related
> + applications to restart the system.
> + To compile thie driver as a module, choose M here: the module will 
> be
> + called ahc1ec0-wdt.
> +
>  # M68K Architecture
>
>  config M54xx_WATCHDOG
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 5c74ee19d441..7190811b1e50 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -145,6 +145,7 @@ obj-$(CONFIG_INTEL_MID_WATCHDOG) += intel-mid_wdt.o
>  obj-$(CONFIG_INTEL_MEI_WDT) += mei_wdt.o
>  obj-$(CONFIG_NI903X_WDT) += ni903x_wdt.o
>  obj-$(CONFIG_NIC7018_WDT) += nic7018_wdt.o
> +obj-$(CONFIG_AHC1EC0_WDT) += ahc1ec0-wdt.o
>  obj-$(CONFIG_MLX_WDT) += mlx_wdt.o
>
>  # M68K Architecture
> diff --git a/drivers/watchdog/ahc1ec0-wdt.c b/drivers/watchdog/ahc1ec0-wdt.c
> new file mode 100644
> index ..4497b6106b24
> --- /dev/null
> +++ b/drivers/watchdog/ahc1ec0-wdt.c
> @@ -0,0 +1,261 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Watchdog Driver for Advantech Embedded Controller chip AHC1EC0
> + *
> + * Copyright 2020, Advantech IIoT Group
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define DRV_NAME  "ahc1ec0-wdt"
> +
> +struct ec_wdt_data {
> +   struct watchdog_device wdtdev;
> +   struct adv_ec_platform_data *adv_ec_data;
> +   struct notifier_block reboot_nb;
> +   struct mutex lock_ioctl;
> +   int is_enable;
> +   int current_timeout;
> +};
> +
> +#define EC_WDT_MIN_TIMEOUT 1   /* The watchdog devices minimum timeout value 
> (in seconds). */
> +#define EC_WDT_MAX_TIMEOUT 600  /* The watchdog devices maximum timeout 
> value (in seconds) */
> +#define EC_WDT_DEFAULT_TIMEOUT 45
> +
> +static int set_delay(struct adv_ec_platform_data *adv_ec_data, unsigned 
> short delay_timeout_in_ms)
> +{
> +   if (write_hw_ram(adv_ec_data, EC_RESET_DELAY_TIME_L, 
> delay_timeout_in_ms & 0x00FF)) {
> +   pr_err("Failed to set Watchdog Retset Time Low byte.");
> +   return -EINVAL;
> +   }
> +
> +   if (write_hw_ram(adv_ec_data, EC_RESET_DELAY_TIME_H, 
> (delay_timeout_in_ms & 0xFF00) >> 8)) {
> +   pr_err("Failed to set Watchdog Retset Time Hight byte.");
> +   return -EINVAL;
> +   }
> +
> +   return 0;
> +}
> +
> +static int advwdt_set_heartbeat(unsigned long t)
> +{
> +   if (t < 1 || t > 6553) {
> +   pr_err("%s: the input timeout is out of range.",  __func__);
> +   pr_err("Please choose valid data between 1 ~ 6553.");
> +   return -EINVAL;
> +   }
> +
> +   return (t * 10);
> +}
> +
> +/* Notifier for system down */
> +static int advwdt_notify_sys(struct notifier_block *nb, unsigned long code, 
> void *data)
> +{
> +   if (code == SYS_DOWN || code == SYS_HALT) {
> +   struct ec_wdt_data *ec_wdt_data;
> +
> +   ec_wdt_data = container_of(nb, struct ec_wdt_data, reboot_nb);
> +   if (!ec_wdt_data)
> +   return NOTIFY_BAD;
> +
> +   /* Turn the WDT off */
> +   if (write_hwram_command(ec_wdt_data->adv_ec_data, 
> EC_WDT_STOP)) {
> +   pr_err("Failed to set Watchdog stop.");
> +   return -EINVAL;
> +   }
> +   ec_wdt_data->is_enable = 0;
> +   pr_info("%s: notify sys shutdown", __func__);
> +   }
> +
> +   return NOTIFY_DONE;
> +}
> +
> +static int ec_wdt_start(struct watchdog_device *wdd)
> +{
> +   int ret;
> +   int timeout, timeout_in_ms;
> +   struct 

Re: [PATCH v3 5/6] mfd: ahc1ec0-hwmon: Add sub-device hwmon for Advantech embedded controller

[AceLan Kao]

Hi Lee,

One question for you.

Lee Jones  於 2020年10月29日 週四 下午9:14寫道：
>
> On Thu, 29 Oct 2020, Shihlun Lin wrote:
>
> > This is one of sub-device driver for Advantech embedded controller
> > AHC1EC0. This driver provides sysfs ABI for Advantech related
> > applications to monitor the system status.
> >
> > Signed-off-by: Shihlun Lin 
> > Reported-by: kernel test robot 
>
> LKP reported that your driver needed upstreaming?
>
> I'm confused!
>
> > ---
> >  drivers/mfd/Kconfig |8 +
> >  drivers/mfd/Makefile|1 +
> >  drivers/mfd/ahc1ec0-hwmon.c | 1514 +++
>
> This obviously belongs in drivers/hwmon.
>
> >  3 files changed, 1523 insertions(+)
> >  create mode 100644 drivers/mfd/ahc1ec0-hwmon.c
> >
> > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> > index 965bcafbe5b2..52ca49b211fc 100644
> > --- a/drivers/mfd/Kconfig
> > +++ b/drivers/mfd/Kconfig
> > @@ -2175,5 +2175,13 @@ config MFD_AHC1EC0
> > provides expose functions for sub-devices to read/write the value
> > to embedded controller.
> >
> > +config MFD_AHC1EC0_HWMON
> > + tristate "Advantech EC Hareware Monitor Function"
> > + depends on MFD_AHC1EC0
> > + help
> > +   This is sub-device for Advantech embedded controller AHC1EC0. This
> > +   driver provides the sysfs attribues for applications to monitor
> > +   the system status.
> > +
> >  endmenu
> >  endif
> > diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> > index 80a9a2bdc3ba..eb645db817b5 100644
> > --- a/drivers/mfd/Makefile
> > +++ b/drivers/mfd/Makefile
> > @@ -269,3 +269,4 @@ obj-$(CONFIG_MFD_SIMPLE_MFD_I2C)  += simple-mfd-i2c.o
> >  obj-$(CONFIG_MFD_INTEL_M10_BMC)   += intel-m10-bmc.o
> >
> >  obj-$(CONFIG_MFD_AHC1EC0)+= ahc1ec0.o
> > +obj-$(CONFIG_MFD_AHC1EC0_HWMON)  += ahc1ec0-hwmon.o
> > diff --git a/drivers/mfd/ahc1ec0-hwmon.c b/drivers/mfd/ahc1ec0-hwmon.c
> > new file mode 100644
> > index ..3e493b040b4a
> > --- /dev/null
> > +++ b/drivers/mfd/ahc1ec0-hwmon.c
> > @@ -0,0 +1,1514 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (c) 2018, Advantech Automation Corp.
>
> You can't just lift a whole driver from downstream code and send it to
> the mailing list as-is.
Could you elaborate more about which part should be fixed, and how it
should look like?

> > + * THIS IS AN UNPUBLISHED WORK CONTAINING CONFIDENTIAL AND PROPRIETARY
> > + * INFORMATION WHICH IS THE PROPERTY OF ADVANTECH AUTOMATION CORP.
> > + *
> > + * ANY DISCLOSURE, USE, OR REPRODUCTION, WITHOUT WRITTEN AUTHORIZATION FROM
> > + * ADVANTECH AUTOMATION CORP., IS STRICTLY PROHIBITED.
> > + 
> > *
>
> This warning is in contradiction to the licence you are proposing.
>
> --
> Lee Jones [李琼斯]
> Senior Technical Lead - Developer Services
> Linaro.org │ Open source software for Arm SoCs
> Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH] iio: dac: ad5593r: Dynamically set AD5593R channel modes

[AceLan Kao]

Hi Andy,

Here is the updated ads5593r.asl, but I still got some troubles when
using configfs.
The modification I made
1. add DefinitionBlock() and External()
2. align _SB.I2C1
3. set _CRS serialized

The issues I got are,
1. The ADS5593 is defined in DSDT table, but I can't compile the asl
code when define it as DSDT table
$ iasl ads5593r.asl

Intel ACPI Component Architecture
ASL+ Optimizing Compiler/Disassembler version 20190509
Copyright (c) 2000 - 2019 Intel Corporation

ads5593r.asl  5: Scope (_SB.I2C1)
Error6148 -   ^ Illegal open scope on external
object from within DSDT

ASL Input: ads5593r.asl -7917 bytes 30 keywords257 source lines

Compilation failed. 1 Errors, 0 Warnings, 0 Remarks
No AML files were generated due to compiler error(s)

2. I got below errors in dmesg if I set it as SSDT
[  410.220993] ACPI: Host-directed Dynamic ACPI Table Load:
[  410.221013] ACPI: SSDT 0x89A4ADDE7C00 00035D (v01
ADS5593R 0001 INTL 20190509)
[  410.221106] ACPI BIOS Error (bug): Failure creating named object
[\_SB.I2C1.I2CG], AE_ALREADY_EXISTS (20190816/dswload2-326)
[  410.221324] ACPI Error: AE_ALREADY_EXISTS, During name
lookup/catalog (20190816/psobject-220)
[  410.221468] ACPI: Skipping parse of AML opcode: Device (0x5B82)

Do you have any suggestions?
We may have a chance to convince BIOS to move ADS5593 to the SSDT
table, do you think it's a good idea?
BTW, the driver set the channels mode while probing, I'm not sure if
configfs will make the driver probe again when new table is loaded
If we can't use configfs, is there any other way we could try?
Thanks.

Andy Shevchenko  於 2020年9月3日 週四 下午6:54寫道：
>
> On Thu, Sep 3, 2020 at 1:42 PM Andy Shevchenko
>  wrote:
> > On Thu, Sep 3, 2020 at 10:37 AM AceLan Kao  wrote:
>
> Couple additional notes.
>
> ...
>
> > > I spent some time studying/reading what you wrote, but I still don't
> > > understand how to leverage meta-acpi.
> >
> > meta-acpi is a Yocto layer to support provided ACPI tables for the
> > build. My point here is to have it as a collection of ASL examples.
> > It's what you asked for below in this email.
>
> > Also we can collect your ASL example under board (presumably new) folder.
>
> Actually it seems Baytrail, so, minnowboard-max is good enough.
>
> ...
>
> > On the first glance I didn't see any issues with it, but on second
> > look here is one. Look into this [5] example.
> > If you noticed it uses the same path in Scope and in the reference in
> > I2cSerialBus() while in your ASL they are different.
>
> Also there is an _ADR value wrong for the second channel (I'm not sure
> if it affects anyhow the rest).
>
> > Do you have issues with loading it (as is and after above addressed)?
> >
> > [5]: 
> > https://github.com/westeri/meta-acpi/blob/master/recipes-bsp/acpi-tables/samples/edison/ft6236.asli
>
> Also a link [6] to our Buildroot repository which allows to create an
> initramfs with ASL compiled. Maybe used as a reference how we created
> initramfs and compile ASLs.
>
> [6]: https://github.com/andy-shev/buildroot/tree/intel/board/intel/common
> ...
>
> > > > One more useful link is SO answers on the topic:
> > > > https://stackoverflow.com/search?tab=newest=prp0001
> > > >
> > > > > [1]: 
> > > > > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > > > > [2]: 
> > > > > https://elixir.bootlin.com/linux/v5.9-rc3/source/Documentation/devicetree/bindings/iio/dac/ad5592r.txt
> > > > > [3]: https://github.com/westeri/meta-acpi
> > > > > [4]: 
> > > > > https://github.com/westeri/meta-acpi/tree/master/recipes-bsp/acpi-tables/samples
>
> > > > > > 1. 
> > > > > > https://www.advantech.com/products/9a0cc561-8fc2-4e22-969c-9df90a3952b5/uno-420/mod_2d6a546b-39e3-4bc4-bbf4-ac89e6b7667c
>
>
>
> --
> With Best Regards,
> Andy Shevchenko


ads5593.asl
Description: Binary data

Re: [PATCH] iio: dac: ad5593r: Dynamically set AD5593R channel modes

[AceLan Kao]

Hi Andy,

I spent some time studying/reading what you wrote, but I still don't
understand how to leverage meta-acpi.

>From what I understand from the following discussion in the thread,
ADS5593 could be used,
so we can keep using it and don't have to introduce PRP0001 in the table, right?

Here is the ADS5593 asl code, but I have no idea how to re-use it
after it's been modified,
the only way I know is to override the ACPI tables via initrd[1].
Could you share some examples in real cases that I can follow?
Thanks.

1. Documentation/admin-guide/acpi/initrd_table_override.rst

Andy Shevchenko  於 2020年8月31日 週一 下午8:48寫道：
>
> On Mon, Aug 31, 2020 at 3:45 PM Andy Shevchenko
>  wrote:
> > On Mon, Aug 31, 2020 at 2:28 PM AceLan Kao  wrote:
> > > This patch is mainly for Advantech's UNO-420[1] which is a x86-based 
> > > platform.
> > > This platform is more like a development platform for customers to
> > > customize their products,
> > > so, specify the channel modes in ACPI table is not generic enough,
> > > that's why William submit this patch.
> > >
> > > Are there other ways to specify or pass values to the module without
> > > using module parameters?
> > > It's good if we can leverage sysfs, but I don't know if there is one
> > > for this scenario.
> >
> > Can we provide DT bindings for that and use then in ACPI? ACPI has a
> > possibility to reuse DT properties and compatible strings [1]. As far
> > as I can see the driver uses fwnode API, so it supports ACPI case
> > already [2]. So, what prevents you to utilize 'adi,mode' property?
> >
> > Also, we accept examples of ASL excerpt in meta-acpi project [3]. It
> > has already plenty of examples [4] how to use PRP0001 for DIY /
> > development boards.
> >
> > So, take all together I think this patch is simple redundant.
>
> One more useful link is SO answers on the topic:
> https://stackoverflow.com/search?tab=newest=prp0001
>
> > [1]: 
> > https://www.kernel.org/doc/html/latest/firmware-guide/acpi/enumeration.html#device-tree-namespace-link-device-id
> > [2]: 
> > https://elixir.bootlin.com/linux/v5.9-rc3/source/Documentation/devicetree/bindings/iio/dac/ad5592r.txt
> > [3]: https://github.com/westeri/meta-acpi
> > [4]: 
> > https://github.com/westeri/meta-acpi/tree/master/recipes-bsp/acpi-tables/samples
> >
> > P.S. Jonathan, it seems this driver has artificial ACPI HID. We
> > probably have to remove it. However, ADS is indeed reserved for Analog
> > Devices in PNP registry. Can we have AD's official answer on this?
> > Cc'ing additional AD people.
> >
> > > 1. 
> > > https://www.advantech.com/products/9a0cc561-8fc2-4e22-969c-9df90a3952b5/uno-420/mod_2d6a546b-39e3-4bc4-bbf4-ac89e6b7667c
>
> --
> With Best Regards,
> Andy Shevchenko
Scope (_SB.I2C1)
{
Device (I2CG)
{
Name (_HID, "ADS5593")  // _HID: Hardware ID
Device (IOP0)
{
Name (_ADR, Zero)  // _ADR: Address
Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
Package (0x03)
{
Package (0x02)
{
"reg", 
Zero
}, 

Package (0x02)
{
"adi,mode", 
0x08
}, 

Package (0x02)
{
"adi,off-state", 
Zero
}
}
})
}

Device (IOP1)
{
Name (_ADR, Zero)  // _ADR: Address
Name (_DSD, Package (0x02)  // _DSD: Device-Specific Data
{
ToUUID ("daffd814-6eba-4d8c-8a91-bc9bbf4aa301") /* Device Properties for _DSD */, 
Package (0x03)
{
Package (0x02)
{
"reg", 
One
}, 

Package (0x02)
{
"adi,mode", 
0x03
}, 

Package (0x02)
{
"adi,off-state", 
Zero
 

Re: [PATCH] iio: dac: ad5593r: Dynamically set AD5593R channel modes

[AceLan Kao]

Hi Jonathan and Andy,

This patch is mainly for Advantech's UNO-420[1] which is a x86-based platform.
This platform is more like a development platform for customers to
customize their products,
so, specify the channel modes in ACPI table is not generic enough,
that's why William submit this patch.

Are there other ways to specify or pass values to the module without
using module parameters?
It's good if we can leverage sysfs, but I don't know if there is one
for this scenario.

1. 
https://www.advantech.com/products/9a0cc561-8fc2-4e22-969c-9df90a3952b5/uno-420/mod_2d6a546b-39e3-4bc4-bbf4-ac89e6b7667c

Andy Shevchenko  於 2020年8月31日 週一 上午1:07寫道：
>
> On Mon, Aug 24, 2020 at 8:54 AM William Sung
>  wrote:
> >
> > To use ad5593r more flexibly, we use the module parameter to setting the
> > channel modes dynamically whenever the module probe up.
>
> > Users can pass
> > the channel modes to the module parameter for allocating the
> > functionality of channels as desired.
>
> NAK. We have a sysfs interface.
>
> > For example:
> > * Use in the kernel command line:
> > Users can add the module parameter in the kernel command line such as
> >
> > "ad5593r.ch_mode_cmdline=88001122"
> >
> > "88001122" means the channel mode setting for each channel. The most
> > left side indicates the mode of channel 7, and the most right side
> > indicates the mode of channel 0.
> >
> > * Use when manually probe the module:
> > Similar to the kernel command line usage, users can enter
> >
> > "modprobe ad5593r ch_mode_probe=88001122"
> >
> > to start the ad5593r module with the desired channel mode setting.
>
> Again NAK, this basically should come from Device Tree or ACPI.
>
> --
> With Best Regards,
> Andy Shevchenko

Re: [PATCH] iio: dac: ad5593r: Dynamically set AD5593R channel modes

[AceLan Kao]

Hi William,

Please read my comment below.

William Sung  於 2020年8月24日 週一 下午1:43寫道：
>
> To use ad5593r more flexibly, we use the module parameter to setting the
> channel modes dynamically whenever the module probe up. Users can pass
> the channel modes to the module parameter for allocating the
> functionality of channels as desired.
>
> For example:
> * Use in the kernel command line:
> Users can add the module parameter in the kernel command line such as
>
> "ad5593r.ch_mode_cmdline=88001122"
>
> "88001122" means the channel mode setting for each channel. The most
> left side indicates the mode of channel 7, and the most right side
> indicates the mode of channel 0.
>
> * Use when manually probe the module:
> Similar to the kernel command line usage, users can enter
>
> "modprobe ad5593r ch_mode_probe=88001122"
The usage of the parameters do not match the one written in the code,
which is "ch_mode"

>
> to start the ad5593r module with the desired channel mode setting.
>
> Signed-off-by: William Sung 
> ---
>  drivers/iio/dac/ad5592r-base.c | 33 ++--
>  drivers/iio/dac/ad5592r-base.h |  4 +++
>  drivers/iio/dac/ad5593r.c  | 55 ++
>  3 files changed, 89 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/dac/ad5592r-base.c b/drivers/iio/dac/ad5592r-base.c
> index cc4875660a69..cd69a34fa21e 100644
> --- a/drivers/iio/dac/ad5592r-base.c
> +++ b/drivers/iio/dac/ad5592r-base.c
> @@ -21,6 +21,10 @@
>
>  #include "ad5592r-base.h"
>
> +/* Parameters for dynamic channel mode setting */
> +static u8 update_channel_mode;
> +static u8 new_channel_modes[AD559XR_CHANNEL_NR];
> +
>  static int ad5592r_gpio_get(struct gpio_chip *chip, unsigned offset)
>  {
> struct ad5592r_state *st = gpiochip_get_data(chip);
> @@ -132,7 +136,7 @@ static int ad5592r_gpio_init(struct ad5592r_state *st)
>
> st->gpiochip.label = dev_name(st->dev);
> st->gpiochip.base = -1;
> -   st->gpiochip.ngpio = 8;
> +   st->gpiochip.ngpio = AD559XR_CHANNEL_NR;
> st->gpiochip.parent = st->dev;
> st->gpiochip.can_sleep = true;
> st->gpiochip.direction_input = ad5592r_gpio_direction_input;
> @@ -287,6 +291,14 @@ static int ad5592r_set_channel_modes(struct 
> ad5592r_state *st)
> return ret;
>  }
>
> +static void ad5592r_set_def_channel_modes(struct ad5592r_state *st)
> +{
> +   int i;
> +
> +   for (i = 0; i < ARRAY_SIZE(st->channel_modes); i++)
> +   st->channel_modes[i] = new_channel_modes[i];
> +}
> +
>  static int ad5592r_reset_channel_modes(struct ad5592r_state *st)
>  {
> int i;
> @@ -532,6 +544,10 @@ static int ad5592r_alloc_channels(struct iio_dev 
> *iio_dev)
> st->channel_offstate[reg] = tmp;
> }
>
> +   /* Update default channel modes set by external module */
> +   if (update_channel_mode == 1)
> +   ad5592r_set_def_channel_modes(st);
> +
> channels = devm_kcalloc(st->dev,
> 1 + 2 * num_channels, sizeof(*channels),
> GFP_KERNEL);
> @@ -567,7 +583,7 @@ static int ad5592r_alloc_channels(struct iio_dev *iio_dev)
> }
>
> channels[curr_channel].type = IIO_TEMP;
> -   channels[curr_channel].channel = 8;
> +   channels[curr_channel].channel = AD559XR_CHANNEL_NR;
> channels[curr_channel].info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
>BIT(IIO_CHAN_INFO_SCALE) |
>BIT(IIO_CHAN_INFO_OFFSET);
> @@ -589,6 +605,17 @@ static void ad5592r_init_scales(struct ad5592r_state 
> *st, int vref_mV)
> div_s64_rem(tmp * 2, 10LL, >scale_avail[1][1]);
>  }
>
> +void ad5592r_update_default_channel_modes(u8 *new_modes)
> +{
> +   int idx = 0;
> +
> +   update_channel_mode = 1;
> +   for (idx = 0; idx < AD559XR_CHANNEL_NR; idx++)
> +   new_channel_modes[idx] = new_modes[idx];
> +
> +}
> +EXPORT_SYMBOL_GPL(ad5592r_update_default_channel_modes);
> +
>  int ad5592r_probe(struct device *dev, const char *name,
> const struct ad5592r_rw_ops *ops)
>  {
> @@ -603,7 +630,7 @@ int ad5592r_probe(struct device *dev, const char *name,
> st = iio_priv(iio_dev);
> st->dev = dev;
> st->ops = ops;
> -   st->num_channels = 8;
> +   st->num_channels = AD559XR_CHANNEL_NR;
> dev_set_drvdata(dev, iio_dev);
>
> st->reg = devm_regulator_get_optional(dev, "vref");
> diff --git a/drivers/iio/dac/ad5592r-base.h b/drivers/iio/dac/ad5592r-base.h
> index 23dac2f1ff8a..40ad6369e660 100644
> --- a/drivers/iio/dac/ad5592r-base.h
> +++ b/drivers/iio/dac/ad5592r-base.h
> @@ -39,6 +39,9 @@ enum ad5592r_registers {
>  #define AD5592R_REG_CTRL_ADC_RANGE BIT(5)
>  #define AD5592R_REG_CTRL_DAC_RANGE BIT(4)
>
> +/* Define quantity of channels of AD5592R/AD5593R */
> +#define AD559XR_CHANNEL_NR 8
> +
>  struct 

[PATCH] USB: serial: option: add Quectel EG95 LTE modem

[AceLan Kao]

Add support for Quectel Wireless Solutions Co., Ltd. EG95 LTE modem

T:  Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  5 Spd=480 MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=2c7c ProdID=0195 Rev=03.18
S:  Manufacturer=Android
S:  Product=Android
C:  #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)

Signed-off-by: AceLan Kao 
---
 drivers/usb/serial/option.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 254a8bbeea67..a12ad91a7e0b 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -245,6 +245,7 @@ static void option_instat_callback(struct urb *urb);
 /* These Quectel products use Quectel's vendor ID */
 #define QUECTEL_PRODUCT_EC21   0x0121
 #define QUECTEL_PRODUCT_EC25   0x0125
+#define QUECTEL_PRODUCT_EG95   0x0195
 #define QUECTEL_PRODUCT_BG96   0x0296
 #define QUECTEL_PRODUCT_EP06   0x0306
 #define QUECTEL_PRODUCT_EM12   0x0512
@@ -1097,6 +1098,8 @@ static const struct usb_device_id option_ids[] = {
  .driver_info = RSVD(4) },
{ USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EC25),
  .driver_info = RSVD(4) },
+   { USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EG95),
+ .driver_info = RSVD(4) },
{ USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_BG96),
  .driver_info = RSVD(4) },
{ USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, 
QUECTEL_PRODUCT_EP06, 0xff, 0xff, 0xff),
-- 
2.25.1

[PATCH] net: usb: qmi_wwan: add support for Quectel EG95 LTE modem

[AceLan Kao]

Add support for Quectel Wireless Solutions Co., Ltd. EG95 LTE modem

T:  Bus=01 Lev=01 Prnt=01 Port=02 Cnt=02 Dev#=  5 Spd=480 MxCh= 0
D:  Ver= 2.00 Cls=ef(misc ) Sub=02 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=2c7c ProdID=0195 Rev=03.18
S:  Manufacturer=Android
S:  Product=Android
C:  #Ifs= 5 Cfg#= 1 Atr=a0 MxPwr=500mA
I:  If#=0x0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)
I:  If#=0x1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x2 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x3 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=00 Prot=00 Driver=(none)
I:  If#=0x4 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none)

Signed-off-by: AceLan Kao 
---
 drivers/net/usb/qmi_wwan.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index 31b1d4b959f6..07c42c0719f5 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1370,6 +1370,7 @@ static const struct usb_device_id products[] = {
{QMI_QUIRK_SET_DTR(0x1e0e, 0x9001, 5)}, /* SIMCom 7100E, 7230E, 7600E 
++ */
{QMI_QUIRK_SET_DTR(0x2c7c, 0x0121, 4)}, /* Quectel EC21 Mini PCIe */
{QMI_QUIRK_SET_DTR(0x2c7c, 0x0191, 4)}, /* Quectel EG91 */
+   {QMI_QUIRK_SET_DTR(0x2c7c, 0x0195, 4)}, /* Quectel EG95 */
{QMI_FIXED_INTF(0x2c7c, 0x0296, 4)},/* Quectel BG96 */
{QMI_QUIRK_SET_DTR(0x2cb7, 0x0104, 4)}, /* Fibocom NL678 series */
{QMI_FIXED_INTF(0x0489, 0xe0b4, 0)},/* Foxconn T77W968 LTE */
-- 
2.25.1

Re: [PATCH v2] mfd: intel-lpss: use devm_ioremap_uc for MMIO

[AceLan Kao]

Confirmed the patch works well on Dell XPS 7390 2-in-1 machine.

Tested-by: AceLan Kao 

Tuowen Zhao  於 2019年10月8日 週二 上午2:43寫道：
>
> Some BIOS erroneously specifies write-combining BAR for intel-lpss-pci
> in MTRR. This will cause the system to hang during boot. If possible,
> this bug could be corrected with a firmware update.
>
> This patch adds devm_ioremap_uc as a new managed wrapper to ioremap_uc
> and with it overwrite the MTRR settings to force the use of strongly
> uncachable pages for intel-lpss.
>
> The BIOS bug is present on Dell XPS 13 7390 2-in-1:
>
> [0.001734]   5 base 40 mask 60 write-combining
>
> 40-7f : PCI Bus :00
>   40-400fff : :00:02.0 (i915)
>   401000-401fff : :00:15.0 (intel-lpss-pci)
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=203485
> Signed-off-by: Tuowen Zhao 
> ---
> Changes from previous version:
>
>   * changed commit message
>
>  drivers/mfd/intel-lpss.c |  2 +-
>  include/linux/io.h   |  2 ++
>  lib/devres.c | 19 +++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mfd/intel-lpss.c b/drivers/mfd/intel-lpss.c
> index bfe4ff337581..b0f0781a6b9c 100644
> --- a/drivers/mfd/intel-lpss.c
> +++ b/drivers/mfd/intel-lpss.c
> @@ -384,7 +384,7 @@ int intel_lpss_probe(struct device *dev,
> if (!lpss)
> return -ENOMEM;
>
> -   lpss->priv = devm_ioremap(dev, info->mem->start + LPSS_PRIV_OFFSET,
> +   lpss->priv = devm_ioremap_uc(dev, info->mem->start + LPSS_PRIV_OFFSET,
>   LPSS_PRIV_SIZE);
> if (!lpss->priv)
> return -ENOMEM;
> diff --git a/include/linux/io.h b/include/linux/io.h
> index accac822336a..a59834bc0a11 100644
> --- a/include/linux/io.h
> +++ b/include/linux/io.h
> @@ -64,6 +64,8 @@ static inline void devm_ioport_unmap(struct device *dev, 
> void __iomem *addr)
>
>  void __iomem *devm_ioremap(struct device *dev, resource_size_t offset,
>resource_size_t size);
> +void __iomem *devm_ioremap_uc(struct device *dev, resource_size_t offset,
> +  resource_size_t size);
>  void __iomem *devm_ioremap_nocache(struct device *dev, resource_size_t 
> offset,
>resource_size_t size);
>  void __iomem *devm_ioremap_wc(struct device *dev, resource_size_t offset,
> diff --git a/lib/devres.c b/lib/devres.c
> index 6a0e9bd6524a..beb0a064b891 100644
> --- a/lib/devres.c
> +++ b/lib/devres.c
> @@ -9,6 +9,7 @@
>  enum devm_ioremap_type {
> DEVM_IOREMAP = 0,
> DEVM_IOREMAP_NC,
> +   DEVM_IOREMAP_UC,
> DEVM_IOREMAP_WC,
>  };
>
> @@ -39,6 +40,9 @@ static void __iomem *__devm_ioremap(struct device *dev, 
> resource_size_t offset,
> case DEVM_IOREMAP_NC:
> addr = ioremap_nocache(offset, size);
> break;
> +   case DEVM_IOREMAP_UC:
> +   addr = ioremap_uc(offset, size);
> +   break;
> case DEVM_IOREMAP_WC:
> addr = ioremap_wc(offset, size);
> break;
> @@ -68,6 +72,21 @@ void __iomem *devm_ioremap(struct device *dev, 
> resource_size_t offset,
>  }
>  EXPORT_SYMBOL(devm_ioremap);
>
> +/**
> + * devm_ioremap_uc - Managed ioremap_uc()
> + * @dev: Generic device to remap IO address for
> + * @offset: Resource address to map
> + * @size: Size of map
> + *
> + * Managed ioremap_uc().  Map is automatically unmapped on driver detach.
> + */
> +void __iomem *devm_ioremap_uc(struct device *dev, resource_size_t offset,
> + resource_size_t size)
> +{
> +   return __devm_ioremap(dev, offset, size, DEVM_IOREMAP_UC);
> +}
> +EXPORT_SYMBOL(devm_ioremap_uc);
> +
>  /**
>   * devm_ioremap_nocache - Managed ioremap_nocache()
>   * @dev: Generic device to remap IO address for
> --
> 2.23.0
>

Re: [PATCH] r8169: add enable_aspm parameter

[AceLan Kao]

Hi Heiner,

I've tried and verified your PCI ASPM patches and it works well.
I've replied the patch thread and hope this can make it get some progress.

BTW, do you think we can revert commit b75bb8a5b755 ("r8169: disable
ASPM again") once the PCI ASPM patches get merged?

Best regards,
AceLan Kao.

AceLan Kao  於 2019年7月9日 週二 上午11:19寫道：
>
> Heiner Kallweit  於 2019年7月9日 週二 上午2:27寫道：
> >
> > On 08.07.2019 08:37, AceLan Kao wrote:
> > > We have many commits in the driver which enable and then disable ASPM
> > > function over and over again.
> > >commit b75bb8a5b755 ("r8169: disable ASPM again")
> > >commit 0866cd15029b ("r8169: enable ASPM on RTL8106E")
> > >commit 94235460f9ea ("r8169: Align ASPM/CLKREQ setting function with 
> > > vendor driver")
> > >commit aa1e7d2c31ef ("r8169: enable ASPM on RTL8168E-VL")
> > >commit f37658da21aa ("r8169: align ASPM entry latency setting with 
> > > vendor driver")
> > >commit a99790bf5c7f ("r8169: Reinstate ASPM Support")
> > >commit 671646c151d4 ("r8169: Don't disable ASPM in the driver")
> > >commit 4521e1a94279 ("Revert "r8169: enable internal ASPM and clock 
> > > request settings".")
> > >commit d64ec841517a ("r8169: enable internal ASPM and clock request 
> > > settings")
> > >
> > > This function is very important for production, and if we can't come out
> > > a solution to make both happy, I'd suggest we add a parameter in the
> > > driver to toggle it.
> > >
> > The usage of a module parameter to control ASPM is discouraged.
> > There have been more such attempts in the past that have been declined.
> >
> > Pending with the PCI maintainers is a series adding ASPM control
> > via sysfs, see here: https://www.spinics.net/lists/linux-pci/msg83228.html
> Cool, I'll try your patches and reply on that thread.
>
> >
> > Also more details than just stating "it's important for production"
> > would have been appreciated in the commit message, e.g. which
> > power-savings you can achieve with ASPM on which systems.
> I should use more specific wordings rather than "important for
> production", thanks.

Re: [PATCH] r8169: add enable_aspm parameter

[AceLan Kao]

Heiner Kallweit  於 2019年7月9日 週二 上午2:27寫道：
>
> On 08.07.2019 08:37, AceLan Kao wrote:
> > We have many commits in the driver which enable and then disable ASPM
> > function over and over again.
> >commit b75bb8a5b755 ("r8169: disable ASPM again")
> >commit 0866cd15029b ("r8169: enable ASPM on RTL8106E")
> >commit 94235460f9ea ("r8169: Align ASPM/CLKREQ setting function with 
> > vendor driver")
> >commit aa1e7d2c31ef ("r8169: enable ASPM on RTL8168E-VL")
> >commit f37658da21aa ("r8169: align ASPM entry latency setting with 
> > vendor driver")
> >commit a99790bf5c7f ("r8169: Reinstate ASPM Support")
> >commit 671646c151d4 ("r8169: Don't disable ASPM in the driver")
> >commit 4521e1a94279 ("Revert "r8169: enable internal ASPM and clock 
> > request settings".")
> >commit d64ec841517a ("r8169: enable internal ASPM and clock request 
> > settings")
> >
> > This function is very important for production, and if we can't come out
> > a solution to make both happy, I'd suggest we add a parameter in the
> > driver to toggle it.
> >
> The usage of a module parameter to control ASPM is discouraged.
> There have been more such attempts in the past that have been declined.
>
> Pending with the PCI maintainers is a series adding ASPM control
> via sysfs, see here: https://www.spinics.net/lists/linux-pci/msg83228.html
Cool, I'll try your patches and reply on that thread.

>
> Also more details than just stating "it's important for production"
> would have been appreciated in the commit message, e.g. which
> power-savings you can achieve with ASPM on which systems.
I should use more specific wordings rather than "important for
production", thanks.

[PATCH] r8169: add enable_aspm parameter

[AceLan Kao]

We have many commits in the driver which enable and then disable ASPM
function over and over again.
   commit b75bb8a5b755 ("r8169: disable ASPM again")
   commit 0866cd15029b ("r8169: enable ASPM on RTL8106E")
   commit 94235460f9ea ("r8169: Align ASPM/CLKREQ setting function with vendor 
driver")
   commit aa1e7d2c31ef ("r8169: enable ASPM on RTL8168E-VL")
   commit f37658da21aa ("r8169: align ASPM entry latency setting with vendor 
driver")
   commit a99790bf5c7f ("r8169: Reinstate ASPM Support")
   commit 671646c151d4 ("r8169: Don't disable ASPM in the driver")
   commit 4521e1a94279 ("Revert "r8169: enable internal ASPM and clock request 
settings".")
   commit d64ec841517a ("r8169: enable internal ASPM and clock request 
settings")

This function is very important for production, and if we can't come out
a solution to make both happy, I'd suggest we add a parameter in the
driver to toggle it.

Signed-off-by: AceLan Kao 
---
 drivers/net/ethernet/realtek/r8169.c | 13 +
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/realtek/r8169.c 
b/drivers/net/ethernet/realtek/r8169.c
index d06a61f00e78..f557cb36e2c6 100644
--- a/drivers/net/ethernet/realtek/r8169.c
+++ b/drivers/net/ethernet/realtek/r8169.c
@@ -702,10 +702,13 @@ struct rtl8169_private {
 
 typedef void (*rtl_generic_fct)(struct rtl8169_private *tp);
 
+static int enable_aspm;
 MODULE_AUTHOR("Realtek and the Linux r8169 crew ");
 MODULE_DESCRIPTION("RealTek RTL-8169 Gigabit Ethernet driver");
 module_param_named(debug, debug.msg_enable, int, 0);
 MODULE_PARM_DESC(debug, "Debug verbosity level (0=none, ..., 16=all)");
+module_param(enable_aspm, int, 0);
+MODULE_PARM_DESC(enable_aspm, "Enable ASPM support (0 = disable, 1 = enable");
 MODULE_SOFTDEP("pre: realtek");
 MODULE_LICENSE("GPL");
 MODULE_FIRMWARE(FIRMWARE_8168D_1);
@@ -7163,10 +7166,12 @@ static int rtl_init_one(struct pci_dev *pdev, const 
struct pci_device_id *ent)
if (rc)
return rc;
 
-   /* Disable ASPM completely as that cause random device stop working
-* problems as well as full system hangs for some PCIe devices users.
-*/
-   pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | PCIE_LINK_STATE_L1);
+   if (!enable_aspm) {
+   /* Disable ASPM completely as that cause random device stop 
working
+* problems as well as full system hangs for some PCIe devices 
users.
+*/
+   pci_disable_link_state(pdev, PCIE_LINK_STATE_L0S | 
PCIE_LINK_STATE_L1);
+   }
 
/* enable device (incl. PCI PM wakeup and hotplug setup) */
rc = pcim_enable_device(pdev);
-- 
2.17.1

Re: [PATCH] i2c: designware: Add disable runtime pm quirk

[AceLan Kao]

Hi Jarkko,

Sorry, I lost track of this thread and didn't understand what you want
me to try.
I'm willing to try it if you can explain it more.

My colleague comes out another solution for this issue
   https://lkml.org/lkml/2019/7/5/17
and it explains why it takes up to 100ms to wake up.
This solution is more aggressive to zero the d3cold_delay and it looks
like the delay is not necessary.

Anyway, we have two different proposed solutions now, my proposed
solution is specific to the listed platforms, but we may have to
extent the list(platforms which uses the old firmware),
the other proposed solution from my colleague is generic and apply to
all platforms which loads intel-lpss-pci driver, it may lead to
unexpected regressions which we don't see now.
Please give some suggestions, thanks.

Best regards,
AceLan Kao.

Jarkko Nikula  於 2019年6月26日 週三 下午2:27寫道：
>
> On 6/26/19 5:32 AM, AceLan Kao wrote:
> > Adding I2C_HID_QUIRK_NO_RUNTIME_PM quirk doesn't help on this issue.
> > Actually, Goodix touchpad already has that PM quirk in the list for other 
> > issue.
> >  { I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
> > I2C_HID_QUIRK_NO_RUNTIME_PM },
> > I also modify the code as you suggested, but no luck.
> >
> Yeah, I realized it won't help as the i2c-hid device is anyway powered
> on constantly when the device is open by the pm_runtime_get_sync() call
> in i2c_hid_open().
>
> > It's not Goodix takes time to wakeup, it's designware I2C controller.
> > Designware doesn't do anything wrong here, it's Goodix set the interrupt 
> > timeout
> > that leads to the issue, so we have to prevent designware from runtime
> > suspended.
> > But only on that bus where Goodix is connected and open by user space.
> What I mean something like below:
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..bbeaa39ddc23 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -795,6 +795,9 @@ static int i2c_hid_open(struct hid_device *hid)
> struct i2c_hid *ihid = i2c_get_clientdata(client);
> int ret = 0;
>
> +   /* some quirk test here */
> +   pm_runtime_get_sync(>adapter->dev);
> +
> ret = pm_runtime_get_sync(>dev);
> if (ret < 0)
> return ret;
> @@ -812,6 +815,9 @@ static void i2c_hid_close(struct hid_device *hid)
>
> /* Save some power */
> pm_runtime_put(>dev);
> +
> +   /* some quirk test here */
> +   pm_runtime_put(>adapter->dev);
>   }
>
>   static int i2c_hid_power(struct hid_device *hid, int lvl)
>
> --
> Jarkko

Re: [PATCH] i2c: designware: Add disable runtime pm quirk

[AceLan Kao]

Adding I2C_HID_QUIRK_NO_RUNTIME_PM quirk doesn't help on this issue.
Actually, Goodix touchpad already has that PM quirk in the list for other issue.
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
   I2C_HID_QUIRK_NO_RUNTIME_PM },
I also modify the code as you suggested, but no luck.

It's not Goodix takes time to wakeup, it's designware I2C controller.
Designware doesn't do anything wrong here, it's Goodix set the interrupt timeout
that leads to the issue, so we have to prevent designware from runtime
suspended.

Jarkko Nikula  於 2019年6月25日 週二 下午9:38寫道：
>
> On 6/25/19 11:30 AM, AceLan Kao wrote:
> > Dell machines come with goodix touchpad IC suffer from the double click
> > issue if the Designware I2C adapter enters runtime suspend.
> >
> > It's because the goodix re-assert the interrupt if host doesn't read the
> > data within 100ms and designware takes a longer time to wake up from
> > runtime suspend. In the case, it got a second interrupt during
> > resuming, so it thinks it's a double click.
> >
> > There is no simple way to fix this, it's a firmware issue and goodix
> > agrees to fix this in their firmware on next release, but this issue
> > still affects the machines that don't come with an updated firmware. So,
> > add a quirk to mark those machines and avoid the designware from
> > entering runtime suspend.
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=202683
> >
> > Signed-off-by: AceLan Kao 
> > ---
> >   drivers/i2c/busses/i2c-designware-master.c | 30 --
> >   1 file changed, 28 insertions(+), 2 deletions(-)
> >
> I think better place to have this fixed is in
> drivers/hid/i2c-hid/i2c-hid-core.c by forcing the adapter device active
> when communicating with such touchpad.
>
> In that way only bus where touchpad is connected stays active, not all
> and makes sure issue is handled also if that touchpad is ever connected
> to any other I2C adapter than Designware.
>
> I did something similar in the commit 72bfcee11cf8 ("i2c: Prevent
> runtime suspend of adapter when Host Notify is required"). Not exactly
> same issue but similar idea.
>
> By looking at i2c-hid-core.c I saw a few i2c-hid devices have
> I2C_HID_QUIRK_NO_RUNTIME_PM. Could you test how does this Goodix behave
> if only i2c-hid device runtime PM is prevented not I2C adapter?
>
> A very quick test would be to comment out those lines below:
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
> b/drivers/hid/i2c-hid/i2c-hid-core.c
> index 90164fed08d3..bd3e6570c45e 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -1156,8 +1156,8 @@ static int i2c_hid_probe(struct i2c_client *client,
> goto err_mem_free;
> }
>
> -   if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> -   pm_runtime_put(>dev);
> +// if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> +// pm_runtime_put(>dev);
>
> return 0;
>
> @@ -1183,8 +1183,8 @@ static int i2c_hid_remove(struct i2c_client *client)
> struct i2c_hid *ihid = i2c_get_clientdata(client);
> struct hid_device *hid;
>
> -   if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> -   pm_runtime_get_sync(>dev);
> +// if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> +// pm_runtime_get_sync(>dev);
> pm_runtime_disable(>dev);
> pm_runtime_set_suspended(>dev);
> pm_runtime_put_noidle(>dev);
>
> --
> Jarkko

[PATCH] i2c: designware: Add disable runtime pm quirk

[AceLan Kao]

Dell machines come with goodix touchpad IC suffer from the double click
issue if the Designware I2C adapter enters runtime suspend.

It's because the goodix re-assert the interrupt if host doesn't read the
data within 100ms and designware takes a longer time to wake up from
runtime suspend. In the case, it got a second interrupt during
resuming, so it thinks it's a double click.

There is no simple way to fix this, it's a firmware issue and goodix
agrees to fix this in their firmware on next release, but this issue
still affects the machines that don't come with an updated firmware. So,
add a quirk to mark those machines and avoid the designware from
entering runtime suspend.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=202683

Signed-off-by: AceLan Kao 
---
 drivers/i2c/busses/i2c-designware-master.c | 30 --
 1 file changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-master.c 
b/drivers/i2c/busses/i2c-designware-master.c
index d464799e40a3..4048a66355f6 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -9,6 +9,7 @@
  * Copyright (C) 2009 Provigent Ltd.
  */
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,6 +23,25 @@
 
 #include "i2c-designware-core.h"
 
+static int no_runtime_pm;
+static const struct dmi_system_id i2c_dw_no_runtime_pm[] = {
+   {
+   .ident = "Dell Inspiron 5390",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 5390"),
+   },
+   },
+   {
+   .ident = "Dell Vostro 5390",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 5390"),
+   },
+   },
+   { }
+};
+
 static void i2c_dw_configure_fifo_master(struct dw_i2c_dev *dev)
 {
/* Configure Tx/Rx FIFO threshold levels */
@@ -424,7 +444,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
 
dev_dbg(dev->dev, "%s: msgs: %d\n", __func__, num);
 
-   pm_runtime_get_sync(dev->dev);
+   if (!no_runtime_pm)
+   pm_runtime_get_sync(dev->dev);
 
if (dev_WARN_ONCE(dev->dev, dev->suspended, "Transfer while 
suspended\n")) {
ret = -ESHUTDOWN;
@@ -501,7 +522,8 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg 
msgs[], int num)
 
 done_nolock:
pm_runtime_mark_last_busy(dev->dev);
-   pm_runtime_put_autosuspend(dev->dev);
+   if (!no_runtime_pm)
+   pm_runtime_put_autosuspend(dev->dev);
 
return ret;
 }
@@ -733,6 +755,10 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
if (ret)
return ret;
 
+   no_runtime_pm = dmi_check_system(i2c_dw_no_runtime_pm);
+   if (no_runtime_pm)
+   __pm_runtime_disable(dev->dev, true);
+
/*
 * Increment PM usage count during adapter registration in order to
 * avoid possible spurious runtime suspend when adapter device is
-- 
2.17.1

Re: [PATCH 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

Christoph Hellwig  於 2018年11月6日 週二 下午3:10寫道：
>
> On Tue, Nov 06, 2018 at 10:04:01AM +0800, AceLan Kao wrote:
> > It leads to the power consumption raises to 2.2W during s2idle, while
> > it consumes less than 1W during long idle if put SK hynix nvme to D3
> > and then enter s2idle.
> > >From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
> > APST feature to do the power management.
> > To leverage its APST feature during s2idle, we can't disable nvme
> > device while suspending, too.
> >
> > BTW, prevent it from entering D3 will increase the power consumtion around
> > 0.13W ~ 0.15W during short/long idle, and the power consumption during
> > s2idle becomes 0.77W.
>
> Please get this fixed in firmware.  And yes, depending on which Windows
> driver you use Windows might be doing very agressive runtime D3, which
> is parted of Windows modern standby, so it'll trip up there eventually
> as well.
Agree, this is not a good fix for Sk hynix nvme, so Dell is still pushing
Sk hynix to fix it from firmware.
But before the firmware is ready, this is still a issue that need to be fixed in
kernel side, and the new firmware may not be applied on the old nvme
modules.
This list won't keep growing, and we'll keep an eye on the new firmware and
co-work with engineers from Sky hynix to make sure this issue won't happen
again.

Re: [PATCH 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

Christoph Hellwig  於 2018年11月6日 週二 下午3:10寫道：
>
> On Tue, Nov 06, 2018 at 10:04:01AM +0800, AceLan Kao wrote:
> > It leads to the power consumption raises to 2.2W during s2idle, while
> > it consumes less than 1W during long idle if put SK hynix nvme to D3
> > and then enter s2idle.
> > >From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
> > APST feature to do the power management.
> > To leverage its APST feature during s2idle, we can't disable nvme
> > device while suspending, too.
> >
> > BTW, prevent it from entering D3 will increase the power consumtion around
> > 0.13W ~ 0.15W during short/long idle, and the power consumption during
> > s2idle becomes 0.77W.
>
> Please get this fixed in firmware.  And yes, depending on which Windows
> driver you use Windows might be doing very agressive runtime D3, which
> is parted of Windows modern standby, so it'll trip up there eventually
> as well.
Agree, this is not a good fix for Sk hynix nvme, so Dell is still pushing
Sk hynix to fix it from firmware.
But before the firmware is ready, this is still a issue that need to be fixed in
kernel side, and the new firmware may not be applied on the old nvme
modules.
This list won't keep growing, and we'll keep an eye on the new firmware and
co-work with engineers from Sky hynix to make sure this issue won't happen
again.

[PATCH v2 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Call nvme_dev_disable() function leads to the power consumption goes
up to 2.2 Watt during suspend-to-idle, and from SK hynix FE, they
suggest us to use its own APST feature to do the power management during
s2idle.
After D3 is diabled and nvme_dev_disable() is not called while
suspending, the power consumption drops to 0.77 Watt during s2idle.

V2:
- replace PCI_DEVICE with PCI_VDEVICE
- replace 0x1c5c with SK_HYNIX

Signed-off-by: AceLan Kao 
---
 drivers/nvme/host/nvme.h | 5 +
 drivers/nvme/host/pci.c  | 8 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cee79cb388af..35d260a4cf46 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -90,6 +90,11 @@ enum nvme_quirks {
 * Set MEDIUM priority on SQ creation
 */
NVME_QUIRK_MEDIUM_PRIO_SQ   = (1 << 7),
+
+   /*
+* Do not disable nvme when suspending(s2idle)
+*/
+   NVME_QUIRK_NO_DISABLE   = (1 << 8),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c33bb201b884..13a2d6b2d047 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "nvme.h"
 
@@ -2612,8 +2613,11 @@ static int nvme_suspend(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct nvme_dev *ndev = pci_get_drvdata(pdev);
+   struct nvme_ctrl *ctrl = >ctrl;
+
+   if (!(pm_suspend_via_s2idle() && (ctrl->quirks & 
NVME_QUIRK_NO_DISABLE)))
+   nvme_dev_disable(ndev, true);
 
-   nvme_dev_disable(ndev, true);
return 0;
 }
 
@@ -2716,6 +2720,8 @@ static const struct pci_device_id nvme_id_table[] = {
.driver_data = NVME_QUIRK_LIGHTNVM, },
{ PCI_DEVICE(0x1d1d, 0x2601),   /* CNEX Granby */
.driver_data = NVME_QUIRK_LIGHTNVM, },
+   { PCI_VDEVICE(SK_HYNIX, 0x1527),   /* Sk Hynix */
+   .driver_data = NVME_QUIRK_NO_DISABLE, },
{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xff) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1

[PATCH v2 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Call nvme_dev_disable() function leads to the power consumption goes
up to 2.2 Watt during suspend-to-idle, and from SK hynix FE, they
suggest us to use its own APST feature to do the power management during
s2idle.
After D3 is diabled and nvme_dev_disable() is not called while
suspending, the power consumption drops to 0.77 Watt during s2idle.

V2:
- replace PCI_DEVICE with PCI_VDEVICE
- replace 0x1c5c with SK_HYNIX

Signed-off-by: AceLan Kao 
---
 drivers/nvme/host/nvme.h | 5 +
 drivers/nvme/host/pci.c  | 8 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cee79cb388af..35d260a4cf46 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -90,6 +90,11 @@ enum nvme_quirks {
 * Set MEDIUM priority on SQ creation
 */
NVME_QUIRK_MEDIUM_PRIO_SQ   = (1 << 7),
+
+   /*
+* Do not disable nvme when suspending(s2idle)
+*/
+   NVME_QUIRK_NO_DISABLE   = (1 << 8),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c33bb201b884..13a2d6b2d047 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "nvme.h"
 
@@ -2612,8 +2613,11 @@ static int nvme_suspend(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct nvme_dev *ndev = pci_get_drvdata(pdev);
+   struct nvme_ctrl *ctrl = >ctrl;
+
+   if (!(pm_suspend_via_s2idle() && (ctrl->quirks & 
NVME_QUIRK_NO_DISABLE)))
+   nvme_dev_disable(ndev, true);
 
-   nvme_dev_disable(ndev, true);
return 0;
 }
 
@@ -2716,6 +2720,8 @@ static const struct pci_device_id nvme_id_table[] = {
.driver_data = NVME_QUIRK_LIGHTNVM, },
{ PCI_DEVICE(0x1d1d, 0x2601),   /* CNEX Granby */
.driver_data = NVME_QUIRK_LIGHTNVM, },
+   { PCI_VDEVICE(SK_HYNIX, 0x1527),   /* Sk Hynix */
+   .driver_data = NVME_QUIRK_NO_DISABLE, },
{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xff) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1

[PATCH v2 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

It leads to the power consumption raises to 2.2W during s2idle, while
it consumes less than 1W during long idle if put SK hynix nvme to D3
and then enter s2idle.
>From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
APST feature to do the power management.
To leverage its APST feature during s2idle, we can't disable nvme
device while suspending, too.

BTW, prevent it from entering D3 will increase the power consumtion around
0.13W ~ 0.15W during short/long idle, and the power consumption during
s2idle becomes 0.77W.

Signed-off-by: AceLan Kao 
---
 drivers/pci/quirks.c| 1 +
 include/linux/pci_ids.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4700d24e5d55..b7e6492e8311 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1332,6 +1332,7 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AL, 
PCI_ANY_ID,
occur when mode detecting */
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SK_HYNIX, 0x1527, quirk_no_ata_d3);
 
 /*
  * This was originally an Alpha-specific thing, but it really fits here.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..5f5adda07de0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3090,4 +3090,6 @@
 
 #define PCI_VENDOR_ID_NCUBE0x10ff
 
+#define PCI_VENDOR_ID_SK_HYNIX 0x1c5c
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
2.17.1

[PATCH v2 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

It leads to the power consumption raises to 2.2W during s2idle, while
it consumes less than 1W during long idle if put SK hynix nvme to D3
and then enter s2idle.
>From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
APST feature to do the power management.
To leverage its APST feature during s2idle, we can't disable nvme
device while suspending, too.

BTW, prevent it from entering D3 will increase the power consumtion around
0.13W ~ 0.15W during short/long idle, and the power consumption during
s2idle becomes 0.77W.

Signed-off-by: AceLan Kao 
---
 drivers/pci/quirks.c| 1 +
 include/linux/pci_ids.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4700d24e5d55..b7e6492e8311 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1332,6 +1332,7 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AL, 
PCI_ANY_ID,
occur when mode detecting */
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SK_HYNIX, 0x1527, quirk_no_ata_d3);
 
 /*
  * This was originally an Alpha-specific thing, but it really fits here.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..5f5adda07de0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3090,4 +3090,6 @@
 
 #define PCI_VENDOR_ID_NCUBE0x10ff
 
+#define PCI_VENDOR_ID_SK_HYNIX 0x1c5c
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
2.17.1

Re: [PATCH 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Right, should replace it with SK_HYNIX.
I'll prepare v2 patch for that.
Sinan Kaya  於 2018年11月6日 週二 上午10:24寫道：
>
> On 11/5/2018 6:04 PM, AceLan Kao wrote:
> > + { PCI_DEVICE(0x1c5c, 0x1527),   /* Sk Hynix */
> > + .driver_data = NVME_QUIRK_NO_DISABLE, },
>
> Now that you added PCI_VENDOR_ID_SK_HYNIX to pci_ids.h, you
> could use PCI_VENDOR_ID_SK_HYNIX above instead of 0x1c5c.

Re: [PATCH 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Right, should replace it with SK_HYNIX.
I'll prepare v2 patch for that.
Sinan Kaya  於 2018年11月6日 週二 上午10:24寫道：
>
> On 11/5/2018 6:04 PM, AceLan Kao wrote:
> > + { PCI_DEVICE(0x1c5c, 0x1527),   /* Sk Hynix */
> > + .driver_data = NVME_QUIRK_NO_DISABLE, },
>
> Now that you added PCI_VENDOR_ID_SK_HYNIX to pci_ids.h, you
> could use PCI_VENDOR_ID_SK_HYNIX above instead of 0x1c5c.

[PATCH 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

It leads to the power consumption raises to 2.2W during s2idle, while
it consumes less than 1W during long idle if put SK hynix nvme to D3
and then enter s2idle.
>From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
APST feature to do the power management.
To leverage its APST feature during s2idle, we can't disable nvme
device while suspending, too.

BTW, prevent it from entering D3 will increase the power consumtion around
0.13W ~ 0.15W during short/long idle, and the power consumption during
s2idle becomes 0.77W.

Signed-off-by: AceLan Kao 
---
 drivers/pci/quirks.c| 1 +
 include/linux/pci_ids.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4700d24e5d55..b7e6492e8311 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1332,6 +1332,7 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AL, 
PCI_ANY_ID,
occur when mode detecting */
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SK_HYNIX, 0x1527, quirk_no_ata_d3);
 
 /*
  * This was originally an Alpha-specific thing, but it really fits here.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..5f5adda07de0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3090,4 +3090,6 @@
 
 #define PCI_VENDOR_ID_NCUBE0x10ff
 
+#define PCI_VENDOR_ID_SK_HYNIX 0x1c5c
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
2.17.1

[PATCH 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Call nvme_dev_disable() function leads to the power consumption goes
up to 2.2 Watt during suspend-to-idle, and from SK hynix FE, they
suggest us to use its own APST feature to do the power management during
s2idle.
After D3 is diabled and nvme_dev_disable() is not called while
suspending, the power consumption drops to 0.77 Watt during s2idle.

Signed-off-by: AceLan Kao 
---
 drivers/nvme/host/nvme.h | 5 +
 drivers/nvme/host/pci.c  | 8 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cee79cb388af..35d260a4cf46 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -90,6 +90,11 @@ enum nvme_quirks {
 * Set MEDIUM priority on SQ creation
 */
NVME_QUIRK_MEDIUM_PRIO_SQ   = (1 << 7),
+
+   /*
+* Do not disable nvme when suspending(s2idle)
+*/
+   NVME_QUIRK_NO_DISABLE   = (1 << 8),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c33bb201b884..586f3516e36b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "nvme.h"
 
@@ -2612,8 +2613,11 @@ static int nvme_suspend(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct nvme_dev *ndev = pci_get_drvdata(pdev);
+   struct nvme_ctrl *ctrl = >ctrl;
+
+   if (!(pm_suspend_via_s2idle() && (ctrl->quirks & 
NVME_QUIRK_NO_DISABLE)))
+   nvme_dev_disable(ndev, true);
 
-   nvme_dev_disable(ndev, true);
return 0;
 }
 
@@ -2716,6 +2720,8 @@ static const struct pci_device_id nvme_id_table[] = {
.driver_data = NVME_QUIRK_LIGHTNVM, },
{ PCI_DEVICE(0x1d1d, 0x2601),   /* CNEX Granby */
.driver_data = NVME_QUIRK_LIGHTNVM, },
+   { PCI_DEVICE(0x1c5c, 0x1527),   /* Sk Hynix */
+   .driver_data = NVME_QUIRK_NO_DISABLE, },
{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xff) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1

[PATCH 1/2] pci: prevent sk hynix nvme from entering D3

[AceLan Kao]

It leads to the power consumption raises to 2.2W during s2idle, while
it consumes less than 1W during long idle if put SK hynix nvme to D3
and then enter s2idle.
>From SK hynix FE, MS Windows doesn't put nvme to D3, and uses its own
APST feature to do the power management.
To leverage its APST feature during s2idle, we can't disable nvme
device while suspending, too.

BTW, prevent it from entering D3 will increase the power consumtion around
0.13W ~ 0.15W during short/long idle, and the power consumption during
s2idle becomes 0.77W.

Signed-off-by: AceLan Kao 
---
 drivers/pci/quirks.c| 1 +
 include/linux/pci_ids.h | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 4700d24e5d55..b7e6492e8311 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -1332,6 +1332,7 @@ DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_AL, 
PCI_ANY_ID,
occur when mode detecting */
 DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_VIA, PCI_ANY_ID,
PCI_CLASS_STORAGE_IDE, 8, quirk_no_ata_d3);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SK_HYNIX, 0x1527, quirk_no_ata_d3);
 
 /*
  * This was originally an Alpha-specific thing, but it really fits here.
diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 69f0abe1ba1a..5f5adda07de0 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -3090,4 +3090,6 @@
 
 #define PCI_VENDOR_ID_NCUBE0x10ff
 
+#define PCI_VENDOR_ID_SK_HYNIX 0x1c5c
+
 #endif /* _LINUX_PCI_IDS_H */
-- 
2.17.1

[PATCH 2/2] nvme: add quirk to not call disable function when suspending

[AceLan Kao]

Call nvme_dev_disable() function leads to the power consumption goes
up to 2.2 Watt during suspend-to-idle, and from SK hynix FE, they
suggest us to use its own APST feature to do the power management during
s2idle.
After D3 is diabled and nvme_dev_disable() is not called while
suspending, the power consumption drops to 0.77 Watt during s2idle.

Signed-off-by: AceLan Kao 
---
 drivers/nvme/host/nvme.h | 5 +
 drivers/nvme/host/pci.c  | 8 +++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index cee79cb388af..35d260a4cf46 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -90,6 +90,11 @@ enum nvme_quirks {
 * Set MEDIUM priority on SQ creation
 */
NVME_QUIRK_MEDIUM_PRIO_SQ   = (1 << 7),
+
+   /*
+* Do not disable nvme when suspending(s2idle)
+*/
+   NVME_QUIRK_NO_DISABLE   = (1 << 8),
 };
 
 /*
diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index c33bb201b884..586f3516e36b 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -31,6 +31,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "nvme.h"
 
@@ -2612,8 +2613,11 @@ static int nvme_suspend(struct device *dev)
 {
struct pci_dev *pdev = to_pci_dev(dev);
struct nvme_dev *ndev = pci_get_drvdata(pdev);
+   struct nvme_ctrl *ctrl = >ctrl;
+
+   if (!(pm_suspend_via_s2idle() && (ctrl->quirks & 
NVME_QUIRK_NO_DISABLE)))
+   nvme_dev_disable(ndev, true);
 
-   nvme_dev_disable(ndev, true);
return 0;
 }
 
@@ -2716,6 +2720,8 @@ static const struct pci_device_id nvme_id_table[] = {
.driver_data = NVME_QUIRK_LIGHTNVM, },
{ PCI_DEVICE(0x1d1d, 0x2601),   /* CNEX Granby */
.driver_data = NVME_QUIRK_LIGHTNVM, },
+   { PCI_DEVICE(0x1c5c, 0x1527),   /* Sk Hynix */
+   .driver_data = NVME_QUIRK_NO_DISABLE, },
{ PCI_DEVICE_CLASS(PCI_CLASS_STORAGE_EXPRESS, 0xff) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2001) },
{ PCI_DEVICE(PCI_VENDOR_ID_APPLE, 0x2003) },
-- 
2.17.1

[PATCH] usb: core: disable USB2 LPM when suspending

[AceLan Kao]

We found a S5 current leakage issue on Dell DW1820 WiFi/BT combo card
which uses Qualcomm QCA6174 SoC. It also comes with WiFi and BT failure
when encountered current leakage issue.
   1. Power on, both WiFi and BT work.
   2. Power off and found a current leakage issue(consumes ~0.5W)
   3. Power on, no WiFi and BT devices can be found in lspci and lsusb.
   4. Power off, there is no current leakage issue at S5.
   5. continue to 1.

>From Qualcomm's report:
Based on the USB sniffer log, the difference between Linux and Windows
is USB LPM setting(no LPM transaction on Windows) which may leads to
the voltage leakage on Linux S5 state.

After checked the LPM related code and found, when system is going to
enter S5, it resumes the USB devices from runtime suspend and enables
USB2 LPM, and then it calls usb_dev_poweroff() -> usb_suspend(), and
leave USB2 LPM stays enabled.

Disable USB2 LPM in usb_suspend() fixes the issue mentioned above,
and try 30 times of s2idle, S3 and S5, the USB devices keep working
well. Disable USB2 LPM seems do no harm to the system.

Signed-off-by: AceLan Kao 
---
 drivers/usb/core/driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index e76e95f62f76..ac5e60d7104f 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1463,6 +1463,9 @@ int usb_suspend(struct device *dev, pm_message_t msg)
struct usb_device   *udev = to_usb_device(dev);
int r;
 
+   if (udev->usb2_hw_lpm_enabled == 1)
+   usb_set_usb2_hardware_lpm(udev, 0);
+
unbind_no_pm_drivers_interfaces(udev);
 
/* From now on we are sure all drivers support suspend/resume
-- 
2.17.1

[PATCH] usb: core: disable USB2 LPM when suspending

[AceLan Kao]

We found a S5 current leakage issue on Dell DW1820 WiFi/BT combo card
which uses Qualcomm QCA6174 SoC. It also comes with WiFi and BT failure
when encountered current leakage issue.
   1. Power on, both WiFi and BT work.
   2. Power off and found a current leakage issue(consumes ~0.5W)
   3. Power on, no WiFi and BT devices can be found in lspci and lsusb.
   4. Power off, there is no current leakage issue at S5.
   5. continue to 1.

>From Qualcomm's report:
Based on the USB sniffer log, the difference between Linux and Windows
is USB LPM setting(no LPM transaction on Windows) which may leads to
the voltage leakage on Linux S5 state.

After checked the LPM related code and found, when system is going to
enter S5, it resumes the USB devices from runtime suspend and enables
USB2 LPM, and then it calls usb_dev_poweroff() -> usb_suspend(), and
leave USB2 LPM stays enabled.

Disable USB2 LPM in usb_suspend() fixes the issue mentioned above,
and try 30 times of s2idle, S3 and S5, the USB devices keep working
well. Disable USB2 LPM seems do no harm to the system.

Signed-off-by: AceLan Kao 
---
 drivers/usb/core/driver.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c
index e76e95f62f76..ac5e60d7104f 100644
--- a/drivers/usb/core/driver.c
+++ b/drivers/usb/core/driver.c
@@ -1463,6 +1463,9 @@ int usb_suspend(struct device *dev, pm_message_t msg)
struct usb_device   *udev = to_usb_device(dev);
int r;
 
+   if (udev->usb2_hw_lpm_enabled == 1)
+   usb_set_usb2_hardware_lpm(udev, 0);
+
unbind_no_pm_drivers_interfaces(udev);
 
/* From now on we are sure all drivers support suspend/resume
-- 
2.17.1

[PATCH] HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd touchscreen

[AceLan Kao]

The incomplete report flooded after S3 and touchscreen becomes
malfunctioned.
[ 1367.646244] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/18785)
[ 1367.649471] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/28743)
[ 1367.651092] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/26757)
[ 1367.652658] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/52280)
[ 1367.654287] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/56059)

Adding device ID, 04F3:30CC, to the quirk to re-send report description
after resume.

Cc: sta...@vger.kernel.org
Signed-off-by: AceLan Kao 
---
 drivers/hid/hid-ids.h | 1 +
 drivers/hid/i2c-hid/i2c-hid.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c7981ddd8776..e80bcd71fe1e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -528,6 +528,7 @@
 
 #define I2C_VENDOR_ID_RAYD 0x2386
 #define I2C_PRODUCT_ID_RAYD_3118   0x3118
+#define I2C_PRODUCT_ID_RAYD_4B33   0x4B33
 
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index eae0cb3ddec6..5fd1159fc095 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -174,6 +174,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
I2C_HID_QUIRK_RESEND_REPORT_DESCR },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
-- 
2.17.1

[PATCH] HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd touchscreen

[AceLan Kao]

The incomplete report flooded after S3 and touchscreen becomes
malfunctioned.
[ 1367.646244] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/18785)
[ 1367.649471] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/28743)
[ 1367.651092] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/26757)
[ 1367.652658] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/52280)
[ 1367.654287] i2c_hid i2c-CUST:00: i2c_hid_get_input: incomplete report 
(58/56059)

Adding device ID, 04F3:30CC, to the quirk to re-send report description
after resume.

Cc: sta...@vger.kernel.org
Signed-off-by: AceLan Kao 
---
 drivers/hid/hid-ids.h | 1 +
 drivers/hid/i2c-hid/i2c-hid.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c7981ddd8776..e80bcd71fe1e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -528,6 +528,7 @@
 
 #define I2C_VENDOR_ID_RAYD 0x2386
 #define I2C_PRODUCT_ID_RAYD_3118   0x3118
+#define I2C_PRODUCT_ID_RAYD_4B33   0x4B33
 
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index eae0cb3ddec6..5fd1159fc095 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -174,6 +174,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
I2C_HID_QUIRK_RESEND_REPORT_DESCR },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
-- 
2.17.1

Re: [PATCH] PCI: Fix calculation of bridge window's size

[AceLan Kao]

Hi Bjorn,

Here is the public bug.
https://bugzilla.kernel.org/show_bug.cgi?id=200769

Best regards,
AceLan Kao.

2018-08-09 2:00 GMT+08:00 Bjorn Helgaas :
> On Wed, Aug 08, 2018 at 01:48:37PM +0800, AceLan Kao wrote:
>> There are some 0 resource size pci devices, and it leads to the
>> accumulator fails to maintain the correct value.
>> It results in a strange issue on my machine that xhci_hcd failed to init.
>>[2.437278] xhci_hcd :05:00.0: init :05:00.0 fail, -16
>>[2.437300] xhci_hcd: probe of :05:00.0 failed with error -16
>
> I don't think it's possible for a PCI device to have a BAR of zero
> size.  Bits 0-3 of memory BARs are read-only, leading to a minimum BAR
> size of 16 bytes; similarly, bits 0-1 of I/O BARs are read-only,
> giving a minimum size of 4 ports.
>
> My guess is that if you have a pci_dev with a resource that looks like
> it's valid but has zero size, that resource came from OF or device
> tree.  If that's the case, I'd say we should fix the OF or DT parsing
> code to reject those resources.  If we do that, we shouldn't need the
> patch below.
>
> Can you open a bugzilla.kernel.org report and attach the complete
> dmesg log leading up to this, as well as the "sudo lspci -vv" output
> for this device?  It should have clues about how we got here.
>
>> To fix this, check if the resource size equals to 0, doesn't increase size.
>>
>> Fixes: c9c75143a596 ("PCI: Fix calculation of bridge window's size and 
>> alignment")
>> CC: sta...@vger.kernel.org # 4.14+
>>
>> Signed-off-by: AceLan Kao 
>> ---
>>  drivers/pci/setup-bus.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 79b1824e83b4..ae05dde8c6e3 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -1061,7 +1061,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned 
>> long mask,
>>   r->flags = 0;
>>   continue;
>>   }
>> - size += max(r_size, align);
>> + if (r_size != 0)
>> + size += max(r_size, align);
>>   /* Exclude ranges with size > align from
>>  calculation of the alignment. */
>>   if (r_size <= align)
>> --
>> 2.17.1
>>

Re: [PATCH] PCI: Fix calculation of bridge window's size

[AceLan Kao]

Hi Bjorn,

Here is the public bug.
https://bugzilla.kernel.org/show_bug.cgi?id=200769

Best regards,
AceLan Kao.

2018-08-09 2:00 GMT+08:00 Bjorn Helgaas :
> On Wed, Aug 08, 2018 at 01:48:37PM +0800, AceLan Kao wrote:
>> There are some 0 resource size pci devices, and it leads to the
>> accumulator fails to maintain the correct value.
>> It results in a strange issue on my machine that xhci_hcd failed to init.
>>[2.437278] xhci_hcd :05:00.0: init :05:00.0 fail, -16
>>[2.437300] xhci_hcd: probe of :05:00.0 failed with error -16
>
> I don't think it's possible for a PCI device to have a BAR of zero
> size.  Bits 0-3 of memory BARs are read-only, leading to a minimum BAR
> size of 16 bytes; similarly, bits 0-1 of I/O BARs are read-only,
> giving a minimum size of 4 ports.
>
> My guess is that if you have a pci_dev with a resource that looks like
> it's valid but has zero size, that resource came from OF or device
> tree.  If that's the case, I'd say we should fix the OF or DT parsing
> code to reject those resources.  If we do that, we shouldn't need the
> patch below.
>
> Can you open a bugzilla.kernel.org report and attach the complete
> dmesg log leading up to this, as well as the "sudo lspci -vv" output
> for this device?  It should have clues about how we got here.
>
>> To fix this, check if the resource size equals to 0, doesn't increase size.
>>
>> Fixes: c9c75143a596 ("PCI: Fix calculation of bridge window's size and 
>> alignment")
>> CC: sta...@vger.kernel.org # 4.14+
>>
>> Signed-off-by: AceLan Kao 
>> ---
>>  drivers/pci/setup-bus.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 79b1824e83b4..ae05dde8c6e3 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -1061,7 +1061,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned 
>> long mask,
>>   r->flags = 0;
>>   continue;
>>   }
>> - size += max(r_size, align);
>> + if (r_size != 0)
>> + size += max(r_size, align);
>>   /* Exclude ranges with size > align from
>>  calculation of the alignment. */
>>   if (r_size <= align)
>> --
>> 2.17.1
>>

[PATCH] PCI: Fix calculation of bridge window's size

[AceLan Kao]

There are some 0 resource size pci devices, and it leads to the
accumulator fails to maintain the correct value.
It results in a strange issue on my machine that xhci_hcd failed to init.
   [2.437278] xhci_hcd :05:00.0: init :05:00.0 fail, -16
   [2.437300] xhci_hcd: probe of :05:00.0 failed with error -16

To fix this, check if the resource size equals to 0, doesn't increase size.

Fixes: c9c75143a596 ("PCI: Fix calculation of bridge window's size and 
alignment")
CC: sta...@vger.kernel.org # 4.14+

Signed-off-by: AceLan Kao 
---
 drivers/pci/setup-bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 79b1824e83b4..ae05dde8c6e3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1061,7 +1061,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned 
long mask,
r->flags = 0;
continue;
}
-   size += max(r_size, align);
+   if (r_size != 0)
+   size += max(r_size, align);
/* Exclude ranges with size > align from
   calculation of the alignment. */
if (r_size <= align)
-- 
2.17.1

[PATCH] PCI: Fix calculation of bridge window's size

[AceLan Kao]

There are some 0 resource size pci devices, and it leads to the
accumulator fails to maintain the correct value.
It results in a strange issue on my machine that xhci_hcd failed to init.
   [2.437278] xhci_hcd :05:00.0: init :05:00.0 fail, -16
   [2.437300] xhci_hcd: probe of :05:00.0 failed with error -16

To fix this, check if the resource size equals to 0, doesn't increase size.

Fixes: c9c75143a596 ("PCI: Fix calculation of bridge window's size and 
alignment")
CC: sta...@vger.kernel.org # 4.14+

Signed-off-by: AceLan Kao 
---
 drivers/pci/setup-bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 79b1824e83b4..ae05dde8c6e3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1061,7 +1061,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned 
long mask,
r->flags = 0;
continue;
}
-   size += max(r_size, align);
+   if (r_size != 0)
+   size += max(r_size, align);
/* Exclude ranges with size > align from
   calculation of the alignment. */
if (r_size <= align)
-- 
2.17.1

Re: [PATCH v2] platform/x86: intel-vbtn: Add support for dock mode detection

[AceLan Kao]

Looks good to me

Reviewed-By: AceLan Kao 

2018-06-25 8:29 GMT+08:00 Matt Delco :
> The Dell laptop I have has an ACPI that sends 0xCB and 0xCC on entering
> tablet mode.  On exiting tablet mode it sends 0xCA and 0xCD. Based on:
>
> http://www.traby.de/medion/DSDT/dsdt.dsl
> https://gist.github.com/jprvita/5737de3cbb670e80973b7d4e51c38ab6
> https://osdn.net/projects/android-x86/scm/git/kernel/commits/
> 7cbe5a330687b851f32dd9f1048a6ce182d0ff44
>
> It appears that 0xCA and 0xCB are about dock mode, which for my
> convertible laptop seems questionably tied to whether I've put
> the laptop in tablet or laptop mode.  I previously proposed no-oping
> 0xCA and 0xCB but this revised change attempts to add support for
> detecting dock mode--this detection will essentially be broken for
> my laptop (the main workaround would be for 0xCA and 0xCB to be used
> to provoke a query of the VGBS method that reports the current dock &
> tablet mode [which is accurately reported on my laptop but based on
> the prior workarounds in the driver it apparently can't be trusted
> for all systems]).
>
> Signed-off-by: Matt Delco 
> Cc: Darren Hart 
> Cc: Andy Shevchenko 
> Cc: platform-driver-...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Dmitry Torokhov 
> ---
>  drivers/platform/x86/intel-vbtn.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/platform/x86/intel-vbtn.c 
> b/drivers/platform/x86/intel-vbtn.c
> index c13780b8dabb..06cd7e818ed5 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -17,6 +17,7 @@
>
>  /* When NOT in tablet mode, VGBS returns with the flag 0x40 */
>  #define TABLET_MODE_FLAG 0x40
> +#define DOCK_MODE_FLAG   0x80
>
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("AceLan Kao");
> @@ -38,6 +39,8 @@ static const struct key_entry intel_vbtn_keymap[] = {
> { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
> release */
> { KE_KEY,0xC8, { KEY_ROTATE_LOCK_TOGGLE } },/* 
> rotate-lock key press */
> { KE_KEY,0xC9, { KEY_ROTATE_LOCK_TOGGLE } },/* 
> rotate-lock key release */
> +   { KE_SW, 0xCA, { .sw = { SW_DOCK, 1 } } },  /* Docked */
> +   { KE_SW, 0xCB, { .sw = { SW_DOCK, 0 } } },  /* Undocked */
> { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet */
> { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop */
> { KE_END },
> @@ -121,6 +124,8 @@ static void detect_tablet_mode(struct platform_device 
> *device)
>
> m = !(obj->integer.value & TABLET_MODE_FLAG);
> input_report_switch(priv->input_dev, SW_TABLET_MODE, m);
> +   m = (obj->integer.value & DOCK_MODE_FLAG) ? 1 : 0;
> +   input_report_switch(priv->input_dev, SW_DOCK, m);
>  out:
> kfree(vgbs_output.pointer);
>  }
> --
> 2.18.0.rc2.346.g013aa6912e-goog
>

Re: [PATCH v2] platform/x86: intel-vbtn: Add support for dock mode detection

[AceLan Kao]

Looks good to me

Reviewed-By: AceLan Kao 

2018-06-25 8:29 GMT+08:00 Matt Delco :
> The Dell laptop I have has an ACPI that sends 0xCB and 0xCC on entering
> tablet mode.  On exiting tablet mode it sends 0xCA and 0xCD. Based on:
>
> http://www.traby.de/medion/DSDT/dsdt.dsl
> https://gist.github.com/jprvita/5737de3cbb670e80973b7d4e51c38ab6
> https://osdn.net/projects/android-x86/scm/git/kernel/commits/
> 7cbe5a330687b851f32dd9f1048a6ce182d0ff44
>
> It appears that 0xCA and 0xCB are about dock mode, which for my
> convertible laptop seems questionably tied to whether I've put
> the laptop in tablet or laptop mode.  I previously proposed no-oping
> 0xCA and 0xCB but this revised change attempts to add support for
> detecting dock mode--this detection will essentially be broken for
> my laptop (the main workaround would be for 0xCA and 0xCB to be used
> to provoke a query of the VGBS method that reports the current dock &
> tablet mode [which is accurately reported on my laptop but based on
> the prior workarounds in the driver it apparently can't be trusted
> for all systems]).
>
> Signed-off-by: Matt Delco 
> Cc: Darren Hart 
> Cc: Andy Shevchenko 
> Cc: platform-driver-...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Dmitry Torokhov 
> ---
>  drivers/platform/x86/intel-vbtn.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/platform/x86/intel-vbtn.c 
> b/drivers/platform/x86/intel-vbtn.c
> index c13780b8dabb..06cd7e818ed5 100644
> --- a/drivers/platform/x86/intel-vbtn.c
> +++ b/drivers/platform/x86/intel-vbtn.c
> @@ -17,6 +17,7 @@
>
>  /* When NOT in tablet mode, VGBS returns with the flag 0x40 */
>  #define TABLET_MODE_FLAG 0x40
> +#define DOCK_MODE_FLAG   0x80
>
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("AceLan Kao");
> @@ -38,6 +39,8 @@ static const struct key_entry intel_vbtn_keymap[] = {
> { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
> release */
> { KE_KEY,0xC8, { KEY_ROTATE_LOCK_TOGGLE } },/* 
> rotate-lock key press */
> { KE_KEY,0xC9, { KEY_ROTATE_LOCK_TOGGLE } },/* 
> rotate-lock key release */
> +   { KE_SW, 0xCA, { .sw = { SW_DOCK, 1 } } },  /* Docked */
> +   { KE_SW, 0xCB, { .sw = { SW_DOCK, 0 } } },  /* Undocked */
> { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet */
> { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop */
> { KE_END },
> @@ -121,6 +124,8 @@ static void detect_tablet_mode(struct platform_device 
> *device)
>
> m = !(obj->integer.value & TABLET_MODE_FLAG);
> input_report_switch(priv->input_dev, SW_TABLET_MODE, m);
> +   m = (obj->integer.value & DOCK_MODE_FLAG) ? 1 : 0;
> +   input_report_switch(priv->input_dev, SW_DOCK, m);
>  out:
> kfree(vgbs_output.pointer);
>  }
> --
> 2.18.0.rc2.346.g013aa6912e-goog
>

Re: [PATCH] platform/x86: intel-vbtn: Ignore extraneous events from tablet mode

[AceLan Kao]

There is a v2 patch.
Please ignore this one.

2018-06-22 4:37 GMT+08:00 Dmitry Torokhov :
> On Thu, Jun 21, 2018 at 01:12:07PM -0700, Matt Delco wrote:
>> The Dell laptop I have has an ACPI that sends 0xCB and 0xCC on entering
>> tablet mode.  On exiting tablet mode it sends 0xCA and 0xCD.  This
>> change has the driver ignore 0xCA and 0xCB.
>>
>> Signed-off-by: Matt Delco 
>
> This looks good to me.
>
> Acked-by: Dmitry Torokhov 
>
>> ---
>>  drivers/platform/x86/intel-vbtn.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>> b/drivers/platform/x86/intel-vbtn.c
>> index c13780b8dabb..4c6f2bb1d1ac 100644
>> --- a/drivers/platform/x86/intel-vbtn.c
>> +++ b/drivers/platform/x86/intel-vbtn.c
>> @@ -38,8 +38,10 @@ static const struct key_entry intel_vbtn_keymap[] = {
>>   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>> release */
>>   { KE_KEY,0xC8, { KEY_ROTATE_LOCK_TOGGLE } },/* rotate-lock 
>> key press */
>>   { KE_KEY,0xC9, { KEY_ROTATE_LOCK_TOGGLE } },/* rotate-lock 
>> key release */
>> - { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet */
>> - { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop */
>> + { KE_IGNORE, 0xCA, { KEY_UNKNOWN } },   /* Laptop key 
>> press */
>> + { KE_IGNORE, 0xCB, { KEY_UNKNOWN } },   /* Tablet key 
>> press */
>> + { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet key 
>> release */
>> + { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop key 
>> release */
>>   { KE_END },
>>  };
>>
>> --
>> 2.18.0.rc2.346.g013aa6912e-goog
>>
>
> --
> Dmitry

Re: [PATCH] platform/x86: intel-vbtn: Ignore extraneous events from tablet mode

[AceLan Kao]

There is a v2 patch.
Please ignore this one.

2018-06-22 4:37 GMT+08:00 Dmitry Torokhov :
> On Thu, Jun 21, 2018 at 01:12:07PM -0700, Matt Delco wrote:
>> The Dell laptop I have has an ACPI that sends 0xCB and 0xCC on entering
>> tablet mode.  On exiting tablet mode it sends 0xCA and 0xCD.  This
>> change has the driver ignore 0xCA and 0xCB.
>>
>> Signed-off-by: Matt Delco 
>
> This looks good to me.
>
> Acked-by: Dmitry Torokhov 
>
>> ---
>>  drivers/platform/x86/intel-vbtn.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>> b/drivers/platform/x86/intel-vbtn.c
>> index c13780b8dabb..4c6f2bb1d1ac 100644
>> --- a/drivers/platform/x86/intel-vbtn.c
>> +++ b/drivers/platform/x86/intel-vbtn.c
>> @@ -38,8 +38,10 @@ static const struct key_entry intel_vbtn_keymap[] = {
>>   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>> release */
>>   { KE_KEY,0xC8, { KEY_ROTATE_LOCK_TOGGLE } },/* rotate-lock 
>> key press */
>>   { KE_KEY,0xC9, { KEY_ROTATE_LOCK_TOGGLE } },/* rotate-lock 
>> key release */
>> - { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet */
>> - { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop */
>> + { KE_IGNORE, 0xCA, { KEY_UNKNOWN } },   /* Laptop key 
>> press */
>> + { KE_IGNORE, 0xCB, { KEY_UNKNOWN } },   /* Tablet key 
>> press */
>> + { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } },   /* Tablet key 
>> release */
>> + { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } },   /* Laptop key 
>> release */
>>   { KE_END },
>>  };
>>
>> --
>> 2.18.0.rc2.346.g013aa6912e-goog
>>
>
> --
> Dmitry

Re: [PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

Hi all,

Just inform you a news reported by a user who confirmed the wake up
issue can't be reproduce by the new kernel.
https://bugzilla.kernel.org/show_bug.cgi?id=61651#c126


Guillaume de Jabrun 2018-05-27 15:12:54 UTC

I am using this patch for a long time. I was experiencing the "wake up
twice" bug, but with recent kernel version, I don't have this issue
anymore.
I can't tell which version actually fix the bug (I don't remember..).


Best regards,
AceLan Kao.

2018-05-21 11:18 GMT+08:00 David Miller <da...@davemloft.net>:
> From: AceLan Kao <acelan@canonical.com>
> Date: Mon, 21 May 2018 11:14:00 +0800
>
>> We are willing to fix the issue, but we don't have a machine to
>> reproduce it, and the WoL feature has been removed 5 years ago, it's
>> hard to find those buggy machines.
>
> Have you bothered to ask the person who did the revert?
>
>> WoL is a feature that is only used by a very small group of people,
>> and the wake up issue looks like only happens on some
>> platforms. Which means only small part of the group of people are
>> affected.
>
> One of those people was the wireless networking stack maintainer.
>
>> So, it's not a serious issue worth to remove it from alx driver.
>
> I disagree.
>
> You must fix the regression solved by the revert, before adding
> WoL support back to the driver.
>
> I'm not going to say this again.
>
> Thank you.
>

Re: [PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

Hi all,

Just inform you a news reported by a user who confirmed the wake up
issue can't be reproduce by the new kernel.
https://bugzilla.kernel.org/show_bug.cgi?id=61651#c126


Guillaume de Jabrun 2018-05-27 15:12:54 UTC

I am using this patch for a long time. I was experiencing the "wake up
twice" bug, but with recent kernel version, I don't have this issue
anymore.
I can't tell which version actually fix the bug (I don't remember..).


Best regards,
AceLan Kao.

2018-05-21 11:18 GMT+08:00 David Miller :
> From: AceLan Kao 
> Date: Mon, 21 May 2018 11:14:00 +0800
>
>> We are willing to fix the issue, but we don't have a machine to
>> reproduce it, and the WoL feature has been removed 5 years ago, it's
>> hard to find those buggy machines.
>
> Have you bothered to ask the person who did the revert?
>
>> WoL is a feature that is only used by a very small group of people,
>> and the wake up issue looks like only happens on some
>> platforms. Which means only small part of the group of people are
>> affected.
>
> One of those people was the wireless networking stack maintainer.
>
>> So, it's not a serious issue worth to remove it from alx driver.
>
> I disagree.
>
> You must fix the regression solved by the revert, before adding
> WoL support back to the driver.
>
> I'm not going to say this again.
>
> Thank you.
>

Re: [PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

Hi David,

We are willing to fix the issue, but we don't have a machine to reproduce it,
and the WoL feature has been removed 5 years ago, it's hard to find
those buggy machines.

WoL is a feature that is only used by a very small group of people,
and the wake up issue
looks like only happens on some platforms. Which means only small part
of the group of people are affected.
So, it's not a serious issue worth to remove it from alx driver.

As the commit describes, WoL is required to pass E-Start 6.1, and
taking secure boot into account,
we can't keep distributing "alx driver with WoL" dkms package, so we
really need this feature to be built in the kernel.

There are some solutions to fix it.
1. Add WoL feature back, and we will try our best to fix the wake up
issue if we encounter it or users report it.
2. Add WoL feature back and add an driver option to disable it by
default, so that it won't create any regression and user can enable it
by kernel cmdline.
3. Add WoL feature back and create a white list in the driver, we'll
add those platforms we tested to the list.
4. or create a blacklist to list machines which are reported buggy.
Could you let me know which solution is more feasible for you?
Thanks.

Best regards,
AceLan Kao.

2018-05-14 21:35 GMT+08:00 David Miller <da...@davemloft.net>:
> From: AceLan Kao <acelan@canonical.com>
> Date: Mon, 14 May 2018 11:28:39 +0800
>
>> This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.
>>
>> The WoL feature is a must to pass Energy Star 6.1 and above,
>> the power consumption will be measured during S3 with WoL is enabled.
>>
>> Reverting "alx: remove WoL support", and will try to fix the unintentional
>> wake up issue when WoL is enabled.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
>>
>> Signed-off-by: AceLan Kao <acelan@canonical.com>
>
> First, we must fix the problem that caused WoL to be disabled.
>
> Then, and only then, can you re-enable it.
>
> Thank you.

Re: [PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

Hi David,

We are willing to fix the issue, but we don't have a machine to reproduce it,
and the WoL feature has been removed 5 years ago, it's hard to find
those buggy machines.

WoL is a feature that is only used by a very small group of people,
and the wake up issue
looks like only happens on some platforms. Which means only small part
of the group of people are affected.
So, it's not a serious issue worth to remove it from alx driver.

As the commit describes, WoL is required to pass E-Start 6.1, and
taking secure boot into account,
we can't keep distributing "alx driver with WoL" dkms package, so we
really need this feature to be built in the kernel.

There are some solutions to fix it.
1. Add WoL feature back, and we will try our best to fix the wake up
issue if we encounter it or users report it.
2. Add WoL feature back and add an driver option to disable it by
default, so that it won't create any regression and user can enable it
by kernel cmdline.
3. Add WoL feature back and create a white list in the driver, we'll
add those platforms we tested to the list.
4. or create a blacklist to list machines which are reported buggy.
Could you let me know which solution is more feasible for you?
Thanks.

Best regards,
AceLan Kao.

2018-05-14 21:35 GMT+08:00 David Miller :
> From: AceLan Kao 
> Date: Mon, 14 May 2018 11:28:39 +0800
>
>> This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.
>>
>> The WoL feature is a must to pass Energy Star 6.1 and above,
>> the power consumption will be measured during S3 with WoL is enabled.
>>
>> Reverting "alx: remove WoL support", and will try to fix the unintentional
>> wake up issue when WoL is enabled.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
>>
>> Signed-off-by: AceLan Kao 
>
> First, we must fix the problem that caused WoL to be disabled.
>
> Then, and only then, can you re-enable it.
>
> Thank you.

[PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.

The WoL feature is a must to pass Energy Star 6.1 and above,
the power consumption will be measured during S3 with WoL is enabled.

Reverting "alx: remove WoL support", and will try to fix the unintentional
wake up issue when WoL is enabled.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  36 +
 drivers/net/ethernet/atheros/alx/hw.c  | 154 -
 drivers/net/ethernet/atheros/alx/hw.h  |   5 +
 drivers/net/ethernet/atheros/alx/main.c| 142 +--
 4 files changed, 326 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 2f4eabf652e8..859e27236ce4 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -310,11 +310,47 @@ static int alx_get_sset_count(struct net_device *netdev, 
int sset)
}
 }
 
+static void alx_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   wol->supported = WAKE_MAGIC | WAKE_PHY;
+   wol->wolopts = 0;
+
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
+   wol->wolopts |= WAKE_MAGIC;
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY)
+   wol->wolopts |= WAKE_PHY;
+}
+
+static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   return -EOPNOTSUPP;
+
+   hw->sleep_ctrl = 0;
+
+   if (wol->wolopts & WAKE_MAGIC)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_MAGIC;
+   if (wol->wolopts & WAKE_PHY)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_PHY;
+
+   device_set_wakeup_enable(>hw.pdev->dev, hw->sleep_ctrl);
+
+   return 0;
+}
+
 const struct ethtool_ops alx_ethtool_ops = {
.get_pauseparam = alx_get_pauseparam,
.set_pauseparam = alx_set_pauseparam,
.get_msglevel   = alx_get_msglevel,
.set_msglevel   = alx_set_msglevel,
+   .get_wol= alx_get_wol,
+   .set_wol= alx_set_wol,
.get_link   = ethtool_op_get_link,
.get_strings= alx_get_strings,
.get_sset_count = alx_get_sset_count,
diff --git a/drivers/net/ethernet/atheros/alx/hw.c 
b/drivers/net/ethernet/atheros/alx/hw.c
index 6ac40b0003a3..f9bf612550ab 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -332,6 +332,16 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
alx_write_mem32(hw, ALX_STAD1, val);
 }
 
+static void alx_enable_osc(struct alx_hw *hw)
+{
+   u32 val;
+
+   /* rising edge */
+   val = alx_read_mem32(hw, ALX_MISC);
+   alx_write_mem32(hw, ALX_MISC, val & ~ALX_MISC_INTNLOSC_OPEN);
+   alx_write_mem32(hw, ALX_MISC, val | ALX_MISC_INTNLOSC_OPEN);
+}
+
 static void alx_reset_osc(struct alx_hw *hw, u8 rev)
 {
u32 val, val2;
@@ -774,7 +784,6 @@ int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, 
u8 flowctrl)
return err;
 }
 
-
 void alx_post_phy_link(struct alx_hw *hw)
 {
u16 phy_val, len, agc;
@@ -848,6 +857,65 @@ void alx_post_phy_link(struct alx_hw *hw)
}
 }
 
+/* NOTE:
+ *1. phy link must be established before calling this function
+ *2. wol option (pattern,magic,link,etc.) is configed before call it.
+ */
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
+{
+   u32 master, mac, phy, val;
+   int err = 0;
+
+   master = alx_read_mem32(hw, ALX_MASTER);
+   master &= ~ALX_MASTER_PCLKSEL_SRDS;
+   mac = hw->rx_ctrl;
+   /* 10/100 half */
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,  ALX_MAC_CTRL_SPEED_10_100);
+   mac &= ~(ALX_MAC_CTRL_FULLD | ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_TX_EN);
+
+   phy = alx_read_mem32(hw, ALX_PHY_CTRL);
+   phy &= ~(ALX_PHY_CTRL_DSPRST_OUT | ALX_PHY_CTRL_CLS);
+   phy |= ALX_PHY_CTRL_RST_ANALOG | ALX_PHY_CTRL_HIB_PULSE |
+  ALX_PHY_CTRL_HIB_EN;
+
+   /* without any activity  */
+   if (!(hw->sleep_ctrl & ALX_SLEEP_ACTIVE)) {
+   err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
+   if (err)
+   return err;
+   phy |= ALX_PHY_CTRL_IDDQ | ALX_PHY_CTRL_POWER_DOWN;
+   } else {
+   if (hw->sleep_ctrl & (ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_CIFS))
+   mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
+   if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
+   mac |= ALX_MAC_CTRL_TX_EN;
+ 

[PATCH v2] Revert "alx: remove WoL support"

[AceLan Kao]

This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.

The WoL feature is a must to pass Energy Star 6.1 and above,
the power consumption will be measured during S3 with WoL is enabled.

Reverting "alx: remove WoL support", and will try to fix the unintentional
wake up issue when WoL is enabled.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651

Signed-off-by: AceLan Kao 
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  36 +
 drivers/net/ethernet/atheros/alx/hw.c  | 154 -
 drivers/net/ethernet/atheros/alx/hw.h  |   5 +
 drivers/net/ethernet/atheros/alx/main.c| 142 +--
 4 files changed, 326 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 2f4eabf652e8..859e27236ce4 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -310,11 +310,47 @@ static int alx_get_sset_count(struct net_device *netdev, 
int sset)
}
 }
 
+static void alx_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   wol->supported = WAKE_MAGIC | WAKE_PHY;
+   wol->wolopts = 0;
+
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
+   wol->wolopts |= WAKE_MAGIC;
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY)
+   wol->wolopts |= WAKE_PHY;
+}
+
+static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   return -EOPNOTSUPP;
+
+   hw->sleep_ctrl = 0;
+
+   if (wol->wolopts & WAKE_MAGIC)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_MAGIC;
+   if (wol->wolopts & WAKE_PHY)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_PHY;
+
+   device_set_wakeup_enable(>hw.pdev->dev, hw->sleep_ctrl);
+
+   return 0;
+}
+
 const struct ethtool_ops alx_ethtool_ops = {
.get_pauseparam = alx_get_pauseparam,
.set_pauseparam = alx_set_pauseparam,
.get_msglevel   = alx_get_msglevel,
.set_msglevel   = alx_set_msglevel,
+   .get_wol= alx_get_wol,
+   .set_wol= alx_set_wol,
.get_link   = ethtool_op_get_link,
.get_strings= alx_get_strings,
.get_sset_count = alx_get_sset_count,
diff --git a/drivers/net/ethernet/atheros/alx/hw.c 
b/drivers/net/ethernet/atheros/alx/hw.c
index 6ac40b0003a3..f9bf612550ab 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -332,6 +332,16 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
alx_write_mem32(hw, ALX_STAD1, val);
 }
 
+static void alx_enable_osc(struct alx_hw *hw)
+{
+   u32 val;
+
+   /* rising edge */
+   val = alx_read_mem32(hw, ALX_MISC);
+   alx_write_mem32(hw, ALX_MISC, val & ~ALX_MISC_INTNLOSC_OPEN);
+   alx_write_mem32(hw, ALX_MISC, val | ALX_MISC_INTNLOSC_OPEN);
+}
+
 static void alx_reset_osc(struct alx_hw *hw, u8 rev)
 {
u32 val, val2;
@@ -774,7 +784,6 @@ int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, 
u8 flowctrl)
return err;
 }
 
-
 void alx_post_phy_link(struct alx_hw *hw)
 {
u16 phy_val, len, agc;
@@ -848,6 +857,65 @@ void alx_post_phy_link(struct alx_hw *hw)
}
 }
 
+/* NOTE:
+ *1. phy link must be established before calling this function
+ *2. wol option (pattern,magic,link,etc.) is configed before call it.
+ */
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
+{
+   u32 master, mac, phy, val;
+   int err = 0;
+
+   master = alx_read_mem32(hw, ALX_MASTER);
+   master &= ~ALX_MASTER_PCLKSEL_SRDS;
+   mac = hw->rx_ctrl;
+   /* 10/100 half */
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,  ALX_MAC_CTRL_SPEED_10_100);
+   mac &= ~(ALX_MAC_CTRL_FULLD | ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_TX_EN);
+
+   phy = alx_read_mem32(hw, ALX_PHY_CTRL);
+   phy &= ~(ALX_PHY_CTRL_DSPRST_OUT | ALX_PHY_CTRL_CLS);
+   phy |= ALX_PHY_CTRL_RST_ANALOG | ALX_PHY_CTRL_HIB_PULSE |
+  ALX_PHY_CTRL_HIB_EN;
+
+   /* without any activity  */
+   if (!(hw->sleep_ctrl & ALX_SLEEP_ACTIVE)) {
+   err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
+   if (err)
+   return err;
+   phy |= ALX_PHY_CTRL_IDDQ | ALX_PHY_CTRL_POWER_DOWN;
+   } else {
+   if (hw->sleep_ctrl & (ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_CIFS))
+   mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
+   if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
+   mac |= ALX_MAC_CTRL_TX_EN;
+   if (duplex == DUPLEX_FULL)
+   

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Okay, I'll submit a new patch with some more description of why we
need this feature.
Thanks.

2018-05-10 20:34 GMT+08:00 Andrew Lunn <and...@lunn.ch>:
> On Thu, May 10, 2018 at 01:58:24PM +0800, AceLan Kao wrote:
>> Hi Andrew,
>>
>> We have some machines using Qualcomm Atheros Killer E2400 Gigabit
>> Ethernet Controller,
>> but none of them has the unintentional wake up issue.
>> We're willing to fix it if we encountered the issue, but before we can
>> do it, we need this feature is supported by the driver.
>>
>> Taking the feature has been removed for 5 years into account, I doubt
>> if we still can reproduce this issue,
>> but again, to verify this issue we need to add back this feature first.
>> Set WoL disabled by default won't introduce any regression but give
>> users and developers a chance to fix it.
>
> The main problem here is the module parameter. That is not going to be
> accepted.
>
> Can you argue the cure is worse than the disease? Is WoL not working
> considered by a lot of people as being a bug? Double wake up is also a
> bug, but not many people care, it does not cause any data corruption,
> etc. So can you argue overall we have a less buggy system, but still
> buggy, if WoL is enabled?
>
> If you can write a convincing Change Message arguing the case, a patch
> simply re-enabling WoL might be accepted.
>
> But you also need to take on the responsibility to help debug the
> failed shutdowns in order to get to the bottom of this problem.
>
>Andrew

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Okay, I'll submit a new patch with some more description of why we
need this feature.
Thanks.

2018-05-10 20:34 GMT+08:00 Andrew Lunn :
> On Thu, May 10, 2018 at 01:58:24PM +0800, AceLan Kao wrote:
>> Hi Andrew,
>>
>> We have some machines using Qualcomm Atheros Killer E2400 Gigabit
>> Ethernet Controller,
>> but none of them has the unintentional wake up issue.
>> We're willing to fix it if we encountered the issue, but before we can
>> do it, we need this feature is supported by the driver.
>>
>> Taking the feature has been removed for 5 years into account, I doubt
>> if we still can reproduce this issue,
>> but again, to verify this issue we need to add back this feature first.
>> Set WoL disabled by default won't introduce any regression but give
>> users and developers a chance to fix it.
>
> The main problem here is the module parameter. That is not going to be
> accepted.
>
> Can you argue the cure is worse than the disease? Is WoL not working
> considered by a lot of people as being a bug? Double wake up is also a
> bug, but not many people care, it does not cause any data corruption,
> etc. So can you argue overall we have a less buggy system, but still
> buggy, if WoL is enabled?
>
> If you can write a convincing Change Message arguing the case, a patch
> simply re-enabling WoL might be accepted.
>
> But you also need to take on the responsibility to help debug the
> failed shutdowns in order to get to the bottom of this problem.
>
>Andrew

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi Andrew,

We have some machines using Qualcomm Atheros Killer E2400 Gigabit
Ethernet Controller,
but none of them has the unintentional wake up issue.
We're willing to fix it if we encountered the issue, but before we can
do it, we need this feature is supported by the driver.

Taking the feature has been removed for 5 years into account, I doubt
if we still can reproduce this issue,
but again, to verify this issue we need to add back this feature first.
Set WoL disabled by default won't introduce any regression but give
users and developers a chance to fix it.

Best regards,
AceLan Kao.

2018-05-09 21:45 GMT+08:00 Andrew Lunn <and...@lunn.ch>:
> On Wed, May 09, 2018 at 10:40:13AM +0800, AceLan Kao wrote:
>> Hi,
>>
>> I didn't get any response around one month.
>
> I didn't notice you posting an results of searching for the true
> problem? Please can you point me at an email archive.
>
>  Andrew

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi Andrew,

We have some machines using Qualcomm Atheros Killer E2400 Gigabit
Ethernet Controller,
but none of them has the unintentional wake up issue.
We're willing to fix it if we encountered the issue, but before we can
do it, we need this feature is supported by the driver.

Taking the feature has been removed for 5 years into account, I doubt
if we still can reproduce this issue,
but again, to verify this issue we need to add back this feature first.
Set WoL disabled by default won't introduce any regression but give
users and developers a chance to fix it.

Best regards,
AceLan Kao.

2018-05-09 21:45 GMT+08:00 Andrew Lunn :
> On Wed, May 09, 2018 at 10:40:13AM +0800, AceLan Kao wrote:
>> Hi,
>>
>> I didn't get any response around one month.
>
> I didn't notice you posting an results of searching for the true
> problem? Please can you point me at an email archive.
>
>  Andrew

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi,

I didn't get any response around one month.
I'm still here hoping you can consider accepting the WoL patch.

Without that patch, people have no chance to bump into the issue and
have no chance to fix it.
Moreover, it leads to the dkms package be spreaded around, and it'll
become a more annoying issue when UEFI secure boot is enabled[1].
Please re-consider it to enable WoL again and set it to disable by default,
so that we/user have a chance to examine the feature and have a chance
to find out a read fix for it.
Thanks.

1. https://bugzilla.kernel.org/show_bug.cgi?id=61651

Best regards,
AceLan Kao.

2018-04-24 11:45 GMT+08:00 AceLan Kao <acelan@canonical.com>:
> Hi,
>
> May I know the final decision of this patch?
> Thanks.
>
> Best regards,
> AceLan Kao.
>
> 2018-04-10 10:40 GMT+08:00 AceLan Kao <acelan@canonical.com>:
>> The problem is I don't have a machine with that wakeup issue, and I
>> need WoL feature.
>> Instead of spreading "alx with WoL" dkms package everywhere, I would
>> like to see it's supported in the driver and is disabled by default.
>>
>> Moreover, the wakeup issue may come from old Atheros chips, or result
>> from buggy BIOS.
>> With the WoL has been removed from the driver, no one will report
>> issue about that, and we don't have any chance to find a fix for it.
>>
>> Adding this feature back is not covering a paper on the issue, it
>> makes people have a chance to examine this feature.
>>
>> 2018-04-09 22:50 GMT+08:00 David Miller <da...@davemloft.net>:
>>> From: Andrew Lunn <and...@lunn.ch>
>>> Date: Mon, 9 Apr 2018 14:39:10 +0200
>>>
>>>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>>>> The WoL feature was reported broken and will lead to
>>>>> the system resume immediately after suspending.
>>>>> This symptom is not happening on every system, so adding
>>>>> disable_wol option and disable WoL by default to prevent the issue from
>>>>> happening again.
>>>>
>>>>>  const char alx_drv_name[] = "alx";
>>>>>
>>>>> +/* disable WoL by default */
>>>>> +bool disable_wol = 1;
>>>>> +module_param(disable_wol, bool, 0);
>>>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>>>> +
>>>>
>>>> Hi AceLan
>>>>
>>>> This seems like you are papering over the cracks. And module
>>>> parameters are not liked.
>>>>
>>>> Please try to find the real problem.
>>>
>>> Agreed.

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi,

I didn't get any response around one month.
I'm still here hoping you can consider accepting the WoL patch.

Without that patch, people have no chance to bump into the issue and
have no chance to fix it.
Moreover, it leads to the dkms package be spreaded around, and it'll
become a more annoying issue when UEFI secure boot is enabled[1].
Please re-consider it to enable WoL again and set it to disable by default,
so that we/user have a chance to examine the feature and have a chance
to find out a read fix for it.
Thanks.

1. https://bugzilla.kernel.org/show_bug.cgi?id=61651

Best regards,
AceLan Kao.

2018-04-24 11:45 GMT+08:00 AceLan Kao :
> Hi,
>
> May I know the final decision of this patch?
> Thanks.
>
> Best regards,
> AceLan Kao.
>
> 2018-04-10 10:40 GMT+08:00 AceLan Kao :
>> The problem is I don't have a machine with that wakeup issue, and I
>> need WoL feature.
>> Instead of spreading "alx with WoL" dkms package everywhere, I would
>> like to see it's supported in the driver and is disabled by default.
>>
>> Moreover, the wakeup issue may come from old Atheros chips, or result
>> from buggy BIOS.
>> With the WoL has been removed from the driver, no one will report
>> issue about that, and we don't have any chance to find a fix for it.
>>
>> Adding this feature back is not covering a paper on the issue, it
>> makes people have a chance to examine this feature.
>>
>> 2018-04-09 22:50 GMT+08:00 David Miller :
>>> From: Andrew Lunn 
>>> Date: Mon, 9 Apr 2018 14:39:10 +0200
>>>
>>>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>>>> The WoL feature was reported broken and will lead to
>>>>> the system resume immediately after suspending.
>>>>> This symptom is not happening on every system, so adding
>>>>> disable_wol option and disable WoL by default to prevent the issue from
>>>>> happening again.
>>>>
>>>>>  const char alx_drv_name[] = "alx";
>>>>>
>>>>> +/* disable WoL by default */
>>>>> +bool disable_wol = 1;
>>>>> +module_param(disable_wol, bool, 0);
>>>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>>>> +
>>>>
>>>> Hi AceLan
>>>>
>>>> This seems like you are papering over the cracks. And module
>>>> parameters are not liked.
>>>>
>>>> Please try to find the real problem.
>>>
>>> Agreed.

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi,

May I know the final decision of this patch?
Thanks.

Best regards,
AceLan Kao.

2018-04-10 10:40 GMT+08:00 AceLan Kao <acelan@canonical.com>:
> The problem is I don't have a machine with that wakeup issue, and I
> need WoL feature.
> Instead of spreading "alx with WoL" dkms package everywhere, I would
> like to see it's supported in the driver and is disabled by default.
>
> Moreover, the wakeup issue may come from old Atheros chips, or result
> from buggy BIOS.
> With the WoL has been removed from the driver, no one will report
> issue about that, and we don't have any chance to find a fix for it.
>
> Adding this feature back is not covering a paper on the issue, it
> makes people have a chance to examine this feature.
>
> 2018-04-09 22:50 GMT+08:00 David Miller <da...@davemloft.net>:
>> From: Andrew Lunn <and...@lunn.ch>
>> Date: Mon, 9 Apr 2018 14:39:10 +0200
>>
>>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>>> The WoL feature was reported broken and will lead to
>>>> the system resume immediately after suspending.
>>>> This symptom is not happening on every system, so adding
>>>> disable_wol option and disable WoL by default to prevent the issue from
>>>> happening again.
>>>
>>>>  const char alx_drv_name[] = "alx";
>>>>
>>>> +/* disable WoL by default */
>>>> +bool disable_wol = 1;
>>>> +module_param(disable_wol, bool, 0);
>>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>>> +
>>>
>>> Hi AceLan
>>>
>>> This seems like you are papering over the cracks. And module
>>> parameters are not liked.
>>>
>>> Please try to find the real problem.
>>
>> Agreed.

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

Hi,

May I know the final decision of this patch?
Thanks.

Best regards,
AceLan Kao.

2018-04-10 10:40 GMT+08:00 AceLan Kao :
> The problem is I don't have a machine with that wakeup issue, and I
> need WoL feature.
> Instead of spreading "alx with WoL" dkms package everywhere, I would
> like to see it's supported in the driver and is disabled by default.
>
> Moreover, the wakeup issue may come from old Atheros chips, or result
> from buggy BIOS.
> With the WoL has been removed from the driver, no one will report
> issue about that, and we don't have any chance to find a fix for it.
>
> Adding this feature back is not covering a paper on the issue, it
> makes people have a chance to examine this feature.
>
> 2018-04-09 22:50 GMT+08:00 David Miller :
>> From: Andrew Lunn 
>> Date: Mon, 9 Apr 2018 14:39:10 +0200
>>
>>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>>> The WoL feature was reported broken and will lead to
>>>> the system resume immediately after suspending.
>>>> This symptom is not happening on every system, so adding
>>>> disable_wol option and disable WoL by default to prevent the issue from
>>>> happening again.
>>>
>>>>  const char alx_drv_name[] = "alx";
>>>>
>>>> +/* disable WoL by default */
>>>> +bool disable_wol = 1;
>>>> +module_param(disable_wol, bool, 0);
>>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>>> +
>>>
>>> Hi AceLan
>>>
>>> This seems like you are papering over the cracks. And module
>>> parameters are not liked.
>>>
>>> Please try to find the real problem.
>>
>> Agreed.

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

The problem is I don't have a machine with that wakeup issue, and I
need WoL feature.
Instead of spreading "alx with WoL" dkms package everywhere, I would
like to see it's supported in the driver and is disabled by default.

Moreover, the wakeup issue may come from old Atheros chips, or result
from buggy BIOS.
With the WoL has been removed from the driver, no one will report
issue about that, and we don't have any chance to find a fix for it.

Adding this feature back is not covering a paper on the issue, it
makes people have a chance to examine this feature.

2018-04-09 22:50 GMT+08:00 David Miller <da...@davemloft.net>:
> From: Andrew Lunn <and...@lunn.ch>
> Date: Mon, 9 Apr 2018 14:39:10 +0200
>
>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>> The WoL feature was reported broken and will lead to
>>> the system resume immediately after suspending.
>>> This symptom is not happening on every system, so adding
>>> disable_wol option and disable WoL by default to prevent the issue from
>>> happening again.
>>
>>>  const char alx_drv_name[] = "alx";
>>>
>>> +/* disable WoL by default */
>>> +bool disable_wol = 1;
>>> +module_param(disable_wol, bool, 0);
>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>> +
>>
>> Hi AceLan
>>
>> This seems like you are papering over the cracks. And module
>> parameters are not liked.
>>
>> Please try to find the real problem.
>
> Agreed.

Re: [PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

The problem is I don't have a machine with that wakeup issue, and I
need WoL feature.
Instead of spreading "alx with WoL" dkms package everywhere, I would
like to see it's supported in the driver and is disabled by default.

Moreover, the wakeup issue may come from old Atheros chips, or result
from buggy BIOS.
With the WoL has been removed from the driver, no one will report
issue about that, and we don't have any chance to find a fix for it.

Adding this feature back is not covering a paper on the issue, it
makes people have a chance to examine this feature.

2018-04-09 22:50 GMT+08:00 David Miller :
> From: Andrew Lunn 
> Date: Mon, 9 Apr 2018 14:39:10 +0200
>
>> On Mon, Apr 09, 2018 at 07:35:14PM +0800, AceLan Kao wrote:
>>> The WoL feature was reported broken and will lead to
>>> the system resume immediately after suspending.
>>> This symptom is not happening on every system, so adding
>>> disable_wol option and disable WoL by default to prevent the issue from
>>> happening again.
>>
>>>  const char alx_drv_name[] = "alx";
>>>
>>> +/* disable WoL by default */
>>> +bool disable_wol = 1;
>>> +module_param(disable_wol, bool, 0);
>>> +MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
>>> +
>>
>> Hi AceLan
>>
>> This seems like you are papering over the cracks. And module
>> parameters are not liked.
>>
>> Please try to find the real problem.
>
> Agreed.

[PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

The WoL feature was reported broken and will lead to
the system resume immediately after suspending.
This symptom is not happening on every system, so adding
disable_wol option and disable WoL by default to prevent the issue from
happening again.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/ethernet/atheros/alx/ethtool.c | 7 ++-
 drivers/net/ethernet/atheros/alx/main.c| 5 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 859e272..e50129d 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -46,6 +46,8 @@
 #include "reg.h"
 #include "hw.h"
 
+extern const bool disable_wol;
+
 /* The order of these strings must match the order of the fields in
  * struct alx_hw_stats
  * See hw.h
@@ -315,6 +317,9 @@ static void alx_get_wol(struct net_device *netdev, struct 
ethtool_wolinfo *wol)
struct alx_priv *alx = netdev_priv(netdev);
struct alx_hw *hw = >hw;
 
+   if (disable_wol)
+   return;
+
wol->supported = WAKE_MAGIC | WAKE_PHY;
wol->wolopts = 0;
 
@@ -329,7 +334,7 @@ static int alx_set_wol(struct net_device *netdev, struct 
ethtool_wolinfo *wol)
struct alx_priv *alx = netdev_priv(netdev);
struct alx_hw *hw = >hw;
 
-   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   if (disable_wol || (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY)))
return -EOPNOTSUPP;
 
hw->sleep_ctrl = 0;
diff --git a/drivers/net/ethernet/atheros/alx/main.c 
b/drivers/net/ethernet/atheros/alx/main.c
index c0e2bb2..c27fdf7 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -51,6 +51,11 @@
 
 const char alx_drv_name[] = "alx";
 
+/* disable WoL by default */
+bool disable_wol = 1;
+module_param(disable_wol, bool, 0);
+MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
+
 static void alx_free_txbuf(struct alx_tx_queue *txq, int entry)
 {
struct alx_buffer *txb = >bufs[entry];
-- 
2.7.4

[PATCH 2/2] alx: add disable_wol paramenter

[AceLan Kao]

The WoL feature was reported broken and will lead to
the system resume immediately after suspending.
This symptom is not happening on every system, so adding
disable_wol option and disable WoL by default to prevent the issue from
happening again.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
Signed-off-by: AceLan Kao 
---
 drivers/net/ethernet/atheros/alx/ethtool.c | 7 ++-
 drivers/net/ethernet/atheros/alx/main.c| 5 +
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 859e272..e50129d 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -46,6 +46,8 @@
 #include "reg.h"
 #include "hw.h"
 
+extern const bool disable_wol;
+
 /* The order of these strings must match the order of the fields in
  * struct alx_hw_stats
  * See hw.h
@@ -315,6 +317,9 @@ static void alx_get_wol(struct net_device *netdev, struct 
ethtool_wolinfo *wol)
struct alx_priv *alx = netdev_priv(netdev);
struct alx_hw *hw = >hw;
 
+   if (disable_wol)
+   return;
+
wol->supported = WAKE_MAGIC | WAKE_PHY;
wol->wolopts = 0;
 
@@ -329,7 +334,7 @@ static int alx_set_wol(struct net_device *netdev, struct 
ethtool_wolinfo *wol)
struct alx_priv *alx = netdev_priv(netdev);
struct alx_hw *hw = >hw;
 
-   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   if (disable_wol || (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY)))
return -EOPNOTSUPP;
 
hw->sleep_ctrl = 0;
diff --git a/drivers/net/ethernet/atheros/alx/main.c 
b/drivers/net/ethernet/atheros/alx/main.c
index c0e2bb2..c27fdf7 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -51,6 +51,11 @@
 
 const char alx_drv_name[] = "alx";
 
+/* disable WoL by default */
+bool disable_wol = 1;
+module_param(disable_wol, bool, 0);
+MODULE_PARM_DESC(disable_wol, "Disable Wake on Lan feature");
+
 static void alx_free_txbuf(struct alx_tx_queue *txq, int entry)
 {
struct alx_buffer *txb = >bufs[entry];
-- 
2.7.4

[PATCH 1/2] Revert "alx: remove WoL support"

[AceLan Kao]

This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.

There are still many people need this feature, so try adding it back.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  36 +++
 drivers/net/ethernet/atheros/alx/hw.c  | 154 -
 drivers/net/ethernet/atheros/alx/hw.h  |   5 +
 drivers/net/ethernet/atheros/alx/main.c| 142 --
 4 files changed, 326 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 2f4eabf..859e272 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -310,11 +310,47 @@ static int alx_get_sset_count(struct net_device *netdev, 
int sset)
}
 }
 
+static void alx_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   wol->supported = WAKE_MAGIC | WAKE_PHY;
+   wol->wolopts = 0;
+
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
+   wol->wolopts |= WAKE_MAGIC;
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY)
+   wol->wolopts |= WAKE_PHY;
+}
+
+static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   return -EOPNOTSUPP;
+
+   hw->sleep_ctrl = 0;
+
+   if (wol->wolopts & WAKE_MAGIC)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_MAGIC;
+   if (wol->wolopts & WAKE_PHY)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_PHY;
+
+   device_set_wakeup_enable(>hw.pdev->dev, hw->sleep_ctrl);
+
+   return 0;
+}
+
 const struct ethtool_ops alx_ethtool_ops = {
.get_pauseparam = alx_get_pauseparam,
.set_pauseparam = alx_set_pauseparam,
.get_msglevel   = alx_get_msglevel,
.set_msglevel   = alx_set_msglevel,
+   .get_wol= alx_get_wol,
+   .set_wol= alx_set_wol,
.get_link   = ethtool_op_get_link,
.get_strings= alx_get_strings,
.get_sset_count = alx_get_sset_count,
diff --git a/drivers/net/ethernet/atheros/alx/hw.c 
b/drivers/net/ethernet/atheros/alx/hw.c
index 6ac40b0..f9bf612 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -332,6 +332,16 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
alx_write_mem32(hw, ALX_STAD1, val);
 }
 
+static void alx_enable_osc(struct alx_hw *hw)
+{
+   u32 val;
+
+   /* rising edge */
+   val = alx_read_mem32(hw, ALX_MISC);
+   alx_write_mem32(hw, ALX_MISC, val & ~ALX_MISC_INTNLOSC_OPEN);
+   alx_write_mem32(hw, ALX_MISC, val | ALX_MISC_INTNLOSC_OPEN);
+}
+
 static void alx_reset_osc(struct alx_hw *hw, u8 rev)
 {
u32 val, val2;
@@ -774,7 +784,6 @@ int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, 
u8 flowctrl)
return err;
 }
 
-
 void alx_post_phy_link(struct alx_hw *hw)
 {
u16 phy_val, len, agc;
@@ -848,6 +857,65 @@ void alx_post_phy_link(struct alx_hw *hw)
}
 }
 
+/* NOTE:
+ *1. phy link must be established before calling this function
+ *2. wol option (pattern,magic,link,etc.) is configed before call it.
+ */
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
+{
+   u32 master, mac, phy, val;
+   int err = 0;
+
+   master = alx_read_mem32(hw, ALX_MASTER);
+   master &= ~ALX_MASTER_PCLKSEL_SRDS;
+   mac = hw->rx_ctrl;
+   /* 10/100 half */
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,  ALX_MAC_CTRL_SPEED_10_100);
+   mac &= ~(ALX_MAC_CTRL_FULLD | ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_TX_EN);
+
+   phy = alx_read_mem32(hw, ALX_PHY_CTRL);
+   phy &= ~(ALX_PHY_CTRL_DSPRST_OUT | ALX_PHY_CTRL_CLS);
+   phy |= ALX_PHY_CTRL_RST_ANALOG | ALX_PHY_CTRL_HIB_PULSE |
+  ALX_PHY_CTRL_HIB_EN;
+
+   /* without any activity  */
+   if (!(hw->sleep_ctrl & ALX_SLEEP_ACTIVE)) {
+   err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
+   if (err)
+   return err;
+   phy |= ALX_PHY_CTRL_IDDQ | ALX_PHY_CTRL_POWER_DOWN;
+   } else {
+   if (hw->sleep_ctrl & (ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_CIFS))
+   mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
+   if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
+   mac |= ALX_MAC_CTRL_TX_EN;
+   if (duplex == DUPLEX_FULL)
+   mac |= ALX_MAC_CTRL_FULLD;
+   if (speed == SPEED_1000)
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_

[PATCH 1/2] Revert "alx: remove WoL support"

[AceLan Kao]

This reverts commit bc2bebe8de8ed4ba6482c9cc370b0dd72ffe8cd2.

There are still many people need this feature, so try adding it back.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=61651
Signed-off-by: AceLan Kao 
---
 drivers/net/ethernet/atheros/alx/ethtool.c |  36 +++
 drivers/net/ethernet/atheros/alx/hw.c  | 154 -
 drivers/net/ethernet/atheros/alx/hw.h  |   5 +
 drivers/net/ethernet/atheros/alx/main.c| 142 --
 4 files changed, 326 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/atheros/alx/ethtool.c 
b/drivers/net/ethernet/atheros/alx/ethtool.c
index 2f4eabf..859e272 100644
--- a/drivers/net/ethernet/atheros/alx/ethtool.c
+++ b/drivers/net/ethernet/atheros/alx/ethtool.c
@@ -310,11 +310,47 @@ static int alx_get_sset_count(struct net_device *netdev, 
int sset)
}
 }
 
+static void alx_get_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   wol->supported = WAKE_MAGIC | WAKE_PHY;
+   wol->wolopts = 0;
+
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_MAGIC)
+   wol->wolopts |= WAKE_MAGIC;
+   if (hw->sleep_ctrl & ALX_SLEEP_WOL_PHY)
+   wol->wolopts |= WAKE_PHY;
+}
+
+static int alx_set_wol(struct net_device *netdev, struct ethtool_wolinfo *wol)
+{
+   struct alx_priv *alx = netdev_priv(netdev);
+   struct alx_hw *hw = >hw;
+
+   if (wol->wolopts & ~(WAKE_MAGIC | WAKE_PHY))
+   return -EOPNOTSUPP;
+
+   hw->sleep_ctrl = 0;
+
+   if (wol->wolopts & WAKE_MAGIC)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_MAGIC;
+   if (wol->wolopts & WAKE_PHY)
+   hw->sleep_ctrl |= ALX_SLEEP_WOL_PHY;
+
+   device_set_wakeup_enable(>hw.pdev->dev, hw->sleep_ctrl);
+
+   return 0;
+}
+
 const struct ethtool_ops alx_ethtool_ops = {
.get_pauseparam = alx_get_pauseparam,
.set_pauseparam = alx_set_pauseparam,
.get_msglevel   = alx_get_msglevel,
.set_msglevel   = alx_set_msglevel,
+   .get_wol= alx_get_wol,
+   .set_wol= alx_set_wol,
.get_link   = ethtool_op_get_link,
.get_strings= alx_get_strings,
.get_sset_count = alx_get_sset_count,
diff --git a/drivers/net/ethernet/atheros/alx/hw.c 
b/drivers/net/ethernet/atheros/alx/hw.c
index 6ac40b0..f9bf612 100644
--- a/drivers/net/ethernet/atheros/alx/hw.c
+++ b/drivers/net/ethernet/atheros/alx/hw.c
@@ -332,6 +332,16 @@ void alx_set_macaddr(struct alx_hw *hw, const u8 *addr)
alx_write_mem32(hw, ALX_STAD1, val);
 }
 
+static void alx_enable_osc(struct alx_hw *hw)
+{
+   u32 val;
+
+   /* rising edge */
+   val = alx_read_mem32(hw, ALX_MISC);
+   alx_write_mem32(hw, ALX_MISC, val & ~ALX_MISC_INTNLOSC_OPEN);
+   alx_write_mem32(hw, ALX_MISC, val | ALX_MISC_INTNLOSC_OPEN);
+}
+
 static void alx_reset_osc(struct alx_hw *hw, u8 rev)
 {
u32 val, val2;
@@ -774,7 +784,6 @@ int alx_setup_speed_duplex(struct alx_hw *hw, u32 ethadv, 
u8 flowctrl)
return err;
 }
 
-
 void alx_post_phy_link(struct alx_hw *hw)
 {
u16 phy_val, len, agc;
@@ -848,6 +857,65 @@ void alx_post_phy_link(struct alx_hw *hw)
}
 }
 
+/* NOTE:
+ *1. phy link must be established before calling this function
+ *2. wol option (pattern,magic,link,etc.) is configed before call it.
+ */
+int alx_pre_suspend(struct alx_hw *hw, int speed, u8 duplex)
+{
+   u32 master, mac, phy, val;
+   int err = 0;
+
+   master = alx_read_mem32(hw, ALX_MASTER);
+   master &= ~ALX_MASTER_PCLKSEL_SRDS;
+   mac = hw->rx_ctrl;
+   /* 10/100 half */
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,  ALX_MAC_CTRL_SPEED_10_100);
+   mac &= ~(ALX_MAC_CTRL_FULLD | ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_TX_EN);
+
+   phy = alx_read_mem32(hw, ALX_PHY_CTRL);
+   phy &= ~(ALX_PHY_CTRL_DSPRST_OUT | ALX_PHY_CTRL_CLS);
+   phy |= ALX_PHY_CTRL_RST_ANALOG | ALX_PHY_CTRL_HIB_PULSE |
+  ALX_PHY_CTRL_HIB_EN;
+
+   /* without any activity  */
+   if (!(hw->sleep_ctrl & ALX_SLEEP_ACTIVE)) {
+   err = alx_write_phy_reg(hw, ALX_MII_IER, 0);
+   if (err)
+   return err;
+   phy |= ALX_PHY_CTRL_IDDQ | ALX_PHY_CTRL_POWER_DOWN;
+   } else {
+   if (hw->sleep_ctrl & (ALX_SLEEP_WOL_MAGIC | ALX_SLEEP_CIFS))
+   mac |= ALX_MAC_CTRL_RX_EN | ALX_MAC_CTRL_BRD_EN;
+   if (hw->sleep_ctrl & ALX_SLEEP_CIFS)
+   mac |= ALX_MAC_CTRL_TX_EN;
+   if (duplex == DUPLEX_FULL)
+   mac |= ALX_MAC_CTRL_FULLD;
+   if (speed == SPEED_1000)
+   ALX_SET_FIELD(mac, ALX_MAC_CTRL_SPEED,
+ ALX_M

[PATCH] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding the following platforms to the quirk.
   Dell Inspiron 24-3460
   Dell Inspiron 3472
   Dell Inspiron 14-3473
   Dell Vostro 3262
   Dell Vostro 15-3572

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 53 +++
 1 file changed, 53 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 43adead..e479fae 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ath9k.h"
@@ -96,6 +97,56 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
 };
 #endif
 
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+   ath9k_use_msi = 1;
+   return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 24-3460",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 3262",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 3472",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 15-3572",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 14-3473",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+   },
+   },
+   {}
+};
+
 static void ath9k_deinit_softc(struct ath_softc *sc);
 
 static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1155,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
 
+   dmi_check_system(ath9k_quirks);
+
return 0;
 
  err_pci_exit:
-- 
2.7.4

[PATCH] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding the following platforms to the quirk.
   Dell Inspiron 24-3460
   Dell Inspiron 3472
   Dell Inspiron 14-3473
   Dell Vostro 3262
   Dell Vostro 15-3572

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 53 +++
 1 file changed, 53 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 43adead..e479fae 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ath9k.h"
@@ -96,6 +97,56 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
 };
 #endif
 
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+   ath9k_use_msi = 1;
+   return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 24-3460",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 3262",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 3472",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 15-3572",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+   },
+   },
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 14-3473",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+   },
+   },
+   {}
+};
+
 static void ath9k_deinit_softc(struct ath_softc *sc);
 
 static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1155,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
 
+   dmi_check_system(ath9k_quirks);
+
return 0;
 
  err_pci_exit:
-- 
2.7.4

[PATCH] Bluetooth: btusb: Add support for 0cf3:e010

[AceLan Kao]

Device 0cf3:e010 is one of the QCA ROME family.

T:  Bus=01 Lev=01 Prnt=01 Port=13 Cnt=03 Dev#=  4 Spd=12  MxCh= 0
D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0cf3 ProdID=e010 Rev=00.01
C:  #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
I:  If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/bluetooth/btusb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f7120c9..9136edb 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -263,6 +263,7 @@ static const struct usb_device_id blacklist_table[] = {
/* QCA ROME chipset */
{ USB_DEVICE(0x0cf3, 0xe007), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe009), .driver_info = BTUSB_QCA_ROME },
+   { USB_DEVICE(0x0cf3, 0xe010), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe300), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe301), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe360), .driver_info = BTUSB_QCA_ROME },
-- 
2.7.4

[PATCH] Bluetooth: btusb: Add support for 0cf3:e010

[AceLan Kao]

Device 0cf3:e010 is one of the QCA ROME family.

T:  Bus=01 Lev=01 Prnt=01 Port=13 Cnt=03 Dev#=  4 Spd=12  MxCh= 0
D:  Ver= 2.01 Cls=e0(wlcon) Sub=01 Prot=01 MxPS=64 #Cfgs=  1
P:  Vendor=0cf3 ProdID=e010 Rev=00.01
C:  #Ifs= 2 Cfg#= 1 Atr=e0 MxPwr=100mA
I:  If#= 0 Alt= 0 #EPs= 3 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb
I:  If#= 1 Alt= 0 #EPs= 2 Cls=e0(wlcon) Sub=01 Prot=01 Driver=btusb

Signed-off-by: AceLan Kao 
---
 drivers/bluetooth/btusb.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index f7120c9..9136edb 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -263,6 +263,7 @@ static const struct usb_device_id blacklist_table[] = {
/* QCA ROME chipset */
{ USB_DEVICE(0x0cf3, 0xe007), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe009), .driver_info = BTUSB_QCA_ROME },
+   { USB_DEVICE(0x0cf3, 0xe010), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe300), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe301), .driver_info = BTUSB_QCA_ROME },
{ USB_DEVICE(0x0cf3, 0xe360), .driver_info = BTUSB_QCA_ROME },
-- 
2.7.4

Re: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

Hi Alex,

2017-10-27 0:10 GMT+08:00 Alan Cox :
>
>> > +static int __init dell_uart_bl_init(void)
>> > +{
>> > +   ftty = filp_open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NDELAY, 0);
>>
>
> You have no idea what name is assigned. This doesn't work.
Yes, I did. The port is defined in BIOS as PNP0501 uart port,
so the port will be registered by 8250 PNP driver as ttyS* device.
And consider it's defined in the BIOS, so it should be the first uart
port on the system,
and ttyS0 would be his name.

>
> This isn't the way to do it any more because the 'tty from kernel'
> problem finally got fixed. You can attach it as a serdev directly to the
> kernel.
>
> Take a look at serdev_tty_port_register/serdev_tty_port_unreg
Yes, I'm studying serdev, but currently there are not many examples
for reference,
so I still have no clues how to implement it by serdev.
I'll try using serdev_tty_port_register/serdev_tty_port_unreg to
re-implement the driver.
Thanks.

>
> Alan



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)

Re: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

Hi Alex,

2017-10-27 0:10 GMT+08:00 Alan Cox :
>
>> > +static int __init dell_uart_bl_init(void)
>> > +{
>> > +   ftty = filp_open("/dev/ttyS0", O_RDWR | O_NOCTTY | O_NDELAY, 0);
>>
>
> You have no idea what name is assigned. This doesn't work.
Yes, I did. The port is defined in BIOS as PNP0501 uart port,
so the port will be registered by 8250 PNP driver as ttyS* device.
And consider it's defined in the BIOS, so it should be the first uart
port on the system,
and ttyS0 would be his name.

>
> This isn't the way to do it any more because the 'tty from kernel'
> problem finally got fixed. You can attach it as a serdev directly to the
> kernel.
>
> Take a look at serdev_tty_port_register/serdev_tty_port_unreg
Yes, I'm studying serdev, but currently there are not many examples
for reference,
so I still have no clues how to implement it by serdev.
I'll try using serdev_tty_port_register/serdev_tty_port_unreg to
re-implement the driver.
Thanks.

>
> Alan



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)

Re: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

Hi Mario,

2017-10-26 23:54 GMT+08:00  <mario.limoncie...@dell.com>:
>> -Original Message-
>> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
>> ow...@vger.kernel.org] On Behalf Of AceLan Kao
>> Sent: Thursday, October 26, 2017 2:54 AM
>> To: Darren Hart <dvh...@infradead.org>; Andy Shevchenko
>> <a...@infradead.org>; linux-kernel@vger.kernel.org; platform-driver-
>> x...@vger.kernel.org
>> Subject: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for 
>> DELL
>> AIO
>>
>> The Dell AIO machines released after 2017 come with a UART interface
>> to communicate with the backlight scalar board. This driver creates
>> a standard backlight interface and talks to the scalar board through
>> UART.
>>
>> In DSDT this uart port will be defined as
>>Name (_HID, "DELL0501")
>>Name (_CID, EisaId ("PNP0501")
>> The 8250 PNP driver will be loaded by default, and this driver uses
>> "DELL0501" to confirm the uart port is a backlight interface and
>> leverages the port created by 8250 PNP driver to communicate with
>> the scalar board.
>>
>> Signed-off-by: AceLan Kao <acelan@canonical.com>
>> ---
>>  MAINTAINERS|   6 +
>>  drivers/platform/x86/Kconfig   |  14 ++
>>  drivers/platform/x86/Makefile  |   1 +
>>  drivers/platform/x86/dell-uart-backlight.c | 361
>> +
>>  drivers/platform/x86/dell-uart-backlight.h |  88 +++
>>  5 files changed, 470 insertions(+)
>>  create mode 100644 drivers/platform/x86/dell-uart-backlight.c
>>  create mode 100644 drivers/platform/x86/dell-uart-backlight.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d85c089..581cb09 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4013,6 +4013,12 @@ M: Pali Rohár <pali.ro...@gmail.com>
>>  S:   Maintained
>>  F:   drivers/platform/x86/dell-wmi.c
>>
>> +DELL AIO UART BACKLIGHT DRIVER
>> +M:   AceLan Kao <acelan@canonical.com>
>> +L:   platform-driver-...@vger.kernel.org
>> +S:   Maintained
>> +F:   drivers/platform/x86/dell-uart-backlight.c
>> +
>>  DELTA ST MEDIA DRIVER
>>  M:   Hugues Fruchet <hugues.fruc...@st.com>
>>  L:   linux-me...@vger.kernel.org
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 80b8795..58e26bc 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -175,6 +175,20 @@ config DELL_RBTN
>> To compile this driver as a module, choose M here: the module will
>> be called dell-rbtn.
>>
>> +config DELL_UART_BACKLIGHT
>> + tristate "Dell AIO UART Backlight driver"
>> + depends on SERIAL_8250
>> + depends on ACPI
>> + ---help---
>> +   Say Y here if you want to support Dell AIO UART backlight interface.
>> +   The Dell AIO machines released after 2017 come with a UART interface
>> +   to communicate with the backlight scalar board. This driver creates
>> +   a standard backlight interface and talks to the scalar board through
>> +   UART to adjust the AIO screen brightness.
>> +
>> +   To compile this driver as a module, choose M here: the module will
>> +   be called dell_uart_backlight.
>> +
>>
>>  config FUJITSU_LAPTOP
>>   tristate "Fujitsu Laptop Extras"
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index 91cec17..3cf576f 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -18,6 +18,7 @@ obj-$(CONFIG_DELL_WMI_AIO)  += dell-wmi-aio.o
>>  obj-$(CONFIG_DELL_WMI_LED)   += dell-wmi-led.o
>>  obj-$(CONFIG_DELL_SMO8800)   += dell-smo8800.o
>>  obj-$(CONFIG_DELL_RBTN)  += dell-rbtn.o
>> +obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o
>>  obj-$(CONFIG_ACER_WMI)   += acer-wmi.o
>>  obj-$(CONFIG_ACERHDF)+= acerhdf.o
>>  obj-$(CONFIG_HP_ACCEL)   += hp_accel.o
>> diff --git a/drivers/platform/x86/dell-uart-backlight.c 
>> b/drivers/platform/x86/dell-
>> uart-backlight.c
>> new file mode 100644
>> index 000..400be77
>> --- /dev/null
>> +++ b/drivers/platform/x86/dell-uart-backlight.c
>> @@ -0,0 +1,361 @@
>> +/*
>> + *  Dell AIO Serial Backlight Driver
>> + *
>> + *  Copyright (C) 2017 AceLan Kao <acelan@canoni

Re: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

Hi Mario,

2017-10-26 23:54 GMT+08:00  :
>> -Original Message-
>> From: platform-driver-x86-ow...@vger.kernel.org [mailto:platform-driver-x86-
>> ow...@vger.kernel.org] On Behalf Of AceLan Kao
>> Sent: Thursday, October 26, 2017 2:54 AM
>> To: Darren Hart ; Andy Shevchenko
>> ; linux-kernel@vger.kernel.org; platform-driver-
>> x...@vger.kernel.org
>> Subject: [PATCH] platform/x86: dell-uart-backlight: new backlight driver for 
>> DELL
>> AIO
>>
>> The Dell AIO machines released after 2017 come with a UART interface
>> to communicate with the backlight scalar board. This driver creates
>> a standard backlight interface and talks to the scalar board through
>> UART.
>>
>> In DSDT this uart port will be defined as
>>Name (_HID, "DELL0501")
>>Name (_CID, EisaId ("PNP0501")
>> The 8250 PNP driver will be loaded by default, and this driver uses
>> "DELL0501" to confirm the uart port is a backlight interface and
>> leverages the port created by 8250 PNP driver to communicate with
>> the scalar board.
>>
>> Signed-off-by: AceLan Kao 
>> ---
>>  MAINTAINERS|   6 +
>>  drivers/platform/x86/Kconfig   |  14 ++
>>  drivers/platform/x86/Makefile  |   1 +
>>  drivers/platform/x86/dell-uart-backlight.c | 361
>> +
>>  drivers/platform/x86/dell-uart-backlight.h |  88 +++
>>  5 files changed, 470 insertions(+)
>>  create mode 100644 drivers/platform/x86/dell-uart-backlight.c
>>  create mode 100644 drivers/platform/x86/dell-uart-backlight.h
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d85c089..581cb09 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -4013,6 +4013,12 @@ M: Pali Rohár 
>>  S:   Maintained
>>  F:   drivers/platform/x86/dell-wmi.c
>>
>> +DELL AIO UART BACKLIGHT DRIVER
>> +M:   AceLan Kao 
>> +L:   platform-driver-...@vger.kernel.org
>> +S:   Maintained
>> +F:   drivers/platform/x86/dell-uart-backlight.c
>> +
>>  DELTA ST MEDIA DRIVER
>>  M:   Hugues Fruchet 
>>  L:   linux-me...@vger.kernel.org
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 80b8795..58e26bc 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -175,6 +175,20 @@ config DELL_RBTN
>> To compile this driver as a module, choose M here: the module will
>> be called dell-rbtn.
>>
>> +config DELL_UART_BACKLIGHT
>> + tristate "Dell AIO UART Backlight driver"
>> + depends on SERIAL_8250
>> + depends on ACPI
>> + ---help---
>> +   Say Y here if you want to support Dell AIO UART backlight interface.
>> +   The Dell AIO machines released after 2017 come with a UART interface
>> +   to communicate with the backlight scalar board. This driver creates
>> +   a standard backlight interface and talks to the scalar board through
>> +   UART to adjust the AIO screen brightness.
>> +
>> +   To compile this driver as a module, choose M here: the module will
>> +   be called dell_uart_backlight.
>> +
>>
>>  config FUJITSU_LAPTOP
>>   tristate "Fujitsu Laptop Extras"
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index 91cec17..3cf576f 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -18,6 +18,7 @@ obj-$(CONFIG_DELL_WMI_AIO)  += dell-wmi-aio.o
>>  obj-$(CONFIG_DELL_WMI_LED)   += dell-wmi-led.o
>>  obj-$(CONFIG_DELL_SMO8800)   += dell-smo8800.o
>>  obj-$(CONFIG_DELL_RBTN)  += dell-rbtn.o
>> +obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o
>>  obj-$(CONFIG_ACER_WMI)   += acer-wmi.o
>>  obj-$(CONFIG_ACERHDF)+= acerhdf.o
>>  obj-$(CONFIG_HP_ACCEL)   += hp_accel.o
>> diff --git a/drivers/platform/x86/dell-uart-backlight.c 
>> b/drivers/platform/x86/dell-
>> uart-backlight.c
>> new file mode 100644
>> index 000..400be77
>> --- /dev/null
>> +++ b/drivers/platform/x86/dell-uart-backlight.c
>> @@ -0,0 +1,361 @@
>> +/*
>> + *  Dell AIO Serial Backlight Driver
>> + *
>> + *  Copyright (C) 2017 AceLan Kao 
>> + *
>> + *  This program is free software; you can redistribute it and/or modify
>> + *  it under the terms of the GNU General Public License as published by
>> + *  the Free Software Foundation;

[PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

The Dell AIO machines released after 2017 come with a UART interface
to communicate with the backlight scalar board. This driver creates
a standard backlight interface and talks to the scalar board through
UART.

In DSDT this uart port will be defined as
   Name (_HID, "DELL0501")
   Name (_CID, EisaId ("PNP0501")
The 8250 PNP driver will be loaded by default, and this driver uses
"DELL0501" to confirm the uart port is a backlight interface and
leverages the port created by 8250 PNP driver to communicate with
the scalar board.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 MAINTAINERS|   6 +
 drivers/platform/x86/Kconfig   |  14 ++
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-uart-backlight.c | 361 +
 drivers/platform/x86/dell-uart-backlight.h |  88 +++
 5 files changed, 470 insertions(+)
 create mode 100644 drivers/platform/x86/dell-uart-backlight.c
 create mode 100644 drivers/platform/x86/dell-uart-backlight.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d85c089..581cb09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4013,6 +4013,12 @@ M:   Pali Rohár <pali.ro...@gmail.com>
 S: Maintained
 F: drivers/platform/x86/dell-wmi.c
 
+DELL AIO UART BACKLIGHT DRIVER
+M: AceLan Kao <acelan@canonical.com>
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-uart-backlight.c
+
 DELTA ST MEDIA DRIVER
 M: Hugues Fruchet <hugues.fruc...@st.com>
 L: linux-me...@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b8795..58e26bc 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -175,6 +175,20 @@ config DELL_RBTN
  To compile this driver as a module, choose M here: the module will
  be called dell-rbtn.
 
+config DELL_UART_BACKLIGHT
+   tristate "Dell AIO UART Backlight driver"
+   depends on SERIAL_8250
+   depends on ACPI
+   ---help---
+ Say Y here if you want to support Dell AIO UART backlight interface.
+ The Dell AIO machines released after 2017 come with a UART interface
+ to communicate with the backlight scalar board. This driver creates
+ a standard backlight interface and talks to the scalar board through
+ UART to adjust the AIO screen brightness.
+
+ To compile this driver as a module, choose M here: the module will
+ be called dell_uart_backlight.
+
 
 config FUJITSU_LAPTOP
tristate "Fujitsu Laptop Extras"
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec17..3cf576f 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DELL_WMI_AIO)+= dell-wmi-aio.o
 obj-$(CONFIG_DELL_WMI_LED) += dell-wmi-led.o
 obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o
 obj-$(CONFIG_DELL_RBTN)+= dell-rbtn.o
+obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o
 obj-$(CONFIG_ACER_WMI) += acer-wmi.o
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
 obj-$(CONFIG_HP_ACCEL) += hp_accel.o
diff --git a/drivers/platform/x86/dell-uart-backlight.c 
b/drivers/platform/x86/dell-uart-backlight.c
new file mode 100644
index 000..400be77
--- /dev/null
+++ b/drivers/platform/x86/dell-uart-backlight.c
@@ -0,0 +1,361 @@
+/*
+ *  Dell AIO Serial Backlight Driver
+ *
+ *  Copyright (C) 2017 AceLan Kao <acelan@canonical.com>
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "dell-uart-backlight.h"
+
+struct dell_uart_backlight {
+   struct device *dev;
+   struct backlight_device *dell_uart_bd;
+   struct mutex brightness_mutex;
+   int line;
+   int bl_power;
+};
+struct uart_8250_port *serial8250_get_port(int line);
+struct tty_struct *tty;
+struct file *ftty;
+
+unsigned int (*io_serial_in)(struct uart_port *p, int offset);
+int (*uart_write)(struct tty_struct *tty, const unsigned char *buf, int count);
+int (*uart_chars_in_buffer)(struct tty_struct *tty);
+
+static int dell_uart_write(struct uart_8250_port *up, __u8 *buf, int len)
+{
+   int actual = 0;
+   struct uart_port *port = >port;
+
+   tty_port_tty_wakeup(>state->port);
+   tty = tty_port_tty_get

[PATCH] platform/x86: dell-uart-backlight: new backlight driver for DELL AIO

[AceLan Kao]

The Dell AIO machines released after 2017 come with a UART interface
to communicate with the backlight scalar board. This driver creates
a standard backlight interface and talks to the scalar board through
UART.

In DSDT this uart port will be defined as
   Name (_HID, "DELL0501")
   Name (_CID, EisaId ("PNP0501")
The 8250 PNP driver will be loaded by default, and this driver uses
"DELL0501" to confirm the uart port is a backlight interface and
leverages the port created by 8250 PNP driver to communicate with
the scalar board.

Signed-off-by: AceLan Kao 
---
 MAINTAINERS|   6 +
 drivers/platform/x86/Kconfig   |  14 ++
 drivers/platform/x86/Makefile  |   1 +
 drivers/platform/x86/dell-uart-backlight.c | 361 +
 drivers/platform/x86/dell-uart-backlight.h |  88 +++
 5 files changed, 470 insertions(+)
 create mode 100644 drivers/platform/x86/dell-uart-backlight.c
 create mode 100644 drivers/platform/x86/dell-uart-backlight.h

diff --git a/MAINTAINERS b/MAINTAINERS
index d85c089..581cb09 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4013,6 +4013,12 @@ M:   Pali Rohár 
 S: Maintained
 F: drivers/platform/x86/dell-wmi.c
 
+DELL AIO UART BACKLIGHT DRIVER
+M: AceLan Kao 
+L: platform-driver-...@vger.kernel.org
+S: Maintained
+F: drivers/platform/x86/dell-uart-backlight.c
+
 DELTA ST MEDIA DRIVER
 M: Hugues Fruchet 
 L: linux-me...@vger.kernel.org
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 80b8795..58e26bc 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -175,6 +175,20 @@ config DELL_RBTN
  To compile this driver as a module, choose M here: the module will
  be called dell-rbtn.
 
+config DELL_UART_BACKLIGHT
+   tristate "Dell AIO UART Backlight driver"
+   depends on SERIAL_8250
+   depends on ACPI
+   ---help---
+ Say Y here if you want to support Dell AIO UART backlight interface.
+ The Dell AIO machines released after 2017 come with a UART interface
+ to communicate with the backlight scalar board. This driver creates
+ a standard backlight interface and talks to the scalar board through
+ UART to adjust the AIO screen brightness.
+
+ To compile this driver as a module, choose M here: the module will
+ be called dell_uart_backlight.
+
 
 config FUJITSU_LAPTOP
tristate "Fujitsu Laptop Extras"
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 91cec17..3cf576f 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -18,6 +18,7 @@ obj-$(CONFIG_DELL_WMI_AIO)+= dell-wmi-aio.o
 obj-$(CONFIG_DELL_WMI_LED) += dell-wmi-led.o
 obj-$(CONFIG_DELL_SMO8800) += dell-smo8800.o
 obj-$(CONFIG_DELL_RBTN)+= dell-rbtn.o
+obj-$(CONFIG_DELL_UART_BACKLIGHT) += dell-uart-backlight.o
 obj-$(CONFIG_ACER_WMI) += acer-wmi.o
 obj-$(CONFIG_ACERHDF)  += acerhdf.o
 obj-$(CONFIG_HP_ACCEL) += hp_accel.o
diff --git a/drivers/platform/x86/dell-uart-backlight.c 
b/drivers/platform/x86/dell-uart-backlight.c
new file mode 100644
index 000..400be77
--- /dev/null
+++ b/drivers/platform/x86/dell-uart-backlight.c
@@ -0,0 +1,361 @@
+/*
+ *  Dell AIO Serial Backlight Driver
+ *
+ *  Copyright (C) 2017 AceLan Kao 
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "dell-uart-backlight.h"
+
+struct dell_uart_backlight {
+   struct device *dev;
+   struct backlight_device *dell_uart_bd;
+   struct mutex brightness_mutex;
+   int line;
+   int bl_power;
+};
+struct uart_8250_port *serial8250_get_port(int line);
+struct tty_struct *tty;
+struct file *ftty;
+
+unsigned int (*io_serial_in)(struct uart_port *p, int offset);
+int (*uart_write)(struct tty_struct *tty, const unsigned char *buf, int count);
+int (*uart_chars_in_buffer)(struct tty_struct *tty);
+
+static int dell_uart_write(struct uart_8250_port *up, __u8 *buf, int len)
+{
+   int actual = 0;
+   struct uart_port *port = >port;
+
+   tty_port_tty_wakeup(>state->port);
+   tty = tty_port_tty_get(>state->port);
+   actual = uart_write(tty, buf, len);
+   while (uart_chars_in_buffer(tty)) {
+   udelay(10);
+   }
+
+

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Daniel,

After applied the 2 commits you mentioned in the email, ath9k works.

https://marc.info/?l=linux-wireless=150631274108016=2
https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657

Best regards,
AceLan Kao.

2017-10-05 14:39 GMT+08:00 AceLan Kao <acelan@canonical.com>:
> Hi all,
>
> Please drop my patches, Qualcomm is working internally and will submit
> the MSI patch by themselves.
> Thanks.
>
> Hi Daniel,
>
> I'll try your patches tomorrow.
>
> Best regards,
> AceLan Kao.
>
> 2017-10-02 12:21 GMT+08:00 Daniel Drake <dr...@endlessm.com>:
>> Hi AceLan,
>>
>> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao <acelan@canonical.com> wrote:
>>> Hi Daniel,
>>>
>>> I've tried your patch, but it doesn't work for me.
>>> Wifi can scan AP, but can't get connected.
>>
>> Can you please clarify which patch(es) you have tried?
>>
>> This is the base patch which adds the infrastructure to request
>> specific MSI IRQ vectors:
>> https://marc.info/?l=linux-wireless=150631274108016=2
>>
>> This is the ath9k MSI patch which makes use of that:
>> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>>
>> If you were already able to use ath9k MSI interrupts without specific
>> consideration for which MSI vector numbers were used, these are the
>> possible explanations that spring to mind:
>>
>> 1. You got lucky and it picked a vector number that is 4-aligned. You
>> can check this in the "lspci -vvv" output. You'll see something like:
>> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
>> Address: fee0300c  Data: 4142
>> The lower number is the vector number. In my example here 0x42 (66) is
>> not 4-aligned so the failure condition will be hit.
>>
>> 2. You are using interrupt remapping, which I suspect may provide a
>> high likelihood of MSI interrupt vectors being 4-aligned. See if
>> /proc/interrupts shows the IRQ type as IR-PCI-MSI
>> Unfortunately interrupt remapping is not available here,
>> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>>
>> 3. My assumption that all ath9k hardware corrupts the MSI vector
>> number could wrong. However we've seen this on different wifi modules
>> in laptops produced by different OEMs and ODMs, so it seems to be a
>> somewhat widespread problem at least.
>>
>> 4. My assumption that ath9k hardware is corrupting the MSI vector
>> number could be wrong; maybe another component is to blame, could it
>> be a BIOS issue? Admittedly I don't really know how I can debug the
>> layers inbetween seeing the MSI Message Data value disagree with the
>> vector number being handled inside do_IRQ().
>>
>> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Daniel,

After applied the 2 commits you mentioned in the email, ath9k works.

https://marc.info/?l=linux-wireless=150631274108016=2
https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657

Best regards,
AceLan Kao.

2017-10-05 14:39 GMT+08:00 AceLan Kao :
> Hi all,
>
> Please drop my patches, Qualcomm is working internally and will submit
> the MSI patch by themselves.
> Thanks.
>
> Hi Daniel,
>
> I'll try your patches tomorrow.
>
> Best regards,
> AceLan Kao.
>
> 2017-10-02 12:21 GMT+08:00 Daniel Drake :
>> Hi AceLan,
>>
>> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao  wrote:
>>> Hi Daniel,
>>>
>>> I've tried your patch, but it doesn't work for me.
>>> Wifi can scan AP, but can't get connected.
>>
>> Can you please clarify which patch(es) you have tried?
>>
>> This is the base patch which adds the infrastructure to request
>> specific MSI IRQ vectors:
>> https://marc.info/?l=linux-wireless=150631274108016=2
>>
>> This is the ath9k MSI patch which makes use of that:
>> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>>
>> If you were already able to use ath9k MSI interrupts without specific
>> consideration for which MSI vector numbers were used, these are the
>> possible explanations that spring to mind:
>>
>> 1. You got lucky and it picked a vector number that is 4-aligned. You
>> can check this in the "lspci -vvv" output. You'll see something like:
>> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
>> Address: fee0300c  Data: 4142
>> The lower number is the vector number. In my example here 0x42 (66) is
>> not 4-aligned so the failure condition will be hit.
>>
>> 2. You are using interrupt remapping, which I suspect may provide a
>> high likelihood of MSI interrupt vectors being 4-aligned. See if
>> /proc/interrupts shows the IRQ type as IR-PCI-MSI
>> Unfortunately interrupt remapping is not available here,
>> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>>
>> 3. My assumption that all ath9k hardware corrupts the MSI vector
>> number could wrong. However we've seen this on different wifi modules
>> in laptops produced by different OEMs and ODMs, so it seems to be a
>> somewhat widespread problem at least.
>>
>> 4. My assumption that ath9k hardware is corrupting the MSI vector
>> number could be wrong; maybe another component is to blame, could it
>> be a BIOS issue? Admittedly I don't really know how I can debug the
>> layers inbetween seeing the MSI Message Data value disagree with the
>> vector number being handled inside do_IRQ().
>>
>> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi all,

Please drop my patches, Qualcomm is working internally and will submit
the MSI patch by themselves.
Thanks.

Hi Daniel,

I'll try your patches tomorrow.

Best regards,
AceLan Kao.

2017-10-02 12:21 GMT+08:00 Daniel Drake <dr...@endlessm.com>:
> Hi AceLan,
>
> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao <acelan@canonical.com> wrote:
>> Hi Daniel,
>>
>> I've tried your patch, but it doesn't work for me.
>> Wifi can scan AP, but can't get connected.
>
> Can you please clarify which patch(es) you have tried?
>
> This is the base patch which adds the infrastructure to request
> specific MSI IRQ vectors:
> https://marc.info/?l=linux-wireless=150631274108016=2
>
> This is the ath9k MSI patch which makes use of that:
> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>
> If you were already able to use ath9k MSI interrupts without specific
> consideration for which MSI vector numbers were used, these are the
> possible explanations that spring to mind:
>
> 1. You got lucky and it picked a vector number that is 4-aligned. You
> can check this in the "lspci -vvv" output. You'll see something like:
> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
> Address: fee0300c  Data: 4142
> The lower number is the vector number. In my example here 0x42 (66) is
> not 4-aligned so the failure condition will be hit.
>
> 2. You are using interrupt remapping, which I suspect may provide a
> high likelihood of MSI interrupt vectors being 4-aligned. See if
> /proc/interrupts shows the IRQ type as IR-PCI-MSI
> Unfortunately interrupt remapping is not available here,
> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>
> 3. My assumption that all ath9k hardware corrupts the MSI vector
> number could wrong. However we've seen this on different wifi modules
> in laptops produced by different OEMs and ODMs, so it seems to be a
> somewhat widespread problem at least.
>
> 4. My assumption that ath9k hardware is corrupting the MSI vector
> number could be wrong; maybe another component is to blame, could it
> be a BIOS issue? Admittedly I don't really know how I can debug the
> layers inbetween seeing the MSI Message Data value disagree with the
> vector number being handled inside do_IRQ().
>
> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi all,

Please drop my patches, Qualcomm is working internally and will submit
the MSI patch by themselves.
Thanks.

Hi Daniel,

I'll try your patches tomorrow.

Best regards,
AceLan Kao.

2017-10-02 12:21 GMT+08:00 Daniel Drake :
> Hi AceLan,
>
> On Thu, Sep 28, 2017 at 4:28 PM, AceLan Kao  wrote:
>> Hi Daniel,
>>
>> I've tried your patch, but it doesn't work for me.
>> Wifi can scan AP, but can't get connected.
>
> Can you please clarify which patch(es) you have tried?
>
> This is the base patch which adds the infrastructure to request
> specific MSI IRQ vectors:
> https://marc.info/?l=linux-wireless=150631274108016=2
>
> This is the ath9k MSI patch which makes use of that:
> https://github.com/endlessm/linux/commit/739c7a924db8f4434a9617657
>
> If you were already able to use ath9k MSI interrupts without specific
> consideration for which MSI vector numbers were used, these are the
> possible explanations that spring to mind:
>
> 1. You got lucky and it picked a vector number that is 4-aligned. You
> can check this in the "lspci -vvv" output. You'll see something like:
> Capabilities: [50] MSI: Enable+ Count=1/4 Maskable+ 64bit+
> Address: fee0300c  Data: 4142
> The lower number is the vector number. In my example here 0x42 (66) is
> not 4-aligned so the failure condition will be hit.
>
> 2. You are using interrupt remapping, which I suspect may provide a
> high likelihood of MSI interrupt vectors being 4-aligned. See if
> /proc/interrupts shows the IRQ type as IR-PCI-MSI
> Unfortunately interrupt remapping is not available here,
> https://lists.linuxfoundation.org/pipermail/iommu/2017-August/023717.html
>
> 3. My assumption that all ath9k hardware corrupts the MSI vector
> number could wrong. However we've seen this on different wifi modules
> in laptops produced by different OEMs and ODMs, so it seems to be a
> somewhat widespread problem at least.
>
> 4. My assumption that ath9k hardware is corrupting the MSI vector
> number could be wrong; maybe another component is to blame, could it
> be a BIOS issue? Admittedly I don't really know how I can debug the
> layers inbetween seeing the MSI Message Data value disagree with the
> vector number being handled inside do_IRQ().
>
> Daniel

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Christoph,

I'm okay with ether ways, but just worrying the regression if we
switch to use MSI by default.

Hi Daniel,

I've tried your patch, but it doesn't work for me.
Wifi can scan AP, but can't get connected.
It keeps showing
[   48.362297] wlp2s0: authenticate with 3c:ce:73:48:0e:40
[   48.374445] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 1/3)
[   49.824071] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 2/3)
[   50.848143] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 3/3)
[   51.840078] wlp2s0: authentication with 3c:ce:73:48:0e:40 timed out
[   52.038561] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   52.058930] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   52.832063] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   53.824135] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)
[   54.816067] wlp2s0: authentication with 3c:ce:73:48:04:80 timed out
[   55.904797] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   55.921771] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   56.800151] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   57.824072] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)

02:00.0 Network controller [0280]: Qualcomm Atheros QCA9565 / AR9565
Wireless Network Adapter [168c:0036] (rev 01)
   Subsystem: Dell QCA9565 / AR9565 Wireless Network Adapter [1028:020e]
   Kernel driver in use: ath9k
   Kernel modules: ath9k

Do you have a chance to see if my patch works on your side?

Best regards,
AceLan Kao.

2017-09-26 22:14 GMT+08:00 Kalle Valo <kv...@codeaurora.org>:
> Christoph Hellwig <h...@infradead.org> writes:
>
>> On Tue, Sep 26, 2017 at 03:26:51PM +0800, AceLan Kao wrote:
>>> Ath9k is an old driver for old chips, and they work fine with legacy INTx.
>>> But some new platforms are using it, so I think we should list those
>>> new platforms which blocks INTx in the driver.
>>
>> And unless they are broken and need a quirk they should work even better
>> with MSI if the device advertises MSI support in the PCI capabilities.
>
> Daniel Drake (CCed) already found problems with the MSI implementation:
>
> https://lkml.kernel.org/r/20170925041153.26574-1-dr...@endlessm.com
>
> --
> Kalle Valo

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Hi Christoph,

I'm okay with ether ways, but just worrying the regression if we
switch to use MSI by default.

Hi Daniel,

I've tried your patch, but it doesn't work for me.
Wifi can scan AP, but can't get connected.
It keeps showing
[   48.362297] wlp2s0: authenticate with 3c:ce:73:48:0e:40
[   48.374445] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 1/3)
[   49.824071] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 2/3)
[   50.848143] wlp2s0: send auth to 3c:ce:73:48:0e:40 (try 3/3)
[   51.840078] wlp2s0: authentication with 3c:ce:73:48:0e:40 timed out
[   52.038561] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   52.058930] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   52.832063] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   53.824135] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)
[   54.816067] wlp2s0: authentication with 3c:ce:73:48:04:80 timed out
[   55.904797] wlp2s0: authenticate with 3c:ce:73:48:04:80
[   55.921771] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 1/3)
[   56.800151] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 2/3)
[   57.824072] wlp2s0: send auth to 3c:ce:73:48:04:80 (try 3/3)

02:00.0 Network controller [0280]: Qualcomm Atheros QCA9565 / AR9565
Wireless Network Adapter [168c:0036] (rev 01)
   Subsystem: Dell QCA9565 / AR9565 Wireless Network Adapter [1028:020e]
   Kernel driver in use: ath9k
   Kernel modules: ath9k

Do you have a chance to see if my patch works on your side?

Best regards,
AceLan Kao.

2017-09-26 22:14 GMT+08:00 Kalle Valo :
> Christoph Hellwig  writes:
>
>> On Tue, Sep 26, 2017 at 03:26:51PM +0800, AceLan Kao wrote:
>>> Ath9k is an old driver for old chips, and they work fine with legacy INTx.
>>> But some new platforms are using it, so I think we should list those
>>> new platforms which blocks INTx in the driver.
>>
>> And unless they are broken and need a quirk they should work even better
>> with MSI if the device advertises MSI support in the PCI capabilities.
>
> Daniel Drake (CCed) already found problems with the MSI implementation:
>
> https://lkml.kernel.org/r/20170925041153.26574-1-dr...@endlessm.com
>
> --
> Kalle Valo

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Ath9k is an old driver for old chips, and they work fine with legacy INTx.
But some new platforms are using it, so I think we should list those
new platforms which blocks INTx in the driver.

BTW, new chips use ath10k driver.

2017-09-26 14:44 GMT+08:00 Christoph Hellwig <h...@infradead.org>:
> On Tue, Sep 26, 2017 at 02:41:35PM +0800, AceLan Kao wrote:
>> Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
>> for WLAN device. So adding a quirk to list those machines and set
>> use_msi automatically.
>> Adding Dell Inspiron 24-3460 to the quirk.
>
> Huh?  Using MSI should be the default, and skipping MSI should be
> a quirk if needed at all (usually it should be autodetected)

Re: [PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Ath9k is an old driver for old chips, and they work fine with legacy INTx.
But some new platforms are using it, so I think we should list those
new platforms which blocks INTx in the driver.

BTW, new chips use ath10k driver.

2017-09-26 14:44 GMT+08:00 Christoph Hellwig :
> On Tue, Sep 26, 2017 at 02:41:35PM +0800, AceLan Kao wrote:
>> Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
>> for WLAN device. So adding a quirk to list those machines and set
>> use_msi automatically.
>> Adding Dell Inspiron 24-3460 to the quirk.
>
> Huh?  Using MSI should be the default, and skipping MSI should be
> a quirk if needed at all (usually it should be autodetected)

[PATCH 5/6] ath9k: set use_msi=1 on Dell Vostro 15-3572

[AceLan Kao]

BIOS on Dell Vostro 15-3572 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index fce9ac7..e1198e1 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -128,6 +128,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 15-3572",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 6/6] ath9k: set use_msi=1 on Dell Inspiron 14-3473

[AceLan Kao]

BIOS on Dell Inspiron 14-3473 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index e1198e1..d03e1fc 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -136,6 +136,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 14-3473",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 5/6] ath9k: set use_msi=1 on Dell Vostro 15-3572

[AceLan Kao]

BIOS on Dell Vostro 15-3572 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index fce9ac7..e1198e1 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -128,6 +128,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 15-3572",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 6/6] ath9k: set use_msi=1 on Dell Inspiron 14-3473

[AceLan Kao]

BIOS on Dell Inspiron 14-3473 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index e1198e1..d03e1fc 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -136,6 +136,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 15-3572"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 14-3473",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 14-3473"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 4/6] ath9k: set use_msi=1 on Dell Inspiron 3472

[AceLan Kao]

BIOS on Dell Inspiron 3472 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 6b5d53c..fce9ac7 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -120,6 +120,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 3472",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 4/6] ath9k: set use_msi=1 on Dell Inspiron 3472

[AceLan Kao]

BIOS on Dell Inspiron 3472 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 6b5d53c..fce9ac7 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -120,6 +120,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 3472",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 3472"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 3/6] ath9k: set use_msi=1 on Dell Vostro 3262

[AceLan Kao]

BIOS on Dell Vostro 3262 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 1667949..6b5d53c 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -112,6 +112,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 3262",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+   },
+   },
{}
 };
 
-- 
2.7.4

[PATCH 1/6] ath9k: add MSI support and use_msi parameter

[AceLan Kao]

Adding MSI support for ath9k devices.
This patch is originally from Qualcomm, but they have no intention of
submitting and maintaining ath9k driver now.
The credit should go to Qualcomm.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/hw.c   | 33 ++--
 drivers/net/wireless/ath/ath9k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath9k/init.c |  4 +++
 drivers/net/wireless/ath/ath9k/mac.c  | 47 +++
 drivers/net/wireless/ath/ath9k/pci.c  | 21 +++-
 drivers/net/wireless/ath/ath9k/reg.h  | 15 +++
 6 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c 
b/drivers/net/wireless/ath/ath9k/hw.c
index 8c5c2dd..cd0f023 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -922,6 +922,7 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
AR_IMR_RXERR |
AR_IMR_RXORN |
AR_IMR_BCNMISC;
+   u32 msi_cfg = 0;
 
if (AR_SREV_9340(ah) || AR_SREV_9550(ah) || AR_SREV_9531(ah) ||
AR_SREV_9561(ah))
@@ -929,22 +930,30 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw 
*ah,
 
if (AR_SREV_9300_20_OR_LATER(ah)) {
imr_reg |= AR_IMR_RXOK_HP;
-   if (ah->config.rx_intr_mitigation)
+   if (ah->config.rx_intr_mitigation) {
imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+   } else {
imr_reg |= AR_IMR_RXOK_LP;
-
+   msi_cfg |= AR_INTCFG_MSI_RXOK;
+   }
} else {
-   if (ah->config.rx_intr_mitigation)
+   if (ah->config.rx_intr_mitigation) {
imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+   } else {
imr_reg |= AR_IMR_RXOK;
+   msi_cfg |= AR_INTCFG_MSI_RXOK;
+   }
}
 
-   if (ah->config.tx_intr_mitigation)
+   if (ah->config.tx_intr_mitigation) {
imr_reg |= AR_IMR_TXINTM | AR_IMR_TXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_TXINTM | AR_INTCFG_MSI_TXMINTR;
+   } else {
imr_reg |= AR_IMR_TXOK;
+   msi_cfg |= AR_INTCFG_MSI_TXOK;
+   }
 
ENABLE_REGWRITE_BUFFER(ah);
 
@@ -952,6 +961,16 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw 
*ah,
ah->imrs2_reg |= AR_IMR_S2_GTT;
REG_WRITE(ah, AR_IMR_S2, ah->imrs2_reg);
 
+   if (ah->msi_enabled) {
+   ah->msi_reg = REG_READ(ah, AR_PCIE_MSI);
+   ah->msi_reg |= AR_PCIE_MSI_HW_DBI_WR_EN;
+   ah->msi_reg &= AR_PCIE_MSI_HW_INT_PENDING_ADDR_MSI_64;
+   REG_WRITE(ah, AR_INTCFG, msi_cfg);
+   ath_dbg(ath9k_hw_common(ah), ANY,
+   "value of AR_INTCFG=0x%X, msi_cfg=0x%X\n",
+   REG_READ(ah, AR_INTCFG), msi_cfg);
+   }
+
if (!AR_SREV_9100(ah)) {
REG_WRITE(ah, AR_INTR_SYNC_CAUSE, 0x);
REG_WRITE(ah, AR_INTR_SYNC_ENABLE, sync_default);
diff --git a/drivers/net/wireless/ath/ath9k/hw.h 
b/drivers/net/wireless/ath/ath9k/hw.h
index 4ac7082..0d6c07c7 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -977,6 +977,9 @@ struct ath_hw {
bool tpc_enabled;
u8 tx_power[Ar5416RateSize];
u8 tx_power_stbc[Ar5416RateSize];
+   bool msi_enabled;
+   u32 msi_mask;
+   u32 msi_reg;
 };
 
 struct ath_bus_ops {
diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index bb79360..b6b7a35 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -75,6 +75,10 @@ MODULE_PARM_DESC(use_chanctx, "Enable channel context for 
concurrency");
 
 #endif /* CONFIG_ATH9K_CHANNEL_CONTEXT */
 
+int ath9k_use_msi;
+module_param_named(use_msi, ath9k_use_msi, int, 0444);
+MODULE_PARM_DESC(use_msi, "Use MSI instead of INTx if possible");
+
 bool is_ath9k_unloaded;
 
 #ifdef CONFIG_MAC80211_LEDS
diff --git a/drivers/net/wireless/ath/ath9k/mac.c 
b/drivers/net/wireless/ath/ath9k/mac.c
index 77c94f9..58d02c1 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -832,6 +832,43 @@ static void __ath9k_hw_enable_interrupts(struct ath_hw *ah)
}
ath_dbg(common, INTERRUPT, "AR_IMR 0x%x IER 0x%x\n",
REG_READ(ah, AR_IMR), REG_READ(ah, AR_IER));
+
+   if (ah->msi_enabled) {
+   u32 _msi_reg = 0;
+   u

[PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding Dell Inspiron 24-3460 to the quirk.

Signed-off-by: AceLan Kao <acelan@canonical.com>
---
 drivers/net/wireless/ath/ath9k/init.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index b6b7a35..1667949 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ath9k.h"
@@ -96,6 +97,24 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
 };
 #endif
 
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+   ath9k_use_msi = 1;
+   return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 24-3460",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+   },
+   },
+   {}
+};
+
 static void ath9k_deinit_softc(struct ath_softc *sc);
 
 static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1123,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
 
+   dmi_check_system(ath9k_quirks);
+
return 0;
 
  err_pci_exit:
-- 
2.7.4

[PATCH 1/6] ath9k: add MSI support and use_msi parameter

[AceLan Kao]

Adding MSI support for ath9k devices.
This patch is originally from Qualcomm, but they have no intention of
submitting and maintaining ath9k driver now.
The credit should go to Qualcomm.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/hw.c   | 33 ++--
 drivers/net/wireless/ath/ath9k/hw.h   |  3 +++
 drivers/net/wireless/ath/ath9k/init.c |  4 +++
 drivers/net/wireless/ath/ath9k/mac.c  | 47 +++
 drivers/net/wireless/ath/ath9k/pci.c  | 21 +++-
 drivers/net/wireless/ath/ath9k/reg.h  | 15 +++
 6 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/hw.c 
b/drivers/net/wireless/ath/ath9k/hw.c
index 8c5c2dd..cd0f023 100644
--- a/drivers/net/wireless/ath/ath9k/hw.c
+++ b/drivers/net/wireless/ath/ath9k/hw.c
@@ -922,6 +922,7 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw *ah,
AR_IMR_RXERR |
AR_IMR_RXORN |
AR_IMR_BCNMISC;
+   u32 msi_cfg = 0;
 
if (AR_SREV_9340(ah) || AR_SREV_9550(ah) || AR_SREV_9531(ah) ||
AR_SREV_9561(ah))
@@ -929,22 +930,30 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw 
*ah,
 
if (AR_SREV_9300_20_OR_LATER(ah)) {
imr_reg |= AR_IMR_RXOK_HP;
-   if (ah->config.rx_intr_mitigation)
+   if (ah->config.rx_intr_mitigation) {
imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+   } else {
imr_reg |= AR_IMR_RXOK_LP;
-
+   msi_cfg |= AR_INTCFG_MSI_RXOK;
+   }
} else {
-   if (ah->config.rx_intr_mitigation)
+   if (ah->config.rx_intr_mitigation) {
imr_reg |= AR_IMR_RXINTM | AR_IMR_RXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_RXINTM | AR_INTCFG_MSI_RXMINTR;
+   } else {
imr_reg |= AR_IMR_RXOK;
+   msi_cfg |= AR_INTCFG_MSI_RXOK;
+   }
}
 
-   if (ah->config.tx_intr_mitigation)
+   if (ah->config.tx_intr_mitigation) {
imr_reg |= AR_IMR_TXINTM | AR_IMR_TXMINTR;
-   else
+   msi_cfg |= AR_INTCFG_MSI_TXINTM | AR_INTCFG_MSI_TXMINTR;
+   } else {
imr_reg |= AR_IMR_TXOK;
+   msi_cfg |= AR_INTCFG_MSI_TXOK;
+   }
 
ENABLE_REGWRITE_BUFFER(ah);
 
@@ -952,6 +961,16 @@ static void ath9k_hw_init_interrupt_masks(struct ath_hw 
*ah,
ah->imrs2_reg |= AR_IMR_S2_GTT;
REG_WRITE(ah, AR_IMR_S2, ah->imrs2_reg);
 
+   if (ah->msi_enabled) {
+   ah->msi_reg = REG_READ(ah, AR_PCIE_MSI);
+   ah->msi_reg |= AR_PCIE_MSI_HW_DBI_WR_EN;
+   ah->msi_reg &= AR_PCIE_MSI_HW_INT_PENDING_ADDR_MSI_64;
+   REG_WRITE(ah, AR_INTCFG, msi_cfg);
+   ath_dbg(ath9k_hw_common(ah), ANY,
+   "value of AR_INTCFG=0x%X, msi_cfg=0x%X\n",
+   REG_READ(ah, AR_INTCFG), msi_cfg);
+   }
+
if (!AR_SREV_9100(ah)) {
REG_WRITE(ah, AR_INTR_SYNC_CAUSE, 0x);
REG_WRITE(ah, AR_INTR_SYNC_ENABLE, sync_default);
diff --git a/drivers/net/wireless/ath/ath9k/hw.h 
b/drivers/net/wireless/ath/ath9k/hw.h
index 4ac7082..0d6c07c7 100644
--- a/drivers/net/wireless/ath/ath9k/hw.h
+++ b/drivers/net/wireless/ath/ath9k/hw.h
@@ -977,6 +977,9 @@ struct ath_hw {
bool tpc_enabled;
u8 tx_power[Ar5416RateSize];
u8 tx_power_stbc[Ar5416RateSize];
+   bool msi_enabled;
+   u32 msi_mask;
+   u32 msi_reg;
 };
 
 struct ath_bus_ops {
diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index bb79360..b6b7a35 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -75,6 +75,10 @@ MODULE_PARM_DESC(use_chanctx, "Enable channel context for 
concurrency");
 
 #endif /* CONFIG_ATH9K_CHANNEL_CONTEXT */
 
+int ath9k_use_msi;
+module_param_named(use_msi, ath9k_use_msi, int, 0444);
+MODULE_PARM_DESC(use_msi, "Use MSI instead of INTx if possible");
+
 bool is_ath9k_unloaded;
 
 #ifdef CONFIG_MAC80211_LEDS
diff --git a/drivers/net/wireless/ath/ath9k/mac.c 
b/drivers/net/wireless/ath/ath9k/mac.c
index 77c94f9..58d02c1 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -832,6 +832,43 @@ static void __ath9k_hw_enable_interrupts(struct ath_hw *ah)
}
ath_dbg(common, INTERRUPT, "AR_IMR 0x%x IER 0x%x\n",
REG_READ(ah, AR_IMR), REG_READ(ah, AR_IER));
+
+   if (ah->msi_enabled) {
+   u32 _msi_reg = 0;
+   u32 i = 0;
+   u32 msi_pend_addr_mask = A

[PATCH 2/6] ath9k: add a quirk to set use_msi automatically

[AceLan Kao]

Some platform(BIOS) blocks legacy interrupts (INTx), and only allows MSI
for WLAN device. So adding a quirk to list those machines and set
use_msi automatically.
Adding Dell Inspiron 24-3460 to the quirk.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 21 +
 1 file changed, 21 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index b6b7a35..1667949 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "ath9k.h"
@@ -96,6 +97,24 @@ static const struct ieee80211_tpt_blink ath9k_tpt_blink[] = {
 };
 #endif
 
+static int __init set_use_msi(const struct dmi_system_id *dmi)
+{
+   ath9k_use_msi = 1;
+   return 1;
+}
+
+static const struct dmi_system_id ath9k_quirks[] __initconst = {
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Inspiron 24-3460",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
+   },
+   },
+   {}
+};
+
 static void ath9k_deinit_softc(struct ath_softc *sc);
 
 static void ath9k_op_ps_wakeup(struct ath_common *common)
@@ -1104,6 +1123,8 @@ static int __init ath9k_init(void)
goto err_pci_exit;
}
 
+   dmi_check_system(ath9k_quirks);
+
return 0;
 
  err_pci_exit:
-- 
2.7.4

[PATCH 3/6] ath9k: set use_msi=1 on Dell Vostro 3262

[AceLan Kao]

BIOS on Dell Vostro 3262 blocks legacy interrupts (INTx),
and only allows MSI for WLAN device.

Signed-off-by: AceLan Kao 
---
 drivers/net/wireless/ath/ath9k/init.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath9k/init.c 
b/drivers/net/wireless/ath/ath9k/init.c
index 1667949..6b5d53c 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -112,6 +112,14 @@ static const struct dmi_system_id ath9k_quirks[] 
__initconst = {
DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 24-3460"),
},
},
+   {
+   .callback = set_use_msi,
+   .ident = "Dell Vostro 3262",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+   DMI_MATCH(DMI_PRODUCT_NAME, "Vostro 3262"),
+   },
+   },
{}
 };
 
-- 
2.7.4

Re: [PATCH] intel-vbtn: match power button on press rather than release

[AceLan Kao]

Looks like I'm one hour late to ack the patch.
Thanks any way for the quick response.

2017-08-07 8:38 GMT+08:00 Rafael J. Wysocki :
> On Mon, Aug 7, 2017 at 1:24 AM, Darren Hart  wrote:
>> On Sun, Aug 06, 2017 at 12:20:22AM +0200, Rafael Wysocki wrote:
>>> On Saturday, August 5, 2017 10:57:53 PM CEST Darren Hart wrote:
>>> > On Sat, Aug 05, 2017 at 01:30:20AM +0200, Rafael Wysocki wrote:
>>> > > On Friday, August 4, 2017 7:29:53 PM CEST Darren Hart wrote:
>>> > > > On Fri, Aug 04, 2017 at 12:00:06PM -0500, Mario Limonciello wrote:
>>> > > > > This fixes a problem where the system gets stuck in a loop
>>> > > > > unable to wakeup via power button in s2idle.
>>> > > > >
>>> > > > > The problem happens because:
>>> > > > >  - press power button:
>>> > > > >- system emits 0xc0 (power press), event ignored
>>> > > > >- system emits 0xc1 (power release), event processed,
>>> > > > >  emited as KEY_POWER
>>> > > > >- set wakeup_mode to true
>>> > > > >- system goes to s2idle
>>> > > > >  - press power button
>>> > > > >- system emits 0xc0 (power press), wakeup_mode is true,
>>> > > > >  system wakes
>>> > > > >- system emits 0xc1 (power release), event processed,
>>> > > > >  emited as KEY_POWER
>>> > > > >- system goes to s2idle again
>>> > > > >
>>> > > > > To avoid this situation, process the presses (which matches what
>>> > > > > intel-hid does too).
>>> > > > >
>>> > > > > Verified on an Dell XPS 9365
>>> > > > >
>>> > > > > Signed-off-by: Mario Limonciello 
>>> > > >
>>> > > > This looks good to me - responding to the release is non-intuitive 
>>> > > > IMHO
>>> > > > anyway.  I'd like to see agreement from AceLan, but this should go 
>>> > > > into
>>> > > > the 4.13 rc cycle (at rc3 currently).
>>> > >
>>> > > Yes, it should, and I'm not sure if the original author's opinion 
>>> > > matters here.
>>> > >
>>> > > The patch makes sense to everyone involved and fixes an annoying issue, 
>>> > > so
>>> > > I don't see any real arguments against applying it.
>>> > >
>>> > > Please feel free to add my ACK to it if that helps.
>>> >
>>> > I always try to give those in MAINTAINERS a chance to respond, but I will 
>>> > queue
>>> > this up to fixes now.
>>>
>>> Thanks!
>>
>> Rafael,
>>
>> Merged to mainline (in case you were waiting on this for the dependent
>> patch you mentioned).
>
> Awesome, thanks!

Re: [PATCH] intel-vbtn: match power button on press rather than release

[AceLan Kao]

Looks like I'm one hour late to ack the patch.
Thanks any way for the quick response.

2017-08-07 8:38 GMT+08:00 Rafael J. Wysocki :
> On Mon, Aug 7, 2017 at 1:24 AM, Darren Hart  wrote:
>> On Sun, Aug 06, 2017 at 12:20:22AM +0200, Rafael Wysocki wrote:
>>> On Saturday, August 5, 2017 10:57:53 PM CEST Darren Hart wrote:
>>> > On Sat, Aug 05, 2017 at 01:30:20AM +0200, Rafael Wysocki wrote:
>>> > > On Friday, August 4, 2017 7:29:53 PM CEST Darren Hart wrote:
>>> > > > On Fri, Aug 04, 2017 at 12:00:06PM -0500, Mario Limonciello wrote:
>>> > > > > This fixes a problem where the system gets stuck in a loop
>>> > > > > unable to wakeup via power button in s2idle.
>>> > > > >
>>> > > > > The problem happens because:
>>> > > > >  - press power button:
>>> > > > >- system emits 0xc0 (power press), event ignored
>>> > > > >- system emits 0xc1 (power release), event processed,
>>> > > > >  emited as KEY_POWER
>>> > > > >- set wakeup_mode to true
>>> > > > >- system goes to s2idle
>>> > > > >  - press power button
>>> > > > >- system emits 0xc0 (power press), wakeup_mode is true,
>>> > > > >  system wakes
>>> > > > >- system emits 0xc1 (power release), event processed,
>>> > > > >  emited as KEY_POWER
>>> > > > >- system goes to s2idle again
>>> > > > >
>>> > > > > To avoid this situation, process the presses (which matches what
>>> > > > > intel-hid does too).
>>> > > > >
>>> > > > > Verified on an Dell XPS 9365
>>> > > > >
>>> > > > > Signed-off-by: Mario Limonciello 
>>> > > >
>>> > > > This looks good to me - responding to the release is non-intuitive 
>>> > > > IMHO
>>> > > > anyway.  I'd like to see agreement from AceLan, but this should go 
>>> > > > into
>>> > > > the 4.13 rc cycle (at rc3 currently).
>>> > >
>>> > > Yes, it should, and I'm not sure if the original author's opinion 
>>> > > matters here.
>>> > >
>>> > > The patch makes sense to everyone involved and fixes an annoying issue, 
>>> > > so
>>> > > I don't see any real arguments against applying it.
>>> > >
>>> > > Please feel free to add my ACK to it if that helps.
>>> >
>>> > I always try to give those in MAINTAINERS a chance to respond, but I will 
>>> > queue
>>> > this up to fixes now.
>>>
>>> Thanks!
>>
>> Rafael,
>>
>> Merged to mainline (in case you were waiting on this for the dependent
>> patch you mentioned).
>
> Awesome, thanks!

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

In the beginning, we just need the power button function, so I didn't
implement all the keys to the driver.
And we didn't get any further requirement from the following projects
we were working on,
so I'm not aware that there are machines other than Dell using this driver.

I'll try filling up all events on the spec later.

2017-04-25 13:01 GMT+08:00 Maarten Maathuis <madman2...@gmail.com>:
> On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao <acelan@canonical.com> wrote:
>> According the spec. I have, the values are correct.
>> Please merge it, thanks.
>>
>
> Is there a reason the whole spec isn't implemented?
> Is it under NDA?
>
>> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis <madman2...@gmail.com>:
>>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>>> <andy.shevche...@gmail.com> wrote:
>>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis <madman2...@gmail.com> 
>>>> wrote:
>>>>> Tested on HP Elite X2 1012 G1.
>>>>> Matches event report of Lenovo Helix 2
>>>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>>>
>>>>
>>>> Much better!
>>>>
>>>>> V2: Fix indent and add sign-off
>>>>
>>>> Usually this line goes after --- (body delimiter).
>>>> No need to resend this time. I would wait a bit for actual
>>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>>> to me.
>>>
>>> The intent is not have this in the commit message?
>>> I'll keep an eye out if i can place it below "---" next time.
>>> Although i suspect the message would end in the actual code diff,
>>> which seems odd.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Maarten Maathuis <madman2...@shikahr.net>
>>>>> ---
>>>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>>>> b/drivers/platform/x86/intel-vbtn.c
>>>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>>>> --- a/drivers/platform/x86/intel-vbtn.c
>>>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release 
>>>>> */
>>>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press 
>>>>> */
>>>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down 
>>>>> key release */
>>>>> { KE_END },
>>>>>  };
>>>>>
>>>>> --
>>>>> 2.12.2
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> With Best Regards,
>>>> Andy Shevchenko
>>>
>>>
>>>
>>> --
>>> Far away from the primal instinct, the song seems to fade away, the
>>> river get wider between your thoughts and the things we do and say.
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

In the beginning, we just need the power button function, so I didn't
implement all the keys to the driver.
And we didn't get any further requirement from the following projects
we were working on,
so I'm not aware that there are machines other than Dell using this driver.

I'll try filling up all events on the spec later.

2017-04-25 13:01 GMT+08:00 Maarten Maathuis :
> On Tue, Apr 25, 2017 at 4:43 AM, AceLan Kao  wrote:
>> According the spec. I have, the values are correct.
>> Please merge it, thanks.
>>
>
> Is there a reason the whole spec isn't implemented?
> Is it under NDA?
>
>> 2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
>>> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>>>  wrote:
>>>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>>>> wrote:
>>>>> Tested on HP Elite X2 1012 G1.
>>>>> Matches event report of Lenovo Helix 2
>>>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>>>
>>>>
>>>> Much better!
>>>>
>>>>> V2: Fix indent and add sign-off
>>>>
>>>> Usually this line goes after --- (body delimiter).
>>>> No need to resend this time. I would wait a bit for actual
>>>> author/driver maintainer to comment. Otherwise patch looks good enough
>>>> to me.
>>>
>>> The intent is not have this in the commit message?
>>> I'll keep an eye out if i can place it below "---" next time.
>>> Although i suspect the message would end in the actual code diff,
>>> which seems odd.
>>>
>>>>
>>>>>
>>>>> Signed-off-by: Maarten Maathuis 
>>>>> ---
>>>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>>>  1 file changed, 4 insertions(+)
>>>>>
>>>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>>>> b/drivers/platform/x86/intel-vbtn.c
>>>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>>>> --- a/drivers/platform/x86/intel-vbtn.c
>>>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release 
>>>>> */
>>>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press 
>>>>> */
>>>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down 
>>>>> key release */
>>>>> { KE_END },
>>>>>  };
>>>>>
>>>>> --
>>>>> 2.12.2
>>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> With Best Regards,
>>>> Andy Shevchenko
>>>
>>>
>>>
>>> --
>>> Far away from the primal instinct, the song seems to fade away, the
>>> river get wider between your thoughts and the things we do and say.
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.



-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

According the spec. I have, the values are correct.
Please merge it, thanks.

2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>  wrote:
>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>> wrote:
>>> Tested on HP Elite X2 1012 G1.
>>> Matches event report of Lenovo Helix 2
>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>
>>
>> Much better!
>>
>>> V2: Fix indent and add sign-off
>>
>> Usually this line goes after --- (body delimiter).
>> No need to resend this time. I would wait a bit for actual
>> author/driver maintainer to comment. Otherwise patch looks good enough
>> to me.
>
> The intent is not have this in the commit message?
> I'll keep an eye out if i can place it below "---" next time.
> Although i suspect the message would end in the actual code diff,
> which seems odd.
>
>>
>>>
>>> Signed-off-by: Maarten Maathuis 
>>> ---
>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>> b/drivers/platform/x86/intel-vbtn.c
>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>> --- a/drivers/platform/x86/intel-vbtn.c
>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>>> release */
>>> { KE_END },
>>>  };
>>>
>>> --
>>> 2.12.2
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

Re: [PATCH] platform/x86/intel-vbtn: add volume up and down

[AceLan Kao]

According the spec. I have, the values are correct.
Please merge it, thanks.

2017-04-25 5:41 GMT+08:00 Maarten Maathuis :
> On Mon, Apr 24, 2017 at 11:37 PM, Andy Shevchenko
>  wrote:
>> On Tue, Apr 25, 2017 at 12:29 AM, Maarten Maathuis  
>> wrote:
>>> Tested on HP Elite X2 1012 G1.
>>> Matches event report of Lenovo Helix 2
>>> (https://www.spinics.net/lists/ibm-acpi-devel/msg03982.html).
>>>
>>
>> Much better!
>>
>>> V2: Fix indent and add sign-off
>>
>> Usually this line goes after --- (body delimiter).
>> No need to resend this time. I would wait a bit for actual
>> author/driver maintainer to comment. Otherwise patch looks good enough
>> to me.
>
> The intent is not have this in the commit message?
> I'll keep an eye out if i can place it below "---" next time.
> Although i suspect the message would end in the actual code diff,
> which seems odd.
>
>>
>>>
>>> Signed-off-by: Maarten Maathuis 
>>> ---
>>>  drivers/platform/x86/intel-vbtn.c | 4 
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/platform/x86/intel-vbtn.c 
>>> b/drivers/platform/x86/intel-vbtn.c
>>> index 554e82ebe83c..1616cb9c4ae5 100644
>>> --- a/drivers/platform/x86/intel-vbtn.c
>>> +++ b/drivers/platform/x86/intel-vbtn.c
>>> @@ -37,6 +37,10 @@ static const struct acpi_device_id intel_vbtn_ids[] = {
>>>  static const struct key_entry intel_vbtn_keymap[] = {
>>> { KE_IGNORE, 0xC0, { KEY_POWER } }, /* power key press */
>>> { KE_KEY, 0xC1, { KEY_POWER } },/* power key release */
>>> +   { KE_KEY, 0xC4, { KEY_VOLUMEUP} },  /* volume-up key press */
>>> +   { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } },  /* volume-up key release */
>>> +   { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } },   /* volume-down key press */
>>> +   { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } },/* volume-down key 
>>> release */
>>> { KE_END },
>>>  };
>>>
>>> --
>>> 2.12.2
>>>
>>
>>
>>
>> --
>> With Best Regards,
>> Andy Shevchenko
>
>
>
> --
> Far away from the primal instinct, the song seems to fade away, the
> river get wider between your thoughts and the things we do and say.

Re: [PATCH] ACPI: Override rev for DELL Latitude 3350

[AceLan Kao]

Hi Rafael,

The patch you mentioned is pretty the same as mine,
I'm happy to see that patch goes into upstream.
Please keep doing this, thanks.

Best regards,
AceLan Kao.


2016-11-22 6:43 GMT+08:00 Rafael J. Wysocki <r...@rjwysocki.net>:
> On Monday, November 21, 2016 10:07:05 AM AceLan Kao wrote:
>> The ethernet network fails to work on DELL Latitude 3350 after this commit
>>ea7d521 Revert 'Revert "ACPICA: Permanently set _REV to the value '2'."'
>>
>> dmesg shows
>>r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
>>r8169 :03:00.0: can't disable ASPM; OS doesn't have ASPM control
>>pci :00:1c.2: PCI INT C: failed to register GSI
>>pci :00:1c.2: Error enabling bridge (-16), continuing
>>r8169 :03:00.0: PCI INT A: failed to register GSI
>>r8169 :03:00.0 (unnamed net_device) (uninitialized): enable failure
>>r8169: probe of :03:00.0 failed with error -16
>>
>> So override the ACPI _REV for this machine to fix the issue.
>>
>> Signed-off-by: AceLan Kao <acelan@canonical.com>
>
> I have something similar queued up in linux-next.  Care to look at that?
>
> http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next=708f5dcc21ae9b35f395865fc154b0105baf4de4
>
>> ---
>>  drivers/acpi/blacklist.c | 8 
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/acpi/blacklist.c b/drivers/acpi/blacklist.c
>> index bdc67ba..a8c10f2 100644
>> --- a/drivers/acpi/blacklist.c
>> +++ b/drivers/acpi/blacklist.c
>> @@ -160,6 +160,14 @@ static struct dmi_system_id acpi_rev_dmi_table[] 
>> __initdata = {
>> DMI_MATCH(DMI_PRODUCT_NAME, "XPS 13 9343"),
>>   },
>>   },
>> + {
>> +  .callback = dmi_enable_rev_override,
>> +  .ident = "DELL Latitude 3350",
>> +  .matches = {
>> +   DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
>> +   DMI_MATCH(DMI_PRODUCT_NAME, "Latitude 3350"),
>> + },
>> + },
>>  #endif
>>   {}
>>  };
>>
>
> Thanks,
> Rafael
>