2012-05-24 08:32:27

by Dmitry Torokhov

[permalink] [raw]
Subject: [git pull] Input updates for 3.5-rc0

Hi Linus,

Please pull from:

git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus

to receive updates for the input subsystem. You will get:

- a bunch of new drivers (DA9052/53 touchscreenn controller, Synaptics
Navpoint, LM8333 keypads, Wacom I2C touhscreen);
- updates to existing touchpad drivers (ALPS, Sntelic);
- Wacom driver now supports Intuos5;
- device-tree bindings in numerous drivers;
- other cleanups and fixes.

You will get a conflict in wacom_wac.c; please resolve taking "my"
chunks or let me know and I'll merge on my side.

Thanks!


Changelog:
---------

Ashish Jangam (1):
Input: add support for DA9052/53 touch screen controller

Axel Lin (5):
Input: serio - add helper macro for serio_driver boilerplate
Input: serio - use module_serio_driver
Input: gameport - add helper macro for gameport_driver boilerplate
Input: gameport - use module_gameport_driver
Input: use module_pci_driver

Daniel Kurtz (5):
Input: atmel_mxt_ts - use CONFIG_PM_SLEEP
Input: atmel_mxt_ts - only allow root to update firmware
Input: atmel_mxt_ts - verify object size in mxt_write_object
Input: atmel_mxt_ts - do not read extra (checksum) byte
Input: atmel_mxt_ts - dump each message on just 1 line

Dmitry Torokhov (21):
Input: xilinx_ps2 - allocate serio port separately
Input: st1232 - switch to using SIMPLE_DEV_PM_OPS
Input: wacom_i2c - do not use irq_to_gpio
Input: ep93xx_keypad - switch to using dev_pm_ops
Input: matrix-keypad - fix 'duplicate const' sparse warning
Input: matrix-keypad - allocate keycodes with keypad structure
Input: matrix-keypad - undo GPIO setup if input_register_device fails
Input: cma3000-d0x - remove unneeded checks
Input: tc3589x-keypad - remove unnecessary checks
Input: serio_raw - ensure we don't block in non-blocking read
Input: wacom - fix sparse warning
Input: wacom - return proper error if usb_get_extra_descriptor() fails
Input: wacom - use dev_xxx() instead of naked printk()s and dbg()s
Input: serio_raw - signal EFAULT even if read/write partially succeeds
Input: evdev - properly access RCU-protected 'grab' data
Input: evdev - properly handle read/write with count 0
Input: ALPS - switch to using input_mt_report_finger_count
MAINTAINERS: adjust input-related patterns
Input: matrix-keymap - uninline and prepare for device tree support
Input: matrix-keymap - wire up device tree support
Input: matrix-keymap - fix building keymaps

George Pantalos (1):
Input: ALPS - add semi-MT support for v4 protocol

JJ Ding (1):
Input: synaptics - fix compile warning

Jason Gerecke (4):
Input: wacom - add basic Intuos5 support
Input: wacom - add Intuos5 Touch Ring/ExpressKey support
Input: wacom - add Intuos5 Touch Ring LED support
Input: wacom - add Intuos5 multitouch sensor support

Jean-Fran?ois Dagenais (1):
Input: adp5588 - add support for gpio names

Jesper Juhl (1):
Input: atkbd - fix language in a printed message

Julia Lawall (1):
Input: aiptek - adjust error-handling code label

Magnus Damm (1):
Input: st1232 - add device tree support

Paul Parsons (1):
Input: Add Synaptics NavPoint (PXA27x SSP/SPI) driver

Peter Ujfalusi (1):
Input: tl6040-vibra - Device Tree support

Ping Cheng (2):
Input: wacom - retrieve maximum number of touch points
Input: wacom - add 0xE5 (MT device) support

Poddar, Sourav (1):
Input: omap-keypad - dynamically handle register offsets

Roland Stigge (2):
Input: lpc32xx_ts - add device tree support
Input: lpc32xx_ts - fix device tree compatible string

Shawn Landden (2):
Input: usbtouchscreen - fix typo
Input: usbtouchscreen - only expose e2i configure option in expert mode

