Re: [PATCH] bcm5974-0.58: name changes, open/close and suspend/resume serialized

Previous thread: [PATCH] add might_sleep() to all implementations of gpio_free by Uwe Kleine-König on Thursday, July 24, 2008 - 12:28 am. (14 messages)

Next thread: [PATCH] move kernel-doc comment for might_sleep directly before its defining block by Uwe Kleine-König on Thursday, July 24, 2008 - 12:42 am. (1 message)
From: Henrik Rydberg
Date: Thursday, July 24, 2008 - 12:37 am

BCM5974: This driver adds support for the multitouch trackpad on the new
Apple Macbook Air and Macbook Pro Penryn laptops. It replaces the
appletouch driver on those computers, and integrates well with the
synaptics driver of the Xorg system.

Signed-off-by: Henrik Rydberg <rydberg@euromail.se>
---

This is version bcm5974-0.58. Changes since 0.57 are in short:

  * Name changes; use usbhid definitions, rename atp to bcm5974
  * Input syncing moved to report functions, event type setup broken out
  * Traffic functions added for open/close and suspend/resume logic
  * Open/close and suspend/resume transition logic corrected
  * Open/close serialized with respect to suspend/resume

Thanks to Dmitry Torokhov for reviewing and clearing up the resume functionality,
and Alan Stern for the resume error state handling.

This patch is against linux-next v2.6.26-9281-gb475aa5.

diff --git a/drivers/input/mouse/Kconfig b/drivers/input/mouse/Kconfig
index 7bbea09..956cc4f 100644
--- a/drivers/input/mouse/Kconfig
+++ b/drivers/input/mouse/Kconfig
@@ -130,6 +130,29 @@ config MOUSE_APPLETOUCH
 	  To compile this driver as a module, choose M here: the
 	  module will be called appletouch.

+config MOUSE_BCM5974
+	tristate "Apple USB BCM5974 Multitouch trackpad support"
+	depends on USB_ARCH_HAS_HCD
+	depends on USB
+	help
+	  Say Y here if you have an Apple USB BCM5974 Multitouch
+	  trackpad.
+	
+	  The BCM5974 is the multitouch trackpad found in the Macbook
+	  Air (JAN2008) and Macbook Pro Penryn (FEB2008) laptops.
+	
+	  It is also found in the IPhone (2007) and Ipod Touch (2008).
+	
+	  This driver provides multitouch functionality together with
+	  the synaptics X11 driver.
+	
+	  The interface is currently identical to the appletouch interface,
+	  for further information, see
+	  <file:Documentation/input/appletouch.txt>.
+	
+	  To compile this driver as a module, choose M here: the
+	  module will be called bcm5974.
+
 config MOUSE_INPORT
 	tristate ...
From: Dmitry Torokhov
Date: Friday, July 25, 2008 - 8:42 am

Hi Henrik,


Thank you for the changes.  You don't have to track whether device is
manually suspended or not - usb_submit_urb will fail and that is it.

Attached are 2 patches. First cleans suspend state tracking as it is not
really needed and does some formatting and other minor changes and 2nd
implements runtime pm for the device. Could you please try them and if
everything still works I will apply the driver.

Thanks!

-- 
Dmitry
From: Henrik Rydberg
Date: Friday, July 25, 2008 - 11:25 am

Patch 1:

  Looking good, just a few comments:

  * I notice that clamp_val is new since 2.6.24.3
  * Regarding the open-fails comment: is EACCES correct?

Patch 2:

  Perfect.

Thank you so much, Dmitry - it's a GO!

Henrik

--

From: Dmitry Torokhov
Date: Friday, July 25, 2008 - 11:37 am

We don't return EACCESS anymore, I don't think. We will return what
bcm5974_start_traffic returns which in turn returns result of
usb_submit_urb and the likes.

I don't think returning -EACCESS was proper - it usually indicates

Great, thank you!

-- 
Dmitry
--

From: Henrik Rydberg
Date: Saturday, July 26, 2008 - 12:49 am

Hi Dmitry,

one thing about the drivers/input/mouse/Kconfig patch:

The original version contained "select USB", which was later changed
to "depends on USB" by Andrew Morton. I saw it reappear as "select USB"
in your latest patch, and simply considered it a change back, but
maybe it was an oversight?

Cheers,
Henrik

--

From: Dmitry Torokhov
Date: Monday, July 28, 2008 - 10:41 pm

Hi Henrik,


I am of the opinion that it is OK to "select" high-level subsystems that
don't have additional dependencies, especially if they are "past" the
original driver in menuconfig. I don't really like the idea of forcing
users revisiting earlier sub-menus after they selected a new subsystem
to see if there are any new options. Plus, if user wants driver for his
touchpad he does not really care whether it is PS/2 or USB, [s]he just
wants it to work.

So the change was intentional on my part.

-- 
Dmitry
--

From: Johannes Berg
Date: Monday, July 28, 2008 - 11:46 pm

USB tends to have implicit dependencies like the ehci/ohci/uhci
controller though, so it's not going to work like that anyway starting
from a minimal config, and all defconfigs probably have USB anyway.

Therefore, I personally would keep it as "depends on" rather than
"select". YMMV.

johannes
From: Dmitry Torokhov
Date: Tuesday, July 29, 2008 - 12:31 am

It should work fine I think. The full dependancy of most USB input
devices is:

        depends on USB_ARCH_HAS_HCD
        select USB

You do need to select the host controller, that is correct, but you
don't need to go to USB menu, select HCD, then go back to input menu
(which you already visited) and select your USB input device.

-- 
Dmitry
--

Previous thread: [PATCH] add might_sleep() to all implementations of gpio_free by Uwe Kleine-König on Thursday, July 24, 2008 - 12:28 am. (14 messages)

Next thread: [PATCH] move kernel-doc comment for might_sleep directly before its defining block by Uwe Kleine-König on Thursday, July 24, 2008 - 12:42 am. (1 message)