Re: [input:next 1310/1311] drivers/input/misc/sparcspkr.c:256:35: error: macro "MODULE_DEVICE_TABLE" requires 2 arguments, but only 1 given

2015-09-04 Thread Luis de Bethencourt 
Hi Dmitry,

I got this error email and confirmed that I missed the 'of' in my patch. I was
about to send a v3 with this fixed. But I just noticed it is already fixed in
your branch.

https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/commit/?h=next=26492f195eed08b95ad5acdfbe625062ad7d86c6

I am assuming you did this. Thank you!

I just wanted to confirm that there isn't anything else related to this you
would like me to do.

Sorry for the inconvenience,
Luis

On Fri, Sep 04, 2015 at 02:27:55PM +0800, kbuild test robot wrote:
> tree:   git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
> head:   ea96e2121490a5083a2f6681042946dd1dfc222a
> commit: 2a29d098e3bb21dc822971eb615569bdb0e4ad0b [1310/1311] Input: sparcspkr 
> - fix module autoload for OF platform drivers
> config: sparc64-defconfig (attached as .config)
> reproduce:
>   wget 
> https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
>  -O ~/bin/make.cross
>   chmod +x ~/bin/make.cross
>   git checkout 2a29d098e3bb21dc822971eb615569bdb0e4ad0b
>   # save the attached .config to linux build tree
>   make.cross ARCH=sparc64 
> 
> All error/warnings (new ones prefixed by >>):
> 
> >> drivers/input/misc/sparcspkr.c:256:35: error: macro "MODULE_DEVICE_TABLE" 
> >> requires 2 arguments, but only 1 given
> MODULE_DEVICE_TABLE(bbc_beep_match);
>   ^
> >> drivers/input/misc/sparcspkr.c:256:1: warning: data definition has no type 
> >> or storage class
> MODULE_DEVICE_TABLE(bbc_beep_match);
> ^
> >> drivers/input/misc/sparcspkr.c:256:1: error: type defaults to 'int' in 
> >> declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
>cc1: some warnings being treated as errors
> 
> vim +/MODULE_DEVICE_TABLE +256 drivers/input/misc/sparcspkr.c
> 
>250{
>251.name = "beep",
>252.compatible = "SUNW,bbc-beep",
>253},
>254{},
>255};
>  > 256MODULE_DEVICE_TABLE(bbc_beep_match);
>257
>258static struct platform_driver bbc_beep_driver = {
>259.driver = {
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation

> #
> # Automatically generated file; DO NOT EDIT.
> # Linux/sparc64 4.2.0-rc3 Kernel Configuration
> #
> CONFIG_64BIT=y
> CONFIG_SPARC=y
> # CONFIG_SPARC32 is not set
> CONFIG_SPARC64=y
> CONFIG_ARCH_DEFCONFIG="arch/sparc/configs/sparc64_defconfig"
> CONFIG_ARCH_PROC_KCORE_TEXT=y
> CONFIG_IOMMU_HELPER=y
> CONFIG_STACKTRACE_SUPPORT=y
> CONFIG_LOCKDEP_SUPPORT=y
> CONFIG_HAVE_LATENCYTOP_SUPPORT=y
> CONFIG_ARCH_HIBERNATION_POSSIBLE=y
> CONFIG_AUDIT_ARCH=y
> CONFIG_HAVE_SETUP_PER_CPU_AREA=y
> CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
> CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
> CONFIG_MMU=y
> CONFIG_ZONE_DMA=y
> CONFIG_NEED_DMA_MAP_STATE=y
> CONFIG_NEED_SG_DMA_LENGTH=y
> CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
> CONFIG_PGTABLE_LEVELS=4
> CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
> CONFIG_IRQ_WORK=y
> 
> #
> # General setup
> #
> CONFIG_INIT_ENV_ARG_LIMIT=32
> CONFIG_CROSS_COMPILE=""
> # CONFIG_COMPILE_TEST is not set
> CONFIG_LOCALVERSION=""
> # CONFIG_LOCALVERSION_AUTO is not set
> CONFIG_DEFAULT_HOSTNAME="(none)"
> CONFIG_SWAP=y
> CONFIG_SYSVIPC=y
> CONFIG_SYSVIPC_SYSCTL=y
> CONFIG_POSIX_MQUEUE=y
> CONFIG_POSIX_MQUEUE_SYSCTL=y
> CONFIG_CROSS_MEMORY_ATTACH=y
> # CONFIG_FHANDLE is not set
> CONFIG_USELIB=y
> # CONFIG_AUDIT is not set
> CONFIG_HAVE_ARCH_AUDITSYSCALL=y
> 
> #
> # IRQ subsystem
> #
> CONFIG_GENERIC_IRQ_SHOW=y
> CONFIG_IRQ_PREFLOW_FASTEOI=y
> CONFIG_GENERIC_MSI_IRQ=y
> CONFIG_SPARSE_IRQ=y
> CONFIG_GENERIC_CLOCKEVENTS=y
> 
> #
> # Timers subsystem
> #
> CONFIG_TICK_ONESHOT=y
> CONFIG_NO_HZ_COMMON=y
> # CONFIG_HZ_PERIODIC is not set
> CONFIG_NO_HZ_IDLE=y
> # CONFIG_NO_HZ_FULL is not set
> CONFIG_NO_HZ=y
> CONFIG_HIGH_RES_TIMERS=y
> 
> #
> # CPU/Task time and stats accounting
> #
> CONFIG_TICK_CPU_ACCOUNTING=y
> # CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
> # CONFIG_BSD_PROCESS_ACCT is not set
> # CONFIG_TASKSTATS is not set
> 
> #
> # RCU Subsystem
> #
> CONFIG_TREE_RCU=y
> # CONFIG_RCU_EXPERT is not set
> CONFIG_SRCU=y
> # CONFIG_TASKS_RCU is not set
> CONFIG_RCU_STALL_COMMON=y
> # CONFIG_TREE_RCU_TRACE is not set
> # CONFIG_RCU_NOCB_CPU is not set
> # CONFIG_RCU_EXPEDITE_BOOT is not set
> # CONFIG_BUILD_BIN2C is not set
> # CONFIG_IKCONFIG is not set
> CONFIG_LOG_BUF_SHIFT=18
> CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
> # CONFIG_CGROUPS is not set
> # CONFIG_CHECKPOINT_RESTORE is not set
> CONFIG_NAMESPACES=y
> CONFIG_UTS_NS=y
> CONFIG_IPC_NS=y
> # CONFIG_USER_NS is not set
> CONFIG_PID_NS=y
> CONFIG_NET_NS=y
> # CONFIG_SCHED_AUTOGROUP is not set
> # CONFIG_SYSFS_DEPRECATED is not set
> CONFIG_RELAY=y
> CONFIG_BLK_DEV_INITRD=y
> CONFIG_INITRAMFS_SOURCE=""
>

[PATCH] Input: i8042 - Lower log level for "no controller" message

2015-09-04 Thread Takashi Iwai 
Nowadays the machines without i8042 controller is popular, and no need
to print "No controller found" message in the error log level, which
annoys at booting in quiet mode.  Let's lower it info level.

Signed-off-by: Takashi Iwai 
---
 drivers/input/serio/i8042.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/serio/i8042.c b/drivers/input/serio/i8042.c
index cb5ece77fd7d..b8cde6912d93 100644
--- a/drivers/input/serio/i8042.c
+++ b/drivers/input/serio/i8042.c
@@ -871,7 +871,7 @@ static int __init i8042_check_aux(void)
 static int i8042_controller_check(void)
 {
if (i8042_flush()) {
-   pr_err("No controller found\n");
+   pr_info("No controller found\n");
return -ENODEV;
}
 
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] hid: enable hid device to suspend/resume asynchronously

2015-09-04 Thread Jiri Kosina 
On Mon, 17 Aug 2015, Fu, Zhonghui wrote:

> Enable hid device to suspend/resume asynchronously. This can improve
> system suspend/resume speed.

How well was this tested?

Power management is notorious for not being really in excellent shape on 
many HID devices.

So I'd like to be careful.

Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4] drivers/hid: Check result of debugfs_create_dir() and debugfs_create_file()

2015-09-04 Thread Alexander Kuleshov 
The debugfs_create_dir() and debugfs_create_file() functions may return -errno
if an error occurs. This patch adds a couple of checks of the result of the
debufs_create_dir() and debugfs_create_file() functions execution in the
hid_debug_register() and othre places.

Signed-off-by: Alexander Kuleshov 
---
Changelog:

v4:

  * Syntax error fixed.

v3:

  * do not check hid_debug before the call of the hid_debug_init()
and hid_debug_exit() from v2.

v2:

  * add check for the result of the debugfs_create_file() calls
  * call hid_debug_init() and hid_debug_exit() only if hid_debug
  * add check for the hid_debug_root in the hid_debug_register()

 drivers/hid/hid-core.c  |  9 ++---
 drivers/hid/hid-debug.c | 34 +-
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index e6fce23..fea2a88 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2599,8 +2599,10 @@ int hid_add_device(struct hid_device *hdev)
ret = device_add(>dev);
if (!ret)
hdev->status |= HID_STAT_ADDED;
-   else
-   hid_debug_unregister(hdev);
+   else {
+   if (hdev->debug)
+   hid_debug_unregister(hdev);
+   }
 
return ret;
 }