Stephen Warren (1):
Input: mpu3050 - set IRQF_ONESHOT when requesting the interrupt

Tai-hwa Liang (1):
Input: sentelic - report device's production serial number

Tatsunosuke Tobita (1):
Input: add support for Wacom Stylus device with I2C interface

Viresh Kumar (2):
Input: spear-keyboard - add device tree bindings
Input: spear-keyboard - document DT bindings

Wolfram Sang (1):
Input: add support for LM8333 keypads


Diffstat:
--------

Documentation/ABI/testing/sysfs-driver-wacom | 15 +-
.../devicetree/bindings/input/spear-keyboard.txt | 20 +
.../bindings/input/touchscreen/lpc32xx-tsc.txt | 16 +
.../devicetree/bindings/input/twl6040-vibra.txt | 37 ++
MAINTAINERS | 2 +
drivers/input/Kconfig | 17 +-
drivers/input/Makefile | 2 +-
drivers/input/evdev.c | 69 +++--
drivers/input/gameport/emu10k1-gp.c | 13 +-
drivers/input/gameport/fm801-gp.c | 16 +-
drivers/input/joystick/a3d.c | 13 +-
drivers/input/joystick/adi.c | 17 +-
drivers/input/joystick/cobra.c | 13 +-
drivers/input/joystick/gf2k.c | 13 +-
drivers/input/joystick/grip.c | 13 +-
drivers/input/joystick/grip_mp.c | 13 +-
drivers/input/joystick/guillemot.c | 13 +-
drivers/input/joystick/interact.c | 13 +-
drivers/input/joystick/joydump.c | 13 +-
drivers/input/joystick/magellan.c | 17 +-
drivers/input/joystick/sidewinder.c | 13 +-
drivers/input/joystick/spaceball.c | 17 +-
drivers/input/joystick/spaceorb.c | 17 +-
drivers/input/joystick/stinger.c | 17 +-
drivers/input/joystick/tmdc.c | 13 +-
drivers/input/joystick/twidjoy.c | 17 +-
drivers/input/joystick/warrior.c | 17 +-
drivers/input/joystick/zhenhua.c | 17 +-
drivers/input/keyboard/Kconfig | 32 ++-
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/adp5588-keys.c | 1 +
drivers/input/keyboard/atkbd.c | 2 +-
drivers/input/keyboard/ep93xx_keypad.c | 44 +--
drivers/input/keyboard/hil_kbd.c | 13 +-
drivers/input/keyboard/imx_keypad.c | 17 +-
drivers/input/keyboard/lkkbd.c | 17 +-
drivers/input/keyboard/lm8333.c | 235 +++++++++++++
drivers/input/keyboard/matrix_keypad.c | 99 +++---
drivers/input/keyboard/newtonkbd.c | 13 +-
drivers/input/keyboard/nomadik-ske-keypad.c | 20 +-
drivers/input/keyboard/omap-keypad.c | 20 +-
drivers/input/keyboard/omap4-keypad.c | 133 ++++++--
drivers/input/keyboard/pmic8xxx-keypad.c | 20 +-
drivers/input/keyboard/samsung-keypad.c | 20 +-
drivers/input/keyboard/spear-keyboard.c | 92 ++++--
drivers/input/keyboard/stmpe-keypad.c | 16 +-
drivers/input/keyboard/stowaway.c | 13 +-
drivers/input/keyboard/sunkbd.c | 17 +-
drivers/input/keyboard/tc3589x-keypad.c | 41 +--
drivers/input/keyboard/tca8418_keypad.c | 15 +-
drivers/input/keyboard/tegra-kbc.c | 68 +++--
drivers/input/keyboard/tnetv107x-keypad.c | 21 +-
drivers/input/keyboard/twl4030_keypad.c | 25 +-
drivers/input/keyboard/w90p910_keypad.c | 27 +-
drivers/input/keyboard/xtkbd.c | 13 +-
drivers/input/matrix-keymap.c | 163 +++++++++
drivers/input/misc/cma3000_d0x.c | 2 +-
drivers/input/misc/mpu3050.c | 2 +-
drivers/input/misc/twl6040-vibra.c | 46 ++-
drivers/input/mouse/Kconfig | 12 +
drivers/input/mouse/Makefile | 1 +
drivers/input/mouse/alps.c | 81 ++++-
drivers/input/mouse/alps.h | 2 +
drivers/input/mouse/navpoint.c | 369 +++++++++++++++++++
drivers/input/mouse/sentelic.c | 34 ++-
drivers/input/mouse/sentelic.h | 8 +
drivers/input/mouse/sermouse.c | 13 +-
drivers/input/mouse/synaptics.c | 20 +-
drivers/input/mouse/vsxxxaa.c | 14 +-
drivers/input/of_keymap.c | 87 -----
drivers/input/serio/pcips2.c | 15 +-
drivers/input/serio/ps2mult.c | 13 +-
drivers/input/serio/serio_raw.c | 65 ++--
drivers/input/serio/xilinx_ps2.c | 35 +-
drivers/input/tablet/aiptek.c | 2 +-
drivers/input/tablet/wacom.h | 4 +-
drivers/input/tablet/wacom_sys.c | 244 ++++++++++----
drivers/input/tablet/wacom_wac.c | 298 ++++++++++++++--
drivers/input/tablet/wacom_wac.h | 13 +
drivers/input/touchscreen/Kconfig | 34 ++-
drivers/input/touchscreen/Makefile | 2 +
drivers/input/touchscreen/atmel_mxt_ts.c | 33 +--
drivers/input/touchscreen/da9052_tsi.c | 370 ++++++++++++++++++++
drivers/input/touchscreen/dynapro.c | 17 +-
drivers/input/touchscreen/elo.c | 17 +-
drivers/input/touchscreen/fujitsu_ts.c | 13 +-
drivers/input/touchscreen/gunze.c | 17 +-
drivers/input/touchscreen/h3600_ts_input.c | 17 +-
drivers/input/touchscreen/hampshire.c | 17 +-
drivers/input/touchscreen/inexio.c | 17 +-
drivers/input/touchscreen/lpc32xx_ts.c | 10 +
drivers/input/touchscreen/mtouch.c | 17 +-
drivers/input/touchscreen/penmount.c | 17 +-
drivers/input/touchscreen/st1232.c | 20 +-
drivers/input/touchscreen/touchit213.c | 17 +-
drivers/input/touchscreen/touchright.c | 17 +-
drivers/input/touchscreen/touchwin.c | 17 +-
drivers/input/touchscreen/tsc40.c | 12 +-
drivers/input/touchscreen/wacom_i2c.c | 282 +++++++++++++++
drivers/input/touchscreen/wacom_w8001.c | 13 +-
include/linux/gameport.h | 13 +
include/linux/i2c/adp5588.h | 1 +
include/linux/input/lm8333.h | 24 ++
include/linux/input/matrix_keypad.h | 54 +---
include/linux/input/navpoint.h | 12 +
include/linux/serio.h | 13 +
106 files changed, 2845 insertions(+), 1299 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/spear-keyboard.txt
create mode 100644 Documentation/devicetree/bindings/input/touchscreen/lpc32xx-tsc.txt
create mode 100644 Documentation/devicetree/bindings/input/twl6040-vibra.txt
create mode 100644 drivers/input/keyboard/lm8333.c
create mode 100644 drivers/input/matrix-keymap.c
create mode 100644 drivers/input/mouse/navpoint.c
delete mode 100644 drivers/input/of_keymap.c
create mode 100644 drivers/input/touchscreen/da9052_tsi.c
create mode 100644 drivers/input/touchscreen/wacom_i2c.c
create mode 100644 include/linux/input/lm8333.h
create mode 100644 include/linux/input/navpoint.h

