Re: [PATCH] xhci-pci: Set AMD Renoir USB controller to D3 when shutdown

2021-02-19 Thread Aaron Ma




On 2/11/21 8:50 PM, Greg Kroah-Hartman wrote:

On Wed, Feb 10, 2021 at 03:13:30PM +0200, Mathias Nyman wrote:

On 9.2.2021 10.37, Greg Kroah-Hartman wrote:

On Fri, Feb 05, 2021 at 02:50:15PM +0800, Kai-Heng Feng wrote:

On Fri, Feb 5, 2021 at 2:45 PM Aaron Ma  wrote:



On 2/5/21 12:27 PM, Kai-Heng Feng wrote:

Can you please test the following patch, which should address the root cause:
https://lore.kernel.org/linux-acpi/20201201213019.1558738-1-furq...@google.com/

It also helps another AMD laptop on S5:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912935



No, this patch doesn't help on ThinkPad AMD platform.


Thanks for the confirmation!

Acked-by: Kai-Heng Feng 


Mathias, want me to take this in my tree now, or are you going to send
me more patches for 5.12-rc1?



Nothing more for 5.12-rc1 from me.

Could this be a PCI quirk instead of xhci?
Maybe there is some PCI flag for this already, haven't checked yet.

We want a specific PCI device to go to PCI D3cold at PCI shutdown...


There probably is.  Kay-Heng, can you look into doing that instead?



There is no such PCI quirk, usually it calls driver to shutdown.

Regards,
Aaron


thanks,

greg k-h



Re: [PATCH] xhci-pci: Set AMD Renoir USB controller to D3 when shutdown

2021-02-04 Thread Aaron Ma



On 2/5/21 12:27 PM, Kai-Heng Feng wrote:

Can you please test the following patch, which should address the root cause:
https://lore.kernel.org/linux-acpi/20201201213019.1558738-1-furq...@google.com/

It also helps another AMD laptop on S5:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1912935



No, this patch doesn't help on ThinkPad AMD platform.

Aaron


We don't need to put bandage on drivers one by one once the patch with
alternative approach is in upstream.

Kai-Heng


[PATCH] xhci-pci: Set AMD Renoir USB controller to D3 when shutdown

2021-02-03 Thread Aaron Ma
On AMD Renoir/Cezanne platforms, when set "Always on USB" to "On" in BIOS,
USB controller will consume more power than 0.03w.

Set it to D3cold when shutdown, S5 power consumption will be 0.03w lower.
The USB can charge other devices as before.
USB controller works fine after power on and reboot.

Signed-off-by: Aaron Ma 
---
 drivers/usb/host/xhci-pci.c | 8 
 drivers/usb/host/xhci.h | 1 +
 2 files changed, 9 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 84da8406d5b4..a31be1ba927f 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -62,6 +62,7 @@
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_30x43ba
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_20x43bb
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_10x43bc
+#define PCI_DEVICE_ID_AMD_RENOIR_USB31 0x1639
 #define PCI_DEVICE_ID_ASMEDIA_1042_XHCI0x1042
 #define PCI_DEVICE_ID_ASMEDIA_1042A_XHCI   0x1142
 #define PCI_DEVICE_ID_ASMEDIA_1142_XHCI0x1242
@@ -171,6 +172,10 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
if (pdev->vendor == PCI_VENDOR_ID_AMD)
xhci->quirks |= XHCI_TRUST_TX_LENGTH;
 
