Hi Hans, On Fri, Jan 31, 2020 at 1:46 PM Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > __check_hid_generic is used to check if there is a driver, other then > the hid-generic driver, which wants to handle the hid-device, in which > case hid_generic_match() will return false so that the other driver can > bind. > > But what if the other driver also has a match callback and that indicates > it does not want to handle the device? Then hid-generic should bind to it > and thus hid_generic_match() should NOT return false in that case. > > This commit makes __check_hid_generic check if a matching driver has > a match callback and if it does makes its check this, so that > hid-generic will bind to devices which have a matching other driver, > but that other driver's match callback rejects the device. I get that part, but I am not sure I'll remember this in 2 months time when/if we need to extend the .match() in another driver. I am especially worried about the potential circular calls where an other driver decides to check on all the other drivers having a match callback... Could you add a little blurb either in hid-generic.c explaining the logic, or (and) in hid.h where .match is defined? Because now, we have 2 callers for .match(): hid-core and hid-generic (and 2 is usually one too many :-/). Cheers, Benjamin > > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > This patch was written while fixing the issues with hid-ite on the > Acer SW5-012, where we only want to bind to one interface. In that > specific case this change is not necessary because hid-multitouch will > pick the hid-device which hid-ite's match callback is rejecting. > --- > drivers/hid/hid-generic.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-generic.c b/drivers/hid/hid-generic.c > index f9db991d3c5a..fe3ca7612750 100644 > --- a/drivers/hid/hid-generic.c > +++ b/drivers/hid/hid-generic.c > @@ -31,7 +31,13 @@ static int __check_hid_generic(struct device_driver *drv, void *data) > if (hdrv == &hid_generic) > return 0; > > - return hid_match_device(hdev, hdrv) != NULL; > + if (!hid_match_device(hdev, hdrv)) > + return 0; > + > + if (!hdrv->match) > + return 1; > + > + return hdrv->match(hdev, false); > } > > static bool hid_generic_match(struct hid_device *hdev, > -- > 2.23.0 >