--
Dmitry


Attachments:
(No filename) (11.52 kB)
(No filename) (836.00 B)
Download all attachments

2012-05-24 17:34:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 1:32 AM, Dmitry Torokhov
<[email protected]> wrote:
> Hi Linus,
>
> to receive updates for the input subsystem. You will get:

I get an annoying conflict, and the reason I call it annoying is not
because it's hard to resolve, it's because doing that shows that you
seem to have preferred using

dev_dbg(&input->dev.parent, ...)

over the much more natural

dev_dbg(&input->dev, ...)

which would seem to make more sense.

Why? Are the input layer device names so bad that using them for debug
output is useless? And if so, why *are* they so bad?

I'm going to take your version over Greg's more straightforward one,
because I assume Greg did things a bit more mindlessly and I think you
presumably had a *reason* for your extra (stupid) ".parent" part. But
I'm unhappy with it, because I suspect the reason you did that implies
that the input layer does something bad.

Hmm?

Linus

2012-05-24 18:02:04

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 10:33:49AM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 1:32 AM, Dmitry Torokhov
> <[email protected]> wrote:
> > Hi Linus,
> >
> > to receive updates for the input subsystem. You will get:
>
> I get an annoying conflict, and the reason I call it annoying is not
> because it's hard to resolve, it's because doing that shows that you
> seem to have preferred using
>
> dev_dbg(&input->dev.parent, ...)
>
> over the much more natural
>
> dev_dbg(&input->dev, ...)
>
> which would seem to make more sense.
>
> Why? Are the input layer device names so bad that using them for debug
> output is useless? And if so, why *are* they so bad?
>
> I'm going to take your version over Greg's more straightforward one,
> because I assume Greg did things a bit more mindlessly and I think you
> presumably had a *reason* for your extra (stupid) ".parent" part. But
> I'm unhappy with it, because I suspect the reason you did that implies
> that the input layer does something bad.