@@ -2644,7 +2646,8 @@ static void hid_remove_device(struct hid_device *hdev)
 {
if (hdev->status & HID_STAT_ADDED) {
device_del(>dev);
-   hid_debug_unregister(hdev);
+   if (hdev->debug)
+   hid_debug_unregister(hdev);
hdev->status &= ~HID_STAT_ADDED;
}
kfree(hdev->dev_rdesc);
diff --git a/drivers/hid/hid-debug.c b/drivers/hid/hid-debug.c
index 2886b64..12fe6b6 100644
--- a/drivers/hid/hid-debug.c
+++ b/drivers/hid/hid-debug.c
@@ -1222,12 +1222,41 @@ static const struct file_operations 
hid_debug_events_fops = {
 
 void hid_debug_register(struct hid_device *hdev, const char *name)
 {
+   if (!hid_debug_root)
+   goto debugfs_root_error;
+
hdev->debug_dir = debugfs_create_dir(name, hid_debug_root);
+
+   if (!hdev->debug_dir) {
+   printk(KERN_WARNING "Could not create '%s' debugfs directory\n",
+   name);
+   return;
+   }
+
hdev->debug_rdesc = debugfs_create_file("rdesc", 0400,
hdev->debug_dir, hdev, _debug_rdesc_fops);
+   if (!hdev->debug_rdesc) {
+   printk(KERN_WARNING "Could not create rdesc debugfs file\n");
+   goto error;
+   }
+
hdev->debug_events = debugfs_create_file("events", 0400,
hdev->debug_dir, hdev, _debug_events_fops);
+   if (!hdev->debug_events) {
+   printk(KERN_WARNING "Could not create events debugfs file\n");
+   goto error;
+   }
+
hdev->debug = 1;
+
+   return;
+
+error:
+   debugfs_remove_recursive(hdev->debug_dir);
+   return;
+
+debugfs_root_error:
+   return;
 }
 
 void hid_debug_unregister(struct hid_device *hdev)
@@ -1242,10 +1271,13 @@ void hid_debug_unregister(struct hid_device *hdev)
 void hid_debug_init(void)
 {
hid_debug_root = debugfs_create_dir("hid", NULL);
+   if (!hid_debug_root)
+   printk(KERN_WARNING "Could not create root 'hid' debugfs 
directory\n");
 }
 
 void hid_debug_exit(void)
 {
-   debugfs_remove_recursive(hid_debug_root);
+   if (hid_debug_root)
+   debugfs_remove_recursive(hid_debug_root);
 }
 
-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] HID: logitech-hidpp: allow to disable tap to click on the K400

2015-09-04 Thread Jiri Kosina 
On Thu, 3 Sep 2015, Benjamin Tissoires wrote:

> Hi guys,
> 
> This series adds a new parameter for the K400 owners. This keyboard has an
> embedded touchpad which defaults to report tap to click. It can be annoying
> given that there are physical buttons, and we can just disable that from the
> host by using the feature 0x6010.
> 
> The first patch might conflict with Simon's current work in progress, so
> that's why Simon is CC-ed to it.

Now in for-4.4/logitech. Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] drivers/hid: Check result of debugfs_create_dir() and debugfs_create_file()

2015-09-04 Thread Alexander Kuleshov 
Yes, sorry, the CONFIG_DEBUG_FS was 'n' in that case that time. Will resend

2015-09-04 19:17 GMT+06:00 Jiri Kosina :
> On Thu, 20 Aug 2015, Alexander Kuleshov wrote:
>
>> The debugfs_create_dir() and debugfs_create_file() functions may return 
>> -errno
>> if an error occurs. This patch adds a couple of checks of the result of the
>> debufs_create_dir() and debugfs_create_file() functions execution in the
>> hid_debug_register() and othre places.
>>
>> Changelog:
>>
>> v3:
>>
>>   * do not check hid_debug before the call of the hid_debug_init()
>> and hid_debug_exit() from v2
>>
>> v2:
>>
>>   * add check for the result of the debugfs_create_file() calls
>>   * call hid_debug_init() and hid_debug_exit() only if hid_debug
>>   * add check for the hid_debug_root in the hid_debug_register()
>>
>> Signed-off-by: Alexander Kuleshov 
>
> Did you try to even compile-test your patch?
>
> --
> Jiri Kosina
> SUSE Labs
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3] drivers/hid: Check result of debugfs_create_dir() and debugfs_create_file()

2015-09-04 Thread Jiri Kosina 
On Thu, 20 Aug 2015, Alexander Kuleshov wrote:

