[PATCH] x86/tsc: mark tsc reliable on CoffeeLake
From: You-Sheng Yang On Intel CoffeeLake it's observed tsc is always marked unstable unexpectedly after entering idle state Package C10(PC10), and then clock source is switched to hpet. This patch marks tsc as reliable when CPUID matches CoffeeLake. Link: https://bugzilla.kernel.org/show_bug.cgi?id=203183 Signed-off-by: You-Sheng Yang --- arch/x86/kernel/tsc.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c index aab0c82e0a0d..2abbadc9cff0 100644 --- a/arch/x86/kernel/tsc.c +++ b/arch/x86/kernel/tsc.c @@ -1161,6 +1161,16 @@ static void __init check_system_tsc_reliable(void) #endif if (boot_cpu_has(X86_FEATURE_TSC_RELIABLE)) tsc_clocksource_reliable = 1; + + /* +* On Intel CoffeeLake, tsc may be marked unstable unexpectedly after +* entering PC10. +*/ + if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && + (boot_cpu_data.x86_model == INTEL_FAM6_KABYLAKE_MOBILE || +boot_cpu_data.x86_model == INTEL_FAM6_KABYLAKE_DESKTOP) && + boot_cpu_data.x86_stepping >= 0x0a) + tsc_clocksource_reliable = 1; } /* -- 2.20.1
Re: [PATCH] x86/tsc: mark tsc reliable on CoffeeLake
On 2019/4/8 8:03 PM, Thomas Gleixner wrote: > On Mon, 8 Apr 2019, You-Sheng Yang wrote: > >> From: You-Sheng Yang >> >> On Intel CoffeeLake it's observed tsc is always marked unstable >> unexpectedly after entering idle state Package C10(PC10), and then clock >> source is switched to hpet. This patch marks tsc as reliable when CPUID >> matches CoffeeLake. > > This lacks a proper analysis: > > 1) Why is it marked unstable Usually the differences between wd_nsec and cs_nsec in function clocksource_watchdog in kernel/time/clocksource.c would be less than a few thousand nanoseconds. However, when CPU is entering deeper idle state, PC10, the hpet clocksource readings starts to give inaccurate values for unknown reason, and the differences to cs_nsec varies from a few hundred nanoseconds to several hundred millisecond, which is larger than WATCHDOG_THRESHOLD (62.5ms) and finally results in tsc being marked unreliable. No HPET overflow is found when this occurs. > 2) Why is it correct to set that for coffeelake So far this strange behaviour is only found on coffeelake. Besides this, no much I can tell actually. This could be probably wrong, but may serve as a start to bring up some more discussion/investigation to solve the problem. I would be more than willing to help verifying further appropriate fixes. Thank you. You-Sheng Yang
[PATCH] HID: intel-ish-hid: ipc: Add Tiger Lake H PCI device ID
Added Tiger Lake H PCI device ID to the supported device list.
Signed-off-by: You-Sheng Yang
---
drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
drivers/hid/intel-ish-hid/ipc/pci-ish.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
index 1fb294ca463e..21b0e6123754 100644
--- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
+++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
@@ -27,6 +27,7 @@
#define CMP_H_DEVICE_ID0x06FC
#define EHL_Ax_DEVICE_ID 0x4BB3
#define TGL_LP_DEVICE_ID 0xA0FC
+#define TGL_H_DEVICE_ID0x43FC
#defineREVISION_ID_CHT_A0 0x6
#defineREVISION_ID_CHT_Ax_SI 0x0
diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index c6d48a8648b7..6dea657b7b15 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -37,6 +37,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, CMP_H_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, EHL_Ax_DEVICE_ID)},
{PCI_DEVICE(PCI_VENDOR_ID_INTEL, TGL_LP_DEVICE_ID)},
+ {PCI_DEVICE(PCI_VENDOR_ID_INTEL, TGL_H_DEVICE_ID)},
{0, }
};
MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
--
2.29.2
Re: [PATCH] HID: intel-ish-hid: ipc: Add Tiger Lake H PCI device ID
On 2/4/21 11:13 PM, Srinivas Pandruvada wrote:
> On Thu, 2021-02-04 at 16:33 +0800, You-Sheng Yang wrote:
>> Added Tiger Lake H PCI device ID to the supported device list.
>>
>> Signed-off-by: You-Sheng Yang
> Did you get chance to verify on a platform?
> Do you see sensors enumerated in /sys/bus/iio?
Yes,
[2.485650] ish-hid {33AECD58-B679-4E54-9BD9-A04D34F0C226}:
[hid-ish]: enum_devices_done OK, num_hid_devices=3
[2.497337] hid-generic 001F:8087:0AC2.0003: hidraw2: HID
v2.00 Device [hid-ishtp 8087:0AC2] on
[2.506945] hid-generic 001F:8087:0AC2.0004: hidraw3: HID
v2.00 Device [hid-ishtp 8087:0AC2] on
[2.512127] hid-generic 001F:8087:0AC2.0005: hidraw4: HID
v2.00 Device [hid-ishtp 8087:0AC2] on
$ ls /sys/bus/iio/devices/
iio:device0 iio:device1 trigger0 trigger1
One of them is an ambient light sensor, and it works only with this
patch applied.
Thanks,
You-Sheng Yang
> Thanks,
> Srinivas
>
>> ---
>> drivers/hid/intel-ish-hid/ipc/hw-ish.h | 1 +
>> drivers/hid/intel-ish-hid/ipc/pci-ish.c | 1 +
>> 2 files changed, 2 insertions(+)
>>
>> diff --git a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
>> b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
>> index 1fb294ca463e..21b0e6123754 100644
>> --- a/drivers/hid/intel-ish-hid/ipc/hw-ish.h
>> +++ b/drivers/hid/intel-ish-hid/ipc/hw-ish.h
>> @@ -27,6 +27,7 @@
>> #define CMP_H_DEVICE_ID 0x06FC
>> #define EHL_Ax_DEVICE_ID0x4BB3
>> #define TGL_LP_DEVICE_ID0xA0FC
>> +#define TGL_H_DEVICE_ID 0x43FC
>>
>> #define REVISION_ID_CHT_A0 0x6
>> #define REVISION_ID_CHT_Ax_SI 0x0
>> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
>> b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
>> index c6d48a8648b7..6dea657b7b15 100644
>> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
>> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
>> @@ -37,6 +37,7 @@ static const struct pci_device_id ish_pci_tbl[] = {
>> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, CMP_H_DEVICE_ID)},
>> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, EHL_Ax_DEVICE_ID)},
>> {PCI_DEVICE(PCI_VENDOR_ID_INTEL, TGL_LP_DEVICE_ID)},
>> +{PCI_DEVICE(PCI_VENDOR_ID_INTEL, TGL_H_DEVICE_ID)},
>> {0, }
>> };
>> MODULE_DEVICE_TABLE(pci, ish_pci_tbl);
>
iwlwifi may wrongly cast a iwl_cfg_trans_params to iwl_cfg
Hi,
With an Intel AX201 Wi-Fi [8086:43f0] subsystem [1a56:1652] pcie card,
device fails to load firmware with following error messages:
Intel(R) Wireless WiFi driver for Linux
iwlwifi :00:14.3: enabling device ( -> 0002)
iwlwifi :00:14.3: Direct firmware load for (efault)128.ucode
failed with error -2
iwlwifi :00:14.3: Direct firmware load for (efault)127.ucode
failed with error -2
...
iwlwifi :00:14.3: Direct firmware load for (efault)0.ucode failed
with error -2
iwlwifi :00:14.3: no suitable firmware found!
iwlwifi :00:14.3: minimum version required: (efault)0
iwlwifi :00:14.3: maximum version supported: (efault)128
This is also reported on some public forums:
*
https://askubuntu.com/questions/1297311/ubuntu-20-04-wireless-not-working-for-intel-ax1650i-lenovo-thinkpad
*
https://www.reddit.com/r/pop_os/comments/jxmnre/wifi_and_bluetooth_issues_in_2010/
In drivers/net/wireless/intel/iwlwifi/pcie/drv.c:
static const struct pci_device_id iwl_hw_card_ids[] = {
{IWL_PCI_DEVICE(0x4232, 0x1201, iwl5100_agn_cfg)},
...
{IWL_PCI_DEVICE(0x43F0, PCI_ANY_ID, iwl_qu_long_latency_trans_cfg)},
...
};
The third argument to IWL_PCI_DEVICE macro will be assigned to
driver_data field of struct pci_device_id. However, iwl5100_agn_cfg has
type struct iwl_cfg, and yet iwl_qu_long_latency_trans_cfg is typed
struct iwl_cfg_trans_params.
struct iwl_cfg_trans_params {
...
};
struct iwl_cfg {
struct iwl_cfg_trans_params trans;
const char *name;
const char *fw_name_pre;
...
};
It's fine to cast a pointer to struct iwl_cfg, but it's not always valid
to cast a struct iwl_cfg_trans_params to struct iwl_cfg.
In function iwl_pci_probe, it tries to find an alternative cfg by
iterating throughout iwl_dev_info_table, but in our case, [8086:43f0]
subsystem [1a56:1652], there will be no match in all of the candidates,
and iwl_qu_long_latency_trans_cfg will be assigned as the ultimate
struct iwl_cfg, which will be certainly wrong when you're trying to
dereference anything beyond sizeof(struct iwl_cfg_trans_params), e.g.
cfg->fw_name_pre.
In this case, ((struct
iwl_cfg_trans_params*)&iwl_qu_long_latency_trans_cfg)->name will be "'",
and ((struct
iwl_cfg_trans_params*)&iwl_qu_long_latency_trans_cfg)->fw_name_pre gives
"(efault)", pure garbage data.
So is there something missed in the iwl_dev_info_table, or better, just
find another solid safe way to handle such trans/cfg mix?
Regards,
You-Sheng Yang
[PATCH] iwlwifi: fw: don't send GEO_TX_POWER_LIMIT command to FW version 29
Follow-up for commit fddbfeece9c7 ("iwlwifi: fw: don't send
GEO_TX_POWER_LIMIT command to FW version 36"). There is no
GEO_TX_POWER_LIMIT command support for all revisions of FW version
29, either.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204151
Signed-off-by: You-Sheng Yang
---
drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
index 32a5e4e5461f..dbba616c19de 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
@@ -889,14 +889,14 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm *mvm)
* firmware versions. Unfortunately, we don't have a TLV API
* flag to rely on, so rely on the major version which is in
* the first byte of ucode_ver. This was implemented
-* initially on version 38 and then backported to29 and 17.
+* initially on version 38 and then backported to 29 and 17.
* The intention was to have it in 36 as well, but not all
* 8000 family got this feature enabled. The 8000 family is
* the only one using version 36, so skip this version
-* entirely.
+* entirely. All revisions of -29 fw still don't have
+* GEO_TX_POWER_LIMIT supported yet.
*/
return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
- IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
}
--
2.20.1
Re: [PATCH] iwlwifi: fw: don't send GEO_TX_POWER_LIMIT command to FW version 29
Tested and commented on the issue page. Thank you for the correction.
--
Cheers,
You-Sheng Yang
On 2019-10-08 15:17, Luciano Coelho wrote:
> On Tue, 2019-10-08 at 14:05 +0800, You-Sheng Yang wrote:
>> Follow-up for commit fddbfeece9c7 ("iwlwifi: fw: don't send
>> GEO_TX_POWER_LIMIT command to FW version 36"). There is no
>> GEO_TX_POWER_LIMIT command support for all revisions of FW version
>> 29, either.
>>
>> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=204151
>> Signed-off-by: You-Sheng Yang
>> ---
>> drivers/net/wireless/intel/iwlwifi/mvm/fw.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
>> b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
>> index 32a5e4e5461f..dbba616c19de 100644
>> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
>> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
>> @@ -889,14 +889,14 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm
>> *mvm)
>> * firmware versions. Unfortunately, we don't have a TLV API
>> * flag to rely on, so rely on the major version which is in
>> * the first byte of ucode_ver. This was implemented
>> - * initially on version 38 and then backported to29 and 17.
>> + * initially on version 38 and then backported to 29 and 17.
>> * The intention was to have it in 36 as well, but not all
>> * 8000 family got this feature enabled. The 8000 family is
>> * the only one using version 36, so skip this version
>> - * entirely.
>> + * entirely. All revisions of -29 fw still don't have
>> + * GEO_TX_POWER_LIMIT supported yet.
>> */
>> return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
>> - IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
>> IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
>> }
>
> Thanks for the patch!
>
> But I have investigated this (even) further and now I see that 3168
> doesn't have this command, but 7265D does. The latter also uses -29,
> so we can't blindly disable all -29 versions.
>
> Can you try this instead?
>
> diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> index 0d2229319261..38d89ee9bd28 100644
> --- a/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> +++ b/drivers/net/wireless/intel/iwlwifi/mvm/fw.c
> @@ -906,8 +906,10 @@ static bool iwl_mvm_sar_geo_support(struct iwl_mvm
> *mvm)
> * entirely.
> */
> return IWL_UCODE_SERIAL(mvm->fw->ucode_ver) >= 38 ||
> - IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 ||
> - IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17;
> + IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 17 ||
> + (IWL_UCODE_SERIAL(mvm->fw->ucode_ver) == 29 &&
> + (mvm->trans->hw_rev &
> +CSR_HW_REV_TYPE_MSK) == CSR_HW_REV_TYPE_7265D);
> }
>
> int iwl_mvm_get_sar_geo_profile(struct iwl_mvm *mvm)
>
>
> --
> Cheers,
> Luca.
>
signature.asc
Description: OpenPGP digital signature
[PATCH 1/2] HID: i2c-hid: allow delay after SET_POWER
According to HID over I2C specification v1.0 section 7.2.8, a device is
allowed to take at most 1 second to make the transition to the specified
power state. On some touchpad devices implements Microsoft Precision
Touchpad, it may fail to execute following set PTP mode command without
the delay and leaves the device in an unsupported Mouse mode.
This change adds a post-setpower-delay-ms device property that allows
specifying the delay after a SET_POWER command is issued.
References: https://bugzilla.kernel.org/show_bug.cgi?id=204991
Signed-off-by: You-Sheng Yang
---
.../bindings/input/hid-over-i2c.txt | 2 +
drivers/hid/i2c-hid/i2c-hid-core.c| 46 +++
include/linux/platform_data/i2c-hid.h | 3 ++
3 files changed, 32 insertions(+), 19 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index c76bafaf98d2f..d82faae335da0 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -32,6 +32,8 @@ device-specific compatible properties, which should be used
in addition to the
- vdd-supply: phandle of the regulator that provides the supply voltage.
- post-power-on-delay-ms: time required by the device after enabling its
regulators
or powering it on, before it is ready for communication.
+- post-setpower-delay-ms: time required by the device after a SET_POWER command
+ before it finished the state transition.
Example:
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index 2a7c6e33bb1c4..a5bc2786dc440 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -168,6 +168,7 @@ static const struct i2c_hid_quirks {
__u16 idVendor;
__u16 idProduct;
__u32 quirks;
+ __u32 post_setpower_delay_ms;
} i2c_hid_quirks[] = {
{ USB_VENDOR_ID_WEIDA, HID_ANY_ID,
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
@@ -189,21 +190,20 @@ static const struct i2c_hid_quirks {
* i2c_hid_lookup_quirk: return any quirks associated with a I2C HID device
* @idVendor: the 16-bit vendor ID
* @idProduct: the 16-bit product ID
- *
- * Returns: a u32 quirks value.
*/
-static u32 i2c_hid_lookup_quirk(const u16 idVendor, const u16 idProduct)
+static void i2c_hid_set_quirk(struct i2c_hid *ihid,
+ const u16 idVendor, const u16 idProduct)
{
- u32 quirks = 0;
int n;
for (n = 0; i2c_hid_quirks[n].idVendor; n++)
if (i2c_hid_quirks[n].idVendor == idVendor &&
(i2c_hid_quirks[n].idProduct == (__u16)HID_ANY_ID ||
-i2c_hid_quirks[n].idProduct == idProduct))
- quirks = i2c_hid_quirks[n].quirks;
-
- return quirks;
+i2c_hid_quirks[n].idProduct == idProduct)) {
+ ihid->quirks = i2c_hid_quirks[n].quirks;
+ ihid->pdata.post_setpower_delay_ms =
+ i2c_hid_quirks[n].post_setpower_delay_ms;
+ }
}
static int __i2c_hid_command(struct i2c_client *client,
@@ -431,8 +431,22 @@ static int i2c_hid_set_power(struct i2c_client *client,
int power_state)
power_state == I2C_HID_PWR_SLEEP)
ihid->sleep_delay = jiffies + msecs_to_jiffies(20);
- if (ret)
+ if (ret) {
dev_err(&client->dev, "failed to change power setting.\n");
+ goto set_pwr_exit;
+ }
+
+ /*
+* The HID over I2C specification states that if a DEVICE needs time
+* after the PWR_ON request, it should utilise CLOCK stretching.
+* However, it has been observered that the Windows driver provides a
+* 1ms sleep between the PWR_ON and RESET requests and that some devices
+* rely on this.
+*/
+ if (ihid->pdata.post_setpower_delay_ms)
+ msleep(ihid->pdata.post_setpower_delay_ms);
+ else
+ usleep_range(1000, 5000);
set_pwr_exit:
return ret;
@@ -456,15 +470,6 @@ static int i2c_hid_hwreset(struct i2c_client *client)
if (ret)
goto out_unlock;
- /*
-* The HID over I2C specification states that if a DEVICE needs time
-* after the PWR_ON request, it should utilise CLOCK stretching.
-* However, it has been observered that the Windows driver provides a
-* 1ms sleep between the PWR_ON and RESET requests and that some devices
-* rely on this.
-*/
- usleep_range(1000, 5000);
-
i2c_hid_dbg(ihid, "resetting...\n");
ret = i2c_hid_command(client, &hid_reset_cmd, NULL, 0);
@@ -1023,6 +1028,9 @@ static void i2c_hid_fwnode_probe(struct i2c_client
*client,
if (!device_property_read_u32(&client->dev, "p
[PATCH 2/2] HID: i2c-hid: add 60ms SET_POWER delay for Goodix touchpad
Goodix touchpad 27C6:01F0 fails to switch to PTP mode when resumed from
suspend. The traffic after resumed looks like:
[ 275.312190] i2c_hid i2c-DELL096E:00: i2c_hid_set_power
[ 275.312191] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 01 08
[ 283.926905] i2c_hid i2c-DELL096E:00: i2c_hid_set_power
[ 283.926910] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 00 08
[ 283.927146] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report
[ 283.927149] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 37 03 06
00 05 00 07 00 00
[ 283.927872] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report
[ 283.927874] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 33 03 06
00 05 00 03 03 00
[ 283.929148] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report
[ 283.929151] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 35 03 06
00 05 00 05 03 00
[ 289.262675] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 00 00 00 00 00 00 00
[ 289.270314] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fe 00 00 00 00 00 00
[ 289.276806] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fd 00 00 00 00 00 00
...
The time delay between i2c_hid_set_power and i2c_hid_set_or_send_report
is less than vendor recommended 60ms, so it failed to complete its power
state transition, ignored i2c_hid_set_or_send_report and is still
operating in legacy mouse mode, and therefore it gives unsupported input
reports.
This change updates the quirk for the device to specifies a 60ms
post-setpower-delay-ms.
References: https://bugzilla.kernel.org/show_bug.cgi?id=204991
Signed-off-by: You-Sheng Yang
---
drivers/hid/i2c-hid/i2c-hid-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
b/drivers/hid/i2c-hid/i2c-hid-core.c
index a5bc2786dc440..8c01ce33f1c61 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -180,7 +180,7 @@ static const struct i2c_hid_quirks {
{ USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
I2C_HID_QUIRK_NO_RUNTIME_PM },
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_01F0,
- I2C_HID_QUIRK_NO_RUNTIME_PM },
+ I2C_HID_QUIRK_NO_RUNTIME_PM, 60 },
{ USB_VENDOR_ID_ELAN, HID_ANY_ID,
I2C_HID_QUIRK_BOGUS_IRQ },
{ 0, 0 }
--
2.23.0
[PATCH 0/2] HID: i2c-hid: add 60ms SET_POWER delay for Goodix touchpad
I2C traffic after resumed from s2idle: [ 275.312190] i2c_hid i2c-DELL096E:00: i2c_hid_set_power [ 275.312191] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 01 08 [ 283.926905] i2c_hid i2c-DELL096E:00: i2c_hid_set_power [ 283.926910] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 00 08 [ 283.927146] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report [ 283.927149] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 37 03 06 00 05 00 07 00 00 [ 283.927872] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report [ 283.927874] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 33 03 06 00 05 00 03 03 00 [ 283.929148] i2c_hid i2c-DELL096E:00: i2c_hid_set_or_send_report [ 283.929151] i2c_hid i2c-DELL096E:00: __i2c_hid_command: cmd=05 00 35 03 06 00 05 00 05 03 00 # when touching touchpad: [ 289.262675] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 00 00 00 00 00 00 00 [ 289.270314] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fe 00 00 00 00 00 00 [ 289.276806] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fd 00 00 00 00 00 00 [ 289.283863] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fb 01 00 00 00 00 00 [ 289.291213] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 fa 02 00 00 00 00 00 [ 289.297932] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 f9 03 00 00 00 00 00 [ 289.304775] i2c_hid i2c-DELL096E:00: input: 0b 00 01 00 f9 03 00 00 00 00 00 On this device (Goodix touchpad 27C6:01F0), both Precision Touchpad mode and a legacy Mouse mode are supported. When I2C SET_POWER ON is issued, it would take as long as 60ms to complete state transition and is then able to receive and execute further commands. On boot the device is running under Mouse mode for legacy platform support, and will be put to PTP mode with a SET_OR_SEND_REPORT command. If somehow the time gap between the first SET_POWER ON and further SET_OR_SEND_REPORT commands is less than 60ms, the device will still operating under Mouse mode and gives reports like "0b 00 01 00 fe 00 00 00 00 00 00". On Linux system boot, it may take 500ms from i2c probe (SET_POWER ON) to hid initialization (SET_OR_SEND_REPORT), so it will always be put to PTP mode successfully. But when the device is put under s2idle suspend and resumed, the hid reset resume process will be right after i2c resume, so the device won't have enough time to complete its state transition, which fails following SET_OR_SEND_REPORT executing and therefore it operates under Mouse mode instead. Currently Linux has a at most 5ms sleep in i2c_hid_hwreset(), but according to HID over I2C Specification[1] section 7.2.8 "SET_POWER": > The DEVICE must ensure that it transitions to the HOST > specified Power State in under 1 second. it can take as long as 1 second. This changeset add a device property post-setpower-delay-ms and use it to specify the delay after a SET_POWER command is issued for this device. References: https://bugzilla.kernel.org/show_bug.cgi?id=204991 You-Sheng Yang (2): HID: i2c-hid: allow delay after SET_POWER HID: i2c-hid: add 60ms SET_POWER delay for Goodix touchpad .../bindings/input/hid-over-i2c.txt | 2 + drivers/hid/i2c-hid/i2c-hid-core.c| 48 +++ include/linux/platform_data/i2c-hid.h | 3 ++ 3 files changed, 33 insertions(+), 20 deletions(-) -- 2.23.0
Re: [PATCH] x86/tsc: mark tsc reliable on CoffeeLake
On 2019/4/8 8:03 PM, Thomas Gleixner wrote: > On Mon, 8 Apr 2019, You-Sheng Yang wrote: >> +/* >> + * On Intel CoffeeLake, tsc may be marked unstable unexpectedly after >> + * entering PC10. >> + */ >> +if (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL && >> +(boot_cpu_data.x86_model == INTEL_FAM6_KABYLAKE_MOBILE || >> + boot_cpu_data.x86_model == INTEL_FAM6_KABYLAKE_DESKTOP) && >> +boot_cpu_data.x86_stepping >= 0x0a) >> +tsc_clocksource_reliable = 1; > > No. We are not starting that family/model/stepping game especially not > with random stepping cutoffs which are pulled out of thin air. That's > going to spiral out of control sooner than later. What about we simply disable clocksource watchdog if this is an invariant TSC? > There must be a better way to do that. Rafael? > > Thanks, > > tglx You-Sheng Yang signature.asc Description: OpenPGP digital signature