A couple of points:

1. A driver should try to use the same device for all its messages and
input devices are not created yet at the time we try to bind USB
interface to a driver. Most (all?) USB probe() methods use interface's
device with dev_xxx() which shows exactly which interface we are dealing
with.

2. Input devices are essentially driver-less (they are class devices) and
therefore do not provide useful information if used in messages as the
format of the message would be:

input inputX: some message

which does not identify neitehr the driver nor hardware device. By
using input->dev.parent we get to the USB interface thus getting more
meaningful messages:

wacom 2-1.2:1.0: some error happened

We had a discussion with Greg about this and he was going to change his
patchset to use USB interfaces in the messages...

Thanks.

--
Dmitry

2012-05-24 18:21:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 11:01 AM, Dmitry Torokhov
<[email protected]> wrote:
>
> A couple of points:

*NEITHER* of your points actually address my issue.

If the "inputX" name is useless (and dammit, I agree), then why do you
continue to use it then?

IOW, why the hell do you set a name that is so useful that no sane
person would ever want to use it?

Basically, my complaint is that the input layer drivers end up doing
stupid things ("dev_dbg(input->dev.parent..") because the input layer
has done stupid things (dev_set_name(&dev->dev, "input%ld").

So let me be more blunt and direct: which part of "that's f*cking
stupid" do you disagree with?

So instead of making drivers do stupid things because you've made the
input layer names so damn useless, why don't you make the input layer
use better naming? Doesn't that seem to be the *smart* thing to do?

Wouldn't everybody be much happier if they saw useful names in the
device tree? And then the drivers could actually *use* those useful
names, instead of clearly saying "our names are so shitty that we have
to look up the parent name just to get a useful printout"?

Do people really *want* to see names like "input2" when working with a
keyboard? Do we have some stupid userlevel interface that requires
these useless and nondescriptive names that are so useless that even
the input layer itself doesn't want to use them?

Wouldn't it be *much* nicer if the input layer would, for example, do
something like

dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));

or whatever? Better yet, make it easy for the driver to say what it
is, so that it really can just say "wacom %s" or whatever.

Because the basic reason you seem to want to use "dev.parent" instead
of just "dev" *all* seem to boil down to a very simple reason:

- other subsystems name their devices better, so you want to use
those better names

Fix the underlying *reason*, instead of the symptoms. That's what I'm saying.

Linus