> The debugfs_create_dir() and debugfs_create_file() functions may return -errno
> if an error occurs. This patch adds a couple of checks of the result of the
> debufs_create_dir() and debugfs_create_file() functions execution in the
> hid_debug_register() and othre places.
> 
> Changelog:
> 
> v3:
> 
>   * do not check hid_debug before the call of the hid_debug_init()
> and hid_debug_exit() from v2
> 
> v2:
> 
>   * add check for the result of the debugfs_create_file() calls
>   * call hid_debug_init() and hid_debug_exit() only if hid_debug
>   * add check for the hid_debug_root in the hid_debug_register()
> 
> Signed-off-by: Alexander Kuleshov 

Did you try to even compile-test your patch?

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4.2] hid_dr: Fixed HID Descriptor

2015-09-04 Thread Jiri Kosina 
On Thu, 3 Sep 2015, Maciej Zuk wrote:

> From: Maciej Zuk 
> 
> Fixed HID descriptor for DragonRise Joystick.
> Replaced default descriptor which doubles Z axis 
> and causes mixing values of X and Z axes.

Applied to for-4.4/dragonrise.

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/1] Corsair Vengeance K90 driver

2015-09-04 Thread Jiri Kosina 
On Sat, 29 Aug 2015, Clément Vuchener wrote:


This is missing changelog and Signed-off-by: line. I know that you have 
provided that in your cover letter, but cover letter never make it to the 
actual GIT commits.

So please fix that and resend the patch, thanks.

> ---
>  Documentation/ABI/testing/sysfs-class-k90_profile  |  55 ++
>  .../ABI/testing/sysfs-driver-hid-corsair-k90   |  15 +
>  drivers/hid/Kconfig|  10 +
>  drivers/hid/Makefile   |   1 +
>  drivers/hid/hid-core.c |   1 +
>  drivers/hid/hid-corsair-k90.c  | 690
> +
>  drivers/hid/hid-ids.h  |   3 +
>  7 files changed, 775 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-class-k90_profile
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
>  create mode 100644 drivers/hid/hid-corsair-k90.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-k90_profile
> b/Documentation/ABI/testing/sysfs-class-k90_profile
> new file mode 100644
> index 000..3275c16
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-class-k90_profile
> @@ -0,0 +1,55 @@
> +What:/sys/class/k90_profile//profile_number
> +Date:August 2015
> +KernelVersion:   4.2
> +Contact: Clement Vuchener 
> +Description: Get the number of the profile.
> +
> +What:/sys/class/k90_profile//bindings
> +Date:August 2015
> +KernelVersion:   4.2
> +Contact: Clement Vuchener 
> +Description: Write bindings to the keyboard on-board profile.
> + The data structure is:
> +  - number of bindings in this structure (1 byte)
> +  - size of this data structure (2 bytes big endian)
> +  - size of the macro data written to
> +/sys/class/k90_profile//data (2 bytes big endian)
> +  - array of bindings referencing the data from
> +/sys/class/k90_profile//data, each containing:
> +* 0x10 for a key usage, 0x20 for a macro (1 byte)
> +* offset of the key usage/macro data (2 bytes big endian)
> +* length of the key usage/macro data (2 bytes big endian)
> +

This looks like violation of one-value-per-attribute rule for sysfs ABI. 
Could you please think about it once again with this aspect on mind?

[ ... snip ... ]
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index e6fce23..f0d9125 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1807,6 +1807,7 @@ static const struct hid_device_id
> hid_have_special_driver[] = {
>   { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY,
> USB_DEVICE_ID_CHICONY_WIRELESS) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY,
> USB_DEVICE_ID_CHICONY_WIRELESS2) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS,
> USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_CP2112) },
>   { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
> USB_DEVICE_ID_CYPRESS_BARCODE_1) },

Your mail client is corrupting long lines, making tha patch application 
impossible. Please fix that for your following submissions.

> diff --git a/drivers/hid/hid-corsair-k90.c b/drivers/hid/hid-corsair-k90.c
> new file mode 100644
> index 000..67c1095
> --- /dev/null
> +++ b/drivers/hid/hid-corsair-k90.c
> @@ -0,0 +1,690 @@
> +/*
> + * HID driver for Corsair Vengeance K90 Keyboard

Usually we try to be a little bit more generic, and name the driver 
according to the vendor (and fold all the vedor-specific stuff into the 
one driver).

So my suggestion would be to name the driver hid-corsair, keeping the 
possibility for adding more devices later open.

[ ... snip ... ]
> +static int k90_init_special_functions(struct hid_device *dev)
> +{
> + int ret, i;
> + struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
> + struct usb_device *usbdev = interface_to_usbdev(usbif);
> + char data[8];
> + struct k90_drvdata *drvdata =
> + kzalloc(sizeof(struct k90_drvdata), GFP_KERNEL);
> + size_t name_sz;
> + char *name;
> + struct k90_led *led;
> +
> + if (!drvdata) {
> + ret = -ENOMEM;
> + goto fail_drvdata;
> + }
> + hid_set_drvdata(dev, drvdata);
> +
> + /* Get current status */
> + ret = usb_control_msg(usbdev, usb_rcvctrlpipe(usbdev, 0),
> +   K90_REQUEST_STATUS,
> +   USB_DIR_IN | USB_TYPE_VENDOR |
> +   USB_RECIP_DEVICE, 0, 0, data, 8,
> +   USB_CTRL_SET_TIMEOUT);

So you apparently also depend on USB ...

> + if

Re: [PATCH] HID: mode button quirk for Mad Catz R.A.T.5

2015-09-04 Thread Jiri Kosina 
On Thu, 3 Sep 2015, Harald Brinkmann wrote:

> Hi all,
> 
> this patch applies to Linux 4.2.0.
> 
> It enables the Saitek HID quirk for the mode button
> of the Mad Catz R.A.T.5 gaming mouse.
> 
> Signed-off-by: Harald Brinkmann 

Applied to for-4.4/upstream. Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v1] HID: sensor-hub: Fixup for Lenovo ThinkPad Helix 2 sensor hub report

2015-09-04 Thread Jiri Kosina 
On Thu, 3 Sep 2015, Srinivas Pandruvada wrote:

> On Thu, 2015-09-03 at 12:56 -0300, Fernando D S Lima wrote:
> > There is an error in the report descriptor of the Thinkpad Helix 2 
> > where
> > logical minimum value (557376) is greater than logical maximum 
> > (491200)
> > for all of the magnetic flux axis data fields. This error results in 
> > a
> > report descriptor parsing failure that causes the sensors attached to 
> > the
> > hub not to be detected.
> > 
> > dmesg excerpt:
> > [   19.866905] drivers/hid/hid-core.c: logical range invalid 0x88140 
> > 0x77ec0
> > [   19.866914] hid-sensor-hub 0018:2047:0855.0007: item 0 1 0 8 
> > parsing failed
> > [   19.866926] hid-sensor-hub 0018:2047:0855.0007: parse failed
> > [   19.866933] hid-sensor-hub: probe of 0018:2047:0855.0007 failed 
> > with error -22
> > 
> > Add a report fixup to change magnetic flux logical minimums to 
> > -557376
> > for the parsing to succeed and the sensors to get detected.
> > After applying the fix the sensors get detected, with corresponding 
> > drivers
> > (hid-accel-3d,hid-gyro-3d,etc) loaded, and its possible to read their 
> > values.
> > 
> > Signed-off-by: Fernando D S Lima 
> Reviewed-by: Srinivas Pandruvada 

