2012-08-15 08:15:39

by Jiri Kosina

Subject: Re: [PATCH 6/7] HID: picoLCD: disable version check during probe

On Mon, 30 Jul 2012, Bruno Prémont wrote:

> Commit 4ea5454203d991ec85264f64f89ca8855fce69b0
> [HID: Fix race condition between driver core and ll-driver] introduced
> new locking around proce/remove functions that prevent any report/reply
> from hardware to reach driver until it returned from probe.
>
> As such, the ask-reply way to checking picoLCD firmware version during
> probe is bound to timeout and let probe fail.
>
> Disabling the check lets driver sucessfully probe again.
>
> Signed-off-by: Bruno Prémont <[email protected]>
> ---
> drivers/hid/hid-picolcd_core.c | 8 ++++----
> 1 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index 2d7ef68..42d0791 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -478,13 +478,13 @@ static int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
> {
> int error;
>
> - error = picolcd_check_version(hdev);
> +/* error = picolcd_check_version(hdev);
> if (error)
> return error;
>
> if (data->version[0] != 0 && data->version[1] != 3)
> hid_info(hdev, "Device with untested firmware revision, please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
> - dev_name(&hdev->dev));
> + dev_name(&hdev->dev)); */

Please just remove it altogether, I don't see a reason to keep the
commented-out code in the in-tree driver.

Once the locking mess is sorted out, we can re-introduce it again as
necessary.

Thanks.

--
Jiri Kosina
SUSE Labs

2012-08-19 16:57:18

by Bruno Prémont

Subject: Re: [PATCH 6/7 v2] HID: picoLCD: drop version check during probe

Commit 4ea5454203d991ec85264f64f89ca8855fce69b0
[HID: Fix race condition between driver core and ll-driver] introduced
new locking around probe/remove functions that prevents any report/reply
from hardware to reach driver until it returned from probe.

As such, the ask-reply way to checking picoLCD firmware version during
probe is bound to timeout and let probe fail.

Drop the check to let driver successfully probe again (until locking issues
are resolved allowing to reinstate the check).

Signed-off-by: Bruno Prémont <[email protected]>
---

Changes since v1:
- drop version check during probe instead of commenting it out.


drivers/hid/hid-picolcd_core.c | 18 ------------------
1 files changed, 0 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
index 7b566ee..e08ffd2 100644
--- a/drivers/hid/hid-picolcd_core.c
+++ b/drivers/hid/hid-picolcd_core.c
@@ -478,14 +478,6 @@ static int picolcd_probe_lcd(struct hid_device *hdev, struct picolcd_data *data)
{
int error;

- error = picolcd_check_version(hdev);
- if (error)
- return error;
-
- if (data->version[0] != 0 && data->version[1] != 3)
- hid_info(hdev, "Device with untested firmware revision, please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
- dev_name(&hdev->dev));
-
/* Setup keypad input device */
error = picolcd_init_keys(data, picolcd_in_report(REPORT_KEY_STATE, hdev));
if (error)
@@ -534,16 +526,6 @@ err:

static int picolcd_probe_bootloader(struct hid_device *hdev, struct picolcd_data *data)
{
- int error;
-
- error = picolcd_check_version(hdev);
- if (error)
- return error;
-
- if (data->version[0] != 1 && data->version[1] != 0)
- hid_info(hdev, "Device with untested bootloader revision, please submit /sys/kernel/debug/hid/%s/rdesc for this device.\n",
- dev_name(&hdev->dev));
-
picolcd_init_devfs(data, NULL, NULL,
picolcd_out_report(REPORT_BL_READ_MEMORY, hdev),
picolcd_out_report(REPORT_BL_WRITE_MEMORY, hdev), NULL);
--
1.7.8.6