2012-05-24 19:59:31

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 11:21:20AM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 11:01 AM, Dmitry Torokhov
> <[email protected]> wrote:
> >
> > A couple of points:
>
> *NEITHER* of your points actually address my issue.
>
> If the "inputX" name is useless (and dammit, I agree), then why do you
> continue to use it then?
>
> IOW, why the hell do you set a name that is so useful that no sane
> person would ever want to use it?
>
> Basically, my complaint is that the input layer drivers end up doing
> stupid things ("dev_dbg(input->dev.parent..") because the input layer
> has done stupid things (dev_set_name(&dev->dev, "input%ld").
>
> So let me be more blunt and direct: which part of "that's f*cking
> stupid" do you disagree with?
>
> So instead of making drivers do stupid things because you've made the
> input layer names so damn useless, why don't you make the input layer
> use better naming? Doesn't that seem to be the *smart* thing to do?
>
> Wouldn't everybody be much happier if they saw useful names in the
> device tree? And then the drivers could actually *use* those useful
> names, instead of clearly saying "our names are so shitty that we have
> to look up the parent name just to get a useful printout"?
>
> Do people really *want* to see names like "input2" when working with a
> keyboard? Do we have some stupid userlevel interface that requires
> these useless and nondescriptive names that are so useless that even
> the input layer itself doesn't want to use them?
>
> Wouldn't it be *much* nicer if the input layer would, for example, do
> something like
>
> dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));
>
> or whatever? Better yet, make it easy for the driver to say what it
> is, so that it really can just say "wacom %s" or whatever.
>
> Because the basic reason you seem to want to use "dev.parent" instead
> of just "dev" *all* seem to boil down to a very simple reason:
>
> - other subsystems name their devices better, so you want to use
> those better names

NO. What you seem to be missing is that the driver does not operate on
an input device, input device is a _product_ of that driver. The error
condition did not arise in input stack or input handler, the error
condition happend on a physical device connected to a given USB port.
And that is why it is proper to associate the error (in this case) with
USB interface and USB driver that controls it. input_dev->dev.parent was
just a convenient way of getting to this object but if it irks you I can
stuff a device pointer in wacom_wac structure so that we use

dev_xxx(wacom->dev, "blah);

instead.

On the topic of naming - nobody cares what input devices are called as
long as they are unique. Userspace does not work with input devices
directly (there is no API), they are working with iterfaces (such as
evdev) that have their own interfaces and names. We could burden drivers
to make sure they provide unique names, but that would not be optimal
and most likely gotten wrong. Considering your

dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));

what happens if I unplug and replug my wacom tablet while event node is
still opened and therefore all kobjects are still pinned into memory?
The parent device's name is stuill the same so we'll fail to cretaed
device with duplicate name. Extending with global counter? We could I
guess, but why bother? If you check /sys/class/ most of the abstract
devices do not care to have unique names. Is /sys/class/block/loop5 or
/sys/class/rtc/rct0 or /sys/class/sound/card0 any better than
/sys/class/input/inputX?

Thanks.

--
Dmitry

2012-05-24 20:11:38

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 12:58 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> Considering your
>
> ? ? ? ?dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));
>
> what happens if I unplug and replug my wacom tablet while event node is
> still opened and therefore all kobjects are still pinned into memory?

I'll let you think about just how stupid that comment was for a moment.

Linus

2012-05-24 20:36:54

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 01:11:14PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 12:58 PM, Dmitry Torokhov
> <[email protected]> wrote:
> >
> > Considering your
> >
> > ? ? ? ?dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));
> >
> > what happens if I unplug and replug my wacom tablet while event node is
> > still opened and therefore all kobjects are still pinned into memory?
>
> I'll let you think about just how stupid that comment was for a moment.

Hmm, I guess we are better now with cleaning and turning devices into
zombies waiting to be reaped. Still have to handle parent have several
input devices (HID devices quite often do it). And I still do not see
the point why we want to bother, the proper layer to report driver
errors is driver and object it is bound to.

--
Dmitry

2012-05-24 20:49:22

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 1:36 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> Hmm, I guess we are better now with cleaning and turning devices into
> zombies waiting to be reaped.

No, that wasn't the silly part of your statement.

The silly part is that

dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));

works perfectly fine if the parent goes away, for a damn simple
reason: it generates the string *once*, and doesn't care one *whit*
about the parent pointer ever again afterwards.

For exactly the same that your current

dev_set_name(&dev->dev, "input%d", atomic_inc_return(&input_no) - 1);

doesn't care *at*all* about that "input_no" variable afterwards -
dev_set_name() will have turned it into a string, and "input_no" can
change as much as it wants, and that won't change the name of the
device.