Applied to for-4.3/upstream-fixes. Thanks,

-- 
Jiri Kosina
SUSE Labs

--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [input:next 1310/1311] drivers/input/misc/sparcspkr.c:256:35: error: macro "MODULE_DEVICE_TABLE" requires 2 arguments, but only 1 given

2015-09-04 Thread Dmitry Torokhov 
Hi Luis,

On Fri, Sep 4, 2015 at 4:25 AM, Luis de Bethencourt
 wrote:
> Hi Dmitry,
>
> I got this error email and confirmed that I missed the 'of' in my patch. I was
> about to send a v3 with this fixed. But I just noticed it is already fixed in
> your branch.
>
> https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/commit/?h=next=26492f195eed08b95ad5acdfbe625062ad7d86c6
>
> I am assuming you did this. Thank you!
>
> I just wanted to confirm that there isn't anything else related to this you
> would like me to do.

No I think we are good now.

Thanks.

-- 
Dmitry
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Potential data race in psmouse_interrupt

2015-09-04 Thread Dmitry Torokhov 
On Tue, Sep 1, 2015 at 11:46 AM, Dmitry Vyukov  wrote:
> On Fri, Aug 28, 2015 at 8:32 PM, Dmitry Torokhov
>  wrote:
>> On Fri, Aug 28, 2015 at 11:08 AM, Dmitry Vyukov  wrote:
>>> On Fri, Aug 28, 2015 at 7:51 PM, Dmitry Torokhov
>>>  wrote:
 On Fri, Aug 28, 2015 at 10:34 AM, Dmitry Vyukov  wrote:
> Hello,
>
> I am looking at this code in __ps2_command again:
>
> /*
> * The reset command takes a long time to execute.
> */
> timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 500);
>
> timeout = wait_event_timeout(ps2dev->wait,
> !(READ_ONCE(ps2dev->flags) & PS2_FLAG_CMD1), timeout);
>
> if (smp_load_acquire(>cmdcnt) &&
> !(smp_load_acquire(>flags) & PS2_FLAG_CMD1)) {
>   timeout = ps2_adjust_timeout(ps2dev, command, timeout);
>   wait_event_timeout(ps2dev->wait,
> !(smp_load_acquire(>flags) &
> PS2_FLAG_CMD), timeout);
> }
>
> if (param)
> for (i = 0; i < receive; i++)
>   param[i] = ps2dev->cmdbuf[(receive - 1) - i];
>
>
> Here are two moments I don't understand:
> 1. The last parameter of ps2_adjust_timeout is timeout in jiffies (it
> is compared against 100ms). However, timeout is assigned to result of
> wait_event_timeout, which returns 0 or 1. This does not make sense to
> me. What am I missing?

 The fact that wait_event_timeout can return value greater than one:

  * Returns:
  * 0 if the @condition evaluated to %false after the @timeout elapsed,
  * 1 if the @condition evaluated to %true after the @timeout elapsed,
  * or the remaining jiffies (at least 1) if the @condition evaluated
   ^
>>>
>>>
>>> OK, makes sense now!
>>>
> 2. This code pays great attention to timeouts, but in the end I don't
> see how it handles timeouts. That is, if a timeout is happened, we
> still copyout (garbage) from cmdbuf. What am I missing here?

 Once upon a time wait_event() did not return positive value when
 timeout expired and then condition satisfied. So we just examine the
 final state (psmpouse->cmdcnt should be 0 if command actually
 succeeded) and even if we copy in garbage nobody should care since
 we'll return error in this case.
>>>
>>>
>>> I see.
>>> But the cmdcnt is re-read after copying out response. So it is
>>> possible that we read garbage response, but then read cmdcnt==0 and
>>> return OK to caller.
>>
>> That assumes that we actually timed out, and while we were copying the
>> data the response finally came.
>
> Right.
>
>>>
>>> So far I have something along the following lines to fix data races in 
>>> libps2.c
>>
>> I don't know, maybe we should simply move call to
>> serio_pause_rx(ps2dev->serio) higher, before we check ps2dev->cmdcnt,
>> and move copying of the buffer down, after checking cmdcnt.
>
> I don't know about serio_pause_rx, but copying of response should be
> done after checking cmdcnt.

It will stop the interrupt handler from running while we are examining
the cmdcnt and copy out the data, thus removing the race.

