All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Tissoires <benjamin.tissoires@redhat.com>
To: Hn Chen <hn.chen@weidahitech.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	"linux-input@vger.kernel.org" <linux-input@vger.kernel.org>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Input: wdt87xx_i2c - support new body WDT8752
Date: Mon, 5 Sep 2016 15:55:45 +0200	[thread overview]
Message-ID: <20160905135545.GL21864@mail.corp.redhat.com> (raw)
In-Reply-To: <22498D53301C4D4A8FFA8F02C7C3C7C06DDD7D42@mail02.WHT.local>

Hi HN,

On Sep 05 2016 or thereabouts, Hn Chen wrote:
> Hi Dmitry,
> 
> >> Considering to be compatible with i2c-hid, WDT8752 has the same way in 
> >> enumerating device.
> >If it is a HID device then I think you should write a HID driver for it (unless existing driver, such as hid-multitouch can already handle it, possibly >with some changes). I'm addng Benjamin to he can comment as well.
> The device can be handled by i2c-hid driver (HID over I2C) already but this proprietary driver still is a must-have for more features.

Then what are those must-have features? From what I can read, only the
reflashing firmware is part of it. Unless there is something else, I
really don't understand why you can't have a hid-weidatech driver that
could handle the specific bits while leaving the rest to i2c-hid.

Also, I am not sure if your driver doesn't interfere with i2c-hid as you
are claiming the device through the ACPI ID "WDHT0001" but there should
be some PNP IDs "PNP0C50" if it were declared as i2c-hid. If both are
set, then the fact your driver is picked up seems to be pure luck: there
will be a race between the probe of your driver and i2c-hid, which is
not something you want.

If there are some issues with i2c-hid, I'd like also to know them
because if we fix them for you there  is a high chance other vendors
will benefit from those fixes too.

Cheers,
Benjamin

> 
> >> And also modify the part of FW update to be more efficiency. The main 
> >> modification is that reducing the amount of data transmitted and using 
> >> polling for time comsuming operation.
> >>
> >> Flash erase will wait 50ms for the operation complete in last driver.
> >> Extend it to 200ms since the spec says the typical is 30ms but the max 
> >> is 200ms.
> >This should be split into a separate patch please.
> Ok, I will resubmit the part of possible-issue fixing and then the driver patch for supporting WDT8752 again.
> Please ignore this submission.
> 
> Hn.chen

  reply	other threads:[~2016-09-05 13:55 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-03 10:16 [PATCH] Input: wdt87xx_i2c - support new body WDT8752 HungNien Chen
2016-08-03 10:16 ` HungNien Chen
2016-09-02  6:40 ` Dmitry Torokhov
2016-09-05  7:32   ` Hn Chen
2016-09-05 13:55     ` Benjamin Tissoires [this message]
2016-09-06 11:47       ` Hn Chen
2016-10-07 23:46         ` Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160905135545.GL21864@mail.corp.redhat.com \
    --to=benjamin.tissoires@redhat.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=hn.chen@weidahitech.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.