See? So no "refcounting parents" or "zombie devices" or any crap like
that. The name is just a string.

Anyway, I can't be bothered to argue. If you think "input1" is such a
great name, and then that results in nobody actually ever *using* it
because everybody agrees it useless and will use something else,
whatever.

Linus

2012-05-24 21:00:00

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 01:48:57PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 1:36 PM, Dmitry Torokhov
> <[email protected]> wrote:
> >
> > Hmm, I guess we are better now with cleaning and turning devices into
> > zombies waiting to be reaped.
>
> No, that wasn't the silly part of your statement.
>
> The silly part is that
>
> dev_set_name(&dev->dev, "input-%s", dev_name(dev->dev.parent));
>
> works perfectly fine if the parent goes away, for a damn simple
> reason: it generates the string *once*, and doesn't care one *whit*
> about the parent pointer ever again afterwards.

I was concerned about the _next_ device (the one that will be created
the moment I plug in the tablet back into the same port) having exact
same name as the one that is half dead and clashing in sysfs and
elsewhere. We used to have issues with this.

>
> For exactly the same that your current
>
> dev_set_name(&dev->dev, "input%d", atomic_inc_return(&input_no) - 1);
>
> doesn't care *at*all* about that "input_no" variable afterwards -
> dev_set_name() will have turned it into a string, and "input_no" can
> change as much as it wants, and that won't change the name of the
> device.

and it guarantees to make the name unique, that is all we need.

>
> See? So no "refcounting parents" or "zombie devices" or any crap like
> that. The name is just a string.
>
> Anyway, I can't be bothered to argue. If you think "input1" is such a
> great name, and then that results in nobody actually ever *using* it
> because everybody agrees it useless and will use something else,
> whatever.
>
> Linus

--
Dmitry

2012-05-24 21:07:52

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 1:59 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> I was concerned about the _next_ device (the one that will be created
> the moment I plug in the tablet back into the same port) having exact
> same name as the one that is half dead and clashing in sysfs and
> elsewhere. We used to have issues with this.

Ok, that's certainly a valid concern.

It's still - I think - really sad/wrong that the device name is then
so useless than the drivers end up basically not using it.

Linus

2012-05-24 21:36:09

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 10:33:49AM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 1:32 AM, Dmitry Torokhov
> <[email protected]> wrote:
> > Hi Linus,
> >
> > to receive updates for the input subsystem. You will get:
>
> I get an annoying conflict, and the reason I call it annoying is not
> because it's hard to resolve, it's because doing that shows that you
> seem to have preferred using
>
> dev_dbg(&input->dev.parent, ...)
>
> over the much more natural
>
> dev_dbg(&input->dev, ...)
>
> which would seem to make more sense.
>
> Why? Are the input layer device names so bad that using them for debug
> output is useless? And if so, why *are* they so bad?
>
> I'm going to take your version over Greg's more straightforward one,
> because I assume Greg did things a bit more mindlessly and I think you
> presumably had a *reason* for your extra (stupid) ".parent" part. But
> I'm unhappy with it, because I suspect the reason you did that implies
> that the input layer does something bad.

I reworked these patches 3 different times, they were anything but
"mindless" (well, maybe the first ones were...) Dmitry and I seem to
disagree on showing useful names to the users, but as he is the one who
needs to handle user issues when they report things, I'll defer to his
version of the changes.

thanks,

greg k-h

2012-05-24 21:38:21

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 02:07:29PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 1:59 PM, Dmitry Torokhov
> <[email protected]> wrote:
> >
> > I was concerned about the _next_ device (the one that will be created
> > the moment I plug in the tablet back into the same port) having exact
> > same name as the one that is half dead and clashing in sysfs and
> > elsewhere. We used to have issues with this.
>
> Ok, that's certainly a valid concern.
>
> It's still - I think - really sad/wrong that the device name is then
> so useless than the drivers end up basically not using it.

I can see the desire for logging, it is just hard to come with good
automatic way of doing this without burdening drivers, given that errors
happen rarely in input core layer and in driver code we already have
perfect device object to report against.

Maybe dev_XXX() should report full sysfs path do the dveice? Still does
not help with the fact that "driver" shown for class devices is not so
interesting as it will always be the class name.