> Also you need to use smp_store_release/smp_load_acquire cmdcnt and
> flags when they have dependent data. And READ_ONCE/WRITE_ONCE on
> shared state otherwise is highly desirable.
>
>>> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
>>> index 7551699..51c747f 100644
>>> --- a/drivers/input/serio/libps2.c
>>> +++ b/drivers/input/serio/libps2.c
>>> @@ -43,7 +43,7 @@ int ps2_sendbyte(struct ps2dev *ps2dev, unsigned
>>> char byte, int timeout)
>>>
>>>  if (serio_write(ps2dev->serio, byte) == 0)
>>>  wait_event_timeout(ps2dev->wait,
>>> -   !(ps2dev->flags & PS2_FLAG_ACK),
>>> +   !(READ_ONCE(ps2dev->flags) & 
>>> PS2_FLAG_ACK),
>>> msecs_to_jiffies(timeout));
>>>
>>>  serio_pause_rx(ps2dev->serio);
>>> @@ -187,6 +187,7 @@ int __ps2_command(struct ps2dev *ps2dev, unsigned
>>> char *param, int command)
>>>  int receive = (command >> 8) & 0xf;
>>>  int rc = -1;
>>>  int i;
>>> +unsigned char cmdcnt;
>>>
>>>  if (receive > sizeof(ps2dev->cmdbuf)) {
>>>  WARN_ON(1);
>>> @@ -225,23 +226,22 @@ int __ps2_command(struct ps2dev *ps2dev,
>>> unsigned char *param, int command)
>>>  timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 
>>> 500);
>>>
>>>  timeout = wait_event_timeout(ps2dev->wait,
>>> - !(ps2dev->flags &
>>> PS2_FLAG_CMD1), timeout);
>>> -
>>> -if (ps2dev->cmdcnt && !(ps2dev->flags & PS2_FLAG_CMD1)) {
>>> +!(READ_ONCE(ps2dev->flags) & PS2_FLAG_CMD1),

Re: [PATCH 1/1] Corsair Vengeance K90 driver

2015-09-04 Thread Clément Vuchener 

On 09/04/2015 03:27 PM, Jiri Kosina wrote:
> On Sat, 29 Aug 2015, Clément Vuchener wrote:
>
>
> This is missing changelog and Signed-off-by: line. I know that you have 
> provided that in your cover letter, but cover letter never make it to the 
> actual GIT commits.
>
> So please fix that and resend the patch, thanks.
>
>> ---
>>  Documentation/ABI/testing/sysfs-class-k90_profile  |  55 ++
>>  .../ABI/testing/sysfs-driver-hid-corsair-k90   |  15 +
>>  drivers/hid/Kconfig|  10 +
>>  drivers/hid/Makefile   |   1 +
>>  drivers/hid/hid-core.c |   1 +
>>  drivers/hid/hid-corsair-k90.c  | 690
>> +
>>  drivers/hid/hid-ids.h  |   3 +
>>  7 files changed, 775 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-class-k90_profile
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-hid-corsair-k90
>>  create mode 100644 drivers/hid/hid-corsair-k90.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-class-k90_profile
>> b/Documentation/ABI/testing/sysfs-class-k90_profile
>> new file mode 100644
>> index 000..3275c16
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-class-k90_profile
>> @@ -0,0 +1,55 @@
>> +What:   /sys/class/k90_profile//profile_number
>> +Date:   August 2015
>> +KernelVersion:  4.2
>> +Contact:Clement Vuchener 
>> +Description:Get the number of the profile.
>> +
>> +What:   /sys/class/k90_profile//bindings
>> +Date:   August 2015
>> +KernelVersion:  4.2
>> +Contact:Clement Vuchener 
>> +Description:Write bindings to the keyboard on-board profile.
>> +The data structure is:
>> + - number of bindings in this structure (1 byte)
>> + - size of this data structure (2 bytes big endian)
>> + - size of the macro data written to
>> +   /sys/class/k90_profile//data (2 bytes big endian)
>> + - array of bindings referencing the data from
>> +   /sys/class/k90_profile//data, each containing:
>> +   * 0x10 for a key usage, 0x20 for a macro (1 byte)
>> +   * offset of the key usage/macro data (2 bytes big endian)
>> +   * length of the key usage/macro data (2 bytes big endian)
>> +
> This looks like violation of one-value-per-attribute rule for sysfs ABI. 
> Could you please think about it once again with this aspect on mind?
Per key attributes would be nice but I don't think I can do that. The profile 
must be written all at once and I don't know any way to read it from the 
hardware (the windows driver I studied does not do it). So writing one value 
only would erase all the others.
I think I will remove this part from the driver. The same thing can easily be 
done in user space through libusb and writing profile should be exceptional 
enough that it is not a problem to detach the driver while doing it. That part 
of the driver is not really useful with the current ABI.
>
> [ ... snip ... ]
>> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
>> index e6fce23..f0d9125 100644
>> --- a/drivers/hid/hid-core.c
>> +++ b/drivers/hid/hid-core.c
>> @@ -1807,6 +1807,7 @@ static const struct hid_device_id
>> hid_have_special_driver[] = {
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY,
>> USB_DEVICE_ID_CHICONY_WIRELESS) },
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY,
>> USB_DEVICE_ID_CHICONY_WIRELESS2) },
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CHICONY, USB_DEVICE_ID_CHICONY_AK1D) },
>> +{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS,
>> USB_DEVICE_ID_PRODIKEYS_PCMIDI) },
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CYGNAL, USB_DEVICE_ID_CYGNAL_CP2112) },
>>  { HID_USB_DEVICE(USB_VENDOR_ID_CYPRESS,
>> USB_DEVICE_ID_CYPRESS_BARCODE_1) },
> Your mail client is corrupting long lines, making tha patch application 
> impossible. Please fix that for your following submissions.
>
>> diff --git a/drivers/hid/hid-corsair-k90.c b/drivers/hid/hid-corsair-k90.c
>> new file mode 100644
>> index 000..67c1095
>> --- /dev/null
>> +++ b/drivers/hid/hid-corsair-k90.c
>> @@ -0,0 +1,690 @@
>> +/*
>> + * HID driver for Corsair Vengeance K90 Keyboard
> Usually we try to be a little bit more generic, and name the driver 
> according to the vendor (and fold all the vedor-specific stuff into the 
> one driver).
>
> So my suggestion would be to name the driver hid-corsair, keeping the 
> possibility for adding more devices later open.
>
> [ ... snip ... ]
>> +static int k90_init_special_functions(struct hid_device *dev)
>> +{
>> +int ret, i;
>> +struct usb_interface *usbif = to_usb_interface(dev->dev.parent);
>> +struct usb_device *usbdev = interface_to_usbdev(usbif);
>> +char data[8];
>> +

Re: Potential data race in psmouse_interrupt

2015-09-04 Thread Dmitry Vyukov 
On Fri, Sep 4, 2015 at 6:56 PM, Dmitry Torokhov
 wrote:
> On Tue, Sep 1, 2015 at 11:46 AM, Dmitry Vyukov  wrote:
>> On Fri, Aug 28, 2015 at 8:32 PM, Dmitry Torokhov
>>  wrote:
>>> On Fri, Aug 28, 2015 at 11:08 AM, Dmitry Vyukov  wrote:
 On Fri, Aug 28, 2015 at 7:51 PM, Dmitry Torokhov
  wrote:
> On Fri, Aug 28, 2015 at 10:34 AM, Dmitry Vyukov  
> wrote:
>> Hello,
>>
>> I am looking at this code in __ps2_command again:
>>
>> /*
>> * The reset command takes a long time to execute.
>> */
>> timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 500);
>>
>> timeout = wait_event_timeout(ps2dev->wait,
>> !(READ_ONCE(ps2dev->flags) & PS2_FLAG_CMD1), timeout);
>>
>> if (smp_load_acquire(>cmdcnt) &&
>> !(smp_load_acquire(>flags) & PS2_FLAG_CMD1)) {
>>   timeout = ps2_adjust_timeout(ps2dev, command, timeout);
>>   wait_event_timeout(ps2dev->wait,
>> !(smp_load_acquire(>flags) &
>> PS2_FLAG_CMD), timeout);
>> }
>>
>> if (param)
>> for (i = 0; i < receive; i++)
>>   param[i] = ps2dev->cmdbuf[(receive - 1) - i];
>>
>>
>> Here are two moments I don't understand:
>> 1. The last parameter of ps2_adjust_timeout is timeout in jiffies (it
>> is compared against 100ms). However, timeout is assigned to result of
>> wait_event_timeout, which returns 0 or 1. This does not make sense to
>> me. What am I missing?
>
> The fact that wait_event_timeout can return value greater than one:
>
>  * Returns:
>  * 0 if the @condition evaluated to %false after the @timeout elapsed,
>  * 1 if the @condition evaluated to %true after the @timeout elapsed,
>  * or the remaining jiffies (at least 1) if the @condition evaluated
>   ^


 OK, makes sense now!