+   if (pdev->vendor == PCI_VENDOR_ID_AMD &&
+   pdev->device == PCI_DEVICE_ID_AMD_RENOIR_USB31)
+   xhci->quirks |= XHCI_SHUTDOWN_D3COLD;
+
if ((pdev->vendor == PCI_VENDOR_ID_AMD) &&
((pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_4) ||
(pdev->device == PCI_DEVICE_ID_AMD_PROMONTORYA_3) ||
@@ -594,6 +599,9 @@ static void xhci_pci_shutdown(struct usb_hcd *hcd)
/* Yet another workaround for spurious wakeups at shutdown with HSW */
if (xhci->quirks & XHCI_SPURIOUS_WAKEUP)
pci_set_power_state(pdev, PCI_D3hot);
+
+   if (xhci->quirks & XHCI_SHUTDOWN_D3COLD)
+   pci_set_power_state(pdev, PCI_D3cold);
 }
 #endif /* CONFIG_PM */
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 25e57bc9c3cc..0684193da4bd 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1883,6 +1883,7 @@ struct xhci_hcd {
 #define XHCI_SKIP_PHY_INIT BIT_ULL(37)
 #define XHCI_DISABLE_SPARSEBIT_ULL(38)
 #define XHCI_SG_TRB_CACHE_SIZE_QUIRK   BIT_ULL(39)
+#define XHCI_SHUTDOWN_D3COLD   BIT_ULL(40)
 
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
-- 
2.30.0



[PATCH 2/2] drm/i915: Force DPCD backlight mode for BOE 2270 panel

2020-10-09 Thread Aaron Ma
BOE 2270 panel failed to control backlight brightness.
Add it in edid quirks to force using DPCD backlight control.
Then the brightness can be controlled.

Signed-off-by: Aaron Ma 
---
 drivers/gpu/drm/drm_dp_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 092c8c985911..417ed10bbf83 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -1324,6 +1324,7 @@ static const struct edid_quirk edid_quirk_list[] = {
{ MFG(0x4d, 0x10), PROD_ID(0xc7, 0x14), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4d, 0x10), PROD_ID(0xe6, 0x14), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
{ MFG(0x4c, 0x83), PROD_ID(0x47, 0x41), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
+   { MFG(0x09, 0xe5), PROD_ID(0xde, 0x08), 
BIT(DP_QUIRK_FORCE_DPCD_BACKLIGHT) },
 };
 
 #undef MFG
-- 
2.25.1



[PATCH 1/2] drm/i915/dpcd_bl: uncheck PWM_PIN_CAP when detect eDP backlight capabilities

2020-10-09 Thread Aaron Ma
BOE panel with ID 2270 claims both PWM_PIN_CAP and AUX_SET_CAP backlight
control bits, but default chip backlight failed to control brightness.

Check AUX_SET_CAP and proceed to check quirks or VBT backlight type.
DPCD can control the brightness of this pannel.

Signed-off-by: Aaron Ma 
---
 drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c 
b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
index acbd7eb66cbe..308b14159b7c 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_aux_backlight.c
@@ -334,8 +334,7 @@ intel_dp_aux_display_control_capable(struct intel_connector 
*connector)
 * the panel can support backlight control over the aux channel
 */
if (intel_dp->edp_dpcd[1] & DP_EDP_TCON_BACKLIGHT_ADJUSTMENT_CAP &&
-   (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP) &&
-   !(intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_PWM_PIN_CAP)) 
{
+   (intel_dp->edp_dpcd[2] & DP_EDP_BACKLIGHT_BRIGHTNESS_AUX_SET_CAP)) {
drm_dbg_kms(>drm, "AUX Backlight Control Supported!\n");
return true;
}
-- 
2.25.1



[PATCH v2] platform/x86: thinkpad_acpi: re-initialize ACPI buffer size when reuse

2020-10-02 Thread Aaron Ma
Evaluating ACPI _BCL could fail, then ACPI buffer size will be set to 0.
When reuse this ACPI buffer, AE_BUFFER_OVERFLOW will be triggered.

Re-initialize buffer size will make ACPI evaluate successfully.

Fixes: 46445b6b896fd ("thinkpad-acpi: fix handle locate for video and query of 
_BCL")
Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/thinkpad_acpi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 9c4df41687a3..477d63c49c04 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6829,8 +6829,10 @@ static int __init tpacpi_query_bcl_levels(acpi_handle 
handle)
list_for_each_entry(child, >children, node) {
acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
  NULL, );
-   if (ACPI_FAILURE(status))
+   if (ACPI_FAILURE(status)) {
+   buffer.length = ACPI_ALLOCATE_BUFFER;
continue;
+   }
 
obj = (union acpi_object *)buffer.pointer;
if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
-- 
2.28.0



[PATCH] platform/x86: thinkpad_acpi: re-initialize acpi buffer size when reuse

2020-09-29 Thread Aaron Ma
Evaluating acpi _BCL could be failed, then acpi buffer size will be set
to 0. When reuse this acpi buffer, AE_BUFFER_OVERFLOW will be triggered.

Re-initialize buffer size will make acpi evaluate successfully.

Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/thinkpad_acpi.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index 9c4df41687a3..477d63c49c04 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6829,8 +6829,10 @@ static int __init tpacpi_query_bcl_levels(acpi_handle 
handle)
list_for_each_entry(child, >children, node) {
acpi_status status = acpi_evaluate_object(child->handle, "_BCL",
  NULL, );
-   if (ACPI_FAILURE(status))
+   if (ACPI_FAILURE(status)) {
+   buffer.length = ACPI_ALLOCATE_BUFFER;
continue;
+   }
 
obj = (union acpi_object *)buffer.pointer;
if (!obj || (obj->type != ACPI_TYPE_PACKAGE)) {
-- 
2.28.0



[v3][PATCH] platform/x86: thinkpad_acpi: not loading brightness_init when _BCL invalid

2020-07-09 Thread Aaron Ma
When _BCL invalid, disable thinkpad_acpi backlight brightness control.

brightness_enable is already checked at the beginning.
Most new thinkpads are using GPU driver to control brightness now,
print notice when enabled brightness control even when brightness_enable = 1.

Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/thinkpad_acpi.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index ff7f0a4f2475..2b36d5416a3b 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6955,10 +6955,13 @@ static int __init brightness_init(struct 
ibm_init_struct *iibm)
pr_warn("Cannot enable backlight brightness support, 
ACPI is already handling it.  Refer to the acpi_backlight kernel parameter.\n");
return 1;
}
-   } else if (tp_features.bright_acpimode && brightness_enable > 1) {
-   pr_notice("Standard ACPI backlight interface not available, 
thinkpad_acpi native brightness control enabled\n");
+   } else if (!tp_features.bright_acpimode) {
+   pr_notice("ACPI backlight interface not available\n");
+   return 1;
}
 
+   pr_notice("ACPI native brightness control enabled\n");
+
/*
 * Check for module parameter bogosity, note that we
 * init brightness_mode to TPACPI_BRGHT_MODE_MAX in order to be
-- 
2.26.2



[PATCH] drm/amd/display: add dmcub check on RENOIR

2020-07-08 Thread Aaron Ma
RENOIR loads dmub fw not dmcu, check dmcu only will prevent loading iram,
it breaks backlight control.

Bug: https://bugzilla.kernel.org/show_bug.cgi?id=208277
Signed-off-by: Aaron Ma 
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c 
b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 10ac8076d4f2..db5e0bb0d935 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1358,7 +1358,7 @@ static int dm_late_init(void *handle)
struct dmcu *dmcu = NULL;
bool ret;
 
-   if (!adev->dm.fw_dmcu)
+   if (!adev->dm.fw_dmcu && !adev->dm.dmub_fw)
return detect_mst_link_for_all_connectors(adev->ddev);
 
dmcu = adev->dm.dc->res_pool->dmcu;
-- 
2.25.1



[v2][PATCH] platform/x86: thinkpad_acpi: not loading brightness_init when _BCL invalid

2020-07-02 Thread Aaron Ma
When _BCL invalid, disable thinkpad_acpi backlight brightness control.

brightness_enable is already checked at the beginning.
Most new thinkpads are using GPU driver to control brightness now,
print notice when enabled brightness control even when brightness_enable = 1.

Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/thinkpad_acpi.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index ff7f0a4f2475..a52d6d457d6c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6955,10 +6955,13 @@ static int __init brightness_init(struct 
ibm_init_struct *iibm)
pr_warn("Cannot enable backlight brightness support, 
ACPI is already handling it.  Refer to the acpi_backlight kernel parameter.\n");
return 1;
}
-   } else if (tp_features.bright_acpimode && brightness_enable > 1) {
-   pr_notice("Standard ACPI backlight interface not available, 
thinkpad_acpi native brightness control enabled\n");
+   } else if (!tp_features.bright_acpimode) {
+   pr_notice("thinkpad_acpi backlight interface not available\n");
+   return 1;
}
 
+   pr_notice("thinkpad_acpi native brightness control enabled\n");
+
/*
 * Check for module parameter bogosity, note that we
 * init brightness_mode to TPACPI_BRGHT_MODE_MAX in order to be
-- 
2.26.2



Re: [PATCH] platform/x86: thinkpad_acpi: not loading brightness_init when _BCL invalid

2020-07-02 Thread Aaron Ma
On 7/2/20 5:30 PM, Andy Shevchenko wrote:
> On Thu, Jul 2, 2020 at 11:55 AM Aaron Ma  wrote:
>>
>> When _BCL invalid, disable thinkpad_acpi backlight brightness control.
>>
>> brightness_enable is already checked at the beginning,
> 
>> Always print notice when enabled brightness control.
> 
> Why?
> 

Default brightness_enable = 2, so this message will always be printed as before
Actually no change here.

> ...
> 
>> +   pr_notice("thinkpad_acpi native brightness control enabled\n");
> 
> 'notice' level is quite high, why do we spam users with this?
> 

Like above.

Another reason is  most thinkpads are using native gpu driver to control
brightness, notice when thinkpad_acpi brightness is enabled.

Aaron


[PATCH] platform/x86: thinkpad_acpi: not loading brightness_init when _BCL invalid

2020-07-02 Thread Aaron Ma
When _BCL invalid, disable thinkpad_acpi backlight brightness control.

brightness_enable is already checked at the beginning,
Always print notice when enabled brightness control.

Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/thinkpad_acpi.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/thinkpad_acpi.c 
b/drivers/platform/x86/thinkpad_acpi.c
index ff7f0a4f2475..a52d6d457d6c 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -6955,10 +6955,13 @@ static int __init brightness_init(struct 
ibm_init_struct *iibm)
pr_warn("Cannot enable backlight brightness support, 
ACPI is already handling it.  Refer to the acpi_backlight kernel parameter.\n");
return 1;
}
-   } else if (tp_features.bright_acpimode && brightness_enable > 1) {
-   pr_notice("Standard ACPI backlight interface not available, 
thinkpad_acpi native brightness control enabled\n");
+   } else if (!tp_features.bright_acpimode) {
+   pr_notice("thinkpad_acpi backlight interface not available\n");
+   return 1;
}
 
+   pr_notice("thinkpad_acpi native brightness control enabled\n");
+
/*
 * Check for module parameter bogosity, note that we
 * init brightness_mode to TPACPI_BRGHT_MODE_MAX in order to be
-- 
2.26.2



[v4][PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-18 Thread Aaron Ma
After 'commit e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
 for ME systems")',
ThinkPad P14s always failed to disable ULP by ME.
'commit 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")'
break out of init phy:

error log:
[   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error

When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.

Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma 
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
-   goto out;
}
 
ret_val = hw->phy.ops.acquire(hw);
-- 
2.26.2



[v3][PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-18 Thread Aaron Ma
After commit: e086ba2fccda4 ("e1000e: disable s0ix entry and exit flows
 for ME systems").
ThinkPad P14s always failed to disable ULP by ME.
commit: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
break out of init phy:

error log:
[   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error

When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.

Fixes: 0c80cdbf3320 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma 
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
-   goto out;
}
 
ret_val = hw->phy.ops.acquire(hw);
-- 
2.26.2



[v2][PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-17 Thread Aaron Ma
After commit: e086ba2fccd ("e1000e: disable s0ix entry and exit flows
 for ME systems").
ThinkPad P14s always failed to disable ULP by ME.
commit: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
break out of init phy:

error log:
[   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error

When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.

Fixes: 0c80cdbf33 ("e1000e: Warn if disabling ULP failed")
Signed-off-by: Aaron Ma 
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..be7475c5529d 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -303,7 +303,6 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
e_warn("Failed to disable ULP\n");
-   goto out;
}
 
ret_val = hw->phy.ops.acquire(hw);
-- 
2.26.2



Re: [PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-16 Thread Aaron Ma



On 6/16/20 7:23 PM, Kai-Heng Feng wrote:
> 
> 
>> On Jun 16, 2020, at 18:05, Aaron Ma  wrote:
>>
>> After commit "e1000e: disable s0ix entry and exit flows for ME systems",
>> some ThinkPads always failed to disable ulp by ME.
>> commit "e1000e: Warn if disabling ULP failed" break out of init phy:
>>
>> error log:
>> [   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
>> [   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast 
>> Packet
>> [   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>>
>> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
>> If continue to init phy like before, it can work as before.
>> iperf test result good too.
>>
>> Chnage e_warn to e_dbg, in case it confuses.
>>
>> Signed-off-by: Aaron Ma 
>> ---
>> drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> index f999cca37a8a..63405819eb83 100644
>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> @@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
>> e1000_hw *hw)
>>  hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
>>  ret_val = e1000_disable_ulp_lpt_lp(hw, true);
> 
> If si0x entry isn't enabled, maybe skip calling e1000_disable_ulp_lpt_lp() 
> altogether?
> We can use e1000e_check_me() to check that.
> 

No, s0ix is different feature with ULP mode.

Aaron

>>  if (ret_val) {
>> -e_warn("Failed to disable ULP\n");
>> -goto out;
>> +e_dbg("Failed to disable ULP\n");
>>  }
> 
> The change of "e1000e: Warn if disabling ULP failed" is intentional to catch 
> bugs like this.
> 
> Kai-Heng
> 
>>
>>  ret_val = hw->phy.ops.acquire(hw);
>> -- 
>> 2.26.2
>>
> 


Re: [Intel-wired-lan] [PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-16 Thread Aaron Ma
On 6/16/20 6:20 PM, Paul Menzel wrote:
> Dear Aaron,
> 
> 
> Thank you for your patch.
> 
> (Rant: Some more fallout from the other patch, which nobody reverted.)
> 

Would you like a revert?

Thanks,
Aaron

> Am 16.06.20 um 12:05 schrieb Aaron Ma:
>> After commit "e1000e: disable s0ix entry and exit flows for ME systems",
>> some ThinkPads always failed to disable ulp by ME.
> 
> Please add the (short) commit hash from the master branch.
> 
> s/ulp/ULP/
> 
> Please list one ThinkPad as example.
> 
>> commit "e1000e: Warn if disabling ULP failed" break out of init phy:
> 
> 1.  Please add the closing quote ".
> 2.  Please add the commit hash.
> 
>> error log:
>> [   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
>> [   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast 
>> Packet
>> [   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error
>>
>> When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
>> If continue to init phy like before, it can work as before.
>> iperf test result good too.
>>
>> Chnage e_warn to e_dbg, in case it confuses.
> 
> s/Chnage/Change/
> 
> Please leave the level warning, and improve the warning message instead, so a 
> user knows what is going on.
> 
> Could you please add a `Fixes:` tag and the URL to the bug report?
> 
>> Signed-off-by: Aaron Ma 
>> ---
>>   drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
>> b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> index f999cca37a8a..63405819eb83 100644
>> --- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> +++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
>> @@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
>> e1000_hw *hw)
>>   hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
>>   ret_val = e1000_disable_ulp_lpt_lp(hw, true);
>>   if (ret_val) {
>> -    e_warn("Failed to disable ULP\n");
>> -    goto out;
>> +    e_dbg("Failed to disable ULP\n");
>>   }
>>     ret_val = hw->phy.ops.acquire(hw);
>>
> 
> Kind regards,
> 
> Paul


[PATCH] e1000e: continue to init phy even when failed to disable ULP

2020-06-16 Thread Aaron Ma
After commit "e1000e: disable s0ix entry and exit flows for ME systems",
some ThinkPads always failed to disable ulp by ME.
commit "e1000e: Warn if disabling ULP failed" break out of init phy:

error log:
[   42.364753] e1000e :00:1f.6 enp0s31f6: Failed to disable ULP
[   42.524626] e1000e :00:1f.6 enp0s31f6: PHY Wakeup cause - Unicast Packet
[   42.822476] e1000e :00:1f.6 enp0s31f6: Hardware Error

When disable s0ix, E1000_FWSM_ULP_CFG_DONE will never be 1.
If continue to init phy like before, it can work as before.
iperf test result good too.

Chnage e_warn to e_dbg, in case it confuses.

Signed-off-by: Aaron Ma 
---
 drivers/net/ethernet/intel/e1000e/ich8lan.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/ich8lan.c 
b/drivers/net/ethernet/intel/e1000e/ich8lan.c
index f999cca37a8a..63405819eb83 100644
--- a/drivers/net/ethernet/intel/e1000e/ich8lan.c
+++ b/drivers/net/ethernet/intel/e1000e/ich8lan.c
@@ -302,8 +302,7 @@ static s32 e1000_init_phy_workarounds_pchlan(struct 
e1000_hw *hw)
hw->dev_spec.ich8lan.ulp_state = e1000_ulp_state_unknown;
ret_val = e1000_disable_ulp_lpt_lp(hw, true);
if (ret_val) {
-   e_warn("Failed to disable ULP\n");
-   goto out;
+   e_dbg("Failed to disable ULP\n");
}
 
ret_val = hw->phy.ops.acquire(hw);
-- 
2.26.2



[PATCH] rtw88: 8822ce: add support for device ID 0xc82f

2020-06-12 Thread Aaron Ma
New device ID 0xc82f found on Lenovo ThinkCenter.
Tested it with c822 driver, works good.

PCI id:
03:00.0 Network controller [0280]: Realtek Semiconductor Co., Ltd.
Device [10ec:c82f]
Subsystem: Lenovo Device [17aa:c02f]

Signed-off-by: Aaron Ma 
---
 drivers/net/wireless/realtek/rtw88/rtw8822ce.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c 
b/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
index 7b6bd990651e..026ac49ce6e3 100644
--- a/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
+++ b/drivers/net/wireless/realtek/rtw88/rtw8822ce.c
@@ -11,6 +11,10 @@ static const struct pci_device_id rtw_8822ce_id_table[] = {
PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0xC822),
.driver_data = (kernel_ulong_t)_hw_spec
},
+   {
+   PCI_DEVICE(PCI_VENDOR_ID_REALTEK, 0xC82F),
+   .driver_data = (kernel_ulong_t)_hw_spec
+   },
{}
 };
 MODULE_DEVICE_TABLE(pci, rtw_8822ce_id_table);
-- 
2.27.0



Re: [PATCH] ALSA: hda/realtek - Fix 2 front mics of codec 0x623

2019-10-23 Thread Aaron Ma
On 10/23/19 4:44 PM, Kailang wrote:
> 
> 
>> -Original Message-
>> From: Takashi Iwai 
>> Sent: Wednesday, October 23, 2019 12:08 AM
>> To: Aaron Ma 
>> Cc: pe...@perex.cz; Kailang ;
>> hui.w...@canonical.com; alsa-de...@alsa-project.org;
>> linux-kernel@vger.kernel.org
>> Subject: Re: [PATCH] ALSA: hda/realtek - Fix 2 front mics of codec 0x623
>>
>> On Tue, 22 Oct 2019 17:38:55 +0200,
>> Aaron Ma wrote:
>>>
>>> These 2 ThinkCentres installed a new realtek codec ID 0x623, it has 2
>>> front mics with the same location on pin 0x18 and 0x19.
>>>
>>> Apply fixup ALC283_FIXUP_HEADSET_MIC to change 1 front mic location to
>>> right, then pulseaudio can handle them.
>>> One "Front Mic" and one "Mic" will be shown, and audio output works
>>> fine.
>>>
>>> Signed-off-by: Aaron Ma 
>>
>> I'd like to have Kailang's review about the new codec before applying.
>>
>> Kailang, could you take a look?
> OK.
> I will post you the patch for ALC623 codec tomorrow.
> Thanks.

Cc me too.

Thank you.
Aaron

> 
>>
>>
>> thanks,
>>
>> Takashi
>>
>>> ---
>>>  sound/pci/hda/patch_realtek.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/sound/pci/hda/patch_realtek.c
>>> b/sound/pci/hda/patch_realtek.c index b000b36ac3c6..c34d8b435f58
>>> 100644
>>> --- a/sound/pci/hda/patch_realtek.c
>>> +++ b/sound/pci/hda/patch_realtek.c
>>> @@ -7186,6 +7186,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[]
>> = {
>>> SND_PCI_QUIRK(0x17aa, 0x312f, "ThinkCentre Station",
>> ALC294_FIXUP_LENOVO_MIC_LOCATION),
>>> SND_PCI_QUIRK(0x17aa, 0x313c, "ThinkCentre Station",
>> ALC294_FIXUP_LENOVO_MIC_LOCATION),
>>> SND_PCI_QUIRK(0x17aa, 0x3151, "ThinkCentre Station",
>>> ALC283_FIXUP_HEADSET_MIC),
>>> +   SND_PCI_QUIRK(0x17aa, 0x3178, "ThinkCentre Station",
>> ALC283_FIXUP_HEADSET_MIC),
>>> +   SND_PCI_QUIRK(0x17aa, 0x3176, "ThinkCentre Station",
>>> +ALC283_FIXUP_HEADSET_MIC),
>>> SND_PCI_QUIRK(0x17aa, 0x3902, "Lenovo E50-80",
>> ALC269_FIXUP_DMIC_THINKPAD_ACPI),
>>> SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210",
>> ALC283_FIXUP_INT_MIC),
>>> SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo B50-70",
>>> ALC269_FIXUP_DMIC_THINKPAD_ACPI), @@ -9187,6 +9189,7 @@ static
>> const struct hda_device_id snd_hda_id_realtek[] = {
>>> HDA_CODEC_ENTRY(0x10ec0298, "ALC298", patch_alc269),
>>> HDA_CODEC_ENTRY(0x10ec0299, "ALC299", patch_alc269),
>>> HDA_CODEC_ENTRY(0x10ec0300, "ALC300", patch_alc269),
>>> +   HDA_CODEC_ENTRY(0x10ec0623, "ALC623", patch_alc269),
>>> HDA_CODEC_REV_ENTRY(0x10ec0861, 0x100340, "ALC660",
>> patch_alc861),
>>> HDA_CODEC_ENTRY(0x10ec0660, "ALC660-VD", patch_alc861vd),
>>> HDA_CODEC_ENTRY(0x10ec0861, "ALC861", patch_alc861),
>>> --
>>> 2.17.1
>>>
>>
>> --Please consider the environment before printing this e-mail.


[PATCH] ALSA: hda/realtek - Fix 2 front mics of codec 0x623

2019-10-22 Thread Aaron Ma
These 2 ThinkCentres installed a new realtek codec ID 0x623,
it has 2 front mics with the same location on pin 0x18 and 0x19.

Apply fixup ALC283_FIXUP_HEADSET_MIC to change 1 front mic
location to right, then pulseaudio can handle them.
One "Front Mic" and one "Mic" will be shown, and audio output works
fine.

Signed-off-by: Aaron Ma 
---
 sound/pci/hda/patch_realtek.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index b000b36ac3c6..c34d8b435f58 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -7186,6 +7186,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x17aa, 0x312f, "ThinkCentre Station", 
ALC294_FIXUP_LENOVO_MIC_LOCATION),
SND_PCI_QUIRK(0x17aa, 0x313c, "ThinkCentre Station", 
ALC294_FIXUP_LENOVO_MIC_LOCATION),
SND_PCI_QUIRK(0x17aa, 0x3151, "ThinkCentre Station", 
ALC283_FIXUP_HEADSET_MIC),
+   SND_PCI_QUIRK(0x17aa, 0x3178, "ThinkCentre Station", 
ALC283_FIXUP_HEADSET_MIC),
+   SND_PCI_QUIRK(0x17aa, 0x3176, "ThinkCentre Station", 
ALC283_FIXUP_HEADSET_MIC),
SND_PCI_QUIRK(0x17aa, 0x3902, "Lenovo E50-80", 
ALC269_FIXUP_DMIC_THINKPAD_ACPI),
SND_PCI_QUIRK(0x17aa, 0x3977, "IdeaPad S210", ALC283_FIXUP_INT_MIC),
SND_PCI_QUIRK(0x17aa, 0x3978, "Lenovo B50-70", 
ALC269_FIXUP_DMIC_THINKPAD_ACPI),
@@ -9187,6 +9189,7 @@ static const struct hda_device_id snd_hda_id_realtek[] = {
HDA_CODEC_ENTRY(0x10ec0298, "ALC298", patch_alc269),
HDA_CODEC_ENTRY(0x10ec0299, "ALC299", patch_alc269),
HDA_CODEC_ENTRY(0x10ec0300, "ALC300", patch_alc269),
+   HDA_CODEC_ENTRY(0x10ec0623, "ALC623", patch_alc269),
HDA_CODEC_REV_ENTRY(0x10ec0861, 0x100340, "ALC660", patch_alc861),
HDA_CODEC_ENTRY(0x10ec0660, "ALC660-VD", patch_alc861vd),
HDA_CODEC_ENTRY(0x10ec0861, "ALC861", patch_alc861),
-- 
2.17.1



Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-06-13 Thread Aaron Ma
On 6/12/19 1:35 AM, Dmitry Torokhov wrote:
> On Tue, Jun 11, 2019 at 12:55:58AM +0800, Aaron Ma wrote:
>> On 6/10/19 12:55 AM, Dmitry Torokhov wrote:
>>> Hi Aaron,
>>>
>>> On Wed, Feb 20, 2019 at 05:41:59PM +0100, Aaron Ma wrote:
>>>> rmi4 got spam data after S3 resume on some ThinkPads.
>>>> Then TrackPoint lost when be detected by psmouse.
>>>> Clear irqs status before set irqs will make TrackPoint back.
>>> Could you please give me an idea as to what this spam data is?
>>>
>> It should be some data 0 during suspend/resume.
>> Actually I don't know how these data 0 is produced.
>> Not all synaptics touchpads have this issue.
>>
>>> In F03 probe we clear all pending data before enabling the function,
>> Yes we did, but not after resume.
> Yes, I understand that. The question I was asking: if we add code
> consuming all pending data to f03->suspend(), similarly to what we are
> doing at probe time, will it fix the issue with trackstick losing
> synchronization and attempting disconnect?
> 

I just do some test via adding code in suspend or resume.
But they didn't work out.

>>> maybe the same needs to be done on resume, instead of changing the way
>>> we handle IRQ bits?
>> This patch is supposed to clear irq status like it in fn probe. Not
>> changing IRQ bits.
> What I meant is changing how we enable IRQ bits. I would really prefer
> we did not lose IRQ state for other functions when we enable interrupts
> for given function.
> 

Not only F03 with problem, F12 too which is touchpad .
User verified this patch fixes problem of F12 too.
Clear IRQ status before enable IRQ should be safe.

Or we can add code before enable IRQ in F03/F12.

Thanks,
Aaron

> Thanks.
> 
> -- Dmitry
> 


Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-06-10 Thread Aaron Ma


On 6/10/19 12:55 AM, Dmitry Torokhov wrote:
> Hi Aaron,
> 
> On Wed, Feb 20, 2019 at 05:41:59PM +0100, Aaron Ma wrote:
>> rmi4 got spam data after S3 resume on some ThinkPads.
>> Then TrackPoint lost when be detected by psmouse.
>> Clear irqs status before set irqs will make TrackPoint back.
> Could you please give me an idea as to what this spam data is?
> 

It should be some data 0 during suspend/resume.
Actually I don't know how these data 0 is produced.
Not all synaptics touchpads have this issue.

> In F03 probe we clear all pending data before enabling the function,

Yes we did, but not after resume.

> maybe the same needs to be done on resume, instead of changing the way
> we handle IRQ bits?

This patch is supposed to clear irq status like it in fn probe. Not
changing IRQ bits.

Thanks,
Aaron

> 
> Thanks,
> 
> -- Dmitry
> 


Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-06-07 Thread Aaron Ma
Hi Dmitry:

Will you apply them?

Thanks,
Aaron

On 6/4/19 1:19 PM, Christopher Heiny wrote:
> Given that, I'm willing to accept the patch as is.
> 
>   Cheers,
>   Chris


Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-06-03 Thread Aaron Ma
Hi Christopher:

Have got time to review these 2 patches?
Users reported it works fine since I sent out this patch.

Thanks,
Aaron

On 4/3/19 9:58 PM, Aaron Ma wrote:
> Sure, take your time, if you have any questions let me know please.
> 
> Thanks,
> Aaron


Re: [PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

2019-05-22 Thread Aaron Ma



On 5/21/19 2:49 PM, Benjamin Tissoires wrote:
> A quick google image search showed that the X240 had 2 versions: one
> with the top software buttons, one without.
> 
> And this definitively rings a bell. I am sure we asked Lenovo and
> Synaptics to change the PnPID when they would do such a change, but
> they "forgot" during the *40 series refresh.
> We have code in place to fix the reported ranges of the coordinates,
> and we had to check against the board id (see min_max_pnpid_table[] in
> synaptics.c).
> Unfortunately, X240 (LEN0035) is not part of this table, so I don't
> know which refresh of the board ID has implemented the non top
> software buttons.

After double confirm from Lenovo, looks like they mixed up with
touchpads on X240/X240s.

For now only one user reported this LEN0035 doesn't work on SMBus mode.
module parameter can be a workaround.
Maybe some touchpads with software top buttons are working well on
SMBus. Let's keep eyes on this issue for now.

Regards,
Aaron

> 
> Cheers,
> Benjamin


[PATCH 1/2] Input: elantech - enable middle button support on 2 ThinkPads

2019-05-19 Thread Aaron Ma
Adding 2 new touchpad PNPIDs to enable middle button support.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index a7f8b1614559..530142b5a115 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1189,6 +1189,8 @@ static const char * const middle_button_pnp_ids[] = {
"LEN2132", /* ThinkPad P52 */
"LEN2133", /* ThinkPad P72 w/ NFC */
"LEN2134", /* ThinkPad P72 */
+   "LEN0407",
+   "LEN0408",
NULL
 };
 
-- 
2.17.1



[PATCH 2/2] Input: synaptics - remove X240 from the topbuttonpad list

2019-05-19 Thread Aaron Ma
Lenovo ThinkPad X240 does not have the top software button.
When this wrong ID in top button list, smbus mode will fail to probe,
so keep it working at PS2 mode.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/synaptics.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
index b6da0c1267e3..6ae7bc92476b 100644
--- a/drivers/input/mouse/synaptics.c
+++ b/drivers/input/mouse/synaptics.c
@@ -140,7 +140,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
"LEN002E",
"LEN0033", /* Helix */
"LEN0034", /* T431s, L440, L540, T540, W540, X1 Carbon 2nd */
-   "LEN0035", /* X240 */
"LEN0036", /* T440 */
"LEN0037", /* X1 Carbon 2nd */
"LEN0038",
-- 
2.17.1



Re: [PATCH] nvme: determine the number of IO queues

2019-04-25 Thread Aaron Ma


On 4/25/19 10:39 PM, Christoph Hellwig wrote:
> Honestly, unless this is a device shiping in a max market consumer
> product already I don't think we should work around this crap at all,
> given that this device has obviously never been tested at all.  It
> really needs a firmware fix instead of a host workaround.


Already pushed this issue to firmware eng team.
They will try to fix it.
As far as I know we don't need this host workaround.

Thanks,
Aaron


Re: [PATCH] nvme: determine the number of IO queues

2019-04-18 Thread Aaron Ma



On 4/18/19 9:48 PM, Keith Busch wrote:
> It does change the default behavior. If I have a degraded controller that
> can't do IO in a machine with 1000's of CPUs, I have to iterate this
> non-standard behavior 1000's of times before the drive is servicable
> again. We currenlty figure that out in just a single try.
> 
> At least the quirks document *why* the driver is doing non-standard
> behavior. We do the IO queue quirks for Macbooks, for example.
> 
> But why don't you file a bug report with the device vendor instead? Surely
> a firmware fix provides the best possible outcome, and would make this
> device work not only in all versions of Linux, but also every standard
> compliant driver for any OS.

I will do it, no v2 for now.

Thanks,
Aaron


Re: [PATCH] nvme: determine the number of IO queues

2019-04-18 Thread Aaron Ma



On 4/18/19 9:33 PM, Minwoo Im wrote:
> If the controller returns error for that command, how can we assure that
> the controller would support a single I/O queue ?

Make sense, I will keep *count = 0 in V2.

Thanks,
Aaron


Re: [PATCH] nvme: determine the number of IO queues

2019-04-18 Thread Aaron Ma



On 4/18/19 8:13 PM, Minwoo Im wrote:
>> Yes the IO queues number is 0's based, but driver would return error and
>> remove the nvme device as dead.
> 
> IMHO, if a controller indicates an error with this set_feature command,
> then
> we need to figure out why the controller was returning the error to host.
> 
> If you really want to use at least a single queue to see an alive I/O
> queue,
> controller should not return the error because as you mentioned above,
> NCQA, NSQA will be returned as 0-based.  If an error is there, that could
> mean that controller may not able to provide even a single queue for I/O.

I was thinking about try to set 1 I/O queue in driver to try to probe
NVME device.
If it works, at least system can bootup to debug instead of just remove
NVME device and kernel boot hang at loading rootfs.

If you still concern this 1 I/O queue I can still set it as
*count = 0;

At least we try all count, NVME device still failed to respond.

Regards,
Aaron

> 
> Thanks,
> Minwoo Im


Re: [PATCH] nvme: determine the number of IO queues

2019-04-18 Thread Aaron Ma
On 4/18/19 5:30 AM, Edmund Nadolski (Microsoft) wrote:
> On 4/17/19 7:12 AM, Aaron Ma wrote:
>> Some controllers support limited IO queues, when over set
>> the number, it will return invalid field error.
>> Then NVME will be removed by driver.
>>
>> Find the max number of IO queues that controller supports.
>> When it still got invalid result, set 1 IO queue at least to
>> bring NVME online.
>>
>> Signed-off-by: Aaron Ma 
>> ---
>>   drivers/nvme/host/core.c | 29 ++---
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index 2c43e12b70af..fb7f05c310c8 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -1134,14 +1134,24 @@ static int nvme_set_features(struct nvme_ctrl
>> *dev, unsigned fid, unsigned dword
>>     int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
>>   {
>> -    u32 q_count = (*count - 1) | ((*count - 1) << 16);
>> +    u32 q_count;
>>   u32 result;
>> -    int status, nr_io_queues;
>> -
>> -    status = nvme_set_features(ctrl, NVME_FEAT_NUM_QUEUES, q_count,
>> NULL, 0,
>> -    );
>> -    if (status < 0)
>> -    return status;
>> +    int status = -1;
>> +    int nr_io_queues;
>> +    int try_count;
>> +
>> +    for (try_count = *count; try_count > 0; try_count--) {
>> +    q_count = (try_count - 1) | ((try_count - 1) << 16);
> 
> A macro here might help readability.

Will add in V2.

> 
>> +    status = nvme_set_features(ctrl, NVME_FEAT_NUM_QUEUES,
>> +    q_count, NULL, 0, );
>> +    if (status < 0)
>> +    return status;
>> +    else if (status == 0) {
> 
> else following return is not needed.

 *  0: successful read
 *  > 0: NVMe error status code
 *  < 0: Linux errno error code

3 conditions should be taken care.
status > 0 will be handled after loop.

else is needed.

> 
> 
>> +    nr_io_queues = min(result & 0x, result >> 16) + 1;
> 
> Likewise, a macro as above.

Will add in V2.

> 
> 
> Ed
> 
>> +    *count = min(try_count, nr_io_queues);
>> +    break;
>> +    }
>> +    }
>>     /*
>>    * Degraded controllers might return an error when setting the
>> queue
>> @@ -1150,10 +1160,7 @@ int nvme_set_queue_count(struct nvme_ctrl
>> *ctrl, int *count)
>>    */
>>   if (status > 0) {
>>   dev_err(ctrl->device, "Could not set queue count (%d)\n",
>> status);
>> -    *count = 0;
>> -    } else {
>> -    nr_io_queues = min(result & 0x, result >> 16) + 1;
>> -    *count = min(*count, nr_io_queues);
>> +    *count = 1;
>>   }
>>     return 0;
>>
> 


Re: [PATCH] nvme: determine the number of IO queues

2019-04-18 Thread Aaron Ma
On 4/18/19 1:33 AM, Maxim Levitsky wrote:
> On Wed, 2019-04-17 at 20:32 +0300, Maxim Levitsky wrote:
>> On Wed, 2019-04-17 at 22:12 +0800, Aaron Ma wrote:
>>> Some controllers support limited IO queues, when over set
>>> the number, it will return invalid field error.
>>> Then NVME will be removed by driver.
>>>
>>> Find the max number of IO queues that controller supports.
>>> When it still got invalid result, set 1 IO queue at least to
>>> bring NVME online.
>> To be honest a spec compliant device should not need this.
>> The spec states:
>>
>> "Number of I/O Completion Queues Requested (NCQR): Indicates the number of 
>> I/O
>> Completion
>> Queues requested by software. This number does not include the Admin
>> Completion
>> Queue. A
>> minimum of one queue shall be requested, reflecting that the minimum support
>> is
>> for one I/O
>> Completion Queue. This is a 0’s based value. The maximum value that may be
>> specified is 65,534
>> (i.e., 65,535 I/O Completion Queues). If the value specified is 65,535, the
>> controller should return
>> an error of Invalid Field in Command."
>>
>>
>> This implies that you can ask for any value and the controller must not
>> respond
>> with an error, but rather indicate how many queues it supports.
>>
>> Maybe its better to add a quirk for the broken device, which needs this?

Adding quirk only makes the code more complicated.
This patch doesn't change the default behavior.
Only handle the NVME error code.

Yes the IO queues number is 0's based, but driver would return error and
remove the nvme device as dead.

So set it as 1 at least the NVME can be probed properly.

Regards,
Aaron

> I forgot to add the relevant paragraph:
> 
> "Note: The value allocated may be smaller or larger than the number of queues
> requested, often in virtualized
> implementations. The controller may not have as many queues to allocate as are
> requested. Alternatively,
> the controller may have an allocation unit of queues (e.g. power of two) and 
> may
> supply more queues to
> host software to satisfy its allocation unit."
> 
> 
> Best regards,
>   Maxim Levitsky
> 


[PATCH] nvme: determine the number of IO queues

2019-04-17 Thread Aaron Ma
Some controllers support limited IO queues, when over set
the number, it will return invalid field error.
Then NVME will be removed by driver.

Find the max number of IO queues that controller supports.
When it still got invalid result, set 1 IO queue at least to
bring NVME online.

Signed-off-by: Aaron Ma 
---
 drivers/nvme/host/core.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index 2c43e12b70af..fb7f05c310c8 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -1134,14 +1134,24 @@ static int nvme_set_features(struct nvme_ctrl *dev, 
unsigned fid, unsigned dword
 
 int nvme_set_queue_count(struct nvme_ctrl *ctrl, int *count)
 {
-   u32 q_count = (*count - 1) | ((*count - 1) << 16);
+   u32 q_count;
u32 result;
-   int status, nr_io_queues;
-
-   status = nvme_set_features(ctrl, NVME_FEAT_NUM_QUEUES, q_count, NULL, 0,
-   );
-   if (status < 0)
-   return status;
+   int status = -1;
+   int nr_io_queues;
+   int try_count;
+
+   for (try_count = *count; try_count > 0; try_count--) {
+   q_count = (try_count - 1) | ((try_count - 1) << 16);
+   status = nvme_set_features(ctrl, NVME_FEAT_NUM_QUEUES,
+   q_count, NULL, 0, );
+   if (status < 0)
+   return status;
+   else if (status == 0) {
+   nr_io_queues = min(result & 0x, result >> 16) + 1;
+   *count = min(try_count, nr_io_queues);
+   break;
+   }
+   }
 
/*
 * Degraded controllers might return an error when setting the queue
@@ -1150,10 +1160,7 @@ int nvme_set_queue_count(struct nvme_ctrl *ctrl, int 
*count)
 */
if (status > 0) {
dev_err(ctrl->device, "Could not set queue count (%d)\n", 
status);
-   *count = 0;
-   } else {
-   nr_io_queues = min(result & 0x, result >> 16) + 1;
-   *count = min(*count, nr_io_queues);
+   *count = 1;
}
 
return 0;
-- 
2.20.1



Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-04-03 Thread Aaron Ma



On 4/3/19 12:16 AM, Christopher Heiny wrote:
> On Thu, 2019-03-28 at 14:02 +0800, Aaron Ma wrote:
>> Hi Dmitry and Chiristopher:
>>
>> Do you have any suggestion about these 2 patches?
>>
>> Many users confirmed that they fixed issues of Trackpoint/Touchpad
>> after S3.
>>
>> Will you consider them be accepted?
> Hi Aaron,
> 
> Sorry - I thought I'd replied on the NO SLEEP portion of these patches,
> but looking back I don't find the draft or the sent email.  Sorry about
> that.  I'll summarize here what I wrote last month.
> 
> This isn't so much a "fix" as a "hacky workaround" for the issue.  From
> the descriptions in the bug you linked in your original patch
> submission, it appears that the root cause is somewhere in SMBus system
> (could be SMBus driver, SMBus hardware, or the devices on the SMBus
> (touch devices or other devices) - it's hard to tell with the info
> available), where the SMBus is failing to come online correctly coming
> out of S3 state.  Anyway, this patch doesn't fix the root cause, but
> merely works around it.

Users confirmed the 1st patch that clear irq status fixed their multiple
issues on Touchpad and Trackpoint.
I think it is a fix.

NO SLEEP patch was tried to give users a choice a fix touchpad issues
that I didn't reproduce.
If you don't like this export, we can drop it now as users confirmed 1st
patch works.

> 
> Setting the NO SLEEP bit will force the touch sensor to remain in a
> high power consumption state while the rest of the system is in S3. 
> While not a lot of power compared to things like the CPU, display, and
> others, it is still non-trivial and will result in shorter time-on-
> battery capability.

Verified on s2idle and S3 and system running idle mode. no difference on
power consumption of whole system with or without set 1 to nosleep.

> 
> If you have access to the power pin(s) for the touch sensor(s)/styk(s),
> it might be interesting to try turning power off on entering S3, and
> restoring it on exit.  That's very hacky, and has the side effect of
> slightly delaying touchpad readiness on exit from S3.  Plus you'll need
> to restore touch sensor configuration settings on exit.  But it
> definitely reduces power consumption.
> 
> 
> Separately, I am still concerned about the possibility of dropped touch
> events in the IRQ clearing.  I'm not convinced that the code is safe
> (as you mentioned in your reply to my earlier comment), so I'll have to
> study the implementation more carefully.

Sure, take your time, if you have any questions let me know please.

Thanks,
Aaron

> 
>   Cheers,
>   Chris
> 
> 
> 
>> Thanks,
>> Aaron
> 


Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-03-28 Thread Aaron Ma
Hi Dmitry and Chiristopher:

Do you have any suggestion about these 2 patches?

Many users confirmed that they fixed issues of Trackpoint/Touchpad after S3.

Will you consider them be accepted?

Thanks,
Aaron


[PATCH] iommu/amd: Fix NULL dereference bug in match_hid_uid

2019-03-09 Thread Aaron Ma
Add a non-NULL check to fix potential NULL pointer dereference
Cleanup code to call function once.

Signed-off-by: Aaron Ma 
---
 drivers/iommu/amd_iommu.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 2a7b78bb98b4..dbda5b86f979 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -140,9 +140,14 @@ static inline int match_hid_uid(struct device *dev,
struct acpihid_map_entry *entry)
 {
const char *hid, *uid;
+   struct acpi_device *adev = ACPI_COMPANION(dev);
 
-   hid = acpi_device_hid(ACPI_COMPANION(dev));
-   uid = acpi_device_uid(ACPI_COMPANION(dev));
+   if (!adev) {
+   return -ENODEV;
+   }
+
+   hid = acpi_device_hid(adev);
+   uid = acpi_device_uid(adev);
 
if (!hid || !(*hid))
return -ENODEV;
-- 
2.20.1



Re: [PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-03-09 Thread Aaron Ma
Hi,

On 3/9/19 7:13 AM, Christopher Heiny wrote:
> I'm not sure this is a safe action, due to a race condition with the
> actual IRQ handler (rmi_process_interrupt_requests from rmi_driver.c). 
> Remember that reading the IRQ status register clears all the IRQ bits. 
> So you're faced with this possible scenario:
> - ATTN asserted, indicating new data in IRQ status register
> - rmi_driver_set_irq_bits called
> - rmi_driver_set_irq_bits reads IRQ status, clearing bits
> - rmi_process_interrupt_requests called
> - rmi_process_interrupt_request reads IRQ status, sees no
>   bits set, nested IRQs are not handled
> This could lead to loss of data or inconsistent system state
> information.  For example, a button up or down event may be lost, with
> consequent weird behavior by the user interface.

rmi_driver_set_irq_bits is only called to config and enable specific
functions of RMI.
Reading IRQ status before set irqs is supposed to clear spam data/irq
status.
spam data make probe/detect touchpad/trackpoint fail.

rmi_smb_resume -> rmi_driver_reset_handler -> fn-config ->
clear_irq_bits -> set_irq_bits -> enable_irq -> irq_handler  ->
rmi_process_interrupt_requests

set_irq_bits will not be in interrupt context, it enables IRQ bits of RMI.

Regards,
Aaron


[PATCH 1/2] Input: synaptics-rmi4 - clear irqs before set irqs

2019-02-20 Thread Aaron Ma
rmi4 got spam data after S3 resume on some ThinkPads.
Then TrackPoint lost when be detected by psmouse.
Clear irqs status before set irqs will make TrackPoint back.

BugLink: https://bugs.launchpad.net/bugs/1791427
Cc: 
Signed-off-by: Aaron Ma 
---
 drivers/input/rmi4/rmi_driver.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index fc3ab93b7aea..20631b272f43 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -374,6 +374,17 @@ static int rmi_driver_set_irq_bits(struct rmi_device 
*rmi_dev,
struct device *dev = _dev->dev;
 
mutex_lock(>irq_mutex);
+
+   /* Dummy read in order to clear irqs */
+   error = rmi_read_block(rmi_dev,
+   data->f01_container->fd.data_base_addr + 1,
+   data->irq_status, data->num_of_irq_regs);
+   if (error < 0) {
+   dev_err(dev, "%s: Failed to read interrupt status!",
+   __func__);
+   goto error_unlock;
+   }
+
bitmap_or(data->new_irq_mask,
  data->current_irq_mask, mask, data->irq_count);
 
-- 
2.17.1



[PATCH 2/2] Input: synaptics-rmi4 - export nosleep of f01 via sysfs

2019-02-20 Thread Aaron Ma
Some of ThinkPad X1C6 touchpads didn't wakeup after resume.
Forcing enable nosleep make touchpad back.
Add nosleep via sysfs, so user can control it to workaround issue.

/sys/devices/rmi4-00/nosleep can be written non-zero will enable
nosleep mode.

BugLink: https://bugs.launchpad.net/bugs/1791427
Cc: 
Signed-off-by: Aaron Ma 
---
 drivers/input/rmi4/rmi_f01.c | 45 
 1 file changed, 45 insertions(+)

diff --git a/drivers/input/rmi4/rmi_f01.c b/drivers/input/rmi4/rmi_f01.c
index 4edaa14fe878..e41d1ec625d9 100644
--- a/drivers/input/rmi4/rmi_f01.c
+++ b/drivers/input/rmi4/rmi_f01.c
@@ -325,12 +325,57 @@ static ssize_t rmi_driver_package_id_show(struct device 
*dev,
 
 static DEVICE_ATTR(package_id, 0444, rmi_driver_package_id_show, NULL);
 
+static ssize_t rmi_driver_nosleep_show(struct device *dev,
+ struct device_attribute *dattr,
+ char *buf)
+{
+   struct rmi_driver_data *data = dev_get_drvdata(dev);
+   struct f01_data *f01 = dev_get_drvdata(>f01_container->dev);
+   int f01_nosleep;
+
+   f01_nosleep = ((f01->device_control.ctrl0 & RMI_F01_CTRL0_NOSLEEP_BIT)
+? 1 : 0);
+
+   return scnprintf(buf, PAGE_SIZE, "%d\n", f01_nosleep);
+}
+
+static ssize_t rmi_driver_nosleep_store(struct device *dev,
+ struct device_attribute *dattr,
+ const char *buf, size_t count)
+{
+   struct rmi_driver_data *data = dev_get_drvdata(dev);
+   struct f01_data *f01 = dev_get_drvdata(>f01_container->dev);
+   int error;
+
+   if (count <= 0)
+   return count;
+
+   if ('0' == *buf) {
+   f01->device_control.ctrl0 &= ~RMI_F01_CTRL0_NOSLEEP_BIT;
+   } else {
+   f01->device_control.ctrl0 |= RMI_F01_CTRL0_NOSLEEP_BIT;
+   }
+
+   error = rmi_write(data->rmi_dev,
+   data->f01_container->fd.control_base_addr,
+ f01->device_control.ctrl0);
+   if (error) {
+   dev_err(dev, "Failed to write nosleep mode: %d.\n", error);
+   }
+
+   return count;
+}
+
+static DEVICE_ATTR(nosleep, 0644,
+   rmi_driver_nosleep_show, rmi_driver_nosleep_store);
+
 static struct attribute *rmi_f01_attrs[] = {
_attr_manufacturer_id.attr,
_attr_date_of_manufacture.attr,
_attr_product_id.attr,
_attr_firmware_id.attr,
_attr_package_id.attr,
+   _attr_nosleep.attr,
NULL
 };
 
-- 
2.17.1



[PATCH] Input: elantech - enable middle button of touchpad on ThinkPad P72

2018-09-07 Thread Aaron Ma
Adding 2 new touchpad IDs to support middle button support.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 44f57cf6675b..2d95e8d93cc7 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1178,6 +1178,8 @@ static const struct dmi_system_id 
elantech_dmi_has_middle_button[] = {
 static const char * const middle_button_pnp_ids[] = {
"LEN2131", /* ThinkPad P52 w/ NFC */
"LEN2132", /* ThinkPad P52 */
+   "LEN2133", /* ThinkPad P72 w/ NFC */
+   "LEN2134", /* ThinkPad P72 */
NULL
 };
 
-- 
2.17.1



[PATCH] Input: elantech - enable middle button of touchpad on ThinkPad P72

2018-09-07 Thread Aaron Ma
Adding 2 new touchpad IDs to support middle button support.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index 44f57cf6675b..2d95e8d93cc7 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1178,6 +1178,8 @@ static const struct dmi_system_id 
elantech_dmi_has_middle_button[] = {
 static const char * const middle_button_pnp_ids[] = {
"LEN2131", /* ThinkPad P52 w/ NFC */
"LEN2132", /* ThinkPad P52 */
+   "LEN2133", /* ThinkPad P72 w/ NFC */
+   "LEN2134", /* ThinkPad P72 */
NULL
 };
 
-- 
2.17.1



[PATCH] ACPI / EC: Use ec_no_wakeup on ThinkPad X1 Yoga 3rd

2018-07-31 Thread Aaron Ma
Like on X1C6, on X1Y3 EC interrupts constantly wake up system from
s2idle, the power consumption is extremely high.
So make ec_no_wakeup be true as default to keep system in s2idle mode
and reduce power consumption.

Power button works when ec_no_wakeup=true.

Signed-off-by: Aaron Ma 
---
 drivers/acpi/ec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 917f77f4cb55..6291d5c77267 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -2045,6 +2045,13 @@ static const struct dmi_system_id acpi_ec_no_wakeup[] = {
DMI_MATCH(DMI_PRODUCT_FAMILY, "Thinkpad X1 Carbon 6th"),
},
},
+   {
+   .ident = "ThinkPad X1 Yoga 3rd",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+   DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad X1 Yoga 3rd"),
+   },
+   },
{ },
 };
 
-- 
2.17.1



[PATCH] ACPI / EC: Use ec_no_wakeup on ThinkPad X1 Yoga 3rd

2018-07-31 Thread Aaron Ma
Like on X1C6, on X1Y3 EC interrupts constantly wake up system from
s2idle, the power consumption is extremely high.
So make ec_no_wakeup be true as default to keep system in s2idle mode
and reduce power consumption.

Power button works when ec_no_wakeup=true.

Signed-off-by: Aaron Ma 
---
 drivers/acpi/ec.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 917f77f4cb55..6291d5c77267 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -2045,6 +2045,13 @@ static const struct dmi_system_id acpi_ec_no_wakeup[] = {
DMI_MATCH(DMI_PRODUCT_FAMILY, "Thinkpad X1 Carbon 6th"),
},
},
+   {
+   .ident = "ThinkPad X1 Yoga 3rd",
+   .matches = {
+   DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
+   DMI_MATCH(DMI_PRODUCT_FAMILY, "ThinkPad X1 Yoga 3rd"),
+   },
+   },
{ },
 };
 
-- 
2.17.1



Re: [PATCH] Input: elan: enable middle button of touchpads on ThinkPad P52

2018-06-19 Thread Aaron Ma
On 06/19/2018 08:21 PM, Benjamin Tissoires wrote:
> On Tue, Jun 12, 2018 at 9:10 AM Aaron Ma  wrote:
>> PNPID is better way to identify the type of touchpads.
>> Enable middle button support on 2 types of touchpads on Lenovo P52.
>>
>> Cc: sta...@vger.kernel.org
>> Cc: KT Liao 
>> Signed-off-by: Aaron Ma 
>> ---
>>  drivers/input/mouse/elantech.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
> Looks good to me:
> Reviewed-by: Benjamin Tissoires 
> 
> Aaron, our internal tests show that the P52 shows a lot of lost
> synchronization in pre-v4.18 kernels. Do you have a fix for that too?
> 

https://lkml.org/lkml/2018/5/28/694
should fix the sync issue.


> In v4.18 we need to fix the elan_i2c module as right now the touchpad
> is muted when booted with such a kernel.

Does this touchpad support elan_i2c?

Regards,
Aaron

> 
> Cheers,
> Benjamin
> 
>> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
>> index fb4d902c4403..67b061dd4494 100644
>> --- a/drivers/input/mouse/elantech.c
>> +++ b/drivers/input/mouse/elantech.c
>> @@ -1175,6 +1175,12 @@ static const struct dmi_system_id 
>> elantech_dmi_has_middle_button[] = {
>> { }
>>  };
>>
>> +static const char * const middle_button_pnp_ids[] = {
>> +   "LEN2131", /* ThinkPad P52 w/ NFC */
>> +   "LEN2132", /* ThinkPad P52 */
>> +   NULL
>> +};
>> +
>>  /*
>>   * Set the appropriate event bits for the input subsystem
>>   */
>> @@ -1194,7 +1200,8 @@ static int elantech_set_input_params(struct psmouse 
>> *psmouse)
>> __clear_bit(EV_REL, dev->evbit);
>>
>> __set_bit(BTN_LEFT, dev->keybit);
>> -   if (dmi_check_system(elantech_dmi_has_middle_button))
>> +   if (dmi_check_system(elantech_dmi_has_middle_button) ||
>> +   psmouse_matches_pnp_id(psmouse, middle_button_pnp_ids))
>> __set_bit(BTN_MIDDLE, dev->keybit);
>> __set_bit(BTN_RIGHT, dev->keybit);
>>
>> --
>> 2.17.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Input: elan: enable middle button of touchpads on ThinkPad P52

2018-06-19 Thread Aaron Ma
On 06/19/2018 08:21 PM, Benjamin Tissoires wrote:
> On Tue, Jun 12, 2018 at 9:10 AM Aaron Ma  wrote:
>> PNPID is better way to identify the type of touchpads.
>> Enable middle button support on 2 types of touchpads on Lenovo P52.
>>
>> Cc: sta...@vger.kernel.org
>> Cc: KT Liao 
>> Signed-off-by: Aaron Ma 
>> ---
>>  drivers/input/mouse/elantech.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
> Looks good to me:
> Reviewed-by: Benjamin Tissoires 
> 
> Aaron, our internal tests show that the P52 shows a lot of lost
> synchronization in pre-v4.18 kernels. Do you have a fix for that too?
> 

https://lkml.org/lkml/2018/5/28/694
should fix the sync issue.


> In v4.18 we need to fix the elan_i2c module as right now the touchpad
> is muted when booted with such a kernel.

Does this touchpad support elan_i2c?

Regards,
Aaron

> 
> Cheers,
> Benjamin
> 
>> diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
>> index fb4d902c4403..67b061dd4494 100644
>> --- a/drivers/input/mouse/elantech.c
>> +++ b/drivers/input/mouse/elantech.c
>> @@ -1175,6 +1175,12 @@ static const struct dmi_system_id 
>> elantech_dmi_has_middle_button[] = {
>> { }
>>  };
>>
>> +static const char * const middle_button_pnp_ids[] = {
>> +   "LEN2131", /* ThinkPad P52 w/ NFC */
>> +   "LEN2132", /* ThinkPad P52 */
>> +   NULL
>> +};
>> +
>>  /*
>>   * Set the appropriate event bits for the input subsystem
>>   */
>> @@ -1194,7 +1200,8 @@ static int elantech_set_input_params(struct psmouse 
>> *psmouse)
>> __clear_bit(EV_REL, dev->evbit);
>>
>> __set_bit(BTN_LEFT, dev->keybit);
>> -   if (dmi_check_system(elantech_dmi_has_middle_button))
>> +   if (dmi_check_system(elantech_dmi_has_middle_button) ||
>> +   psmouse_matches_pnp_id(psmouse, middle_button_pnp_ids))
>> __set_bit(BTN_MIDDLE, dev->keybit);
>> __set_bit(BTN_RIGHT, dev->keybit);
>>
>> --
>> 2.17.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-input" in
>> the body of a message to majord...@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] Input: elan: enable middle button of touchpads on ThinkPad P52

2018-06-12 Thread Aaron Ma
PNPID is better way to identify the type of touchpads.
Enable middle button support on 2 types of touchpads on Lenovo P52.

Cc: sta...@vger.kernel.org
Cc: KT Liao 
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index fb4d902c4403..67b061dd4494 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1175,6 +1175,12 @@ static const struct dmi_system_id 
elantech_dmi_has_middle_button[] = {
{ }
 };
 
+static const char * const middle_button_pnp_ids[] = {
+   "LEN2131", /* ThinkPad P52 w/ NFC */
+   "LEN2132", /* ThinkPad P52 */
+   NULL
+};
+
 /*
  * Set the appropriate event bits for the input subsystem
  */
@@ -1194,7 +1200,8 @@ static int elantech_set_input_params(struct psmouse 
*psmouse)
__clear_bit(EV_REL, dev->evbit);
 
__set_bit(BTN_LEFT, dev->keybit);
-   if (dmi_check_system(elantech_dmi_has_middle_button))
+   if (dmi_check_system(elantech_dmi_has_middle_button) ||
+   psmouse_matches_pnp_id(psmouse, middle_button_pnp_ids))
__set_bit(BTN_MIDDLE, dev->keybit);
__set_bit(BTN_RIGHT, dev->keybit);
 
-- 
2.17.1



[PATCH] Input: elan: enable middle button of touchpads on ThinkPad P52

2018-06-12 Thread Aaron Ma
PNPID is better way to identify the type of touchpads.
Enable middle button support on 2 types of touchpads on Lenovo P52.

Cc: sta...@vger.kernel.org
Cc: KT Liao 
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index fb4d902c4403..67b061dd4494 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1175,6 +1175,12 @@ static const struct dmi_system_id 
elantech_dmi_has_middle_button[] = {
{ }
 };
 
+static const char * const middle_button_pnp_ids[] = {
+   "LEN2131", /* ThinkPad P52 w/ NFC */
+   "LEN2132", /* ThinkPad P52 */
+   NULL
+};
+
 /*
  * Set the appropriate event bits for the input subsystem
  */
@@ -1194,7 +1200,8 @@ static int elantech_set_input_params(struct psmouse 
*psmouse)
__clear_bit(EV_REL, dev->evbit);
 
__set_bit(BTN_LEFT, dev->keybit);
-   if (dmi_check_system(elantech_dmi_has_middle_button))
+   if (dmi_check_system(elantech_dmi_has_middle_button) ||
+   psmouse_matches_pnp_id(psmouse, middle_button_pnp_ids))
__set_bit(BTN_MIDDLE, dev->keybit);
__set_bit(BTN_RIGHT, dev->keybit);
 
-- 
2.17.1



Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)

2018-05-16 Thread Aaron Ma
On 05/16/2018 03:09 PM, Teika Kazura wrote:
> From: Aaron Ma <aaron...@canonical.com>
> Date:   Tue, 17 Apr 2018 19:42:27 +0800
> 
>> Could you apply my patch too?
>>
>> It add LEN0096 that Benjamin's patch doesn't include.
>>
>> +"LEN0096", /* X280 */
> Aaron, in your original patch in last Oct [1], both *LEN0092 and* LEN0096 
> were aded. Which should be the case, both two, or only LEN0096?

Hi Teika:

Both LEN0096/LENO0092 are needed, so I think my original patch should be
merged, Benjamin's patch only include one of them.

Regards,
Aaron

> 
> [1] https://www.spinics.net/lists/kernel/msg2625450.html
> 
> Teika (Teika kazura)
> 


Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)

2018-05-16 Thread Aaron Ma
On 05/16/2018 03:09 PM, Teika Kazura wrote:
> From: Aaron Ma 
> Date:   Tue, 17 Apr 2018 19:42:27 +0800
> 
>> Could you apply my patch too?
>>
>> It add LEN0096 that Benjamin's patch doesn't include.
>>
>> +"LEN0096", /* X280 */
> Aaron, in your original patch in last Oct [1], both *LEN0092 and* LEN0096 
> were aded. Which should be the case, both two, or only LEN0096?

Hi Teika:

Both LEN0096/LENO0092 are needed, so I think my original patch should be
merged, Benjamin's patch only include one of them.

Regards,
Aaron

> 
> [1] https://www.spinics.net/lists/kernel/msg2625450.html
> 
> Teika (Teika kazura)
> 


[PATCH v2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-18 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

V2:
The V1 patch leads to the following static checker warning:

drivers/hid/i2c-hid/i2c-hid.c:1244 i2c_hid_resume()
info: return a literal instead of 'ret'

This test was inverted in V1 patch.
Change the condition of condition.

Cc: sta...@vger.kernel.org
Cc: Dan Carpenter <dan.carpen...@oracle.com>
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-ids.h |  3 +++
 drivers/hid/i2c-hid/i2c-hid.c | 13 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a3a7ead3012..0b5cc910f62e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 97689e98e53f..963328674e93 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1220,6 +1223,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



[PATCH v2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-18 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

V2:
The V1 patch leads to the following static checker warning:

drivers/hid/i2c-hid/i2c-hid.c:1244 i2c_hid_resume()
info: return a literal instead of 'ret'

This test was inverted in V1 patch.
Change the condition of condition.

Cc: sta...@vger.kernel.org
Cc: Dan Carpenter 
Signed-off-by: Aaron Ma 
---
 drivers/hid/hid-ids.h |  3 +++
 drivers/hid/i2c-hid/i2c-hid.c | 13 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a3a7ead3012..0b5cc910f62e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 97689e98e53f..963328674e93 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1220,6 +1223,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)

2018-04-17 Thread Aaron Ma
Hi Dmitry and Benjamin:

Could you apply my patch too?

It add LEN0096 that Benjamin's patch doesn't include.

+   "LEN0096", /* X280 */

Regards,
Aaron


Re: [PATCH v2 0/9] Input: support for latest Lenovo thinkpads (series 80)

2018-04-17 Thread Aaron Ma
Hi Dmitry and Benjamin:

Could you apply my patch too?

It add LEN0096 that Benjamin's patch doesn't include.

+   "LEN0096", /* X280 */

Regards,
Aaron


[PATCH] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-09 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-ids.h |  3 +++
 drivers/hid/i2c-hid/i2c-hid.c | 13 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a3a7ead3012..0b5cc910f62e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 97689e98e53f..615a91ac93bd 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1220,6 +1223,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (!ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



[PATCH] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-09 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

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

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5a3a7ead3012..0b5cc910f62e 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -525,6 +525,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 97689e98e53f..615a91ac93bd 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1220,6 +1223,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (!ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-09 Thread Aaron Ma
Hi Jiri:

Re-send this patch.

Thanks,
Aaron

On 01/03/2018 01:30 AM, Aaron Ma wrote:
> When Rayd touchscreen resumed from S3, it issues too many errors like:
> i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)
> 
> And all the report data are corrupted, touchscreen is unresponsive.
> 
> Fix this by re-sending report description command after resume.
> Add device ID as a quirk.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Aaron Ma <aaron...@canonical.com>
> ---
>  drivers/hid/hid-ids.h |  3 +++
>  drivers/hid/i2c-hid/i2c-hid.c | 13 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5da3d6256d25..753cc10aa699 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -516,6 +516,9 @@
>  #define I2C_VENDOR_ID_HANTICK0x0911
>  #define I2C_PRODUCT_ID_HANTICK_5288  0x5288
>  
> +#define I2C_VENDOR_ID_RAYD   0x2386
> +#define I2C_PRODUCT_ID_RAYD_3118 0x3118
> +
>  #define USB_VENDOR_ID_HANWANG0x0b57
>  #define USB_DEVICE_ID_HANWANG_TABLET_FIRST   0x5000
>  #define USB_DEVICE_ID_HANWANG_TABLET_LAST0x8fff
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 09404ffdb08b..57a447a9d40e 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -47,6 +47,7 @@
>  /* quirks to control the device */
>  #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
>  #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> +#define I2C_HID_QUIRK_RESEND_REPORT_DESCRBIT(2)
>  
>  /* flags */
>  #define I2C_HID_STARTED  0
> @@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
>   I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
>   { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
>   I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> + { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> + I2C_HID_QUIRK_RESEND_REPORT_DESCR },
>   { 0, 0 }
>  };
>  
> @@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev)
>   if (ret)
>   return ret;
>  
> + /* RAYDIUM device (2386:3118) need to re-send report descr cmd
> +  * after resume, after this it will be back normal.
> +  * otherwise it issues too many incomplete reports.
> +  */
> + if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
> + ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
> + if (!ret)
> + return ret;
> + }
> +
>   if (hid->driver && hid->driver->reset_resume) {
>   ret = hid->driver->reset_resume(hid);
>   return ret;
> -- 2.14.3
> 


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-09 Thread Aaron Ma
Hi Jiri:

Re-send this patch.

Thanks,
Aaron

On 01/03/2018 01:30 AM, Aaron Ma wrote:
> When Rayd touchscreen resumed from S3, it issues too many errors like:
> i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)
> 
> And all the report data are corrupted, touchscreen is unresponsive.
> 
> Fix this by re-sending report description command after resume.
> Add device ID as a quirk.
> 
> Cc: sta...@vger.kernel.org
> Signed-off-by: Aaron Ma 
> ---
>  drivers/hid/hid-ids.h |  3 +++
>  drivers/hid/i2c-hid/i2c-hid.c | 13 +
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5da3d6256d25..753cc10aa699 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -516,6 +516,9 @@
>  #define I2C_VENDOR_ID_HANTICK0x0911
>  #define I2C_PRODUCT_ID_HANTICK_5288  0x5288
>  
> +#define I2C_VENDOR_ID_RAYD   0x2386
> +#define I2C_PRODUCT_ID_RAYD_3118 0x3118
> +
>  #define USB_VENDOR_ID_HANWANG0x0b57
>  #define USB_DEVICE_ID_HANWANG_TABLET_FIRST   0x5000
>  #define USB_DEVICE_ID_HANWANG_TABLET_LAST0x8fff
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 09404ffdb08b..57a447a9d40e 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -47,6 +47,7 @@
>  /* quirks to control the device */
>  #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
>  #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
> +#define I2C_HID_QUIRK_RESEND_REPORT_DESCRBIT(2)
>  
>  /* flags */
>  #define I2C_HID_STARTED  0
> @@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
>   I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
>   { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
>   I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> + { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> + I2C_HID_QUIRK_RESEND_REPORT_DESCR },
>   { 0, 0 }
>  };
>  
> @@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev)
>   if (ret)
>   return ret;
>  
> + /* RAYDIUM device (2386:3118) need to re-send report descr cmd
> +  * after resume, after this it will be back normal.
> +  * otherwise it issues too many incomplete reports.
> +  */
> + if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
> + ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
> + if (!ret)
> + return ret;
> + }
> +
>   if (hid->driver && hid->driver->reset_resume) {
>   ret = hid->driver->reset_resume(hid);
>   return ret;
> -- 2.14.3
> 


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-03 Thread Aaron Ma
Hi Jiri:

This patch is pending for long time.

Could you merge this single patch to upstream?

Regards,
Aaron


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-04-03 Thread Aaron Ma
Hi Jiri:

This patch is pending for long time.

Could you merge this single patch to upstream?

Regards,
Aaron


[PATCH] platform/x86: ideapad-laptop: Increase timeout to wait for EC answer

2018-02-11 Thread Aaron Ma
Lenovo E41-20 needs more time than 100ms to read VPC,
the funtion keys always failed responding.
Increase timeout to get the value from VPC, then
the funtion keys like mic mute key work well.

Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/platform/x86/ideapad-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c 
b/drivers/platform/x86/ideapad-laptop.c
index 5b6f18b18801..535199c9e6bc 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -113,7 +113,7 @@ MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
 /*
  * ACPI Helpers
  */
-#define IDEAPAD_EC_TIMEOUT (100) /* in ms */
+#define IDEAPAD_EC_TIMEOUT (200) /* in ms */
 
 static int read_method_int(acpi_handle handle, const char *method, int *val)
 {
-- 
2.14.3



[PATCH] platform/x86: ideapad-laptop: Increase timeout to wait for EC answer

2018-02-11 Thread Aaron Ma
Lenovo E41-20 needs more time than 100ms to read VPC,
the funtion keys always failed responding.
Increase timeout to get the value from VPC, then
the funtion keys like mic mute key work well.

Signed-off-by: Aaron Ma 
---
 drivers/platform/x86/ideapad-laptop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/ideapad-laptop.c 
b/drivers/platform/x86/ideapad-laptop.c
index 5b6f18b18801..535199c9e6bc 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -113,7 +113,7 @@ MODULE_PARM_DESC(no_bt_rfkill, "No rfkill for bluetooth.");
 /*
  * ACPI Helpers
  */
-#define IDEAPAD_EC_TIMEOUT (100) /* in ms */
+#define IDEAPAD_EC_TIMEOUT (200) /* in ms */
 
 static int read_method_int(acpi_handle handle, const char *method, int *val)
 {
-- 
2.14.3



[PATCH] HID: Fix hid_report_len usage

2018-02-03 Thread Aaron Ma
Follow the change of return type u32 of hid_report_len,
fix all the types of variables those get the return value of
hid_report_len to u32, and all other code already uses u32.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-input.c  | 3 ++-
 drivers/hid/hid-multitouch.c | 5 +++--
 drivers/hid/hid-rmi.c| 4 ++--
 drivers/hid/wacom_sys.c  | 4 ++--
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 04d01b57d94c..d86398755b0d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1368,7 +1368,8 @@ static void hidinput_led_worker(struct work_struct *work)
  led_work);
struct hid_field *field;
struct hid_report *report;
-   int len, ret;
+   int ret;
+   u32 len;
__u8 *buf;
 
field = hidinput_get_led_field(hid);
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3b4739bde05d..2e1736ba2444 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -370,7 +370,8 @@ static const struct attribute_group mt_attribute_group = {
 static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 {
struct mt_device *td = hid_get_drvdata(hdev);
-   int ret, size = hid_report_len(report);
+   int ret;
+   u32 size = hid_report_len(report);
u8 *buf;
 
/*
@@ -1183,7 +1184,7 @@ static void mt_set_input_mode(struct hid_device *hdev)
struct hid_report_enum *re;
struct mt_class *cls = >mtclass;
char *buf;
-   int report_len;
+   u32 report_len;
 
if (td->inputmode < 0)
return;
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index c6c05df3e8d2..9c9362149641 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -89,8 +89,8 @@ struct rmi_data {
u8 *writeReport;
u8 *readReport;
 
-   int input_report_size;
-   int output_report_size;
+   u32 input_report_size;
+   u32 output_report_size;
 
unsigned long flags;
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 409543160af7..b54ef1ffcbec 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -219,7 +219,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
u8 *data;
int ret;
-   int n;
+   u32 n;
 
switch (equivalent_usage) {
case HID_DG_CONTACTMAX:
@@ -519,7 +519,7 @@ static int wacom_set_device_mode(struct hid_device *hdev,
u8 *rep_data;
struct hid_report *r;
struct hid_report_enum *re;
-   int length;
+   u32 length;
int error = -ENOMEM, limit = 0;
 
if (wacom_wac->mode_report < 0)
-- 
2.14.3



[PATCH] HID: Fix hid_report_len usage

2018-02-03 Thread Aaron Ma
Follow the change of return type u32 of hid_report_len,
fix all the types of variables those get the return value of
hid_report_len to u32, and all other code already uses u32.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/hid/hid-input.c  | 3 ++-
 drivers/hid/hid-multitouch.c | 5 +++--
 drivers/hid/hid-rmi.c| 4 ++--
 drivers/hid/wacom_sys.c  | 4 ++--
 4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 04d01b57d94c..d86398755b0d 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -1368,7 +1368,8 @@ static void hidinput_led_worker(struct work_struct *work)
  led_work);
struct hid_field *field;
struct hid_report *report;
-   int len, ret;
+   int ret;
+   u32 len;
__u8 *buf;
 
field = hidinput_get_led_field(hid);
diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 3b4739bde05d..2e1736ba2444 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -370,7 +370,8 @@ static const struct attribute_group mt_attribute_group = {
 static void mt_get_feature(struct hid_device *hdev, struct hid_report *report)
 {
struct mt_device *td = hid_get_drvdata(hdev);
-   int ret, size = hid_report_len(report);
+   int ret;
+   u32 size = hid_report_len(report);
u8 *buf;
 
/*
@@ -1183,7 +1184,7 @@ static void mt_set_input_mode(struct hid_device *hdev)
struct hid_report_enum *re;
struct mt_class *cls = >mtclass;
char *buf;
-   int report_len;
+   u32 report_len;
 
if (td->inputmode < 0)
return;
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index c6c05df3e8d2..9c9362149641 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -89,8 +89,8 @@ struct rmi_data {
u8 *writeReport;
u8 *readReport;
 
-   int input_report_size;
-   int output_report_size;
+   u32 input_report_size;
+   u32 output_report_size;
 
unsigned long flags;
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 409543160af7..b54ef1ffcbec 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -219,7 +219,7 @@ static void wacom_feature_mapping(struct hid_device *hdev,
unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
u8 *data;
int ret;
-   int n;
+   u32 n;
 
switch (equivalent_usage) {
case HID_DG_CONTACTMAX:
@@ -519,7 +519,7 @@ static int wacom_set_device_mode(struct hid_device *hdev,
u8 *rep_data;
struct hid_report *r;
struct hid_report_enum *re;
-   int length;
+   u32 length;
int error = -ENOMEM, limit = 0;
 
if (wacom_wac->mode_report < 0)
-- 
2.14.3



Re: [PATCH v2 2/2] HID: core: Fix size as type u32

2018-02-03 Thread Aaron Ma
On 02/03/2018 03:55 PM, Marcus Folkesson wrote:
> Hi Aaron,
> 
> On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote:
>> When size is negative, calling memset will make segment fault.
>> Declare the size as type u32 to keep memset safe.
>>
>> size in struct hid_report is unsigned, fix return type of
>> hid_report_len to u32.
>>
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Aaron Ma <aaron...@canonical.com>
>> ---
>>  drivers/hid/hid-core.c | 10 +-
>>  include/linux/hid.h|  6 +++---
>>  2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 0c3f608131cf..cf81c53e3b98 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, 
>> gfp_t flags)
>>   * of implement() working on 8 byte chunks
>>   */
>>  
>> -int len = hid_report_len(report) + 7;
>> +u32 len = hid_report_len(report) + 7;
>>  
>>  return kmalloc(len, flags);
>>  }
>> @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct 
>> hid_report *report,
>>  {
>>  char *buf;
>>  int ret;
>> -int len;
>> +u32 len;
>>  
>>  buf = hid_alloc_report_buf(report, GFP_KERNEL);
>>  if (!buf)
>> @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct 
>> hid_report *report,
>>  }
>>  EXPORT_SYMBOL_GPL(__hid_request);
>>  
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int 
>> size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 
>> size,
>>  int interrupt)
>>  {
>>  struct hid_report_enum *report_enum = hid->report_enum + type;
>>  struct hid_report *report;
>>  struct hid_driver *hdrv;
>>  unsigned int a;
>> -int rsize, csize = size;
>> +u32 rsize, csize = size;
>>  u8 *cdata = data;
>>  int ret = 0;
>>  
>> @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
>>   *
>>   * This is data entry for lower layers.
>>   */
>> -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, 
>> int interrupt)
>> +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, 
>> int interrupt)
>>  {
>>  struct hid_report_enum *report_enum;
>>  struct hid_driver *hdrv;
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index d491027a7c22..9bc296eebc98 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, 
>> unsigned int force);
>>  extern void hidinput_disconnect(struct hid_device *);
>>  
>>  int hid_set_field(struct hid_field *, unsigned, __s32);
>> -int hid_input_report(struct hid_device *, int type, u8 *, int, int);
>> +int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
>>  int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned 
>> int code, struct hid_field **field);
>>  struct hid_field *hidinput_get_led_field(struct hid_device *hid);
>>  unsigned int hidinput_count_leds(struct hid_device *hid);
>> @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device 
>> *hdev)
>>   *
>>   * @report: the report we want to know the length
>>   */
>> -static inline int hid_report_len(struct hid_report *report)
>> +static inline u32 hid_report_len(struct hid_report *report)
> hid_report_len() is used in several files.
> If we think it is a good idea to change the return type, we should fix
> these files as well.
> 
> [08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len
> drivers/hid/hid-core.c
> drivers/hid/hid-input.c
> drivers/hid/hid-multitouch.c
> drivers/hid/hid-rmi.c
> drivers/hid/usbhid/hid-core.c
> drivers/hid/wacom_sys.c
> drivers/staging/greybus/hid.c
> include/linux/hid.h


Sure, I will fix these return type in a new patch.

Thanks,
Aaron


> 
>>  {
>>  /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
>>  return ((report->size - 1) >> 3) + 1 + (report->id > 0);
>>  }
>>  
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int 
>> size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 
>> size,
>>  int interrupt);
>>  
>>  /* HID quirks API */
>> -- 
>> 2.14.3
> Best regards
> Marcus Folkesson


Re: [PATCH v2 2/2] HID: core: Fix size as type u32

2018-02-03 Thread Aaron Ma
On 02/03/2018 03:55 PM, Marcus Folkesson wrote:
> Hi Aaron,
> 
> On Mon, Jan 08, 2018 at 10:41:41AM +0800, Aaron Ma wrote:
>> When size is negative, calling memset will make segment fault.
>> Declare the size as type u32 to keep memset safe.
>>
>> size in struct hid_report is unsigned, fix return type of
>> hid_report_len to u32.
>>
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Aaron Ma 
>> ---
>>  drivers/hid/hid-core.c | 10 +-
>>  include/linux/hid.h|  6 +++---
>>  2 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index 0c3f608131cf..cf81c53e3b98 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, 
>> gfp_t flags)
>>   * of implement() working on 8 byte chunks
>>   */
>>  
>> -int len = hid_report_len(report) + 7;
>> +u32 len = hid_report_len(report) + 7;
>>  
>>  return kmalloc(len, flags);
>>  }
>> @@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct 
>> hid_report *report,
>>  {
>>  char *buf;
>>  int ret;
>> -int len;
>> +u32 len;
>>  
>>  buf = hid_alloc_report_buf(report, GFP_KERNEL);
>>  if (!buf)
>> @@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct 
>> hid_report *report,
>>  }
>>  EXPORT_SYMBOL_GPL(__hid_request);
>>  
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int 
>> size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 
>> size,
>>  int interrupt)
>>  {
>>  struct hid_report_enum *report_enum = hid->report_enum + type;
>>  struct hid_report *report;
>>  struct hid_driver *hdrv;
>>  unsigned int a;
>> -int rsize, csize = size;
>> +u32 rsize, csize = size;
>>  u8 *cdata = data;
>>  int ret = 0;
>>  
>> @@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
>>   *
>>   * This is data entry for lower layers.
>>   */
>> -int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, 
>> int interrupt)
>> +int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, 
>> int interrupt)
>>  {
>>  struct hid_report_enum *report_enum;
>>  struct hid_driver *hdrv;
>> diff --git a/include/linux/hid.h b/include/linux/hid.h
>> index d491027a7c22..9bc296eebc98 100644
>> --- a/include/linux/hid.h
>> +++ b/include/linux/hid.h
>> @@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, 
>> unsigned int force);
>>  extern void hidinput_disconnect(struct hid_device *);
>>  
>>  int hid_set_field(struct hid_field *, unsigned, __s32);
>> -int hid_input_report(struct hid_device *, int type, u8 *, int, int);
>> +int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
>>  int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned 
>> int code, struct hid_field **field);
>>  struct hid_field *hidinput_get_led_field(struct hid_device *hid);
>>  unsigned int hidinput_count_leds(struct hid_device *hid);
>> @@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device 
>> *hdev)
>>   *
>>   * @report: the report we want to know the length
>>   */
>> -static inline int hid_report_len(struct hid_report *report)
>> +static inline u32 hid_report_len(struct hid_report *report)
> hid_report_len() is used in several files.
> If we think it is a good idea to change the return type, we should fix
> these files as well.
> 
> [08:47:56]marcus@little:~/git/linux$ git grep -l hid_report_len
> drivers/hid/hid-core.c
> drivers/hid/hid-input.c
> drivers/hid/hid-multitouch.c
> drivers/hid/hid-rmi.c
> drivers/hid/usbhid/hid-core.c
> drivers/hid/wacom_sys.c
> drivers/staging/greybus/hid.c
> include/linux/hid.h