--
Dmitry

2012-05-24 21:44:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 2:07 PM, Linus Torvalds
<[email protected]> wrote:
> On Thu, May 24, 2012 at 1:59 PM, Dmitry Torokhov
> <[email protected]> wrote:
>>
>> I was concerned about the _next_ device (the one that will be created
>> the moment I plug in the tablet back into the same port) having exact
>> same name as the one that is half dead and clashing in sysfs and
>> elsewhere. We used to have issues with this.
>
> Ok, that's certainly a valid concern.
>
> It's still - I think - really sad/wrong that the device name is then
> so useless than the drivers end up basically not using it.

Ok, so I wonder if we could solve the issue at least partly by
separating the "print name for kernel messages" from the "name used
for /sysfs etc".

Because you're right: the sysfs uniqueness rules does make it very
hard to do a good job on descriptive names.

Also, in sysfs, you by definition see the parent (hey, it's part of
the path), so in sysfs, duplicating parent data would be useless and
just ugly.

But for dev_dbg(), those sysfs rules actually act against us: the name
of a device is often tied to the parent bus location.

So I wonder if we could teach dev_printk() to use something more
interesting than "dev_name()" when appropriate? Greg?

Linus

2012-05-24 21:56:55

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 2:38 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> Maybe dev_XXX() should report full sysfs path do the dveice? Still does
> not help with the fact that "driver" shown for class devices is not so
> interesting as it will always be the class name.

Well, the dev_printk logic could certainly have simple rules to avoid
duplicate information. And perhaps pick the most relevant parts using
some heuristics.

Right now it just prints out "%s %s" for dev_driver_string(dev) and
dev_name(dev) respectively. That really isn't all that wonderfully
beautiful, and results in things like

hid-generic 0003:046D:C526.0006: xyz

for my Logitech USB Receiver. That's kind of ugly, and really doesn't
clarify anything at all, I think.

The "driver + dev_name()" works fine for things like PCI devices, but
even then it's partly because the PCI layer has made sure to make the
device name pretty - and the device name contains the (redundant) data
of which bus number it is on, even though that's strictly a matter of
the parent thing (but the device name has been prettified and contains
redundant information exactly so that it would print out nicely).

Linus

2012-05-24 21:57:04

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 02:44:31PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 2:07 PM, Linus Torvalds
> <[email protected]> wrote:
> > On Thu, May 24, 2012 at 1:59 PM, Dmitry Torokhov
> > <[email protected]> wrote:
> >>
> >> I was concerned about the _next_ device (the one that will be created
> >> the moment I plug in the tablet back into the same port) having exact
> >> same name as the one that is half dead and clashing in sysfs and
> >> elsewhere. We used to have issues with this.
> >
> > Ok, that's certainly a valid concern.
> >
> > It's still - I think - really sad/wrong that the device name is then
> > so useless than the drivers end up basically not using it.
>
> Ok, so I wonder if we could solve the issue at least partly by
> separating the "print name for kernel messages" from the "name used
> for /sysfs etc".
>
> Because you're right: the sysfs uniqueness rules does make it very
> hard to do a good job on descriptive names.
>
> Also, in sysfs, you by definition see the parent (hey, it's part of
> the path), so in sysfs, duplicating parent data would be useless and
> just ugly.
>
> But for dev_dbg(), those sysfs rules actually act against us: the name
> of a device is often tied to the parent bus location.
>
> So I wonder if we could teach dev_printk() to use something more
> interesting than "dev_name()" when appropriate? Greg?

Maybe we could traverse up the tree till we find first non-class device
(i.e. device with real driver bound to it)? Then we'd have:

wacom 2-1.2:1.0/inputX/eventY: some error happened

type of messages...

--
Dmitry

2012-05-24 21:59:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 2:56 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> Maybe we could traverse up the tree till we find first non-class device
> (i.e. device with real driver bound to it)? Then we'd have:
>
> ? ? ? ?wacom 2-1.2:1.0/inputX/eventY: some error happened
>
> type of messages...

That really sounds quite nice. It would make sense to show both the
actual device information, and the "software layer" on top of it.