>> 2. This code pays great attention to timeouts, but in the end I don't
>> see how it handles timeouts. That is, if a timeout is happened, we
>> still copyout (garbage) from cmdbuf. What am I missing here?
>
> Once upon a time wait_event() did not return positive value when
> timeout expired and then condition satisfied. So we just examine the
> final state (psmpouse->cmdcnt should be 0 if command actually
> succeeded) and even if we copy in garbage nobody should care since
> we'll return error in this case.


 I see.
 But the cmdcnt is re-read after copying out response. So it is
 possible that we read garbage response, but then read cmdcnt==0 and
 return OK to caller.
>>>
>>> That assumes that we actually timed out, and while we were copying the
>>> data the response finally came.
>>
>> Right.
>>

 So far I have something along the following lines to fix data races in 
 libps2.c
>>>
>>> I don't know, maybe we should simply move call to
>>> serio_pause_rx(ps2dev->serio) higher, before we check ps2dev->cmdcnt,
>>> and move copying of the buffer down, after checking cmdcnt.
>>
>> I don't know about serio_pause_rx, but copying of response should be
>> done after checking cmdcnt.
>
> It will stop the interrupt handler from running while we are examining
> the cmdcnt and copy out the data, thus removing the race.
>
>> Also you need to use smp_store_release/smp_load_acquire cmdcnt and
>> flags when they have dependent data. And READ_ONCE/WRITE_ONCE on
>> shared state otherwise is highly desirable.
>>
 diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
 index 7551699..51c747f 100644
 --- a/drivers/input/serio/libps2.c
 +++ b/drivers/input/serio/libps2.c
 @@ -43,7 +43,7 @@ int ps2_sendbyte(struct ps2dev *ps2dev, unsigned
 char byte, int timeout)

  if (serio_write(ps2dev->serio, byte) == 0)
  wait_event_timeout(ps2dev->wait,
 -   !(ps2dev->flags & PS2_FLAG_ACK),
 +   !(READ_ONCE(ps2dev->flags) & 
 PS2_FLAG_ACK),
 msecs_to_jiffies(timeout));

  serio_pause_rx(ps2dev->serio);
 @@ -187,6 +187,7 @@ int __ps2_command(struct ps2dev *ps2dev, unsigned
 char *param, int command)
  int receive = (command >> 8) & 0xf;
  int rc = -1;
  int i;
 +unsigned char cmdcnt;

  if (receive > sizeof(ps2dev->cmdbuf)) {
  WARN_ON(1);
 @@ -225,23 +226,22 @@ int __ps2_command(struct ps2dev *ps2dev,
 unsigned char *param, int command)
  timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 
 500);

  timeout = wait_event_timeout(ps2dev->wait,
 -

[git pull] Input updates for 4.3-rc0

2015-09-04 Thread Dmitry Torokhov 
Hi Linus,

Please pull from:

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

to receive first round of updates for the input subsystem. Drivers,
drivers, drivers...  No interesting input core changes this time.

Changelog:
-

Anshul Garg (3):
  Input: joydev - use for_each_set_bit where appropriate
  Input: ff-core - use for_each_set_bit where appropriate
  Input: uinput - switch to using for_each_set_bit()

Axel Lin (1):
  Input: drv260x/drv2665/drv2667 - constify reg_default tables

Benjamin Tissoires (1):
  Input: elan_i2c - enable ELAN0600 acpi panels

Benson Leung (1):
  Input: elan_i2c - fix typos for validpage_count

Dan Carpenter (1):
  Input: sentelic - silence some underflow warnings

Dirk Behme (3):
  Input: zforce_ts - convert to use the gpiod interface
  Input: zforce - don't invert the interrupt GPIO
  Input: zforce - make the interrupt GPIO optional

Dmitry Torokhov (20):
  Input: of_touchscreen - always issue warning if axis is not set up
  Input: of_touchscreen - fix setting max values on X/Y axis
  Input: of_touchscreen - switch to using device properties
  Input: pixcir_i2c_ts - move platform data
  Input: pixcir_i2c_ts - switch the device over to gpiod
  Input: pixcir_i2c_ts - allow using with GPIO expanders
  Input: pixcir_i2c_ts - simplify input device initialization
  Input: pixcir_i2c_ts - use standard OF touchscreen parsing code
  Input: matrix_keypad - change name of wakeup property to "wakeup-source"
  Input: ads7846 - change name of wakeup property to "wakeup-source"
  Input: pmic8xxx-keypad - change name of wakeup property
  Input: gpio_keys[_polled] - change name of wakeup property
  Input: samsung-keypad - change name of wakeup property
  Input: tc3589x-keypad - change name of wakeup property
  Input: arizona-haptic - convert to use managed input devices
  Input: do not emit unneeded EV_SYN when suspending
  Input: elants_i2c - wire up regulator support
  Input: elants_i2c - enable asynchronous probing
  Input: elan_i2c - enable asynchronous probing
  Input: synaptics - fix handling of disabling gesture mode

Dudley Du (8):
  Input: cyapa - rename 'gen5' to 'pip' for chared code
  Input: cyapa - add gen6 device module support
  Input: cyapa - add proximity support for gen5 and gen6 modules
  Input: cyapa - fully support runtime suspend power management
  Input: cyapa - add ACPI HID CYAP0002 for Gen6 devices
  Input: cyapa - add regulator vcc support
  Input: cyapa - introduce device tree binding
  Input: cyapa - do not try to enable proximity reporting on older devices

Duson Lin (1):
  Input: elan_i2c - use iap_version to get firmware information

Geert Uytterhoeven (1):
  Input: Allow compile test of GPIO consumers if !GPIOLIB

Himangi Saraogi (1):
  Input: tc3589x-keypad - switch to using managed resources

HungNien Chen (2):
  Input: wdt87xx_i2c - populate vendor and product in input device
  Input: wdt87xx_i2c - change the sleep time to 2500ms after the sw reset

James Chen (1):
  Input: elants_i2c - disable idle mode before updating firmware

Javier Martinez Canillas (4):
  Input: export I2C module alias information in missing drivers
  Input: touchscreen - export OF module alias information
  Input: cros_ec_keyb - replace KEYBOARD_CROS_EC dependency
  Input: max8997_haptic - fix module alias

Julia Lawall (1):
  Input: sur40 - fix error return code

Krzysztof Kozlowski (1):
  Input: drop owner assignment from i2c_driver

Martin Kepplinger (1):
  Input: bma150 - use sign_extend32() for sign extending

Matt Ranostay (1):
  Input: cap11xx - add LED support

Michele Curti (1):
  Input: elan_i2c - enable ELAN0100 acpi panels

Nick Dyer (7):
  Input: atmel_mxt_ts - use deep sleep mode when stopped
  Input: atmel_mxt_ts - remove unused defines
  Input: atmel_mxt_ts - improve device tree parsing
  Input: atmel_mxt_ts - disable interrupt for 50ms after reset
  Input: atmel_mxt_ts - initialise input slots with INPUT_MT_DIRECT
  Input: atmel_mxt_ts - remove warning on zero T44 count
  MAINTAINERS: Add maintainer for atmel_mxt_ts

Pan Xinhui (1):
  Input: atmel_mxt_ts - suspend/resume causes panic if input_dev fails to 
init

Peng Fan (1):
  Input: gpio-keys - report error when disabling unsupported key

Roger Quadros (1):
  Input: pixcir_i2c_ts - add RESET gpio

Sebastian Reichel (5):
  Input: tsc2005 - improve readability of register defines
  Input: tsc2005 - fix Kconfig indentation
  Input: tsc2005 - convert to regmap
  Input: tsc2005 - simplify drvdata acquisition
  Input: tsc2005 - convert to gpiod

Stefan Assmann (1):
  Input: psmouse - add small delay for IBM trackpoint pass-through mode

Stephen Boyd (1):
  Input: pmic8xxx-pwrkey - support shutdown

Re: Potential data race in psmouse_interrupt

2015-09-04 Thread Dmitry Torokhov 
On Fri, Sep 4, 2015 at 12:32 PM, Dmitry Vyukov  wrote:
> On Fri, Sep 4, 2015 at 6:56 PM, Dmitry Torokhov
>  wrote:
>> On Tue, Sep 1, 2015 at 11:46 AM, Dmitry Vyukov  wrote:
>>> On Fri, Aug 28, 2015 at 8:32 PM, Dmitry Torokhov
>>>  wrote:
 On Fri, Aug 28, 2015 at 11:08 AM, Dmitry Vyukov  wrote:
> On Fri, Aug 28, 2015 at 7:51 PM, Dmitry Torokhov
>  wrote:
>> On Fri, Aug 28, 2015 at 10:34 AM, Dmitry Vyukov  
>> wrote:
>>> Hello,
>>>
>>> I am looking at this code in __ps2_command again:
>>>
>>> /*
>>> * The reset command takes a long time to execute.
>>> */
>>> timeout = msecs_to_jiffies(command == PS2_CMD_RESET_BAT ? 4000 : 500);
>>>
>>> timeout = wait_event_timeout(ps2dev->wait,
>>> !(READ_ONCE(ps2dev->flags) & PS2_FLAG_CMD1), timeout);
>>>
>>> if (smp_load_acquire(>cmdcnt) &&
>>> !(smp_load_acquire(>flags) & PS2_FLAG_CMD1)) {
>>>   timeout = ps2_adjust_timeout(ps2dev, command, timeout);
>>>   wait_event_timeout(ps2dev->wait,
>>> !(smp_load_acquire(>flags) &
>>> PS2_FLAG_CMD), timeout);
>>> }
>>>
>>> if (param)
>>> for (i = 0; i < receive; i++)
>>>   param[i] = ps2dev->cmdbuf[(receive - 1) - i];
>>>
>>>
>>> Here are two moments I don't understand:
>>> 1. The last parameter of ps2_adjust_timeout is timeout in jiffies (it
>>> is compared against 100ms). However, timeout is assigned to result of
>>> wait_event_timeout, which returns 0 or 1. This does not make sense to
>>> me. What am I missing?
>>
>> The fact that wait_event_timeout can return value greater than one:
>>
>>  * Returns:
>>  * 0 if the @condition evaluated to %false after the @timeout elapsed,
>>  * 1 if the @condition evaluated to %true after the @timeout elapsed,
>>  * or the remaining jiffies (at least 1) if the @condition evaluated
>>   ^
>
>
> OK, makes sense now!
>
>>> 2. This code pays great attention to timeouts, but in the end I don't
>>> see how it handles timeouts. That is, if a timeout is happened, we
>>> still copyout (garbage) from cmdbuf. What am I missing here?
>>
>> Once upon a time wait_event() did not return positive value when
>> timeout expired and then condition satisfied. So we just examine the
>> final state (psmpouse->cmdcnt should be 0 if command actually
>> succeeded) and even if we copy in garbage nobody should care since
>> we'll return error in this case.
>
>
> I see.
> But the cmdcnt is re-read after copying out response. So it is
> possible that we read garbage response, but then read cmdcnt==0 and
> return OK to caller.

 That assumes that we actually timed out, and while we were copying the
 data the response finally came.
>>>
>>> Right.
>>>
>
> So far I have something along the following lines to fix data races in 
> libps2.c

 I don't know, maybe we should simply move call to
 serio_pause_rx(ps2dev->serio) higher, before we check ps2dev->cmdcnt,
 and move copying of the buffer down, after checking cmdcnt.
>>>
>>> I don't know about serio_pause_rx, but copying of response should be
>>> done after checking cmdcnt.
>>
>> It will stop the interrupt handler from running while we are examining
>> the cmdcnt and copy out the data, thus removing the race.
>>
>>> Also you need to use smp_store_release/smp_load_acquire cmdcnt and
>>> flags when they have dependent data. And READ_ONCE/WRITE_ONCE on
>>> shared state otherwise is highly desirable.
>>>
> diff --git a/drivers/input/serio/libps2.c b/drivers/input/serio/libps2.c
> index 7551699..51c747f 100644
> --- a/drivers/input/serio/libps2.c
> +++ b/drivers/input/serio/libps2.c
> @@ -43,7 +43,7 @@ int ps2_sendbyte(struct ps2dev *ps2dev, unsigned
> char byte, int timeout)
>
>  if (serio_write(ps2dev->serio, byte) == 0)
>  wait_event_timeout(ps2dev->wait,
> -   !(ps2dev->flags & PS2_FLAG_ACK),
> +   !(READ_ONCE(ps2dev->flags) & 
> PS2_FLAG_ACK),
> msecs_to_jiffies(timeout));
>
>  serio_pause_rx(ps2dev->serio);
> @@ -187,6 +187,7 @@ int __ps2_command(struct ps2dev *ps2dev, unsigned
> char *param, int command)
>  int receive = (command >> 8) & 0xf;
>  int rc = -1;
>  int i;
> +unsigned char cmdcnt;
>
>  if (receive > sizeof(ps2dev->cmdbuf)) {
>  WARN_ON(1);
> @@ -225,23 +226,22 @@ int __ps2_command(struct ps2dev *ps2dev,
> unsigned char *param,

[input:next 1310/1311] drivers/input/misc/sparcspkr.c:256:35: error: macro "MODULE_DEVICE_TABLE" requires 2 arguments, but only 1 given

2015-09-04 Thread kbuild test robot 
tree:   git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
head:   ea96e2121490a5083a2f6681042946dd1dfc222a
commit: 2a29d098e3bb21dc822971eb615569bdb0e4ad0b [1310/1311] Input: sparcspkr - 
fix module autoload for OF platform drivers
config: sparc64-defconfig (attached as .config)
reproduce:
  wget 
https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross
 -O ~/bin/make.cross
  chmod +x ~/bin/make.cross
  git checkout 2a29d098e3bb21dc822971eb615569bdb0e4ad0b
  # save the attached .config to linux build tree
  make.cross ARCH=sparc64 

All error/warnings (new ones prefixed by >>):

>> drivers/input/misc/sparcspkr.c:256:35: error: macro "MODULE_DEVICE_TABLE" 
>> requires 2 arguments, but only 1 given
MODULE_DEVICE_TABLE(bbc_beep_match);
  ^
>> drivers/input/misc/sparcspkr.c:256:1: warning: data definition has no type 
>> or storage class
MODULE_DEVICE_TABLE(bbc_beep_match);
^
>> drivers/input/misc/sparcspkr.c:256:1: error: type defaults to 'int' in 
>> declaration of 'MODULE_DEVICE_TABLE' [-Werror=implicit-int]
   cc1: some warnings being treated as errors

vim +/MODULE_DEVICE_TABLE +256 drivers/input/misc/sparcspkr.c

   250  {
   251  .name = "beep",
   252  .compatible = "SUNW,bbc-beep",
   253  },
   254  {},
   255  };
 > 256  MODULE_DEVICE_TABLE(bbc_beep_match);
   257  
   258  static struct platform_driver bbc_beep_driver = {
   259  .driver = {

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation
#
# Automatically generated file; DO NOT EDIT.
# Linux/sparc64 4.2.0-rc3 Kernel Configuration
#
CONFIG_64BIT=y
CONFIG_SPARC=y
# CONFIG_SPARC32 is not set
CONFIG_SPARC64=y
CONFIG_ARCH_DEFCONFIG="arch/sparc/configs/sparc64_defconfig"
CONFIG_ARCH_PROC_KCORE_TEXT=y
CONFIG_IOMMU_HELPER=y
CONFIG_STACKTRACE_SUPPORT=y
CONFIG_LOCKDEP_SUPPORT=y
CONFIG_HAVE_LATENCYTOP_SUPPORT=y
CONFIG_ARCH_HIBERNATION_POSSIBLE=y
CONFIG_AUDIT_ARCH=y
CONFIG_HAVE_SETUP_PER_CPU_AREA=y
CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y
CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y
CONFIG_MMU=y
CONFIG_ZONE_DMA=y
CONFIG_NEED_DMA_MAP_STATE=y
CONFIG_NEED_SG_DMA_LENGTH=y
CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y
CONFIG_PGTABLE_LEVELS=4
CONFIG_DEFCONFIG_LIST="/lib/modules/$UNAME_RELEASE/.config"
CONFIG_IRQ_WORK=y

#
# General setup
#
CONFIG_INIT_ENV_ARG_LIMIT=32
CONFIG_CROSS_COMPILE=""
# CONFIG_COMPILE_TEST is not set
CONFIG_LOCALVERSION=""
# CONFIG_LOCALVERSION_AUTO is not set
CONFIG_DEFAULT_HOSTNAME="(none)"
CONFIG_SWAP=y
CONFIG_SYSVIPC=y
CONFIG_SYSVIPC_SYSCTL=y
CONFIG_POSIX_MQUEUE=y
CONFIG_POSIX_MQUEUE_SYSCTL=y
CONFIG_CROSS_MEMORY_ATTACH=y
# CONFIG_FHANDLE is not set
CONFIG_USELIB=y
# CONFIG_AUDIT is not set
CONFIG_HAVE_ARCH_AUDITSYSCALL=y

#
# IRQ subsystem
#
CONFIG_GENERIC_IRQ_SHOW=y
CONFIG_IRQ_PREFLOW_FASTEOI=y
CONFIG_GENERIC_MSI_IRQ=y
CONFIG_SPARSE_IRQ=y
CONFIG_GENERIC_CLOCKEVENTS=y

#
# Timers subsystem
#
CONFIG_TICK_ONESHOT=y
CONFIG_NO_HZ_COMMON=y
# CONFIG_HZ_PERIODIC is not set
CONFIG_NO_HZ_IDLE=y
# CONFIG_NO_HZ_FULL is not set
CONFIG_NO_HZ=y
CONFIG_HIGH_RES_TIMERS=y

#
# CPU/Task time and stats accounting
#
CONFIG_TICK_CPU_ACCOUNTING=y
# CONFIG_VIRT_CPU_ACCOUNTING_GEN is not set
# CONFIG_BSD_PROCESS_ACCT is not set
# CONFIG_TASKSTATS is not set

#
# RCU Subsystem
#
CONFIG_TREE_RCU=y
# CONFIG_RCU_EXPERT is not set
CONFIG_SRCU=y
# CONFIG_TASKS_RCU is not set
CONFIG_RCU_STALL_COMMON=y
# CONFIG_TREE_RCU_TRACE is not set
# CONFIG_RCU_NOCB_CPU is not set
# CONFIG_RCU_EXPEDITE_BOOT is not set
# CONFIG_BUILD_BIN2C is not set
# CONFIG_IKCONFIG is not set
CONFIG_LOG_BUF_SHIFT=18
CONFIG_LOG_CPU_MAX_BUF_SHIFT=12
# CONFIG_CGROUPS is not set
# CONFIG_CHECKPOINT_RESTORE is not set
CONFIG_NAMESPACES=y
CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
# CONFIG_USER_NS is not set
CONFIG_PID_NS=y
CONFIG_NET_NS=y
# CONFIG_SCHED_AUTOGROUP is not set
# CONFIG_SYSFS_DEPRECATED is not set
CONFIG_RELAY=y
CONFIG_BLK_DEV_INITRD=y
CONFIG_INITRAMFS_SOURCE=""
CONFIG_RD_GZIP=y
CONFIG_RD_BZIP2=y
CONFIG_RD_LZMA=y
CONFIG_RD_XZ=y
CONFIG_RD_LZO=y
CONFIG_RD_LZ4=y
# CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
CONFIG_SYSCTL=y
CONFIG_ANON_INODES=y
CONFIG_HAVE_UID16=y
CONFIG_SYSCTL_EXCEPTION_TRACE=y
CONFIG_BPF=y
# CONFIG_EXPERT is not set
CONFIG_UID16=y
CONFIG_MULTIUSER=y
CONFIG_SGETMASK_SYSCALL=y
CONFIG_SYSFS_SYSCALL=y
# CONFIG_SYSCTL_SYSCALL is not set
CONFIG_KALLSYMS=y
# CONFIG_KALLSYMS_ALL is not set
CONFIG_PRINTK=y
CONFIG_BUG=y
CONFIG_ELF_CORE=y
CONFIG_BASE_FULL=y
CONFIG_FUTEX=y
CONFIG_EPOLL=y
CONFIG_SIGNALFD=y
CONFIG_TIMERFD=y
CONFIG_EVENTFD=y
# CONFIG_BPF_SYSCALL is not set
CONFIG_SHMEM=y
CONFIG_AIO=y
CONFIG_ADVISE_SYSCALLS=y
CONFIG_PCI_QUIRKS=y
# CONFIG_EMBEDDED is not set
CONFIG_HAVE_PERF_EVENTS=y
CONFIG_PERF_USE_VMALLOC=y

#
# Kernel Performance Events And Counters
#
CONFIG_PERF_EVENTS=y
# CONFIG_DEBUG_PERF_USE_VMALLOC is not set