Sure, I will fix these return type in a new patch.

Thanks,
Aaron


> 
>>  {
>>  /* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
>>  return ((report->size - 1) >> 3) + 1 + (report->id > 0);
>>  }
>>  
>> -int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int 
>> size,
>> +int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 
>> size,
>>  int interrupt);
>>  
>>  /* HID quirks API */
>> -- 
>> 2.14.3
> Best regards
> Marcus Folkesson


Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-02-03 Thread Aaron Ma
Hi Marcus:

This patch is replaced by v2 patches you just reviewed.

Thanks,
Aaron



Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-02-03 Thread Aaron Ma
Hi Marcus:

This patch is replaced by v2 patches you just reviewed.

Thanks,
Aaron



Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-02-02 Thread Aaron Ma
Hi

Could anyone review an apply this single patch?

The 2nd patch had been sent as v2.

Regards,
Aaron


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-02-02 Thread Aaron Ma
Hi

Could anyone review an apply this single patch?

The 2nd patch had been sent as v2.

Regards,
Aaron


Re: [PATCH v2 2/2] HID: core: Fix size as type u32

2018-02-02 Thread Aaron Ma
Hi:

Could anyone review and apply these 2 patch?

Regards,
Aaron


Re: [PATCH v2 2/2] HID: core: Fix size as type u32

2018-02-02 Thread Aaron Ma
Hi:

Could anyone review and apply these 2 patch?

Regards,
Aaron


Re: [PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-12 Thread Aaron Ma
Will your patch go to stable kernel?
If yes, that's fine.


Re: [PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-12 Thread Aaron Ma
Will your patch go to stable kernel?
If yes, that's fine.


Re: [PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-12 Thread Aaron Ma
Only this laptop had been confirmed is ALPS: 0102 – FF02 trackpoint, it
return 0 of extended button.
I saw the other device that returned 0, but don't have it now, so the ID
can not be checked.

ThinkPad always have 3 buttons installed, I suggest always set 0x33 for
button info when the button info returned 0.


Re: [PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-12 Thread Aaron Ma
Only this laptop had been confirmed is ALPS: 0102 – FF02 trackpoint, it
return 0 of extended button.
I saw the other device that returned 0, but don't have it now, so the ID
can not be checked.

ThinkPad always have 3 buttons installed, I suggest always set 0x33 for
button info when the button info returned 0.


[PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-11 Thread Aaron Ma
Lenovo introduced trackpoint compatible sticks with minimum PS/2 commands.
Some of these sticks with 3 buttons always return 0 when reading
extended button info, set it as 3 buttons to enable middle button.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/input/mouse/trackpoint.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 0871010f18d5..00c0d1706567 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -383,6 +383,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool 
set_properties)
if (trackpoint_read(ps2dev, TP_EXT_BTN, _info)) {
psmouse_warn(psmouse, "failed to get extended button data, 
assuming 3 buttons\n");
button_info = 0x33;
+   } else if (!button_info) {
+   psmouse_warn(psmouse,
+   "got no extended button data, assuming 3 buttons\n");
+   button_info = 0x33;
}
 
psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);
-- 
2.14.3



[PATCH] Input: trackpoint - force 3 buttons if 0 button is reported

2018-01-11 Thread Aaron Ma
Lenovo introduced trackpoint compatible sticks with minimum PS/2 commands.
Some of these sticks with 3 buttons always return 0 when reading
extended button info, set it as 3 buttons to enable middle button.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/trackpoint.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/input/mouse/trackpoint.c b/drivers/input/mouse/trackpoint.c
index 0871010f18d5..00c0d1706567 100644
--- a/drivers/input/mouse/trackpoint.c
+++ b/drivers/input/mouse/trackpoint.c
@@ -383,6 +383,10 @@ int trackpoint_detect(struct psmouse *psmouse, bool 
set_properties)
if (trackpoint_read(ps2dev, TP_EXT_BTN, _info)) {
psmouse_warn(psmouse, "failed to get extended button data, 
assuming 3 buttons\n");
button_info = 0x33;
+   } else if (!button_info) {
+   psmouse_warn(psmouse,
+   "got no extended button data, assuming 3 buttons\n");
+   button_info = 0x33;
}
 
psmouse->private = kzalloc(sizeof(struct trackpoint_data), GFP_KERNEL);
-- 
2.14.3



[PATCH v2 1/2] HID: i2c-hid: fix size check and type usage

2018-01-07 Thread Aaron Ma
When convert char array with signed int, if the inbuf[x] is negative then
upper bits will be set to 1. Fix this by using u8 instead of char.

ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/i2c-hid/i2c-hid.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index e054ee43c1e2..d17d1950392b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -144,10 +144,10 @@ struct i2c_hid {
   * register of the HID
   * descriptor. */
unsigned intbufsize;/* i2c buffer size */
-   char*inbuf; /* Input buffer */
-   char*rawbuf;/* Raw Input buffer */
-   char*cmdbuf;/* Command buffer */
-   char*argsbuf;   /* Command arguments buffer */
+   u8  *inbuf; /* Input buffer */
+   u8  *rawbuf;/* Raw Input buffer */
+   u8  *cmdbuf;/* Command buffer */
+   u8  *argsbuf;   /* Command arguments buffer */
 
unsigned long   flags;  /* device flags */
unsigned long   quirks; /* Various quirks */
@@ -455,7 +455,8 @@ static int i2c_hid_hwreset(struct i2c_client *client)
 
 static void i2c_hid_get_input(struct i2c_hid *ihid)
 {
-   int ret, ret_size;
+   int ret;
+   u32 ret_size;
int size = le16_to_cpu(ihid->hdesc.wMaxInputLength);
 
if (size > ihid->bufsize)
@@ -480,7 +481,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
 
-   if (ret_size > size) {
+   if ((ret_size > size) || (ret_size <= 2)) {
dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
return;
-- 
2.14.3



[PATCH v2 2/2] HID: core: Fix size as type u32

2018-01-07 Thread Aaron Ma
When size is negative, calling memset will make segment fault.
Declare the size as type u32 to keep memset safe.

size in struct hid_report is unsigned, fix return type of
hid_report_len to u32.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-core.c | 10 +-
 include/linux/hid.h|  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 0c3f608131cf..cf81c53e3b98 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t 
flags)
 * of implement() working on 8 byte chunks
 */
 
-   int len = hid_report_len(report) + 7;
+   u32 len = hid_report_len(report) + 7;
 
return kmalloc(len, flags);
 }
@@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct 
hid_report *report,
 {
char *buf;
int ret;
-   int len;
+   u32 len;
 
buf = hid_alloc_report_buf(report, GFP_KERNEL);
if (!buf)
@@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct 
hid_report *report,
 }
 EXPORT_SYMBOL_GPL(__hid_request);
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int interrupt)
 {
struct hid_report_enum *report_enum = hid->report_enum + type;
struct hid_report *report;
struct hid_driver *hdrv;
unsigned int a;
-   int rsize, csize = size;
+   u32 rsize, csize = size;
u8 *cdata = data;
int ret = 0;
 
@@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
  *
  * This is data entry for lower layers.
  */
-int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int 
interrupt)
+int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int 
interrupt)
 {
struct hid_report_enum *report_enum;
struct hid_driver *hdrv;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d491027a7c22..9bc296eebc98 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, 
unsigned int force);
 extern void hidinput_disconnect(struct hid_device *);
 
 int hid_set_field(struct hid_field *, unsigned, __s32);
-int hid_input_report(struct hid_device *, int type, u8 *, int, int);
+int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
 int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned 
int code, struct hid_field **field);
 struct hid_field *hidinput_get_led_field(struct hid_device *hid);
 unsigned int hidinput_count_leds(struct hid_device *hid);
@@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev)
  *
  * @report: the report we want to know the length
  */
-static inline int hid_report_len(struct hid_report *report)
+static inline u32 hid_report_len(struct hid_report *report)
 {
/* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
return ((report->size - 1) >> 3) + 1 + (report->id > 0);
 }
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int interrupt);
 
 /* HID quirks API */
-- 
2.14.3



[PATCH v2 1/2] HID: i2c-hid: fix size check and type usage

2018-01-07 Thread Aaron Ma
When convert char array with signed int, if the inbuf[x] is negative then
upper bits will be set to 1. Fix this by using u8 instead of char.

ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/hid/i2c-hid/i2c-hid.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index e054ee43c1e2..d17d1950392b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -144,10 +144,10 @@ struct i2c_hid {
   * register of the HID
   * descriptor. */
unsigned intbufsize;/* i2c buffer size */
-   char*inbuf; /* Input buffer */
-   char*rawbuf;/* Raw Input buffer */
-   char*cmdbuf;/* Command buffer */
-   char*argsbuf;   /* Command arguments buffer */
+   u8  *inbuf; /* Input buffer */
+   u8  *rawbuf;/* Raw Input buffer */
+   u8  *cmdbuf;/* Command buffer */
+   u8  *argsbuf;   /* Command arguments buffer */
 
unsigned long   flags;  /* device flags */
unsigned long   quirks; /* Various quirks */
@@ -455,7 +455,8 @@ static int i2c_hid_hwreset(struct i2c_client *client)
 
 static void i2c_hid_get_input(struct i2c_hid *ihid)
 {
-   int ret, ret_size;
+   int ret;
+   u32 ret_size;
int size = le16_to_cpu(ihid->hdesc.wMaxInputLength);
 
if (size > ihid->bufsize)
@@ -480,7 +481,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
return;
}
 
-   if (ret_size > size) {
+   if ((ret_size > size) || (ret_size <= 2)) {
dev_err(>client->dev, "%s: incomplete report (%d/%d)\n",
__func__, size, ret_size);
return;
-- 
2.14.3



[PATCH v2 2/2] HID: core: Fix size as type u32

2018-01-07 Thread Aaron Ma
When size is negative, calling memset will make segment fault.
Declare the size as type u32 to keep memset safe.

size in struct hid_report is unsigned, fix return type of
hid_report_len to u32.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/hid/hid-core.c | 10 +-
 include/linux/hid.h|  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 0c3f608131cf..cf81c53e3b98 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1390,7 +1390,7 @@ u8 *hid_alloc_report_buf(struct hid_report *report, gfp_t 
flags)
 * of implement() working on 8 byte chunks
 */
 
-   int len = hid_report_len(report) + 7;
+   u32 len = hid_report_len(report) + 7;
 
return kmalloc(len, flags);
 }
@@ -1455,7 +1455,7 @@ void __hid_request(struct hid_device *hid, struct 
hid_report *report,
 {
char *buf;
int ret;
-   int len;
+   u32 len;
 
buf = hid_alloc_report_buf(report, GFP_KERNEL);
if (!buf)
@@ -1481,14 +1481,14 @@ void __hid_request(struct hid_device *hid, struct 
hid_report *report,
 }
 EXPORT_SYMBOL_GPL(__hid_request);
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int interrupt)
 {
struct hid_report_enum *report_enum = hid->report_enum + type;
struct hid_report *report;
struct hid_driver *hdrv;
unsigned int a;
-   int rsize, csize = size;
+   u32 rsize, csize = size;
u8 *cdata = data;
int ret = 0;
 
@@ -1546,7 +1546,7 @@ EXPORT_SYMBOL_GPL(hid_report_raw_event);
  *
  * This is data entry for lower layers.
  */
-int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int 
interrupt)
+int hid_input_report(struct hid_device *hid, int type, u8 *data, u32 size, int 
interrupt)
 {
struct hid_report_enum *report_enum;
struct hid_driver *hdrv;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d491027a7c22..9bc296eebc98 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -841,7 +841,7 @@ extern int hidinput_connect(struct hid_device *hid, 
unsigned int force);
 extern void hidinput_disconnect(struct hid_device *);
 
 int hid_set_field(struct hid_field *, unsigned, __s32);
-int hid_input_report(struct hid_device *, int type, u8 *, int, int);
+int hid_input_report(struct hid_device *, int type, u8 *, u32, int);
 int hidinput_find_field(struct hid_device *hid, unsigned int type, unsigned 
int code, struct hid_field **field);
 struct hid_field *hidinput_get_led_field(struct hid_device *hid);
 unsigned int hidinput_count_leds(struct hid_device *hid);
@@ -1088,13 +1088,13 @@ static inline void hid_hw_wait(struct hid_device *hdev)
  *
  * @report: the report we want to know the length
  */
-static inline int hid_report_len(struct hid_report *report)
+static inline u32 hid_report_len(struct hid_report *report)
 {
/* equivalent to DIV_ROUND_UP(report->size, 8) + !!(report->id > 0) */
return ((report->size - 1) >> 3) + 1 + (report->id > 0);
 }
 
-int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
+int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, u32 size,
int interrupt);
 
 /* HID quirks API */
-- 
2.14.3



Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-01-04 Thread Aaron Ma
I will follow your advice and send V2.

Thanks,
Aaron


Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-01-04 Thread Aaron Ma
I will follow your advice and send V2.

Thanks,
Aaron


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-01-04 Thread Aaron Ma
Hi Benjamin:

Thanks for reviewing my patches.

This issue only happened on this RayD 2386:3118 touchscreen.
No other devices found, so I think it should be in quirk.


Re: [PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-01-04 Thread Aaron Ma
Hi Benjamin:

Thanks for reviewing my patches.

This issue only happened on this RayD 2386:3118 touchscreen.
No other devices found, so I think it should be in quirk.


[PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-01-02 Thread Aaron Ma
When convert char array with signed int, if the inbuf[x] is negative then
upper bits will be set to 1. Fix this by using u8 instead of char.

ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.

size should be more than 0 to keep memset safe.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-core.c|  4 ++--
 drivers/hid/i2c-hid/i2c-hid.c | 10 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 0c3f608131cf..992547771d96 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int 
type, u8 *data, int size,
if (rsize > HID_MAX_BUFFER_SIZE)
rsize = HID_MAX_BUFFER_SIZE;
 
-   if (csize < rsize) {
+   if ((csize < rsize) && (csize > 0)) {
dbg_hid("report %d is too short, (%d < %d)\n", report->id,
csize, rsize);
memset(cdata + csize, 0, rsize - csize);
@@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 
*data, int size, int i
report_enum = hid->report_enum + type;
hdrv = hid->driver;
 
-   if (!size) {
+   if (size <= 0) {
dbg_hid("empty report\n");
ret = -1;
goto unlock;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index e054ee43c1e2..09404ffdb08b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -144,10 +144,10 @@ struct i2c_hid {
   * register of the HID
   * descriptor. */
unsigned intbufsize;/* i2c buffer size */
-   char*inbuf; /* Input buffer */
-   char*rawbuf;/* Raw Input buffer */
-   char*cmdbuf;/* Command buffer */
-   char*argsbuf;   /* Command arguments buffer */
+   u8  *inbuf; /* Input buffer */
+   u8  *rawbuf;/* Raw Input buffer */
+   u8  *cmdbuf;/* Command buffer */
+   u8  *argsbuf;   /* Command arguments buffer */
 
unsigned long   flags;  /* device flags */
unsigned long   quirks; /* Various quirks */
@@ -473,7 +473,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
 
ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
 
-   if (!ret_size) {
+   if (ret_size <= 2) {
/* host or device initiated RESET completed */
if (test_and_clear_bit(I2C_HID_RESET_PENDING, >flags))
wake_up(>wait);
-- 
2.14.3



[PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

2018-01-02 Thread Aaron Ma
When convert char array with signed int, if the inbuf[x] is negative then
upper bits will be set to 1. Fix this by using u8 instead of char.

ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.

size should be more than 0 to keep memset safe.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/hid/hid-core.c|  4 ++--
 drivers/hid/i2c-hid/i2c-hid.c | 10 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 0c3f608131cf..992547771d96 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int 
type, u8 *data, int size,
if (rsize > HID_MAX_BUFFER_SIZE)
rsize = HID_MAX_BUFFER_SIZE;
 
-   if (csize < rsize) {
+   if ((csize < rsize) && (csize > 0)) {
dbg_hid("report %d is too short, (%d < %d)\n", report->id,
csize, rsize);
memset(cdata + csize, 0, rsize - csize);
@@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 
*data, int size, int i
report_enum = hid->report_enum + type;
hdrv = hid->driver;
 
-   if (!size) {
+   if (size <= 0) {
dbg_hid("empty report\n");
ret = -1;
goto unlock;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index e054ee43c1e2..09404ffdb08b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -144,10 +144,10 @@ struct i2c_hid {
   * register of the HID
   * descriptor. */
unsigned intbufsize;/* i2c buffer size */
-   char*inbuf; /* Input buffer */
-   char*rawbuf;/* Raw Input buffer */
-   char*cmdbuf;/* Command buffer */
-   char*argsbuf;   /* Command arguments buffer */
+   u8  *inbuf; /* Input buffer */
+   u8  *rawbuf;/* Raw Input buffer */
+   u8  *cmdbuf;/* Command buffer */
+   u8  *argsbuf;   /* Command arguments buffer */
 
unsigned long   flags;  /* device flags */
unsigned long   quirks; /* Various quirks */
@@ -473,7 +473,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
 
ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
 
-   if (!ret_size) {
+   if (ret_size <= 2) {
/* host or device initiated RESET completed */
if (test_and_clear_bit(I2C_HID_RESET_PENDING, >flags))
wake_up(>wait);
-- 
2.14.3



[PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-01-02 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/hid/hid-ids.h |  3 +++
 drivers/hid/i2c-hid/i2c-hid.c | 13 +
 2 files changed, 16 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5da3d6256d25..753cc10aa699 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -516,6 +516,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 09404ffdb08b..57a447a9d40e 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (!ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



[PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

2018-01-02 Thread Aaron Ma
When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

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

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5da3d6256d25..753cc10aa699 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -516,6 +516,9 @@
 #define I2C_VENDOR_ID_HANTICK  0x0911
 #define I2C_PRODUCT_ID_HANTICK_52880x5288
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118   0x3118
+
 #define USB_VENDOR_ID_HANWANG  0x0b57
 #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
 #define USB_DEVICE_ID_HANWANG_TABLET_LAST  0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 09404ffdb08b..57a447a9d40e 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -47,6 +47,7 @@
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV   BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET   BIT(1)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR  BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED0
@@ -171,6 +172,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+   { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
 };
 
@@ -1211,6 +1214,16 @@ static int i2c_hid_resume(struct device *dev)
if (ret)
return ret;
 
+   /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+* after resume, after this it will be back normal.
+* otherwise it issues too many incomplete reports.
+*/
+   if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+   ret = i2c_hid_command(client, _report_descr_cmd, NULL, 0);
+   if (!ret)
+   return ret;
+   }
+
if (hid->driver && hid->driver->reset_resume) {
ret = hid->driver->reset_resume(hid);
return ret;
-- 
2.14.3



Re: [PATCH] Support TrackStick of Thinkpad L570

2017-11-29 Thread Aaron Ma
Please add the patch version next time.

The patch make trackstick work on L570.

Tested-by: Aaron Ma <aaron...@canonical.com>

On 11/29/2017 04:33 PM, Masaki Ota wrote:
> From: Masaki Ota <masaki@jp.alps.com>
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota <masaki@jp.alps.com>
> ---
>  drivers/input/mouse/alps.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..6f092bdd9fc5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,31 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + if (!is_dual) {
> + /* For support TrackStick of Thinkpad L/E series */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse,
> + 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2588,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> 


Re: [PATCH] Support TrackStick of Thinkpad L570

2017-11-29 Thread Aaron Ma
Please add the patch version next time.

The patch make trackstick work on L570.

Tested-by: Aaron Ma 

On 11/29/2017 04:33 PM, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 24 +---
>  1 file changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..6f092bdd9fc5 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,31 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + if (!is_dual) {
> + /* For support TrackStick of Thinkpad L/E series */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse,
> + 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2588,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> 


Re: [PATCH] Support TrackStick of Thinkpad L570

2017-11-20 Thread Aaron Ma
Tested-by: Aaron Ma <aaron...@canonical.com>

On 11/20/2017 03:55 PM, Masaki Ota wrote:
> From: Masaki Ota <masaki@jp.alps.com>
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota <masaki@jp.alps.com>
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> 


Re: [PATCH] Support TrackStick of Thinkpad L570

2017-11-20 Thread Aaron Ma
Tested-by: Aaron Ma 

On 11/20/2017 03:55 PM, Masaki Ota wrote:
> From: Masaki Ota 
> - The issue is that Thinkpad L570 TrackStick does not work. Because the main 
> interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface 
> Firmware setting of TrackStick. The detail is that TrackStick otp bit is 
> disabled.
> - Add the code that checks 0xD7 address value. This value is device number 
> information, so we can identify the device by checking this value.
> - If we check 0xD7 value, we need to enable Command mode and after check the 
> value we need to disable Command mode, then we have to enable the device(0xF4 
> command).
> - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable 
> ALPS_DUALPOINT flag.
> 
> Signed-off-by: Masaki Ota 
> ---
>  drivers/input/mouse/alps.c | 21 ++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index 850b00e3ad8e..cce52104ed5a 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -2541,13 +2541,28 @@ static int alps_update_btn_info_ss4_v2(unsigned char 
> otp[][4],
>  }
>  
>  static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> -struct alps_data *priv)
> +struct alps_data *priv,
> + struct psmouse *psmouse)
>  {
>   bool is_dual = false;
> + int reg_val = 0;
> + struct ps2dev *ps2dev = >ps2dev;
>  
> - if (IS_SS4PLUS_DEV(priv->dev_id))
> + if (IS_SS4PLUS_DEV(priv->dev_id)) {
>   is_dual = (otp[0][0] >> 4) & 0x01;
>  
> + /* For support TrackStick of Thinkpad L570 device */
> + if (alps_exit_command_mode(psmouse) == 0 &&
> + alps_enter_command_mode(psmouse) == 0) {
> + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> + }
> + alps_exit_command_mode(psmouse);
> + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> +
> + if (reg_val == 0x0C || reg_val == 0x1D)
> + is_dual = true;
> + }
> +
>   if (is_dual)
>   priv->flags |= ALPS_DUALPOINT |
>   ALPS_DUALPOINT_WITH_PRESSURE;
> @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
> *psmouse,
>  
>   alps_update_btn_info_ss4_v2(otp, priv);
>  
> - alps_update_dual_info_ss4_v2(otp, priv);
> + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
>  
>   return 0;
>  }
> 


[PATCH] Input: elantech - add new icbody type 15

2017-11-19 Thread Aaron Ma
The touchpad of Lenovo Thinkpad L480 reports it's version as 15.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/input/mouse/elantech.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index b84cd978fce2..a4aaa748e987 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1613,7 +1613,7 @@ static int elantech_set_properties(struct elantech_data 
*etd)
case 5:
etd->hw_version = 3;
break;
-   case 6 ... 14:
+   case 6 ... 15:
etd->hw_version = 4;
break;
default:
-- 
2.13.6



[PATCH] Input: elantech - add new icbody type 15

2017-11-19 Thread Aaron Ma
The touchpad of Lenovo Thinkpad L480 reports it's version as 15.

Cc: sta...@vger.kernel.org
Signed-off-by: Aaron Ma 
---
 drivers/input/mouse/elantech.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/mouse/elantech.c b/drivers/input/mouse/elantech.c
index b84cd978fce2..a4aaa748e987 100644
--- a/drivers/input/mouse/elantech.c
+++ b/drivers/input/mouse/elantech.c
@@ -1613,7 +1613,7 @@ static int elantech_set_properties(struct elantech_data 
*etd)
case 5:
etd->hw_version = 3;
break;
-   case 6 ... 14:
+   case 6 ... 15:
etd->hw_version = 4;
break;
default:
-- 
2.13.6



Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices

2017-11-16 Thread Aaron Ma
Hi Masaki Ota:

Yes, the laptop is L570.

If your code is right, the workaround code for L570 must be done.

Please fix it ASAP.

Regards,
Aaron

On 11/16/2017 03:27 PM, Masaki Ota wrote:
> Hi, Pali, Aaron,
> 
> Current code is correct device setting, previous code is wrong.
> If the trackstick does not work(DUALPOINT flag disable), Device Firmware 
> setting is wrong.
> 
> But recently I received the same report from Thinkpad L570 user, and I 
> checked this device and found this device Firmware setting is wrong. Sorry 
> for our mistake.
> Is your laptop L570 ?
> 
> I will add code that supports the trackstick for this device.
> 
> Best Regards,
> Masaki Ota
> -Original Message-
> From: Pali Rohár [mailto:pali.ro...@gmail.com] 
> Sent: Wednesday, November 15, 2017 5:35 PM
> To: 太田 真喜 Masaki Ota <masaki@jp.alps.com>
> Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> dmitry.torok...@gmail.com; Aaron Ma <aaron...@canonical.com>
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> 
> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
>> There is a regression of commit 4a646580f793 ("Input: ALPS - fix 
>> two-finger scroll breakage"), ALPS device fails with log:
>>
>> psmouse serio1: alps: Rejected trackstick packet from non DualPoint 
>> device
>>
>> ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit 
>> 4a646580f793, after restore the OTP reading order, it becomes to 0x10 
>> as before and reports the right flag.
>>
>> Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
>> Cc: <sta...@vger.kernel.org>
>> Signed-off-by: Aaron Ma <aaron...@canonical.com>
>> ---
>>  drivers/input/mouse/alps.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
>> index 579b899add26..c59b8f7ca2fc 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 
>> psmouse *psmouse,
>>  
>>  memset(otp, 0, sizeof(otp));
>>  
>> -if (alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]) ||
>> -alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]))
>> +if (alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]) ||
>> +alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]))
>>  return -1;
>>  
>>  alps_update_device_area_ss4_v2(otp, priv);
> 
> Masaki Ota, please look at this patch as it partially revert your commit
> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something 
> smells here.
> 
> --
> Pali Rohár
> pali.ro...@gmail.com
> 


Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices

2017-11-16 Thread Aaron Ma
Hi Masaki Ota:

Yes, the laptop is L570.

If your code is right, the workaround code for L570 must be done.

Please fix it ASAP.

Regards,
Aaron

On 11/16/2017 03:27 PM, Masaki Ota wrote:
> Hi, Pali, Aaron,
> 
> Current code is correct device setting, previous code is wrong.
> If the trackstick does not work(DUALPOINT flag disable), Device Firmware 
> setting is wrong.
> 
> But recently I received the same report from Thinkpad L570 user, and I 
> checked this device and found this device Firmware setting is wrong. Sorry 
> for our mistake.
> Is your laptop L570 ?
> 
> I will add code that supports the trackstick for this device.
> 
> Best Regards,
> Masaki Ota
> -Original Message-
> From: Pali Rohár [mailto:pali.ro...@gmail.com] 
> Sent: Wednesday, November 15, 2017 5:35 PM
> To: 太田 真喜 Masaki Ota 
> Cc: linux-in...@vger.kernel.org; linux-kernel@vger.kernel.org; 
> dmitry.torok...@gmail.com; Aaron Ma 
> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> 
> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
>> There is a regression of commit 4a646580f793 ("Input: ALPS - fix 
>> two-finger scroll breakage"), ALPS device fails with log:
>>
>> psmouse serio1: alps: Rejected trackstick packet from non DualPoint 
>> device
>>
>> ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit 
>> 4a646580f793, after restore the OTP reading order, it becomes to 0x10 
>> as before and reports the right flag.
>>
>> Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
>> Cc: 
>> Signed-off-by: Aaron Ma 
>> ---
>>  drivers/input/mouse/alps.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c 
>> index 579b899add26..c59b8f7ca2fc 100644
>> --- a/drivers/input/mouse/alps.c
>> +++ b/drivers/input/mouse/alps.c
>> @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct 
>> psmouse *psmouse,
>>  
>>  memset(otp, 0, sizeof(otp));
>>  
>> -if (alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]) ||
>> -alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]))
>> +if (alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]) ||
>> +alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]))
>>  return -1;
>>  
>>  alps_update_device_area_ss4_v2(otp, priv);
> 
> Masaki Ota, please look at this patch as it partially revert your commit
> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something 
> smells here.
> 
> --
> Pali Rohár
> pali.ro...@gmail.com
> 


[PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices

2017-11-14 Thread Aaron Ma
There is a regression of commit 4a646580f793 ("Input: ALPS - fix
two-finger scroll breakage"), ALPS device fails with log:

psmouse serio1: alps: Rejected trackstick packet from non DualPoint device

ALPS device with id "74 03 28" report OTP[0] data 0xCE after
commit 4a646580f793, after restore the OTP reading order,
it becomes to 0x10 as before and reports the right flag.

Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
Cc: <sta...@vger.kernel.org>
Signed-off-by: Aaron Ma <aaron...@canonical.com>
---
 drivers/input/mouse/alps.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index 579b899add26..c59b8f7ca2fc 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct psmouse 
*psmouse,
 
memset(otp, 0, sizeof(otp));
 
-   if (alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]) ||
-   alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]))
+   if (alps_get_otp_values_ss4_v2(psmouse, 0, [0][0]) ||
+   alps_get_otp_values_ss4_v2(psmouse, 1, [1][0]))
return -1;
 
alps_update_device_area_ss4_v2(otp, priv);
-- 
2.13.6



  1   2   >