Except maybe it then would be absolutely disgusting for some other
case. Hmm. I really can't think of any bad situation off-hand, though.

Linus

2012-05-24 22:01:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 02:44:31PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 2:07 PM, Linus Torvalds
> <[email protected]> wrote:
> > On Thu, May 24, 2012 at 1:59 PM, Dmitry Torokhov
> > <[email protected]> wrote:
> >>
> >> I was concerned about the _next_ device (the one that will be created
> >> the moment I plug in the tablet back into the same port) having exact
> >> same name as the one that is half dead and clashing in sysfs and
> >> elsewhere. We used to have issues with this.
> >
> > Ok, that's certainly a valid concern.
> >
> > It's still - I think - really sad/wrong that the device name is then
> > so useless than the drivers end up basically not using it.
>
> Ok, so I wonder if we could solve the issue at least partly by
> separating the "print name for kernel messages" from the "name used
> for /sysfs etc".
>
> Because you're right: the sysfs uniqueness rules does make it very
> hard to do a good job on descriptive names.
>
> Also, in sysfs, you by definition see the parent (hey, it's part of
> the path), so in sysfs, duplicating parent data would be useless and
> just ugly.
>
> But for dev_dbg(), those sysfs rules actually act against us: the name
> of a device is often tied to the parent bus location.
>
> So I wonder if we could teach dev_printk() to use something more
> interesting than "dev_name()" when appropriate? Greg?

I'm open to ideas on what to change it to. A full sysfs path?
Something more "unique"? I don't know what works for everything here.

thanks,

greg k-h

2012-05-24 22:05:32

by Linus Torvalds

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 3:01 PM, Greg Kroah-Hartman
<[email protected]> wrote:
>
> I'm open to ideas on what to change it to. ?A full sysfs path?

A full sysfs path really looks pretty ugly, partly because we'd have
to really change all our naming (look at PCI devices now: they end up
looking like

pci0000:00/0000:00:1a.0

where that "0000:00" is duplicated because we wanted to make
dev_name() look nice).

I think Dmitry's idea to stop when we hit the actual "physical" device
might just work in practice. So we'd show the dev_name() of the actual
piece of hardware, and then perhaps one or two "layering" details on
top of it. And it wouldn't change anything for the current common case
where people tend to pass in the physical device as-is.

Linus

2012-05-25 01:39:37

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On Thu, May 24, 2012 at 03:05:09PM -0700, Linus Torvalds wrote:
> On Thu, May 24, 2012 at 3:01 PM, Greg Kroah-Hartman
> <[email protected]> wrote:
> >
> > I'm open to ideas on what to change it to. ?A full sysfs path?
>
> A full sysfs path really looks pretty ugly, partly because we'd have
> to really change all our naming (look at PCI devices now: they end up
> looking like
>
> pci0000:00/0000:00:1a.0
>
> where that "0000:00" is duplicated because we wanted to make
> dev_name() look nice).
>
> I think Dmitry's idea to stop when we hit the actual "physical" device
> might just work in practice. So we'd show the dev_name() of the actual
> piece of hardware, and then perhaps one or two "layering" details on
> top of it. And it wouldn't change anything for the current common case
> where people tend to pass in the physical device as-is.

Ok, the "trick" is going to be to figure out where the "physical" device
is in the parent chain (look at the scsi craziness for an example of
that.)

I'm on the road for the next week or so, with lots of international
plane rides with time to kill, so I'll try to figure something out on
them and post it.

thanks,

greg k-h

2012-05-25 19:14:43

by Mark Lord

[permalink] [raw]
Subject: Re: [git pull] Input updates for 3.5-rc0

On 12-05-24 03:58 PM, Dmitry Torokhov wrote:
>
> On the topic of naming - nobody cares what input devices are called as
> long as they are unique. Userspace does not work with input devices
> directly (there is no API), they are working with iterfaces (such as
> evdev) that have their own interfaces and names.

I regularly work with input devices from userspace,
using the "lsinput" "xinput", and "input-kbd" commands (among others)
from various shell scripts.

So I guess that makes me one of those anonymous "nobody" types
who "don't care" about it.

Usually the device naming I see there makes some kind of sense.