public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Yann Cantin <yann.cantin@laposte.net>,
	linux-input@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	jkosina@suse.cz
Subject: Re: [RFC ebeam PATCH 2/2] input: misc: New USB eBeam input driver
Date: Mon, 20 Jul 2015 15:40:33 -0700	[thread overview]
Message-ID: <20150720224033.GA23884@kroah.com> (raw)
In-Reply-To: <20150720222640.GD30252@dtor-ws>

On Mon, Jul 20, 2015 at 03:26:40PM -0700, Dmitry Torokhov wrote:
> On Mon, Jul 20, 2015 at 02:59:56PM -0700, Greg KH wrote:
> > On Mon, Jul 20, 2015 at 11:03:19PM +0200, Yann Cantin wrote:
> > > Signed-off-by: Yann Cantin <yann.cantin@laposte.net>
> > 
> > > +
> > > +	/* sysfs setup */
> > > +	err = sysfs_create_group(&intf->dev.kobj, &ebeam_attr_group);
> > 
> > Ick, you just added the sysfs files to the USB device, not your input
> > device, are you sure you tested this?
> > 
> > And there should be a race-free way to add an attribute group to an
> > input device, as this is, you are adding them to the device _after_ it
> > is created, so userspace will not see them at creation time, causing a
> > race.
> 
> No, there are no driver-specific attributed on input devices themselves,
> they belong to the actual hardware devices. The input devices only
> export standard attributes applicable to every and all input devices
> in the system.

Then the Documentation in this patch better be fixed up, as it points to
the input device as having these sysfs files :)

But as these are input device attributes, and not USB device interface
attributes, putting them on the USB interface doesn't make much sense,
and as I pointed out, is racy.  Why can't input devices have
driver-specific attributes?  Why does input devices add their own
attributes sometimes (like in psmouse, which does so in a racy way) yet
this driver shouldn't do that?

Hm, nevermind about psmouse, that happens on the parent device too it
seems (a serio device), not the input device, so it is "consistent", but
not something I really like...

thanks,

greg k-h

  reply	other threads:[~2015-07-20 22:40 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-20 21:03 [RFC ebeam PATCH 0/3] new USB eBeam input driver Yann Cantin
2015-07-20 21:03 ` [RFC ebeam PATCH 1/2] hid: Blacklist eBeam devices Yann Cantin
2015-07-20 21:03 ` [RFC ebeam PATCH 2/2] input: misc: New USB eBeam input driver Yann Cantin
2015-07-20 21:59   ` Greg KH
2015-07-20 22:26     ` Dmitry Torokhov
2015-07-20 22:40       ` Greg KH [this message]
2015-07-20 23:09         ` Dmitry Torokhov
2015-07-21 13:54         ` Yann Cantin
2015-07-21 12:18     ` Yann Cantin
2015-07-21  8:19   ` Oliver Neukum
2015-07-21 12:38     ` Yann Cantin
  -- strict thread matches above, loose matches on Subject: below --
2013-07-24 23:52 [RFC ebeam PATCH 0/2] new " Yann Cantin
2013-07-24 23:52 ` [RFC ebeam PATCH 2/2] input: misc: New " Yann Cantin

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=20150720224033.GA23884@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jkosina@suse.cz \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=yann.cantin@laposte.net \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox