Linux Input/HID development
 help / color / mirror / Atom feed
* Re: conflict of wiimote and hid-sony
From: Jiri Kosina @ 2013-11-19 10:30 UTC (permalink / raw)
  To: David Herrmann; +Cc: Michel Kraus, open list:HID CORE LAYER
In-Reply-To: <CANq1E4TVhxDk7JvNOMQG2Qddktdp2zzRayeAhQ0Zc=vghyGw7g@mail.gmail.com>

On Sun, 17 Nov 2013, David Herrmann wrote:

> > commit 86b84167 HID: wiimote: add LEGO-wiimote VID
> >
> > introduced a VID/PID conflict with its original owner: hid-wiimote got
> > hid:b0005g*v0000054Cp00000306 added but hid-sony already has this id for
> > the PS3 Remote.
> >
> > I think the original and legitimate owner should remain the default for this
> > VID/PID combination.
> 
> Thanks for the report! It's indeed officially allocated as USB ID to Sony.
> 
> @Jiri, can you revert the following commit (note: it was already
> backported to stable):
> 
> commit 86b84167d4e67372376a57ea9955c5d53dae232f
> Author: David Herrmann <dh.herrmann@gmail.com>
> Date:   Fri Oct 18 16:26:22 2013 +0200
> 
>     HID: wiimote: add LEGO-wiimote VID
> 
> I will write a bluez-patch to force the official wiimote-id on the
> device, based on a name-comparison. This should allow us to support
> both devices.

Thanks a lot for the report. I have queued the revert for 3.13.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCH] usbkbd.c: trivial typo ('released' -> 'pressed')
From: Jiri Kosina @ 2013-11-19 10:32 UTC (permalink / raw)
  To: Adam Cozzette; +Cc: linux-usb, linux-input, linux-kernel
In-Reply-To: <1384725083-3963-1-git-send-email-acozzette@gmail.com>

On Sun, 17 Nov 2013, Adam Cozzette wrote:

> It looks like this typo was introduced by a mistake in a copy-and-paste in
> commit ddbe32491951.
> 
> Signed-off-by: Adam Cozzette <acozzette@gmail.com>
> ---
>  drivers/hid/usbhid/usbkbd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/usbhid/usbkbd.c b/drivers/hid/usbhid/usbkbd.c
> index 7960869..9a332e6 100644
> --- a/drivers/hid/usbhid/usbkbd.c
> +++ b/drivers/hid/usbhid/usbkbd.c
> @@ -146,7 +146,7 @@ static void usb_kbd_irq(struct urb *urb)
>  				input_report_key(kbd->dev, usb_kbd_keycode[kbd->new[i]], 1);
>  			else
>  				hid_info(urb->dev,
> -					 "Unknown key (scancode %#x) released.\n",
> +					 "Unknown key (scancode %#x) pressed.\n",
>  					 kbd->new[i]);
>  		}
>  	}

Applied to hid.git, thanks.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Jiri Kosina @ 2013-11-19 10:35 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: linux-input, simon
In-Reply-To: <2436222.LsGn6ZW5hr@bentobox>

On Tue, 19 Nov 2013, Sven Eckelmann wrote:

> > Now applied for 3.13. Thanks.
> 
> Ok, but would have been nice when the v3 version have been applied. 

I apparently got confused by this thread, sorry.

> Now I have to resubmit the LED patchset to include the function/variable 
> rename. I will do this tonight or tomorrow.

That'd be nice, thanks. Also I think really only the first patch (atomic 
context fix) is really applicable for this (3.13) merge window.

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCHv2] HID: sony: Send FF commands in non-atomic context
From: Sven Eckelmann @ 2013-11-19 10:44 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input, simon
In-Reply-To: <alpine.LNX.2.00.1311191133550.6657@pobox.suse.cz>

[-- Attachment #1: Type: text/plain, Size: 439 bytes --]

On Tuesday 19 November 2013 11:35:27 Jiri Kosina wrote:
> > Now I have to resubmit the LED patchset to include the function/variable
> > rename. I will do this tonight or tomorrow.
> 
> That'd be nice, thanks. Also I think really only the first patch (atomic
> context fix) is really applicable for this (3.13) merge window.

Yes, only the first one was the fix and only the rest is for later because it 
is a feature.

Kind regards,
	Sven

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv3 1/2] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-11-19 12:50 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Grant Likely,
	devicetree, linux-doc, linux-kernel
In-Reply-To: <20131118035815.GA3311@core.coreip.homeip.net>

Hi!

> > > DRIVER              CODE                        DOCUMENTATION
> > > twl4030-keypad      linux,input-no-autorepeat   linux,keypad-no-autorepeat
> > > omap4-keypad        linux,input-no-autorepeat   linux,keypad-no-autorepeat
> > > samsung-keypad      linux,input-no-autorepeat   linux,keypad-no-autorepeat
> > > stmpe-keypad        st,no-autorepeat            st,no-autorepeat
> > > spear-keyboard      autorepeat                  autorepeat
> > > tca8418-keypad      keypad,autorepeat           --- not documented ---
> > > gpio-matrix-keypad  linux,no-autorepeat         linux,no-autorepeat
> > > gpio-keys-polled    autorepeat                  autorepeat
> > > gpio-keys           autorepeat                  --- no documentation ---
> > > 
> > > Any suggestions how to continue fixing this mess? I guess first of
> > > all the documentation of omap4-keypad, samsung-keypad and of course
> > > the new twl4030-keypad driver should be fixed.
> > > 
> > > Next it would be nice to choose one standard property name for this
> > > and use it for twl4030-keypad. I suggest to use "linux,input-no-autorepeat".
> > 
> > I'd suggest just simple "autorepeat", so that we get rid of ugly double-negation.
> The idea was that majority of setups want autorepeat so in the absence
> of the property autorepeat is turned on.

I see... but autorepeat on by default makes sense on devices that are
usually querty keybaords, but does not make sense on devices that are
usually phone keypads or power buttons.

So my proposal is:

1) driver decides if it makes sense to autorepeat by default or not.

2) dts says autorepeat=0 or autorepeat=1

That way, we get dts that get chance to work on other OSes, get rid of
double negations, and get right defaults when autorepeat is not
specified.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [appleir] BUG: unable to handle kernel NULL pointer dereference
From: Jiri Kosina @ 2013-11-19 14:33 UTC (permalink / raw)
  To: Benjamin Tissoires, James Henstridge
  Cc: Luis Henriques, linux-kernel, linux-input, Fabien André,
	Bastien Nocera
In-Reply-To: <527BB695.9040209@redhat.com>

On Thu, 7 Nov 2013, Benjamin Tissoires wrote:

> >> [ adding some more CCs ]
> >>
> >> Okay, so apparently we didn't register with input, but only hiddev /
> >> hidraw.
> >>
> >> appleir 0003:05AC:8240.0005: hiddev0,hidraw4: USB HID v1.11 Device [Apple Computer, Inc. IR Receiver] on usb-0000:00:1d.3-2/input0
> >>
> >> Therefore ->input_configured() callback has never been called, and thus we
> >> oops due to appleir->input_dev being NULL when the first raw event is
> >> reported.
> >>
> >> Could you please provide report descriptor of the device?
> >>
> >> The driver apparently relies on it being registered with hid-input, but
> >> for some reason that doesn't happen.
> > 
> > Here is the relevant lsusb output that I think contains what you're
> > asking for (I had to unbind usbhid for it to include the descriptor):
> > 
> > Bus 005 Device 003: ID 05ac:8240 Apple, Inc. Built-in IR Receiver
> > Device Descriptor:
> >   bLength                18
> >   bDescriptorType         1
> >   bcdUSB               2.00
> > ...
> 
> Ok, thanks for the report. Could you please test the following patch
> which should solve your problem (hopefully)?

James,

any reults from testing Benjamin's patch, please?

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina

Hi,

this is a resent of the patchset "[PATCHv3 0/5] HID: sony: Dualshock3 USB FF
deadlock fix and LED support". It is necessary because the patches 2-5 of that
patchset cannot be applied anymore. This happened because the v2 of the patch
"HID: sony: Send FF commands in non-atomic context" instead of the v3 was
applied in the repository jikos/hid.git. The differences between these two
patch versions can now be found in the new patch "[PATCHv4 1/5] HID: sony:
Rename rumble_* functions/variables to state_*"

Kind regards,
	Sven


^ permalink raw reply

* [PATCHv4 1/5] HID: sony: Rename rumble_* functions/variables to state_*
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Sven Eckelmann
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

The commands used to modify the rumble motor state also modifies the LEDs at
the same time. The functionality used to modify this state in the driver has to
be shared between the rumble and LED part. It is therefore better to replace
the "rumble" part of the names with "state".

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 098af2f8..28b847a 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -226,7 +226,7 @@ struct sony_sc {
 	unsigned long quirks;
 
 #ifdef CONFIG_SONY_FF
-	struct work_struct rumble_worker;
+	struct work_struct state_worker;
 	struct hid_device *hdev;
 	__u8 left;
 	__u8 right;
@@ -622,9 +622,9 @@ static void buzz_remove(struct hid_device *hdev)
 }
 
 #ifdef CONFIG_SONY_FF
-static void sony_rumble_worker(struct work_struct *work)
+static void sony_state_worker(struct work_struct *work)
 {
-	struct sony_sc *sc = container_of(work, struct sony_sc, rumble_worker);
+	struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
 	unsigned char buf[] = {
 		0x01,
 		0x00, 0xff, 0x00, 0xff, 0x00,
@@ -655,7 +655,7 @@ static int sony_play_effect(struct input_dev *dev, void *data,
 	sc->left = effect->u.rumble.strong_magnitude / 256;
 	sc->right = effect->u.rumble.weak_magnitude ? 1 : 0;
 
-	schedule_work(&sc->rumble_worker);
+	schedule_work(&sc->state_worker);
 	return 0;
 }
 
@@ -667,7 +667,7 @@ static int sony_init_ff(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	sc->hdev = hdev;
-	INIT_WORK(&sc->rumble_worker, sony_rumble_worker);
+	INIT_WORK(&sc->state_worker, sony_state_worker);
 
 	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -677,7 +677,7 @@ static void sony_destroy_ff(struct hid_device *hdev)
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	cancel_work_sync(&sc->rumble_worker);
+	cancel_work_sync(&sc->state_worker);
 }
 
 #else
-- 
1.8.4.3


^ permalink raw reply related

* [PATCHv4 2/5] HID: sony: Use BIT(x) macro to define quirks constants
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Sven Eckelmann
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 28b847a..0677292 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -33,11 +33,11 @@
 
 #include "hid-ids.h"
 
-#define VAIO_RDESC_CONSTANT     (1 << 0)
-#define SIXAXIS_CONTROLLER_USB  (1 << 1)
-#define SIXAXIS_CONTROLLER_BT   (1 << 2)
-#define BUZZ_CONTROLLER         (1 << 3)
-#define PS3REMOTE		(1 << 4)
+#define VAIO_RDESC_CONSTANT     BIT(0)
+#define SIXAXIS_CONTROLLER_USB  BIT(1)
+#define SIXAXIS_CONTROLLER_BT   BIT(2)
+#define BUZZ_CONTROLLER         BIT(3)
+#define PS3REMOTE		BIT(4)
 
 static const u8 sixaxis_rdesc_fixup[] = {
 	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
-- 
1.8.4.3


^ permalink raw reply related

* [PATCHv4 3/5] HID: sony: Rename buzz_* functions to sony_led_*
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Sven Eckelmann
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

More controllers managed by the hid-sony module have 4 LEDs. These can share
most of the functionality provided by the buzz functions.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 0677292..cdb2419 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -466,7 +466,7 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
-static void buzz_led_set_brightness(struct led_classdev *led,
+static void sony_led_set_brightness(struct led_classdev *led,
 				    enum led_brightness value)
 {
 	struct device *dev = led->dev->parent;
@@ -498,7 +498,7 @@ static void buzz_led_set_brightness(struct led_classdev *led,
 	}
 }
 
-static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
+static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 {
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
@@ -525,7 +525,7 @@ static enum led_brightness buzz_led_get_brightness(struct led_classdev *led)
 	return on ? LED_FULL : LED_OFF;
 }
 
-static int buzz_init(struct hid_device *hdev)
+static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
 	struct buzz_extra *buzz;
@@ -567,8 +567,8 @@ static int buzz_init(struct hid_device *hdev)
 		led->name = name;
 		led->brightness = 0;
 		led->max_brightness = 1;
-		led->brightness_get = buzz_led_get_brightness;
-		led->brightness_set = buzz_led_set_brightness;
+		led->brightness_get = sony_led_get_brightness;
+		led->brightness_set = sony_led_set_brightness;
 
 		if (led_classdev_register(&hdev->dev, led)) {
 			hid_err(hdev, "Failed to register LED %d\n", n);
@@ -596,7 +596,7 @@ error_leds:
 	return ret;
 }
 
-static void buzz_remove(struct hid_device *hdev)
+static void sony_leds_remove(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
 	struct buzz_extra *buzz;
@@ -733,7 +733,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
 	else if (sc->quirks & BUZZ_CONTROLLER)
-		ret = buzz_init(hdev);
+		ret = sony_leds_init(hdev);
 	else
 		ret = 0;
 
@@ -755,7 +755,7 @@ static void sony_remove(struct hid_device *hdev)
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
 	if (sc->quirks & BUZZ_CONTROLLER)
-		buzz_remove(hdev);
+		sony_leds_remove(hdev);
 
 	sony_destroy_ff(hdev);
 
-- 
1.8.4.3


^ permalink raw reply related

* [PATCHv4 4/5] HID: sony: Move LED data to the main structure
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Sven Eckelmann
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

It is not necessary to keep the LED information in an extra struct which is
only used by the Buzz device. It can also be used by other devices.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
 drivers/hid/hid-sony.c | 58 +++++++++++++++-----------------------------------
 1 file changed, 17 insertions(+), 41 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index cdb2419..2f93aab 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -223,6 +223,7 @@ static const unsigned int buzz_keymap[] = {
 };
 
 struct sony_sc {
+	struct led_classdev *leds[4];
 	unsigned long quirks;
 
 #ifdef CONFIG_SONY_FF
@@ -232,12 +233,7 @@ struct sony_sc {
 	__u8 right;
 #endif
 
-	void *extra;
-};
-
-struct buzz_extra {
-	int led_state;
-	struct led_classdev *leds[4];
+	__u8 led_state;
 };
 
 static __u8 *ps3remote_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -472,26 +468,24 @@ static void sony_led_set_brightness(struct led_classdev *led,
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 
 	int n;
 
 	drv_data = hid_get_drvdata(hdev);
-	if (!drv_data || !drv_data->extra) {
+	if (!drv_data) {
 		hid_err(hdev, "No device data\n");
 		return;
 	}
-	buzz = drv_data->extra;
 
 	for (n = 0; n < 4; n++) {
-		if (led == buzz->leds[n]) {
-			int on = !! (buzz->led_state & (1 << n));
+		if (led == drv_data->leds[n]) {
+			int on = !!(drv_data->led_state & (1 << n));
 			if (value == LED_OFF && on) {
-				buzz->led_state &= ~(1 << n);
-				buzz_set_leds(hdev, buzz->led_state);
+				drv_data->led_state &= ~(1 << n);
+				buzz_set_leds(hdev, drv_data->led_state);
 			} else if (value != LED_OFF && !on) {
-				buzz->led_state |= (1 << n);
-				buzz_set_leds(hdev, buzz->led_state);
+				drv_data->led_state |= (1 << n);
+				buzz_set_leds(hdev, drv_data->led_state);
 			}
 			break;
 		}
@@ -503,21 +497,19 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	struct device *dev = led->dev->parent;
 	struct hid_device *hdev = container_of(dev, struct hid_device, dev);
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 
 	int n;
 	int on = 0;
 
 	drv_data = hid_get_drvdata(hdev);
-	if (!drv_data || !drv_data->extra) {
+	if (!drv_data) {
 		hid_err(hdev, "No device data\n");
 		return LED_OFF;
 	}
-	buzz = drv_data->extra;
 
 	for (n = 0; n < 4; n++) {
-		if (led == buzz->leds[n]) {
-			on = !! (buzz->led_state & (1 << n));
+		if (led == drv_data->leds[n]) {
+			on = !!(drv_data->led_state & (1 << n));
 			break;
 		}
 	}
@@ -528,7 +520,6 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 	int n, ret = 0;
 	struct led_classdev *led;
 	size_t name_sz;
@@ -541,13 +532,6 @@ static int sony_leds_init(struct hid_device *hdev)
 	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
 		return -ENODEV;
 
-	buzz = kzalloc(sizeof(*buzz), GFP_KERNEL);
-	if (!buzz) {
-		hid_err(hdev, "Insufficient memory, cannot allocate driver data\n");
-		return -ENOMEM;
-	}
-	drv_data->extra = buzz;
-
 	/* Clear LEDs as we have no way of reading their initial state. This is
 	 * only relevant if the driver is loaded after somebody actively set the
 	 * LEDs to on */
@@ -576,49 +560,41 @@ static int sony_leds_init(struct hid_device *hdev)
 			goto error_leds;
 		}
 
-		buzz->leds[n] = led;
+		drv_data->leds[n] = led;
 	}
 
 	return ret;
 
 error_leds:
 	for (n = 0; n < 4; n++) {
-		led = buzz->leds[n];
-		buzz->leds[n] = NULL;
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
 		if (!led)
 			continue;
 		led_classdev_unregister(led);
 		kfree(led);
 	}
 
-	kfree(drv_data->extra);
-	drv_data->extra = NULL;
 	return ret;
 }
 
 static void sony_leds_remove(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
-	struct buzz_extra *buzz;
 	struct led_classdev *led;
 	int n;
 
 	drv_data = hid_get_drvdata(hdev);
 	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
 
-	buzz = drv_data->extra;
-
 	for (n = 0; n < 4; n++) {
-		led = buzz->leds[n];
-		buzz->leds[n] = NULL;
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
 		if (!led)
 			continue;
 		led_classdev_unregister(led);
 		kfree(led);
 	}
-
-	kfree(drv_data->extra);
-	drv_data->extra = NULL;
 }
 
 #ifdef CONFIG_SONY_FF
-- 
1.8.4.3


^ permalink raw reply related

* [PATCHv4 5/5] HID: sony: Add LED support for Sixaxis/Dualshock3 USB
From: Sven Eckelmann @ 2013-11-19 19:26 UTC (permalink / raw)
  To: linux-input; +Cc: Jiri Kosina, Sven Eckelmann
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

The PS3 Sixaxis controller has 4 LEDs which can be controlled using the same
command as the rumble functionality. It seems not to be possible to only change
the LED without modifying the rumble motor state. Thus both have to be stored
on the host and retransmitted when either the LED or rumble state is changed.

Third party controllers may not support to disable all LEDs at once. These
controllers automatically switch to blinking of all LEDs when no LED is active
anymore.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
Tested-by: Simon Wood <simon@mungewell.org>
---
 drivers/hid/hid-sony.c | 118 +++++++++++++++++++++++++++++--------------------
 1 file changed, 71 insertions(+), 47 deletions(-)

diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index 2f93aab..b60bc38 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -39,6 +39,8 @@
 #define BUZZ_CONTROLLER         BIT(3)
 #define PS3REMOTE		BIT(4)
 
+#define SONY_LED_SUPPORT (SIXAXIS_CONTROLLER_USB | BUZZ_CONTROLLER)
+
 static const u8 sixaxis_rdesc_fixup[] = {
 	0x95, 0x13, 0x09, 0x01, 0x81, 0x02, 0x95, 0x0C,
 	0x81, 0x01, 0x75, 0x10, 0x95, 0x04, 0x26, 0xFF,
@@ -223,12 +225,12 @@ static const unsigned int buzz_keymap[] = {
 };
 
 struct sony_sc {
+	struct hid_device *hdev;
 	struct led_classdev *leds[4];
 	unsigned long quirks;
+	struct work_struct state_worker;
 
 #ifdef CONFIG_SONY_FF
-	struct work_struct state_worker;
-	struct hid_device *hdev;
 	__u8 left;
 	__u8 right;
 #endif
@@ -462,6 +464,18 @@ static void buzz_set_leds(struct hid_device *hdev, int leds)
 	hid_hw_request(hdev, report, HID_REQ_SET_REPORT);
 }
 
+static void sony_set_leds(struct hid_device *hdev, __u8 leds)
+{
+	struct sony_sc *drv_data = hid_get_drvdata(hdev);
+
+	if (drv_data->quirks & BUZZ_CONTROLLER) {
+		buzz_set_leds(hdev, leds);
+	} else if (drv_data->quirks & SIXAXIS_CONTROLLER_USB) {
+		drv_data->led_state = leds;
+		schedule_work(&drv_data->state_worker);
+	}
+}
+
 static void sony_led_set_brightness(struct led_classdev *led,
 				    enum led_brightness value)
 {
@@ -482,10 +496,10 @@ static void sony_led_set_brightness(struct led_classdev *led,
 			int on = !!(drv_data->led_state & (1 << n));
 			if (value == LED_OFF && on) {
 				drv_data->led_state &= ~(1 << n);
-				buzz_set_leds(hdev, drv_data->led_state);
+				sony_set_leds(hdev, drv_data->led_state);
 			} else if (value != LED_OFF && !on) {
 				drv_data->led_state |= (1 << n);
-				buzz_set_leds(hdev, drv_data->led_state);
+				sony_set_leds(hdev, drv_data->led_state);
 			}
 			break;
 		}
@@ -517,6 +531,25 @@ static enum led_brightness sony_led_get_brightness(struct led_classdev *led)
 	return on ? LED_FULL : LED_OFF;
 }
 
+static void sony_leds_remove(struct hid_device *hdev)
+{
+	struct sony_sc *drv_data;
+	struct led_classdev *led;
+	int n;
+
+	drv_data = hid_get_drvdata(hdev);
+	BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
+
+	for (n = 0; n < 4; n++) {
+		led = drv_data->leds[n];
+		drv_data->leds[n] = NULL;
+		if (!led)
+			continue;
+		led_classdev_unregister(led);
+		kfree(led);
+	}
+}
+
 static int sony_leds_init(struct hid_device *hdev)
 {
 	struct sony_sc *drv_data;
@@ -524,20 +557,29 @@ static int sony_leds_init(struct hid_device *hdev)
 	struct led_classdev *led;
 	size_t name_sz;
 	char *name;
+	size_t name_len;
+	const char *name_fmt;
 
 	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
+	BUG_ON(!(drv_data->quirks & SONY_LED_SUPPORT));
 
-	/* Validate expected report characteristics. */
-	if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
-		return -ENODEV;
+	if (drv_data->quirks & BUZZ_CONTROLLER) {
+		name_len = strlen("::buzz#");
+		name_fmt = "%s::buzz%d";
+		/* Validate expected report characteristics. */
+		if (!hid_validate_values(hdev, HID_OUTPUT_REPORT, 0, 0, 7))
+			return -ENODEV;
+	} else {
+		name_len = strlen("::sony#");
+		name_fmt = "%s::sony%d";
+	}
 
 	/* Clear LEDs as we have no way of reading their initial state. This is
 	 * only relevant if the driver is loaded after somebody actively set the
 	 * LEDs to on */
-	buzz_set_leds(hdev, 0x00);
+	sony_set_leds(hdev, 0x00);
 
-	name_sz = strlen(dev_name(&hdev->dev)) + strlen("::buzz#") + 1;
+	name_sz = strlen(dev_name(&hdev->dev)) + name_len + 1;
 
 	for (n = 0; n < 4; n++) {
 		led = kzalloc(sizeof(struct led_classdev) + name_sz, GFP_KERNEL);
@@ -547,7 +589,7 @@ static int sony_leds_init(struct hid_device *hdev)
 		}
 
 		name = (void *)(&led[1]);
-		snprintf(name, name_sz, "%s::buzz%d", dev_name(&hdev->dev), n + 1);
+		snprintf(name, name_sz, name_fmt, dev_name(&hdev->dev), n + 1);
 		led->name = name;
 		led->brightness = 0;
 		led->max_brightness = 1;
@@ -566,45 +608,18 @@ static int sony_leds_init(struct hid_device *hdev)
 	return ret;
 
 error_leds:
-	for (n = 0; n < 4; n++) {
-		led = drv_data->leds[n];
-		drv_data->leds[n] = NULL;
-		if (!led)
-			continue;
-		led_classdev_unregister(led);
-		kfree(led);
-	}
+	sony_leds_remove(hdev);
 
 	return ret;
 }
 
-static void sony_leds_remove(struct hid_device *hdev)
-{
-	struct sony_sc *drv_data;
-	struct led_classdev *led;
-	int n;
-
-	drv_data = hid_get_drvdata(hdev);
-	BUG_ON(!(drv_data->quirks & BUZZ_CONTROLLER));
-
-	for (n = 0; n < 4; n++) {
-		led = drv_data->leds[n];
-		drv_data->leds[n] = NULL;
-		if (!led)
-			continue;
-		led_classdev_unregister(led);
-		kfree(led);
-	}
-}
-
-#ifdef CONFIG_SONY_FF
 static void sony_state_worker(struct work_struct *work)
 {
 	struct sony_sc *sc = container_of(work, struct sony_sc, state_worker);
 	unsigned char buf[] = {
 		0x01,
 		0x00, 0xff, 0x00, 0xff, 0x00,
-		0x00, 0x00, 0x00, 0x00, 0x03,
+		0x00, 0x00, 0x00, 0x00, 0x00,
 		0xff, 0x27, 0x10, 0x00, 0x32,
 		0xff, 0x27, 0x10, 0x00, 0x32,
 		0xff, 0x27, 0x10, 0x00, 0x32,
@@ -612,13 +627,18 @@ static void sony_state_worker(struct work_struct *work)
 		0x00, 0x00, 0x00, 0x00, 0x00
 	};
 
+#ifdef CONFIG_SONY_FF
 	buf[3] = sc->right;
 	buf[5] = sc->left;
+#endif
+
+	buf[10] |= (sc->led_state & 0xf) << 1;
 
 	sc->hdev->hid_output_raw_report(sc->hdev, buf, sizeof(buf),
 					HID_OUTPUT_REPORT);
 }
 
+#ifdef CONFIG_SONY_FF
 static int sony_play_effect(struct input_dev *dev, void *data,
 			    struct ff_effect *effect)
 {
@@ -640,10 +660,6 @@ static int sony_init_ff(struct hid_device *hdev)
 	struct hid_input *hidinput = list_entry(hdev->inputs.next,
 						struct hid_input, list);
 	struct input_dev *input_dev = hidinput->input;
-	struct sony_sc *sc = hid_get_drvdata(hdev);
-
-	sc->hdev = hdev;
-	INIT_WORK(&sc->state_worker, sony_state_worker);
 
 	input_set_capability(input_dev, EV_FF, FF_RUMBLE);
 	return input_ff_create_memless(input_dev, NULL, sony_play_effect);
@@ -682,6 +698,7 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	sc->quirks = quirks;
 	hid_set_drvdata(hdev, sc);
+	sc->hdev = hdev;
 
 	ret = hid_parse(hdev);
 	if (ret) {
@@ -705,23 +722,30 @@ static int sony_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	if (sc->quirks & SIXAXIS_CONTROLLER_USB) {
 		hdev->hid_output_raw_report = sixaxis_usb_output_raw_report;
 		ret = sixaxis_set_operational_usb(hdev);
+		INIT_WORK(&sc->state_worker, sony_state_worker);
 	}
 	else if (sc->quirks & SIXAXIS_CONTROLLER_BT)
 		ret = sixaxis_set_operational_bt(hdev);
-	else if (sc->quirks & BUZZ_CONTROLLER)
-		ret = sony_leds_init(hdev);
 	else
 		ret = 0;
 
 	if (ret < 0)
 		goto err_stop;
 
+	if (sc->quirks & SONY_LED_SUPPORT) {
+		ret = sony_leds_init(hdev);
+		if (ret < 0)
+			goto err_stop;
+	}
+
 	ret = sony_init_ff(hdev);
 	if (ret < 0)
 		goto err_stop;
 
 	return 0;
 err_stop:
+	if (sc->quirks & SONY_LED_SUPPORT)
+		sony_leds_remove(hdev);
 	hid_hw_stop(hdev);
 	return ret;
 }
@@ -730,7 +754,7 @@ static void sony_remove(struct hid_device *hdev)
 {
 	struct sony_sc *sc = hid_get_drvdata(hdev);
 
-	if (sc->quirks & BUZZ_CONTROLLER)
+	if (sc->quirks & SONY_LED_SUPPORT)
 		sony_leds_remove(hdev);
 
 	sony_destroy_ff(hdev);
-- 
1.8.4.3


^ permalink raw reply related

* Re: input question: ambient light sensor button
From: Dmitry Torokhov @ 2013-11-19 21:27 UTC (permalink / raw)
  To: Pali Rohár; +Cc: Jiri Kosina, linux-kernel, linux-input
In-Reply-To: <201311141302.33384@pali>

On Thu, Nov 14, 2013 at 01:02:31PM +0100, Pali Rohár wrote:
> On Wednesday 13 November 2013 17:28:40 Dmitry Torokhov wrote:
> > On Wed, Nov 13, 2013 at 11:47:18AM +0100, Jiri Kosina wrote:
> > > On Sun, 15 Sep 2013, Pali Rohár wrote:
> > > > I do not know where to ask this question, but I think that
> > > > kernel developers could help me.
> > > > 
> > > > I have notebook with one special button on keyboard which
> > > > is designed for turning ambient light sensor on and off.
> > > > By default pressing button do nothing (I can turn ambient
> > > > light sensor on/off via sysfs platform wmi module).
> > > > Button press is reported by kernel input device AT
> > > > Translated Set 2 keyboard and reports it as button
> > > > "touchpad off".
> > > > 
> > > > Of course "touchpad off" is incorrect and I'd like to ask
> > > > which kernel key or button from
> > > > /usr/include/linux/input.h should be mapped for my
> > > > ambient light sensor button? Is there already some? And
> > > > what is strategy for allocating KEY_* and BTN_* numbers?
> > > 
> > > This is maintained by Dmitry, so he's the one to answer
> > > this. I am adding him to CC.
> > 
> > I guess we need patch adding
> > 
> > 	#define KEY_ALS_TOGGLE	0x230
> > 
> > Thanks.
> 
> So there is no good key for als yet?

No, but if you send me a patch adding it I will gladly apply it.

Thanks.

-- 
Dmitry

^ permalink raw reply

* Re: [patch] Input: hp_sdc_rtc - unlock on error in hp_sdc_rtc_read_i8042timer()
From: Dmitry Torokhov @ 2013-11-19 21:28 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Helge Deller, David Howells, linux-input, kernel-janitors
In-Reply-To: <20131114081808.GA8150@elgon.mountain>

On Thu, Nov 14, 2013 at 11:18:08AM +0300, Dan Carpenter wrote:
> The transaction task here is hp_sdc_tasklet() and it releases the lock.
> The problem is if we aren't able to queue the transaction then we need
> to release the lock ourselves.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied, thank you.

> 
> diff --git a/drivers/input/misc/hp_sdc_rtc.c b/drivers/input/misc/hp_sdc_rtc.c
> index 86b8228..45e0e3e 100644
> --- a/drivers/input/misc/hp_sdc_rtc.c
> +++ b/drivers/input/misc/hp_sdc_rtc.c
> @@ -180,7 +180,10 @@ static int64_t hp_sdc_rtc_read_i8042timer (uint8_t loadcmd, int numreg)
>  	if (WARN_ON(down_interruptible(&i8042tregs)))
>  		return -1;
>  
> -	if (hp_sdc_enqueue_transaction(&t)) return -1;
> +	if (hp_sdc_enqueue_transaction(&t)) {
> +		up(&i8042tregs);
> +		return -1;
> +	}
>  	
>  	/* Sleep until results come back. */
>  	if (WARN_ON(down_interruptible(&i8042tregs)))

-- 
Dmitry

^ permalink raw reply

* Re: [PATCHv4 0/5] HID: sony: Dualshock3 USB and LED support
From: Jiri Kosina @ 2013-11-20  9:02 UTC (permalink / raw)
  To: Sven Eckelmann; +Cc: linux-input
In-Reply-To: <1384889192-18488-1-git-send-email-sven@narfation.org>

On Tue, 19 Nov 2013, Sven Eckelmann wrote:

> this is a resent of the patchset "[PATCHv3 0/5] HID: sony: Dualshock3 USB FF
> deadlock fix and LED support". It is necessary because the patches 2-5 of that
> patchset cannot be applied anymore. This happened because the v2 of the patch
> "HID: sony: Send FF commands in non-atomic context" instead of the v3 was
> applied in the repository jikos/hid.git. The differences between these two
> patch versions can now be found in the new patch "[PATCHv4 1/5] HID: sony:
> Rename rumble_* functions/variables to state_*"

Queued for 3.14. Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply

* Re: [PATCHv9][ 1/3] Input: tsc2007: Add device tree support.
From: Dmitry Torokhov @ 2013-11-19 21:11 UTC (permalink / raw)
  To: Denis Carikli
  Cc: linux-input, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
	linux-fbdev, Shawn Guo, Sascha Hauer, devicetree, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Grant Likely, linux-arm-kernel, Lothar Waßmann,
	Eric Bénard
In-Reply-To: <1383916659-9988-1-git-send-email-denis@eukrea.com>

Hi Denis,

On Fri, Nov 08, 2013 at 02:17:37PM +0100, Denis Carikli wrote:
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: Grant Likely <grant.likely@linaro.org>
> Cc: devicetree@vger.kernel.org
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Lothar Waßmann <LW@KARO-electronics.de>
> Cc: Shawn Guo <shawn.guo@linaro.org>
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> ---
> ChangeLog v8->v9:
> - Added Grant Likely in the Cc list.
> - Removed the mention of the pinctrl properties in the documentation.
> 
> ChangeLog v7->v8:
> - Fixed the lack of x and z fuzz properties.
> - The pendown gpio is better documented.
> - Added Shawn Guo in the cc list.

Does the device still work if you drop the patch below on top of yours?

Thanks!

-- 
Dmitry

Input: tsc2007 misc fixes

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 arch/arm/mach-imx/mach-cpuimx35.c    |    2 -
 arch/arm/mach-imx/mach-cpuimx51sd.c  |    2 -
 arch/sh/boards/mach-ecovec24/setup.c |    2 -
 drivers/input/touchscreen/tsc2007.c  |  126 +++++++++++++---------------------
 include/linux/i2c/tsc2007.h          |    6 +-
 5 files changed, 55 insertions(+), 83 deletions(-)

diff --git a/arch/arm/mach-imx/mach-cpuimx35.c b/arch/arm/mach-imx/mach-cpuimx35.c
index 771362d..65e4c53 100644
--- a/arch/arm/mach-imx/mach-cpuimx35.c
+++ b/arch/arm/mach-imx/mach-cpuimx35.c
@@ -53,7 +53,7 @@ static const struct imxi2c_platform_data
 };
 
 #define TSC2007_IRQGPIO		IMX_GPIO_NR(3, 2)
-static int tsc2007_get_pendown_state(void)
+static int tsc2007_get_pendown_state(struct device *dev)
 {
 	return !gpio_get_value(TSC2007_IRQGPIO);
 }
diff --git a/arch/arm/mach-imx/mach-cpuimx51sd.c b/arch/arm/mach-imx/mach-cpuimx51sd.c
index 9b5ddf5..1fba2b8 100644
--- a/arch/arm/mach-imx/mach-cpuimx51sd.c
+++ b/arch/arm/mach-imx/mach-cpuimx51sd.c
@@ -121,7 +121,7 @@ static const struct imxuart_platform_data uart_pdata __initconst = {
 	.flags = IMXUART_HAVE_RTSCTS,
 };
 
-static int tsc2007_get_pendown_state(void)
+static int tsc2007_get_pendown_state(struct device *dev)
 {
 	if (mx51_revision() < IMX_CHIP_REVISION_3_0)
 		return !gpio_get_value(TSC2007_IRQGPIO_REV2);
diff --git a/arch/sh/boards/mach-ecovec24/setup.c b/arch/sh/boards/mach-ecovec24/setup.c
index 1fa8be4..23d7e45 100644
--- a/arch/sh/boards/mach-ecovec24/setup.c
+++ b/arch/sh/boards/mach-ecovec24/setup.c
@@ -501,7 +501,7 @@ static struct platform_device keysc_device = {
 /* TouchScreen */
 #define IRQ0 evt2irq(0x600)
 
-static int ts_get_pendown_state(void)
+static int ts_get_pendown_state(struct device *dev)
 {
 	int val = 0;
 	gpio_free(GPIO_FN_INTC_IRQ0);
diff --git a/drivers/input/touchscreen/tsc2007.c b/drivers/input/touchscreen/tsc2007.c
index 3168a99..390148c 100644
--- a/drivers/input/touchscreen/tsc2007.c
+++ b/drivers/input/touchscreen/tsc2007.c
@@ -88,15 +88,10 @@ struct tsc2007 {
 	wait_queue_head_t	wait;
 	bool			stopped;
 
-	int			(*get_pendown_state)(void);
+	int			(*get_pendown_state)(struct device *);
 	void			(*clear_penirq)(void);
 };
 
-static int tsc2007_get_pendown_state_dt(struct tsc2007 *ts)
-{
-	return !gpio_get_value(ts->gpio);
-}
-
 static inline int tsc2007_xfer(struct tsc2007 *tsc, u8 cmd)
 {
 	s32 data;
@@ -155,14 +150,6 @@ static u32 tsc2007_calculate_pressure(struct tsc2007 *tsc, struct ts_event *tc)
 	return rt;
 }
 
-static bool tsc2007_is_pen_down_valid(struct tsc2007 *ts)
-{
-	if (ts->of)
-		return gpio_is_valid(ts->gpio);
-	else
-		return ts->get_pendown_state ? true : false;
-}
-
 static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 {
 	/*
@@ -179,13 +166,10 @@ static bool tsc2007_is_pen_down(struct tsc2007 *ts)
 	 * to fall back on the pressure reading.
 	 */
 
-	if (!tsc2007_is_pen_down_valid(ts))
+	if (!ts->get_pendown_state)
 		return true;
 
-	if (ts->of)
-		return tsc2007_get_pendown_state_dt(ts);
-	else
-		return ts->get_pendown_state();
+	return ts->get_pendown_state(&ts->client->dev);
 }
 
 static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
@@ -202,7 +186,7 @@ static irqreturn_t tsc2007_soft_irq(int irq, void *handle)
 
 		rt = tsc2007_calculate_pressure(ts, &tc);
 
-		if (!rt && !tsc2007_is_pen_down_valid(ts)) {
+		if (!rt && !ts->get_pendown_state) {
 			/*
 			 * If pressure reported is 0 and we don't have
 			 * callback to check pendown state, we have to
@@ -298,13 +282,25 @@ static void tsc2007_close(struct input_dev *input_dev)
 }
 
 #ifdef CONFIG_OF
-static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
-			    struct device_node *np)
+static int tsc2007_get_pendown_state_gpio(struct device *dev)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	struct tsc2007 *ts = i2c_get_clientdata(client);
+
+	return !gpio_get_value(ts->gpio);
+}
+
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
 {
-	int err = 0;
+	struct device_node *np = client->dev.of_node;
 	u32 val32;
 	u64 val64;
 
+	if (!np) {
+		dev_err(&client->dev, "missing device tree data\n");
+		return -EINVAL;
+	}
+
 	if (!of_property_read_u32(np, "ti,max-rt", &val32))
 		ts->max_rt = val32;
 	else
@@ -327,42 +323,32 @@ static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
 	if (!of_property_read_u32(np, "ti,x-plate-ohms", &val32)) {
 		ts->x_plate_ohms = val32;
 	} else {
-		dev_err(&client->dev,
-			"Error: lacking ti,x-plate-ohms devicetree property. (err %d).",
-			err);
+		dev_err(&client->dev, "missing ti,x-plate-ohms devicetree property.");
 		return -EINVAL;
 	}
 
 	ts->gpio = of_get_gpio(np, 0);
-	if (!gpio_is_valid(ts->gpio))
-		dev_err(&client->dev,
-			"GPIO not found (of_get_gpio returned %d)\n",
-			ts->gpio);
-
-	/* Used to detect if it is probed trough the device tree,
-	 * in order to be able to use that information in the IRQ handler.
-	 */
-	ts->of = 1;
+	if (gpio_is_valid(ts->gpio))
+		ts->get_pendown_state = tsc2007_get_pendown_state_gpio;
+	else
+		dev_warn(&client->dev,
+			 "GPIO not specified in DT (of_get_gpio returned %d)\n",
+			 ts->gpio);
 
 	return 0;
 }
 #else
-static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts,
-			    struct device_node *np)
+static int tsc2007_probe_dt(struct i2c_client *client, struct tsc2007 *ts)
 {
-	return -ENODEV;
+	dev_err(&client->dev, "platform data is required!\n");
+	return -EINVAL;
 }
 #endif
 
 static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
-			      struct tsc2007_platform_data *pdata,
+			      const struct tsc2007_platform_data *pdata,
 			      const struct i2c_device_id *id)
 {
-	if (!pdata) {
-		dev_err(&client->dev, "platform data is required!\n");
-		return -EINVAL;
-	}
-
 	ts->model             = pdata->model;
 	ts->x_plate_ohms      = pdata->x_plate_ohms;
 	ts->max_rt            = pdata->max_rt ? : MAX_12BIT;
@@ -379,45 +365,40 @@ static int tsc2007_probe_pdev(struct i2c_client *client, struct tsc2007 *ts,
 		return -EINVAL;
 	}
 
-	/* Used to detect if it is probed trough the device tree,
-	 * in order to be able to use that information in the IRQ handler.
-	 */
-	ts->of = 0;
-
 	return 0;
 }
 
 static int tsc2007_probe(struct i2c_client *client,
 			 const struct i2c_device_id *id)
 {
-	struct device_node *np = client->dev.of_node;
-	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tsc2007 *ts;
 	struct input_dev *input_dev;
-	int err = 0;
+	int err;
+
+	if (!i2c_check_functionality(client->adapter,
+				     I2C_FUNC_SMBUS_READ_WORD_DATA))
+		return -EIO;
 
 	ts = devm_kzalloc(&client->dev, sizeof(struct tsc2007), GFP_KERNEL);
 	if (!ts)
 		return -ENOMEM;
 
-	if (np)
-		err = tsc2007_probe_dt(client, ts, np);
-	else
+	if (pdata)
 		err = tsc2007_probe_pdev(client, ts, pdata, id);
-
+	else
+		err = tsc2007_probe_dt(client, ts);
 	if (err)
 		return err;
 
-	if (!i2c_check_functionality(client->adapter,
-				     I2C_FUNC_SMBUS_READ_WORD_DATA))
-		return -EIO;
-
 	input_dev = input_allocate_device();
 	if (!input_dev) {
 		err = -ENOMEM;
 		goto err_free_input;
 	};
 
+	i2c_set_clientdata(client, ts);
+
 	ts->client = client;
 	ts->irq = client->irq;
 	ts->input = input_dev;
@@ -443,10 +424,8 @@ static int tsc2007_probe(struct i2c_client *client,
 	input_set_abs_params(input_dev, ABS_PRESSURE, 0, MAX_12BIT,
 			     ts->fuzzz, 0);
 
-	if (!np) {
-		if (pdata->init_platform_hw)
-			pdata->init_platform_hw();
-	}
+	if (pdata && pdata->init_platform_hw)
+		pdata->init_platform_hw();
 
 	err = request_threaded_irq(ts->irq, tsc2007_hard_irq, tsc2007_soft_irq,
 				   IRQF_ONESHOT, client->dev.driver->name, ts);
@@ -461,16 +440,12 @@ static int tsc2007_probe(struct i2c_client *client,
 	if (err)
 		goto err_free_irq;
 
-	i2c_set_clientdata(client, ts);
-
 	return 0;
 
  err_free_irq:
 	free_irq(ts->irq, ts);
-	if (!np) {
-		if (pdata->exit_platform_hw)
-			pdata->exit_platform_hw();
-	}
+	if (pdata && pdata->exit_platform_hw)
+		pdata->exit_platform_hw();
  err_free_input:
 	input_free_device(input_dev);
 	return err;
@@ -478,16 +453,13 @@ static int tsc2007_probe(struct i2c_client *client,
 
 static int tsc2007_remove(struct i2c_client *client)
 {
-	struct device_node *np = client->dev.of_node;
-	struct tsc2007	*ts = i2c_get_clientdata(client);
-	struct tsc2007_platform_data *pdata = client->dev.platform_data;
+	const struct tsc2007_platform_data *pdata = dev_get_platdata(&client->dev);
+	struct tsc2007 *ts = i2c_get_clientdata(client);
 
 	free_irq(ts->irq, ts);
 
-	if (!np) {
-		if (pdata->exit_platform_hw)
-			pdata->exit_platform_hw();
-	}
+	if (pdata && pdata->exit_platform_hw)
+		pdata->exit_platform_hw();
 
 	input_unregister_device(ts->input);
 	kfree(ts);
diff --git a/include/linux/i2c/tsc2007.h b/include/linux/i2c/tsc2007.h
index 506a9f7..041c8e8 100644
--- a/include/linux/i2c/tsc2007.h
+++ b/include/linux/i2c/tsc2007.h
@@ -14,9 +14,9 @@ struct tsc2007_platform_data {
 	int	fuzzy;
 	int	fuzzz;
 
-	int	(*get_pendown_state)(void);
-	void	(*clear_penirq)(void);		/* If needed, clear 2nd level
-						   interrupt source */
+	int	(*get_pendown_state)(struct device *);
+	/* If needed, clear 2nd level interrupt source */
+	void	(*clear_penirq)(void);
 	int	(*init_platform_hw)(void);
 	void	(*exit_platform_hw)(void);
 };
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* Re: [PATCH 26/31] Input: tegra-kbc - use reset framework
From: Dmitry Torokhov @ 2013-11-19 21:17 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Stephen Warren, treding, pdeschrijver, linux-tegra,
	linux-arm-kernel, linux-input
In-Reply-To: <1384548866-13141-27-git-send-email-swarren@wwwdotorg.org>

On Fri, Nov 15, 2013 at 01:54:21PM -0700, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Tegra's clock driver now provides an implementation of the common
> reset API (include/linux/reset.h). Use this instead of the old Tegra-
> specific API; that will soon be removed.
> 
> Cc: treding@nvidia.com
> Cc: pdeschrijver@nvidia.com
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Dmitry Torokhov <dtor@mail.ru>
> Cc: linux-input@vger.kernel.org
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

> ---
> This patch is part of a series with strong internal depdendencies. I'm
> looking for an ack so that I can take the entire series through the Tegra
> and arm-soc trees. The series will be part of a stable branch that can be
> merged into other subsystems if needed to avoid/resolve dependencies.
> ---
>  drivers/input/keyboard/tegra-kbc.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/input/keyboard/tegra-kbc.c b/drivers/input/keyboard/tegra-kbc.c
> index 8508879f6faf..9757a58bc897 100644
> --- a/drivers/input/keyboard/tegra-kbc.c
> +++ b/drivers/input/keyboard/tegra-kbc.c
> @@ -31,7 +31,7 @@
>  #include <linux/clk.h>
>  #include <linux/slab.h>
>  #include <linux/input/matrix_keypad.h>
> -#include <linux/clk/tegra.h>
> +#include <linux/reset.h>
>  #include <linux/err.h>
>  
>  #define KBC_MAX_KPENT	8
> @@ -116,6 +116,7 @@ struct tegra_kbc {
>  	u32 wakeup_key;
>  	struct timer_list timer;
>  	struct clk *clk;
> +	struct reset_control *rst;
>  	const struct tegra_kbc_hw_support *hw_support;
>  	int max_keys;
>  	int num_rows_and_columns;
> @@ -373,9 +374,9 @@ static int tegra_kbc_start(struct tegra_kbc *kbc)
>  	clk_prepare_enable(kbc->clk);
>  
>  	/* Reset the KBC controller to clear all previous status.*/
> -	tegra_periph_reset_assert(kbc->clk);
> +	reset_control_assert(kbc->rst);
>  	udelay(100);
> -	tegra_periph_reset_deassert(kbc->clk);
> +	reset_control_assert(kbc->rst);
>  	udelay(100);
>  
>  	tegra_kbc_config_pins(kbc);
> @@ -663,6 +664,12 @@ static int tegra_kbc_probe(struct platform_device *pdev)
>  		return PTR_ERR(kbc->clk);
>  	}
>  
> +	kbc->rst = devm_reset_control_get(&pdev->dev, "kbc");
> +	if (IS_ERR(kbc->rst)) {
> +		dev_err(&pdev->dev, "failed to get keyboard reset\n");
> +		return PTR_ERR(kbc->rst);
> +	}
> +
>  	/*
>  	 * The time delay between two consecutive reads of the FIFO is
>  	 * the sum of the repeat time and the time taken for scanning
> -- 
> 1.8.1.5
> 

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] Input: Add new driver for GPIO beeper
From: Dmitry Torokhov @ 2013-11-19 21:32 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Mark Rutland, Alexander Shiyan, linux-input@vger.kernel.org,
	devicetree@vger.kernel.org, rob.herring@calxeda.com, Pawel Moll,
	Ian Campbell
In-Reply-To: <52828042.4050501@wwwdotorg.org>

On Tue, Nov 12, 2013 at 12:23:46PM -0700, Stephen Warren wrote:
> On 11/12/2013 03:59 AM, Mark Rutland wrote:
> > On Tue, Nov 12, 2013 at 10:47:57AM +0000, Alexander Shiyan wrote:
> >> Hello.
> >>
> >>> On Tue, Nov 12, 2013 at 10:07:13AM +0000, Alexander Shiyan wrote:
> >>>> This patch adds a new driver for the beeper controlled via GPIO pin.
> >>>> The driver does not depend on the architecture and is positioned as
> >>>> a replacement for the specific drivers that are used for this function.
> >>>>
> >>>> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> >> ...
> >>>> diff --git a/Documentation/devicetree/bindings/input/gpio-beeper.txt b/Documentation/devicetree/bindings/input/gpio-beeper.txt
> >> ...
> >>>> +Example:
> >>>> +
> >>>> +beeper: input@0 {
> >>>> +	compatible = "gpio-beeper";
> >>>> +	reg = <0>;
> >>>> +	gpios = <&gpio3 23 0>;
> >>>> +};
> >>>
> >>> What are the reg / unit-address for?
> >>
> >> Just an example from "simple-bus" container.
> > 
> > If they have no meaning, they should go. They're unnecessary and make
> > things more confusing.
> > 
> > I'd expect the example to be:
> > 
> > beeper: beeper {
> > 	compatible = "gpio-beeper";
> > 	gpios - <&gpio3 23 0>;
> > };
> > 
> > And if we have multiple beepers, something like:
> > 
> > beeper0: beeper0 { ... };
> > beeper1: beeper1 { ... };
> 
> DT node names aren't meant to encode identity though. What we've done in
> the past for nodes without a reg where multiple instances were desired
> is to put them into simple-bus and add a reg, so:
> 
> beeper0: beeper@0 { reg = <0>; ... };
> beeper1: beeper@1 { reg = <1>; ... };
> 
> Of course, if there's only one of them, then it could just be "beeper"
> with no reg. The binding and example should probably reflect that simple
> case.

So do we have an agreement on bindings? Otherwise the driver looks good
to me.

-- 
Dmitry

^ permalink raw reply

* [PATCH 2/2] HID: kye: fix unresponsive keyboard
From: Benjamin Tissoires @ 2013-11-20 14:49 UTC (permalink / raw)
  To: Benjamin Tissoires, Jiri Kosina, Adam Kulagowski, linux-input,
	linux-kernel
In-Reply-To: <1384958982-23099-1-git-send-email-benjamin.tissoires@redhat.com>

The manticore keyboard requires that all usb EP are opened at least
once to be fully functional. The third EP forwards to the user space
some vendor specific information about the keyboard state, but are useless
currently for the kernel.

Opening them and closing them makes the keyboard responsive again.

Reported-and-tested-by: Adam Kulagowski <fidor@fidor.org>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---

Hi Jiri,

well, this one was a long shot as I do not have the hardware to actually
spot the exact call which enables the keyboard. Adam told me that just
opening and closing once all the /dev/hidrawX attached to the keyboard makes
the keyboard functional, so, here is the kernel version of this.
Anyway, it just works, so I think we can take this one as well.

Cheers,
Benjamin

 drivers/hid/hid-kye.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c
index 35a4f9c..ecb5ca6 100644
--- a/drivers/hid/hid-kye.c
+++ b/drivers/hid/hid-kye.c
@@ -421,6 +421,14 @@ static int kye_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			goto enabling_err;
 		}
 		break;
+	case USB_DEVICE_ID_GENIUS_MANTICORE:
+		/*
+		 * The manticore keyboard needs to have all the interfaces
+		 * opened at least once to be fully functional.
+		 */
+		if (hid_hw_open(hdev))
+			hid_hw_close(hdev);
+		break;
 	}
 
 	return 0;
-- 
1.8.3.1

^ permalink raw reply related

* [PATCH 1/2] HID: kye: Add report fixup for Genius Manticore Keyboard
From: Benjamin Tissoires @ 2013-11-20 14:49 UTC (permalink / raw)
  To: Benjamin Tissoires, Jiri Kosina, Adam Kulagowski, linux-input,
	linux-kernel

Genius Manticore Keyboard presents the same problem in its report
descriptors than Genius Gila Gaming Mouse and Genius Imperator Keyboard.
Use the same fixup.

Reported-and-tested-by: Adam Kulagowski <fidor@fidor.org>
Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/hid-core.c | 1 +
 drivers/hid/hid-ids.h  | 1 +
 drivers/hid/hid-kye.c  | 5 +++++
 3 files changed, 7 insertions(+)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 9bbbb72..0a82fdb 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1722,6 +1722,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KENSINGTON, USB_DEVICE_ID_KS_SLIMBLADE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KEYTOUCH, USB_DEVICE_ID_KEYTOUCH_IEC) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_GENIUS_MANTICORE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_GENIUS_GX_IMPERATOR) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_ERGO_525V) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE, USB_DEVICE_ID_KYE_EASYPEN_I405X) },
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7655962..c8c7cb1 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -489,6 +489,7 @@
 #define USB_VENDOR_ID_KYE		0x0458
 #define USB_DEVICE_ID_KYE_ERGO_525V	0x0087
 #define USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE	0x0138
+#define USB_DEVICE_ID_GENIUS_MANTICORE	0x0153
 #define USB_DEVICE_ID_GENIUS_GX_IMPERATOR	0x4018
 #define USB_DEVICE_ID_KYE_GPEN_560	0x5003
 #define USB_DEVICE_ID_KYE_EASYPEN_I405X	0x5010
diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c
index 7384512..35a4f9c 100644
--- a/drivers/hid/hid-kye.c
+++ b/drivers/hid/hid-kye.c
@@ -341,6 +341,9 @@ static __u8 *kye_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	case USB_DEVICE_ID_GENIUS_GX_IMPERATOR:
 		rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 83,
 					"Genius Gx Imperator Keyboard");
+	case USB_DEVICE_ID_GENIUS_MANTICORE:
+		rdesc = kye_consumer_control_fixup(hdev, rdesc, rsize, 104,
+					"Genius Manticore Keyboard");
 		break;
 	}
 	return rdesc;
@@ -439,6 +442,8 @@ static const struct hid_device_id kye_devices[] = {
 				USB_DEVICE_ID_GENIUS_GILA_GAMING_MOUSE) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE,
 				USB_DEVICE_ID_GENIUS_GX_IMPERATOR) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_KYE,
+				USB_DEVICE_ID_GENIUS_MANTICORE) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, kye_devices);
-- 
1.8.3.1


^ permalink raw reply related

* Re: input question: ambient light sensor button
From: Pali Rohár @ 2013-11-20 14:50 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: Jiri Kosina, linux-kernel, linux-input
In-Reply-To: <20131119212736.GA25784@core.coreip.homeip.net>

[-- Attachment #1: Type: Text/Plain, Size: 2284 bytes --]

On Tuesday 19 November 2013 22:27:36 Dmitry Torokhov wrote:
> On Thu, Nov 14, 2013 at 01:02:31PM +0100, Pali Rohár wrote:
> > On Wednesday 13 November 2013 17:28:40 Dmitry Torokhov wrote:
> > > On Wed, Nov 13, 2013 at 11:47:18AM +0100, Jiri Kosina wrote:
> > > > On Sun, 15 Sep 2013, Pali Rohár wrote:
> > > > > I do not know where to ask this question, but I think
> > > > > that kernel developers could help me.
> > > > > 
> > > > > I have notebook with one special button on keyboard
> > > > > which is designed for turning ambient light sensor on
> > > > > and off. By default pressing button do nothing (I can
> > > > > turn ambient light sensor on/off via sysfs platform
> > > > > wmi module). Button press is reported by kernel input
> > > > > device AT Translated Set 2 keyboard and reports it as
> > > > > button "touchpad off".
> > > > > 
> > > > > Of course "touchpad off" is incorrect and I'd like to
> > > > > ask which kernel key or button from
> > > > > /usr/include/linux/input.h should be mapped for my
> > > > > ambient light sensor button? Is there already some?
> > > > > And what is strategy for allocating KEY_* and BTN_*
> > > > > numbers?
> > > > 
> > > > This is maintained by Dmitry, so he's the one to answer
> > > > this. I am adding him to CC.
> > > 
> > > I guess we need patch adding
> > > 
> > > 	#define KEY_ALS_TOGGLE	0x230
> > > 
> > > Thanks.
> > 
> > So there is no good key for als yet?
> 
> No, but if you send me a patch adding it I will gladly apply
> it.
> 
> Thanks.

Ok, here is patch:

Add key code for ambient light sensor button

More notebooks have special button for enabling/disabling ambient
light sensor. This patch adding new als code to input.h header file.

Signed-off-by: Pali Rohár <pali.rohar@gmail.com>

diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
index a372627..1562f10 100644
--- a/include/uapi/linux/input.h
+++ b/include/uapi/linux/input.h
@@ -719,6 +719,8 @@ struct input_keymap_entry {
 #define BTN_DPAD_LEFT		0x222
 #define BTN_DPAD_RIGHT		0x223
 
+#define KEY_ALS_TOGGLE		0x230
+
 #define BTN_TRIGGER_HAPPY		0x2c0
 #define BTN_TRIGGER_HAPPY1		0x2c0
 #define BTN_TRIGGER_HAPPY2		0x2c1


-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply related

* Re: [PATCH 00/31] ARM: tegra: use common reset and DMA bindings
From: Arnd Bergmann @ 2013-11-20 15:37 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Mark Rutland, alsa-devel, Dmitry Torokhov, Wolfram Sang,
	David Airlie, linux-pci, dri-devel, Marc Dietrich, Bjorn Helgaas,
	linux-i2c, ac100, devel, Stephen Warren, Mike Turquette,
	Ian Campbell, Alan Stern, linux-serial, linux-input, treding,
	devicetree, Pawel Moll, Stephen Warren, Julian Andres Klode,
	Rob Herring, Mark Brown, linux-tegra, Terje
In-Reply-To: <1384548866-13141-1-git-send-email-swarren@wwwdotorg.org>

On Friday 15 November 2013, Stephen Warren wrote:
> This series implements a common reset framework driver for Tegra, and
> updates all relevant Tegra drivers to use it. It also removes the custom
> DMA bindings and replaced them with the standard DMA DT bindings.

The series is rather long, so I may have missed it, but I think you need one
more patch to the apbdma binding to document the use of #dma-cells, what
value it has, and what the format of the dma specifiers in slave drivers
needs to be.

	Arnd

^ permalink raw reply

* Re: input question: ambient light sensor button
From: Benjamin Tissoires @ 2013-11-20 15:59 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Dmitry Torokhov, Jiri Kosina, linux-kernel@vger.kernel.org,
	linux-input
In-Reply-To: <201311201550.05800@pali>

Hi,

On Wed, Nov 20, 2013 at 9:50 AM, Pali Rohár <pali.rohar@gmail.com> wrote:
>> > > I guess we need patch adding
>> > >
>> > >   #define KEY_ALS_TOGGLE  0x230
>> > >
>> > > Thanks.
>> >
>> > So there is no good key for als yet?
>>
>> No, but if you send me a patch adding it I will gladly apply
>> it.
>>
>> Thanks.
>
> Ok, here is patch:
>
> Add key code for ambient light sensor button
>
> More notebooks have special button for enabling/disabling ambient
> light sensor. This patch adding new als code to input.h header file.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
>
> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> index a372627..1562f10 100644
> --- a/include/uapi/linux/input.h
> +++ b/include/uapi/linux/input.h
> @@ -719,6 +719,8 @@ struct input_keymap_entry {
>  #define BTN_DPAD_LEFT          0x222
>  #define BTN_DPAD_RIGHT         0x223
>
> +#define KEY_ALS_TOGGLE         0x230

Could you just add a comment explaining that ALS is ambiant light
sensor? Otherwise, I'm sure someone else will use this event code for
an other thing... :)

Cheers,
Benjamin

^ permalink raw reply

* Re: [PATCH 00/31] ARM: tegra: use common reset and DMA bindings
From: Stephen Warren @ 2013-11-20 16:45 UTC (permalink / raw)
  To: Arnd Bergmann, linux-arm-kernel
  Cc: Mark Rutland, alsa-devel, Dmitry Torokhov, Wolfram Sang,
	David Airlie, linux-pci, dri-devel, Marc Dietrich, Bjorn Helgaas,
	linux-i2c, ac100, devel, Stephen Warren, Mike Turquette,
	Alan Stern, linux-serial, linux-input, treding, devicetree,
	Pawel Moll, Ian Campbell, Julian Andres Klode, Rob Herring,
	Mark Brown, linux-tegra
In-Reply-To: <201311201637.36733.arnd@arndb.de>

On 11/20/2013 08:37 AM, Arnd Bergmann wrote:
> On Friday 15 November 2013, Stephen Warren wrote:
>> This series implements a common reset framework driver for Tegra, and
>> updates all relevant Tegra drivers to use it. It also removes the custom
>> DMA bindings and replaced them with the standard DMA DT bindings.
> 
> The series is rather long, so I may have missed it, but I think you need one
> more patch to the apbdma binding to document the use of #dma-cells, what
> value it has, and what the format of the dma specifiers in slave drivers
> needs to be.

Yes, you're right. I will fold the following into "ARM: tegra: document
use of standard DMA DT bindings":

> diff --git a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> index 0b1e577ab9d3..0b0f9498e265 100644
> --- a/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> +++ b/Documentation/devicetree/bindings/dma/tegra20-apbdma.txt
> @@ -11,6 +11,10 @@ Required properties:
>    See ../reset/reset.txt for details.
>  - reset-names : Must include the following entries:
>    - dma
> +- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
> +  client nodes' dmas properties. The specifier represents the DMA request
> +  select value for the peripheral. For more details, consult the Tegra TRM's
> +  documentation of the APB DMA channel control register REQ_SEL field.
>  
>  Examples:
>  
> @@ -36,4 +40,5 @@ apbdma: dma@6000a000 {
>  	clocks = <&tegra_car 34>;
>  	resets = <&tegra_car 34>;
>  	reset-names = "dma";
> +	#iommu-cells = <1>;
>  };

^ permalink raw reply

* Re: [PATCH 00/31] ARM: tegra: use common reset and DMA bindings
From: Arnd Bergmann @ 2013-11-20 17:03 UTC (permalink / raw)
  To: Stephen Warren
  Cc: Mark Rutland, alsa-devel, Dmitry Torokhov, Wolfram Sang,
	David Airlie, linux-pci, dri-devel, Bjorn Helgaas, linux-i2c,
	ac100, devel, Stephen Warren, Mike Turquette, Alan Stern,
	linux-serial, linux-input, treding, devicetree, Pawel Moll,
	Ian Campbell, Rob Herring, Mark Brown, linux-tegra,
	Terje Bergström, Dan Williams, linux-arm-kernel
In-Reply-To: <528CE71D.5090300@wwwdotorg.org>

On Wednesday 20 November 2013, Stephen Warren wrote:
> > +- #iommu-cells : Must be <1>. This dictates the length of DMA specifiers in
> > +  client nodes' dmas properties. The specifier represents the DMA request
> > +  select value for the peripheral. For more details, consult the Tegra TRM's
> > +  documentation of the APB DMA channel control register REQ_SEL field.
> >  
> >  Examples:
> >  
> > @@ -36,4 +40,5 @@ apbdma: dma@6000a000 {
> >       clocks = <&tegra_car 34>;
> >       resets = <&tegra_car 34>;
> >       reset-names = "dma";
> > +     #iommu-cells = <1>;


s/iommu/dma/

Otherwise looks good. The dts files are correct, so I guess it's just
a typo here.

	Arnd

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox