Linux Input/HID development
 help / color / mirror / Atom feed
* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Tony Lindgren @ 2013-10-27 12:23 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-input, 'Beno?t Cousson', Rob Herring, Pawel Moll,
	Mark Rutland, Stephen Warren, Ian Campbell, Rob Landley,
	Russell King, Dmitry Torokhov, Grant Likely, devicetree,
	linux-doc, linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <20131027114753.GB14901@amd.pavel.ucw.cz>

* Pavel Machek <pavel@ucw.cz> [131027 04:48]:
> 
> > > > +#if IS_ENABLED(CONFIG_OF)
> > > I'm probably missing something here, but why not #ifdef CONFIG_OF?
> > 
> > I have been told for other drivers, that IS_ENABLED() is
> > the prefered way to check for configuration these days.
> 
> CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong.

Good point. Looks like there's IS_BUILTIN that's for boolean options.

Regards,

Tony

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-10-27 11:47 UTC (permalink / raw)
  To: linux-input-u79uwXL29TY76Z2rM5mHXA, 'Beno?t Cousson',
	Tony Lindgren, Rob Herring, Pawel Moll, Mark Rutland,
	Stephen Warren, Ian Campbell, Rob Landley, Russell King,
	Dmitry Torokhov, Grant Likely, devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20131027114026.GB14091-SfvFxonMDyemK9LvCR3Hrw@public.gmane.org>

Hi!

> > > + * keypad,num-rows and keypad,num-columns are required.
> > Is "keypad," prefix neccessary here?
> 
> See Documentation/devicetree/bindings/input/matrix-keymap.txt
> 
> > > +Optional Properties specific to linux:
> > > +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> > 
> > "do not autorepeat". Plus I do not see what is Linux specifc about not
> > autorepeating... Other systems will likely know about autorepeat, too.
> 
> This property has already been used like this for
> samsung-keypad, stmpe-keypad, omap-keypad and
> gpio-matrix-keypad.

Ok. But you still have a typo. "do no enable" => "do not enable".

> > > +#if IS_ENABLED(CONFIG_OF)
> > I'm probably missing something here, but why not #ifdef CONFIG_OF?
> 
> I have been told for other drivers, that IS_ENABLED() is
> the prefered way to check for configuration these days.

CONFIG_OF can not be module, using IS_ENABLED() on it is just wrong.

> > > @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
> > >  
> > >  	input_set_capability(input, EV_MSC, MSC_SCAN);
> > >  	/* Enable auto repeat feature of Linux input subsystem */
> > > -	if (pdata->rep)
> > > +	if (!kp->no_autorepeat)
> > >  		__set_bit(EV_REP, input->evbit);
> > >
> > 
> > Double negation is nasty to read. I believe code would be more
> > readable if you switched logic to kp->autorepeat.
> 
> I was thinking about the same when writing it, but decided
> against it, since it will just move the double negation to
> the variable initialization.

Well, the property should be linux,keypad-autorepeat in the first
place, but it is too late to change that.

Thanks,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Sebastian Reichel @ 2013-10-27 11:40 UTC (permalink / raw)
  To: Pavel Machek
  Cc: linux-input, 'Beno?t Cousson', Tony Lindgren, Rob Herring,
	Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell,
	Rob Landley, Russell King, Dmitry Torokhov, Grant Likely,
	devicetree, linux-doc, linux-kernel, linux-arm-kernel, linux-omap
In-Reply-To: <20131027111715.GA2437@xo-6d-61-c0.localdomain>

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

Hi Pavel,

On Sun, Oct 27, 2013 at 12:17:15PM +0100, Pavel Machek wrote:
> > Add device tree support for twl4030 keypad driver and update the
> > Documentation with twl4030 keypad device tree binding information.
> > 
> > Tested on Nokia N900.
> 
> It looks pretty good.

Thanks.

> > + * keypad,num-rows and keypad,num-columns are required.
> Is "keypad," prefix neccessary here?

See Documentation/devicetree/bindings/input/matrix-keymap.txt

> > +Optional Properties specific to linux:
> > +- linux,keypad-no-autorepeat: do no enable autorepeat feature.
> 
> "do not autorepeat". Plus I do not see what is Linux specifc about not
> autorepeating... Other systems will likely know about autorepeat, too.

This property has already been used like this for
samsung-keypad, stmpe-keypad, omap-keypad and
gpio-matrix-keypad.

> > +#if IS_ENABLED(CONFIG_OF)
> I'm probably missing something here, but why not #ifdef CONFIG_OF?

I have been told for other drivers, that IS_ENABLED() is
the prefered way to check for configuration these days.

> > @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
> >  
> >  	input_set_capability(input, EV_MSC, MSC_SCAN);
> >  	/* Enable auto repeat feature of Linux input subsystem */
> > -	if (pdata->rep)
> > +	if (!kp->no_autorepeat)
> >  		__set_bit(EV_REP, input->evbit);
> >
> 
> Double negation is nasty to read. I believe code would be more
> readable if you switched logic to kp->autorepeat.

I was thinking about the same when writing it, but decided
against it, since it will just move the double negation to
the variable initialization.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv2 1/3] Input: twl4030-keypad - add device tree support
From: Pavel Machek @ 2013-10-27 11:17 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Russell King,
	Rob Landley, Pawel Moll, Stephen Warren, Tony Lindgren,
	Ian Campbell, linux-doc, linux-kernel, Rob Herring,
	Sebastian Reichel, 'Beno?t Cousson', linux-input,
	Grant Likely, linux-omap, linux-arm-kernel
In-Reply-To: <1382446042-27099-2-git-send-email-sre@debian.org>

Hi!

> Add device tree support for twl4030 keypad driver and update the
> Documentation with twl4030 keypad device tree binding information.
> 
> Tested on Nokia N900.

It looks pretty good.

> +++ b/Documentation/devicetree/bindings/input/twl4030-keypad.txt
> @@ -0,0 +1,31 @@
> +* TWL4030's Keypad Controller device tree bindings
> +
> +TWL4030's Keypad controller is used to interface a SoC with a matrix-type
> +keypad device. The keypad controller supports multiple row and column lines.
> +A key can be placed at each intersection of a unique row and a unique column.
> +The keypad controller can sense a key-press and key-release and report the
> +event using a interrupt to the cpu.
> +
> +This binding is based on the matrix-keymap binding with the following
> +changes:
> +
> + * keypad,num-rows and keypad,num-columns are required.

Is "keypad," prefix neccessary here?

> +Optional Properties specific to linux:
> +- linux,keypad-no-autorepeat: do no enable autorepeat feature.

"do not autorepeat". Plus I do not see what is Linux specifc about not
autorepeating... Other systems will likely know about autorepeat, too.

> @@ -324,6 +326,31 @@ static int twl4030_kp_program(struct twl4030_keypad *kp)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)

I'm probably missing something here, but why not #ifdef CONFIG_OF?

> @@ -381,7 +426,7 @@ static int twl4030_kp_probe(struct platform_device *pdev)
>  
>  	input_set_capability(input, EV_MSC, MSC_SCAN);
>  	/* Enable auto repeat feature of Linux input subsystem */
> -	if (pdata->rep)
> +	if (!kp->no_autorepeat)
>  		__set_bit(EV_REP, input->evbit);
>

Double negation is nasty to read. I believe code would be more
readable if you switched logic to kp->autorepeat.

Thanks,
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* [PATCH] HID: hid-sensor-hub: fix report size
From: Srinivas Pandruvada @ 2013-10-26 17:04 UTC (permalink / raw)
  To: jkosina; +Cc: linux-input, Srinivas Pandruvada

Most of the hid sensor field size is reported in report_size field
in the report descriptor. For rotation fusion sensor the quaternion
data is 16 byte field, the report size was set to 4 and report
count field is set to 4. So the total size is 16 bytes. But the current
driver has a bug and not taking account for report count field. This
causes user space to see only 4 bytes of data sent via IIO interface. 
The number of bytes in a field needs to take account of report_count
field. Need to multiply report_size and report_count to get total
number of bytes.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/hid-sensor-hub.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c
index 88fc5ae..a184e19 100644
--- a/drivers/hid/hid-sensor-hub.c
+++ b/drivers/hid/hid-sensor-hub.c
@@ -326,7 +326,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 				field->logical == attr_usage_id) {
 				sensor_hub_fill_attr_info(info, i, report->id,
 					field->unit, field->unit_exponent,
-					field->report_size);
+					field->report_size *
+							field->report_count);
 				ret = 0;
 			} else {
 				for (j = 0; j < field->maxusage; ++j) {
@@ -338,7 +339,8 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 							i, report->id,
 							field->unit,
 							field->unit_exponent,
-							field->report_size);
+							field->report_size *
+							field->report_count);
 						ret = 0;
 						break;
 					}
@@ -425,9 +427,10 @@ static int sensor_hub_raw_event(struct hid_device *hdev,
 		hid_dbg(hdev, "%d collection_index:%x hid:%x sz:%x\n",
 				i, report->field[i]->usage->collection_index,
 				report->field[i]->usage->hid,
-				report->field[i]->report_size/8);
-
-		sz = report->field[i]->report_size/8;
+				(report->field[i]->report_size *
+					report->field[i]->report_count)/8);
+		sz = (report->field[i]->report_size *
+					report->field[i]->report_count)/8;
 		if (pdata->pending.status && pdata->pending.attr_usage_id ==
 				report->field[i]->usage->hid) {
 			hid_dbg(hdev, "data was pending ...\n");
-- 
1.8.3.2


^ permalink raw reply related

* my subject
From: Mr. X @ 2013-10-26  8:37 UTC (permalink / raw)
  To: Recipients


Need a Loan, Loans from $5000 - $10,000,000 00. Get your no obligation FREE quote now! Repayments up to 54 Months. No Collateral, Money paid into your account within 24 hours after approval. For more Info contact us today  guaranteeloancompany1@gmail.com



^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-26 11:31 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <085234CB-BD53-4120-AE77-B06FA9314E96@codeaurora.org>

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

On Sat, Oct 26, 2013 at 01:37:57AM -0500, Kumar Gala wrote:
> 
> On Oct 25, 2013, at 5:18 PM, Sebastian Reichel wrote:
> 
> > On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
> >> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
> >>> +- interrupt: should be one of the following
> >>> +   - <8>: For controllers compatible with twl4030
> >> 
> >> Just checking, but the interrupt is always 8 for this device?
> > 
> > Yes. It's currently hardcoded in drivers/mfd/twl-core.c.
> 
> The fact that is hard coded in the driver does not imply that it
> should be in the device tree binding.  Is there an interrupt
> controller as part of the TWL4030?

Hardware looks like this:

&twl4030 {
    compatible = "ti,twl4030";
	interrupt-controller;
	#interrupt-cells = <1>;

    twl_pwrbutton: pwrbutton {
		compatible = "ti,twl4030-pwrbutton";
		interrupts = <8>; /* 8th interrupt from the twl4030 */
	};
};

Simplified the initialization of twl4030 stuff works
like this for non DT boot:

twl4030_init(...) {
    init_subdev(...);
    init_subdev("twl4030-pwrbutton", ..., irq=8, ...);
    init_subdev(...);
};

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* my subject
From: Mr. X @ 2013-10-26  8:37 UTC (permalink / raw)
  To: Recipients


Need a Loan, Loans from $5000 - $10,000,000 00. Get your no obligation FREE quote now! Repayments up to 54 Months. No Collateral, Money paid into your account within 24 hours after approval. For more Info contact us today  guaranteeloancompany1@gmail.com



^ permalink raw reply

* Re: [PATCH] Input: Adding support for touchpad on Dell XT2 model
From: Niels de Vos @ 2013-10-26  7:18 UTC (permalink / raw)
  To: Yunkang Tang
  Cc: dmitry.torokhov, cernekee, dturvene, linux-input, gaspard, jclift,
	yunkang.tang
In-Reply-To: <1382715581-2584-1-git-send-email-yunkang.tang@cn.alps.com>

On Fri, Oct 25, 2013 at 11:39:41PM +0800, Yunkang Tang wrote:
> Hi all,
> 
> This patch adding the support for touchpad on Dell XT2 model.
> It's a dual device with device ID: 73, 00, 14, that comply with "ALPS_PROTO_V2".
> 
> 
> Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>

Tested-by: Gaspard Jankowiak <gaspard@oknaj.eu>
Reviewed-by: Niels de Vos <ndevos@redhat.com>
    (well, for as far a one-line review goes)

Thanks!

> ---
>  drivers/input/mouse/alps.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index ca7a26f..24b3626 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -103,6 +103,7 @@ static const struct alps_model_info alps_model_data[] = {
>  	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
>  	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
> +	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_DUALPOINT },		/* Dell XT2 */
>  	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
>  	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
>  		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
> -- 
> 1.8.1.2
> 

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Kumar Gala @ 2013-10-26  6:37 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <20131025221850.GA8657@earth.universe>


On Oct 25, 2013, at 5:18 PM, Sebastian Reichel wrote:

> On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
>> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
>>> +- interrupt: should be one of the following
>>> +   - <8>: For controllers compatible with twl4030
>> 
>> Just checking, but the interrupt is always 8 for this device?
> 
> Yes. It's currently hardcoded in drivers/mfd/twl-core.c.

The fact that is hard coded in the driver does not imply that it should be in the device tree binding.  Is there an interrupt controller as part of the TWL4030?

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 23:40 UTC (permalink / raw)
  To: Grant Likely
  Cc: Dmitry Torokhov, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <20131025190904.3EE93C403B8@trevor.secretlab.ca>

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

On Fri, Oct 25, 2013 at 08:09:04PM +0100, Grant Likely wrote:
> On Thu, 24 Oct 2013 16:48:44 +0200, Sebastian Reichel <sre@debian.org> wrote:
> > Add device tree support for twl4030 power button driver.
> 
> The above commit text is insufficient. There are changes in the patch
> that aren't described here and have nothing to do with device tree
> bindings.

I will update the description in PATCHv7.

> [...]
> > +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> Can all of the TWL or TWL4030 funciton bindings be collected into a
> single file please? It is a single device after all. All of it should be
> in bindings/mfd/twl-family.txt

I guess this should be done in another patch? There's also a typo in
twl-family.txt's filename. My suggestion is to leave the patchset in
its current state. I will create another patch, which combines all
the twl4030 bindings descriptions into one file.

> [...]
> > +- interrupt: should be one of the following
> 
> Spelling: s/interrupt/interrupts/

fixed.

> [...]
> >  static struct platform_driver twl4030_pwrbutton_driver = {
> > +	.probe		= twl4030_pwrbutton_probe,
> >  	.remove		= __exit_p(twl4030_pwrbutton_remove),
> 
> Remove the __exit_p() wrapper. __exit is for module exit functions, not
> remove hooks (I know, that's not actually this patch, but the code is
> definitely wrong here).

On of the following patches converts the driver to devm, which
results in complete removal of the twl4030_pwrbutton_remove
function.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Grant Likely @ 2013-10-25 19:09 UTC (permalink / raw)
  To: Sebastian Reichel, Dmitry Torokhov
  Cc: Rob Herring, Sebastian Reichel, Peter Ujfalusi, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>

On Thu, 24 Oct 2013 16:48:44 +0200, Sebastian Reichel <sre@debian.org> wrote:
> Add device tree support for twl4030 power button driver.

The above commit text is insufficient. There are changes in the patch
that aren't described here and have nothing to do with device tree
bindings.

> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
>  .../devicetree/bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++++++++++++
>  drivers/input/misc/twl4030-pwrbutton.c              | 16 ++++++++++++----
>  2 files changed, 33 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> new file mode 100644
> index 0000000..4375646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments TWL family (twl4030) pwrbutton module

Can all of the TWL or TWL4030 funciton bindings be collected into a
single file please? It is a single device after all. All of it should be
in bindings/mfd/twl-family.txt

> +
> +This module is part of the TWL4030. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/twl-familly.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> +- interrupt: should be one of the following

Spelling: s/interrupt/interrupts/

> +   - <8>: For controllers compatible with twl4030
> +
> +Example:
> +
> +&twl {
> +	twl_pwrbutton: pwrbutton {
> +		compatible = "ti,twl4030-pwrbutton";
> +		interrupts = <8>;
> +	};
> +};
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index b9a05fd..a3a0fe3 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
> +static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  {
>  	struct input_dev *pwr;
>  	int irq = platform_get_irq(pdev, 0);
> @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)
> +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> +       { .compatible = "ti,twl4030-pwrbutton" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
> +#endif
> +
>  static struct platform_driver twl4030_pwrbutton_driver = {
> +	.probe		= twl4030_pwrbutton_probe,
>  	.remove		= __exit_p(twl4030_pwrbutton_remove),

Remove the __exit_p() wrapper. __exit is for module exit functions, not
remove hooks (I know, that's not actually this patch, but the code is
definitely wrong here).

>  	.driver		= {
>  		.name	= "twl4030_pwrbutton",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
>  	},
>  };
> -
> -module_platform_driver_probe(twl4030_pwrbutton_driver,
> -			twl4030_pwrbutton_probe);
> +module_platform_driver(twl4030_pwrbutton_driver);
>  
>  MODULE_ALIAS("platform:twl4030_pwrbutton");
>  MODULE_DESCRIPTION("Triton2 Power Button");
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 22:18 UTC (permalink / raw)
  To: Kumar Gala
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Peter Ujfalusi,
	Sachin Kamat, Florian Vaussard, linux-input, linux-kernel,
	devicetree
In-Reply-To: <FC6F0077-7E61-4F64-94E5-10E8FEF1F1E4@codeaurora.org>

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

On Fri, Oct 25, 2013 at 04:41:20PM -0500, Kumar Gala wrote:
> On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:
> > +- interrupt: should be one of the following
> > +   - <8>: For controllers compatible with twl4030
> 
> Just checking, but the interrupt is always 8 for this device?

Yes. It's currently hardcoded in drivers/mfd/twl-core.c.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Kumar Gala @ 2013-10-25 21:41 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard, linux-input,
	linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>


On Oct 24, 2013, at 9:48 AM, Sebastian Reichel wrote:

> Add device tree support for twl4030 power button driver.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
> .../devicetree/bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++++++++++++
> drivers/input/misc/twl4030-pwrbutton.c              | 16 ++++++++++++----
> 2 files changed, 33 insertions(+), 4 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> new file mode 100644
> index 0000000..4375646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments TWL family (twl4030) pwrbutton module
> +
> +This module is part of the TWL4030. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/twl-familly.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> +- interrupt: should be one of the following
> +   - <8>: For controllers compatible with twl4030

Just checking, but the interrupt is always 8 for this device?

> +
> +Example:
> +
> +&twl {
> +	twl_pwrbutton: pwrbutton {
> +		compatible = "ti,twl4030-pwrbutton";
> +		interrupts = <8>;
> +	};
> +};

Otherwise Ack on binding:

Acked-by: Kumar Gala <galak@codeaurora.org>

- k

-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation


^ permalink raw reply

* Re: [PATCHv6 3/3] Input: twl4030-pwrbutton: simplify driver using devm_*
From: Aaro Koskinen @ 2013-10-25 20:05 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1382626126-12565-4-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

On Thu, Oct 24, 2013 at 04:48:46PM +0200, Sebastian Reichel wrote:
> Use managed irq resource to simplify the driver.
> 
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Reviewed-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>

> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 26 ++++----------------------
>  1 file changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index 48639ff..be1759c 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -58,7 +58,7 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  	int irq = platform_get_irq(pdev, 0);
>  	int err;
>  
> -	pwr = input_allocate_device();
> +	pwr = devm_input_allocate_device(&pdev->dev);
>  	if (!pwr) {
>  		dev_dbg(&pdev->dev, "Can't allocate power button\n");
>  		return -ENOMEM;
> @@ -70,40 +70,23 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  	pwr->phys = "twl4030_pwrbutton/input0";
>  	pwr->dev.parent = &pdev->dev;
>  
> -	err = request_threaded_irq(irq, NULL, powerbutton_irq,
> +	err = devm_request_threaded_irq(&pwr->dev, irq, NULL, powerbutton_irq,
>  			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
>  			"twl4030_pwrbutton", pwr);
>  	if (err < 0) {
>  		dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
> -		goto free_input_dev;
> +		return err;
>  	}
>  
>  	err = input_register_device(pwr);
>  	if (err) {
>  		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
> -		goto free_irq;
> +		return err;
>  	}
>  
>  	platform_set_drvdata(pdev, pwr);
>  
>  	return 0;
> -
> -free_irq:
> -	free_irq(irq, pwr);
> -free_input_dev:
> -	input_free_device(pwr);
> -	return err;
> -}
> -
> -static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
> -{
> -	struct input_dev *pwr = platform_get_drvdata(pdev);
> -	int irq = platform_get_irq(pdev, 0);
> -
> -	free_irq(irq, pwr);
> -	input_unregister_device(pwr);
> -
> -	return 0;
>  }
>  
>  #if IS_ENABLED(CONFIG_OF)
> @@ -116,7 +99,6 @@ MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
>  
>  static struct platform_driver twl4030_pwrbutton_driver = {
>  	.probe		= twl4030_pwrbutton_probe,
> -	.remove		= __exit_p(twl4030_pwrbutton_remove),
>  	.driver		= {
>  		.name	= "twl4030_pwrbutton",
>  		.owner	= THIS_MODULE,
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv6 2/3] Input: twl4030-pwrbutton: use dev_err for errors
From: Aaro Koskinen @ 2013-10-25 20:05 UTC (permalink / raw)
  To: Sebastian Reichel
  Cc: Sebastian Reichel, Dmitry Torokhov, Grant Likely, Rob Herring,
	Peter Ujfalusi, Sachin Kamat, Florian Vaussard,
	linux-input-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1382626126-12565-3-git-send-email-sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Hi,

On Thu, Oct 24, 2013 at 04:48:45PM +0200, Sebastian Reichel wrote:
> Use dev_err() to output errors instead of dev_dbg().
> 
> Signed-off-by: Sebastian Reichel <sre-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>

Reviewed-by: Aaro Koskinen <aaro.koskinen-X3B1VOXEql0@public.gmane.org>

> ---
>  drivers/input/misc/twl4030-pwrbutton.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index a3a0fe3..48639ff 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -74,13 +74,13 @@ static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  			IRQF_TRIGGER_FALLING | IRQF_TRIGGER_RISING,
>  			"twl4030_pwrbutton", pwr);
>  	if (err < 0) {
> -		dev_dbg(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
> +		dev_err(&pdev->dev, "Can't get IRQ for pwrbutton: %d\n", err);
>  		goto free_input_dev;
>  	}
>  
>  	err = input_register_device(pwr);
>  	if (err) {
> -		dev_dbg(&pdev->dev, "Can't register power button: %d\n", err);
> +		dev_err(&pdev->dev, "Can't register power button: %d\n", err);
>  		goto free_irq;
>  	}
>  
> -- 
> 1.8.4.rc3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCHv5 1/3] Input: twl4030-pwrbutton - add device tree support
From: Grant Likely @ 2013-10-25 19:40 UTC (permalink / raw)
  To: Sebastian Reichel, Rob Herring
  Cc: Dmitry Torokhov, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	linux-input, linux-kernel, devicetree
In-Reply-To: <20131023224940.GA19522@earth.universe>

On Thu, 24 Oct 2013 00:49:42 +0200, Sebastian Reichel <sre@debian.org> wrote:
> On Wed, Oct 23, 2013 at 05:24:14PM -0500, Rob Herring wrote:
> > So a twl4030 device is only a power button? DT should describe the h/w
> > not a node for a sub-function of a device.
> 
> No. TWL4030 is a companion chip for the OMAP3 processor. It provides
> miscellaneous functionality, e.g.:
> 
>  * RTC
>  * Watchdog
>  * Regulators
>  * Keypad Matrix
>  * USB
>  * Audio
>  * Vibrator
>  * GPIO
>  * ...
> 
> One part of the functionality is the power button. The patch
> assumes, that the twl4030-pwrbutton node is used as follows:
> 
> twl {
>     /* ... common stuff ... */
> 
>     pwrbutton {
>         compatible = "ti,twl4030-pwrbutton";
> 		interrupts = <8>;
>     };
> };
> 
> See also:
> * Documentation/devicetree/bindings/mfd/twl-familly.txt
> * Documentation/devicetree/bindings/watchdog/twl4030-wdt.txt
> * Documentation/devicetree/bindings/sound/omap-twl4030.txt
> * Documentation/devicetree/bindings/mfd/twl4030-power.txt
> * Documentation/devicetree/bindings/mfd/twl4030-audio.txt
> * Documentation/devicetree/bindings/gpio/gpio-twl4030.txt

Wow, that's crazy! It is all one device so put all the bindings into a
single file.

g,

^ permalink raw reply

* Re: [PATCHv7][ 1/4] Input: tsc2007: Add device tree support.
From: Grant Likely @ 2013-10-25 19:11 UTC (permalink / raw)
  Cc: Mark Rutland, devicetree, Dmitry Torokhov, Eric BXXnard,
	Pawel Moll, Stephen Warren, Ian Campbell, Rob Herring,
	Denis Carikli, Thierry Reding, Sascha Hauer, linux-input,
	Shawn Guo, linux-arm-kernel
In-Reply-To: <20131025075639.75f45fb0@ipc1.ka-ro>

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

On Fri, 25 Oct 2013 07:56:39 +0200, Lothar Waßmann <LW@KARO-electronics.de> wrote:
> Hi,
> 
> Grant Likely wrote:
> > On Thu, 24 Oct 2013 14:42:13 +0200, Denis Carikli <denis@eukrea.com> 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: 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: Eric Bénard <eric@eukrea.com>
> > > Signed-off-by: Denis Carikli <denis@eukrea.com>
> > > ---
> > > ChangeLog v6->v7:
> > > - One small whitespace cleanup.
> > > - The properties specific to that driver are now prefixed with "ti,".
> > > - The ti,fuzzy property has now better documentation.
> > > ---
> > >  .../bindings/input/touchscreen/tsc2007.txt         |   45 +++++
> > >  drivers/input/touchscreen/tsc2007.c                |  194 +++++++++++++++-----
> > >  2 files changed, 198 insertions(+), 41 deletions(-)
> > >  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > new file mode 100644
> > > index 0000000..516b63b
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/input/touchscreen/tsc2007.txt
> > > @@ -0,0 +1,45 @@
> > > +* Texas Instruments tsc2007 touchscreen controller
> > > +
> > > +Required properties:
> > > +- compatible: must be "ti,tsc2007".
> > > +- reg: I2C address of the chip.
> > > +- ti,x-plate-ohms: X-plate resistance in ohms.
> > > +
> > > +Optional properties:
> > > +- gpios: the interrupt gpio the chip is connected to (trough the penirq pin)
> > > +  (see GPIO binding[2] for more details).
> > 
> > Hmmm, why is this needed? Is the line used for anything other than
> > finding the irq number? If it is just an interrupt then an interrupts
> > property is all that should be specified.
> > 
> It's also the pendown detect GPIO. It asserts an interrupt when first
> activated and is then polled in the driver until it goes inactive again.

That should definitely be in the document then. Otherwise the binding
looks okay to me.

g.



[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply

* [PATCH] Input: Adding support for touchpad on Dell XT2 model
From: Yunkang Tang @ 2013-10-25 15:39 UTC (permalink / raw)
  To: dmitry.torokhov, cernekee, dturvene
  Cc: linux-input, ndevos, gaspard, jclift, yunkang.tang

Hi all,

This patch adding the support for touchpad on Dell XT2 model.
It's a dual device with device ID: 73, 00, 14, that comply with "ALPS_PROTO_V2".


Signed-off-by: Yunkang Tang <yunkang.tang@cn.alps.com>
---
 drivers/input/mouse/alps.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index ca7a26f..24b3626 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -103,6 +103,7 @@ static const struct alps_model_info alps_model_data[] = {
 	/* Dell Latitude E5500, E6400, E6500, Precision M4400 */
 	{ { 0x62, 0x02, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf,
 		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },
+	{ { 0x73, 0x00, 0x14 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_DUALPOINT },		/* Dell XT2 */
 	{ { 0x73, 0x02, 0x50 }, 0x00, ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS },		/* Dell Vostro 1400 */
 	{ { 0x52, 0x01, 0x14 }, 0x00, ALPS_PROTO_V2, 0xff, 0xff,
 		ALPS_PASS | ALPS_DUALPOINT | ALPS_PS2_INTERLEAVED },				/* Toshiba Tecra A11-11L */
-- 
1.8.1.2


^ permalink raw reply related

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Sebastian Reichel @ 2013-10-25 14:44 UTC (permalink / raw)
  To: Peter Ujfalusi
  Cc: Dmitry Torokhov, Grant Likely, Rob Herring, Sachin Kamat,
	Florian Vaussard, linux-input, linux-kernel, devicetree
In-Reply-To: <526A680A.8090308@ti.com>

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

Hi Péter,

On Fri, Oct 25, 2013 at 03:46:02PM +0300, Peter Ujfalusi wrote:
> > [...]
> > +#if IS_ENABLED(CONFIG_OF)
> 
> You don't need to do this.

It's done like this in all the other drivers.

> > +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> > +       { .compatible = "ti,twl4030-pwrbutton" },
> > +       {},
> > +};
> > +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
> > +#endif
> > +
> >  static struct platform_driver twl4030_pwrbutton_driver = {
> > +	.probe		= twl4030_pwrbutton_probe,
> >  	.remove		= __exit_p(twl4030_pwrbutton_remove),
> >  	.driver		= {
> >  		.name	= "twl4030_pwrbutton",
> >  		.owner	= THIS_MODULE,
> > +		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),
> 
> If you try to compile this driver with config !CONFIG_OF it will not work in
> this way.

For !CONFIG_OF of_match_ptr is defined as follows (in "include/linux/of.h"):

#define of_match_ptr(_ptr)  NULL

So the preprocessor will remove the undefined symbol.

-- Sebastian

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: changes to ati_remote to support ATI Remote Wonder Plus RF Remote Control
From: Torrey Hoffman @ 2013-10-25 14:28 UTC (permalink / raw)
  To: sfo; +Cc: linux-input
In-Reply-To: <34245.198.84.133.226.1382707162.squirrel@cargocult.ca>

I should mention that of course, I don't get to 'decide' who the
official maintainer of anything is :-)
But given that Stephen is willing to work on it, he has my blessing,
for what that's worth.

^ permalink raw reply

* changes to ati_remote to support ATI Remote Wonder Plus RF Remote Control
From: Stephen Oberski @ 2013-10-25 13:19 UTC (permalink / raw)
  To: linux-input; +Cc: sfo

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

Attached is a text document describing proposed changes to the ati_remote kernel module to support
the ATI Remote Wonder Plus RF Remote Control.

I've forwarded these proposed changes to the listed authors of the module for review (Anssi
Hannula <anssi.hannula@XXX.YYY> and Torrey Hoffman <thoffman@XXX.YYY>) and have not heard back
from Anssi Hannul and I include Torrey Hoffman's response below.

Thanks,

-- 
Stephen Oberski sfo@cargocult.ca
289 430 5076
http://ca.linkedin.com/in/stephenoberski/


---------------------------------------- Original Message ----------------------------------------
Subject: Re: [Fwd: changes to ati_remote to support ATI Remote Wonder Plus RF Remote Control]
From:    "Torrey Hoffman" <thoffman@XXX.YYY>
Date:    Mon, October 21, 2013 12:21 pm
To:      sfo@cargocult.ca
--------------------------------------------------------------------------------------------------

Thanks for the heads up, and for working on this.

Unfortunately I no longer have any relevant hardware to test with.  If you
would like to be the official maintainer of this module, that would be
excellent.

Best wishes,

Torrey


On Mon, Oct 21, 2013 at 5:18 PM, Stephen Oberski <sfo@cargocult.ca> wrote:

> Hi Torrey,
>
> Attached is a text document describing proposed changes to the ati_remote
> kernel module to support
> the ATI Remote Wonder Plus RF Remote Control.
>
> I am sending this to you as the author of the module before posting to the
> linux-input mailing
> list and would appreciate any feedback.
>
> Thanks,
>
> --
> Stephen Oberski sfo@cargocult.ca
> 289 430 5076
> http://ca.linkedin.com/in/stephenoberski/
>
>
> ---------------------------------------- Original Message
> ----------------------------------------
> Subject: changes to ati_remote to support ATI Remote Wonder Plus RF Remote
>      Control
> From:    "Stephen Oberski" <sfo@cargocult.ca>
> Date:    Sat, October 12, 2013 9:53 pm
> To:      anssi.hannula@XXX.YYY
> Cc:      sfo@cargocult.ca
>
> --------------------------------------------------------------------------------------------------
>
> Hi Anssi,
>
> Attached is a text document describing proposed changes to the ati_remote
> kernel module to support
> the ATI Remote Wonder Plus RF Remote Control.
>
> I am sending this to you as the author of the module before posting to the
> linux-input mailing
> list and would appreciated any feedback.
>
> Thanks,
>
> --
> Stephen Oberski sfo@cargocult.ca
> 289 430 5076
>

[-- Attachment #2: linux-input_ati_remote-problem.txt --]
[-- Type: text/plain, Size: 12887 bytes --]

Proposed changes to the ati_remote kernel loadable module (drivers/media/rc/ati_remote.c) to support the ATI Remote Wonder Plus RF Remote Control
-------------------------------------------------------------------------------------------------------------------------------------------------

Date: Thu Oct 10 10:53:59 EDT 2013

By: Stephen Oberski (sfo@cargocult.ca)

Overview
========

The ati_remote.c USB ATI Remote support loadable kernel module (Version 2.2.0, Linux 3.8.0-31-generic) 
supports the SnapStream Media Firefly PC Remote Wireless Receiver Model R1000-1 receiver (FF receiver) with the Firefly RF Remote Control Model R1000 (FF remote).

It also supports the ATI USB RF Wireless Remote Receiver Part # 5000027000G receiver (ATI receiver) with the Firefly RF Remote Control Model R1000.

However neither the SnapStream Media Firefly PC Remote Wireless Receiver Model R1000-1 receiver nor the ATI USB RF Wireless Remote Receiver Part # 5000027000G receiver
are supported with the ATI Remote Wonder Plus RF Remote Control Part # 5000026900G (ATI remote).

A simple change to the ati_remote.c module results in the support of all combinations of the FF and ATI RF receivers and remotes.

The RF receiver initialization sequence is modified and the processing of the 4 byte remote inputs by the driver is modified to support a leading byte value of 0x15 as well as the current 0x14.

These changes appear to be backward compatible and do no seem to impact the existing behaviour of the module when presented with data from currently supported RF receiver and remote combinations
while allowing currently unsupported combinations to be used.

Problem Scope
=============

It appears that data generated by the ATI remote is never being received by the ati_remote kernel module:

============================== CUT HERE ==============================
# terminal window 1
# modprobe usbmon
# cd /sys/kernel/debug/usb/usbmon
# lsusb
...
Bus 004 Device 008: ID 0bc7:0004 X10 Wireless Technology, Inc. X10 Receiver
...
# rmmod ati_remote
# modprobe ati_remote

# terminal window 2
# grep --line-buffered '4:008' 4u
ffff88018a00d3c0 1361036384 C Ii:4:008:1 -108:8 0
ffff88018a00d240 1385420709 S Io:4:008:2 -115:8 5 = 80010020 14
ffff88018a00d240 1385424393 C Io:4:008:2 0:8 5 >
ffff88018a00d240 1385424401 S Io:4:008:2 -115:8 8 = 80010020 14202020
ffff88018a00d240 1385432392 C Io:4:008:2 0:8 8 >
ffff88018a00da80 1385432519 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1385440394 C Ii:4:008:1 0:8 1 = ff
ffff88018a00da80 1385440399 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415136406 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415136432 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415176406 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415176413 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415216405 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415216411 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415256404 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415256410 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415296405 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415296412 S Ii:4:008:1 -115:8 8 <
ffff88018a00da80 1415336404 C Ii:4:008:1 0:8 4 = 14658010
ffff88018a00da80 1415336410 S Ii:4:008:1 -115:8 8 <
============================== CUT HERE ==============================

We log USB data for the RF receiver in window 2.

In window 1 we enable USB monitoring and unload and reload the ati_remote module.

We can see the 2 initialization strings being sent to the RF receiver via the interrupt out (Io) lines
and then when we press the Firefly key on the FF remote we see the repeated USB packets sent by the RF remote (14658010).

However when we press any keys on the ATI remote no data is logged by usbmon.

This indicates that the RF receiver is ignoring data sent by the ATI remote when initialized by the ati_remote modules.

However when we load the modified version of the ati_remote module we see:

============================== CUT HERE ==============================
# rmmod ati_remote
# insmod ./ati_remote_plus.ko
# grep --line-buffered '4:008' 4u
ffff88018a00da80 2183228699 C Ii:4:008:1 -108:8 0
ffff880181305a80 2197203596 S Io:4:008:2 -115:8 9 = 8080051b 15142024 15
ffff880181305a80 2197216703 C Io:4:008:2 0:8 9 >
ffff880181305a80 2197216761 S Io:4:008:2 -115:8 3 = 808303
ffff880181305a80 2197224702 C Io:4:008:2 0:8 3 >
ffff880181305a80 2197224714 S Io:4:008:2 -115:8 4 = 8084d720
ffff880181305a80 2197232701 C Io:4:008:2 0:8 4 >
ffff8801813050c0 2197232835 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2197240704 C Ii:4:008:1 0:8 1 = 00
ffff8801813050c0 2197240711 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2219904713 C Ii:4:008:1 0:8 4 = 14658010
ffff8801813050c0 2219904740 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2219944712 C Ii:4:008:1 0:8 4 = 14658010
ffff8801813050c0 2219944720 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2219984712 C Ii:4:008:1 0:8 4 = 14658010
ffff8801813050c0 2219984718 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2220024715 C Ii:4:008:1 0:8 4 = 14658010
ffff8801813050c0 2220024722 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2220064715 C Ii:4:008:1 0:8 4 = 14658010
ffff8801813050c0 2220064721 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2221952713 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2221952740 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2221992712 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2221992720 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2222032713 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2222032720 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2222072715 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2222072721 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2222120710 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2222120716 S Ii:4:008:1 -115:8 8 <
ffff8801813050c0 2222160714 C Ii:4:008:1 0:8 4 = 15222d20
ffff8801813050c0 2222160720 S Ii:4:008:1 -115:8 8 <
============================== CUT HERE ==============================

Here we can see the modified initialization sequence being sent to the RF remote and when we press the Firefly key on the FF remote we see the repeated USB packets sent by the FF remote (14658010)
and when we press the ATI key on the ATI remote we see the repeated USB packets sent by the ATI remote (15222d20).

The conclusion is that the RF receiver initialization must be changed in the ati_remote module in order to support the ATI remote.

History
=======

The changes made to the ati_remote.c module are based on changes made to the LIRC lirc_atiusb.c code [1] 
by Cymen Vig in Feb 2006, who used a USB packet snooping program on Windows, these changes now seems to have disappeared from the current version of LIRC (0.9.0)
and a patch made to the drivers/media/rc/ati_remote.c file for the 2.6.31 kernel [2] which was never committed into the Linux source tree.

Hardware
========

- ATI USB RF Wireless Remote Receiver Part # 5000027000G:

Oct 10 16:07:08 acer-Veriton-N282G kernel: [695612.752080] usb 2-2: new low-speed USB device number 4 using uhci_hcd
Oct 10 16:07:08 acer-Veriton-N282G kernel: [695612.929218] usb 2-2: New USB device found, idVendor=0bc7, idProduct=0004
Oct 10 16:07:08 acer-Veriton-N282G kernel: [695612.929227] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=0
Oct 10 16:07:08 acer-Veriton-N282G kernel: [695612.929233] usb 2-2: Product: USB Receiver
Oct 10 16:07:08 acer-Veriton-N282G kernel: [695612.929239] usb 2-2: Manufacturer: X10 Wireless Technology Inc
Oct 10 16:07:08 acer-Veriton-N282G kernel: [695613.044063] Registered IR keymap rc-ati-x10

- SnapStream Media Firefly PC Remote Wireless Receiver Model R1000-1:

Oct 10 19:14:59 OptiPlex-755 kernel: [534490.804065] usb 4-1: new low-speed USB device number 6 using uhci_hcd
Oct 10 19:14:59 OptiPlex-755 kernel: [534490.974451] usb 4-1: New USB device found, idVendor=0bc7, idProduct=0008
Oct 10 19:14:59 OptiPlex-755 kernel: [534490.974455] usb 4-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0
Oct 10 19:14:59 OptiPlex-755 kernel: [534490.974458] usb 4-1: Product: USB Receiver
Oct 10 19:14:59 OptiPlex-755 kernel: [534490.974461] usb 4-1: Manufacturer: X10 Wireless Technology Inc
Oct 10 19:15:00 OptiPlex-755 kernel: [534491.032019] Registered IR keymap rc-snapstream-firefly

- Firefly RF Remote Control Model R1000

- ATI Remote Wonder Plus RF Remote Control Part # 5000026900G

Product ID Test Coverage
========================

+--------------------------+------+-----+------+-----------------------+---------------------+
|Product Name              +ID    +FF RC+ATI RC+3.8.0-31-generic/x86_64+3.8.0-31-generic/i686|
+--------------------------+------+-----+------+-----------------------+---------------------+
|LOLA_REMOTE_PRODUCT_ID    |0x0002|No   |No    |No                     |No                   |
|LOLA2_REMOTE_PRODUCT_ID   |0x0003|No   |No    |No                     |No                   |
|ATI_REMOTE_PRODUCT_ID     |0x0004|Yes  |Yes   |Yes                    |Yes                  |
|NVIDIA_REMOTE_PRODUCT_ID  |0x0005|No   |No    |No                     |No                   |
|MEDION_REMOTE_PRODUCT_ID  |0x0006|No   |No    |No                     |No                   |
|FIREFLY_REMOTE_PRODUCT_ID |0x0008|Yes  |Yes   |Yes                    |Yes                  |
+--------------------------+------+-----+------+-----------------------+---------------------+

FF RC: Firefly Remote Control
ATI RC: ATI Remote Control
3.8.0-31-generic/x86_64: 64 bit kernel
3.8.0-31-generic/i686: 32 bit kernel
Yes: Successful test
No: Not tested

Userspace Test
==============

A userspace test program [3] was developed to test the Remote receiver and remote control combinations.
It locates the X10 receiver by searching the USB devices for a Vendor ID of 0xbc7, any Product ID is allowed.
It then initializes the X10 receiver via interrupt writes using the new initialization strings discovered by Cymen Vig [1]:

static char init1[] = { 0x80, 0x05, 0x1b, 0x15, 0x14, 0x20, 0x24, 0x15 };
static char init2[] = { 0x83, 0x03 };
static char init3[] = { 0x84, 0xd7, 0x020 };

It then enters a loop and displays all packets received via interrupt reads, along with the computed checksum.

It is assumed that the libusb-dev: "userspace USB programming library development files" package is installed.

Compile and link as follows:

$ gcc -c testlibusb1.c
$ gcc testlibusb1.o -o testlibusb1 -lusb

Unload the "ati_remote" module if loaded:

$ sudo rmmod ati_remote

Run the test program:

$ sudo ./testlibusb1
...
found X10 0xbc7:0x8
found 1 interfaces
found 2 endpoints
endpoint 0 direction IN 0x81
endpoint 1 direction OUT 0x2
X10 open() OK
X10 set configuration OK
X10 claim interface 0 OK
X10 write 8 bytes to endpoint 0x2 OK with 8 bytes written
X10 write 2 bytes to endpoint 0x2 OK with 2 bytes written
X10 write 3 bytes to endpoint 0x2 OK with 3 bytes written
X10 read from endpoint 0x1 returned 4
 0:14  1:e5  2:00  3:10 checksum e5 
X10 read from endpoint 0x1 returned 4
 0:14  1:e5  2:00  3:10 checksum e5 
X10 read from endpoint 0x1 returned 4
 0:14  1:e5  2:00  3:10 checksum e5 
X10 read from endpoint 0x1 returned 4
X10 read from endpoint 0x1 returned 4
 0:15  1:a2  2:ad  3:20 checksum a2 
X10 read from endpoint 0x1 returned 4
 0:15  1:a2  2:ad  3:20 checksum a2 
X10 read from endpoint 0x1 returned 4
 0:15  1:a2  2:ad  3:20 checksum a2 
X10 read from endpoint 0x1 returned 4
 0:15  1:a2  2:ad  3:20 checksum a2
...
^C

In the sample output above the SnapStream Media Firefly PC Remote Wireless Receiver Model R1000-1 was connected as can be seen by the Vendor ID value of 0x8.
Each remote key press generates 4 usb packets, each USB packet is always 4 bytes in length.

The first 4 packets were from the Firefly button on the Firefly RF Remote Control Model R1000, byte 0 is always 0x14 for this remote.

The second 4 packets were from the ATI button on the ATI Remote Wonder Plus RF Remote Control Part # 5000026900G, byte 0 is always 0x15 for this remote.

Other than the value of byte 0 the format of the the USB packets generated by the Firefly and ATI remotes are identical, including the channel mask (byte 3) and checksum calculation.

Exactly the same results are obtained when the ATI USB RF Wireless Remote Receiver Part # 5000027000G is used.

Conclusions
===========

The modified version of the ati_remote driver [4] and the corresponding patch file [5] are provided.

The untested Product ID and remote control combinations need to be tested.
I have no access to the untested RF receivers corresponding to the Product IDs so perhaps users on the linux-input and linux-media 
could test this patch using other RF receiver and remote combinations.

References
==========

[1] http://www.mythtv.org/pipermail/mythtv-users/2006-February/121861.html
[2] http://www.mythtv.org/wiki/ATI_Remote_Wonder_Plus
[3] http://cargocult.ca/testlibusb1.c
[4] http://cargocult.ca/ati_remote_plus.c
[5] http://cargocult.ca/ati_remote.patch

^ permalink raw reply

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Peter Ujfalusi @ 2013-10-25 12:46 UTC (permalink / raw)
  To: Sebastian Reichel, Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Sachin Kamat, Florian Vaussard,
	linux-input, linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>

On 10/24/2013 05:48 PM, Sebastian Reichel wrote:
> Add device tree support for twl4030 power button driver.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>
> ---
>  .../devicetree/bindings/input/twl4030-pwrbutton.txt | 21 +++++++++++++++++++++
>  drivers/input/misc/twl4030-pwrbutton.c              | 16 ++++++++++++----
>  2 files changed, 33 insertions(+), 4 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> new file mode 100644
> index 0000000..4375646
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/twl4030-pwrbutton.txt
> @@ -0,0 +1,21 @@
> +Texas Instruments TWL family (twl4030) pwrbutton module
> +
> +This module is part of the TWL4030. For more details about the whole
> +chip see Documentation/devicetree/bindings/mfd/twl-familly.txt.
> +
> +This module provides a simple power button event via an Interrupt.
> +
> +Required properties:
> +- compatible: should be one of the following
> +   - "ti,twl4030-pwrbutton": For controllers compatible with twl4030
> +- interrupt: should be one of the following
> +   - <8>: For controllers compatible with twl4030
> +
> +Example:
> +
> +&twl {
> +	twl_pwrbutton: pwrbutton {
> +		compatible = "ti,twl4030-pwrbutton";
> +		interrupts = <8>;
> +	};
> +};
> diff --git a/drivers/input/misc/twl4030-pwrbutton.c b/drivers/input/misc/twl4030-pwrbutton.c
> index b9a05fd..a3a0fe3 100644
> --- a/drivers/input/misc/twl4030-pwrbutton.c
> +++ b/drivers/input/misc/twl4030-pwrbutton.c
> @@ -52,7 +52,7 @@ static irqreturn_t powerbutton_irq(int irq, void *_pwr)
>  	return IRQ_HANDLED;
>  }
>  
> -static int __init twl4030_pwrbutton_probe(struct platform_device *pdev)
> +static int twl4030_pwrbutton_probe(struct platform_device *pdev)
>  {
>  	struct input_dev *pwr;
>  	int irq = platform_get_irq(pdev, 0);
> @@ -106,16 +106,24 @@ static int __exit twl4030_pwrbutton_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#if IS_ENABLED(CONFIG_OF)

You don't need to do this.

> +static const struct of_device_id twl4030_pwrbutton_dt_match_table[] = {
> +       { .compatible = "ti,twl4030-pwrbutton" },
> +       {},
> +};
> +MODULE_DEVICE_TABLE(of, twl4030_pwrbutton_dt_match_table);
> +#endif
> +
>  static struct platform_driver twl4030_pwrbutton_driver = {
> +	.probe		= twl4030_pwrbutton_probe,
>  	.remove		= __exit_p(twl4030_pwrbutton_remove),
>  	.driver		= {
>  		.name	= "twl4030_pwrbutton",
>  		.owner	= THIS_MODULE,
> +		.of_match_table = of_match_ptr(twl4030_pwrbutton_dt_match_table),

If you try to compile this driver with config !CONFIG_OF it will not work in
this way.

>  	},
>  };
> -
> -module_platform_driver_probe(twl4030_pwrbutton_driver,
> -			twl4030_pwrbutton_probe);
> +module_platform_driver(twl4030_pwrbutton_driver);
>  
>  MODULE_ALIAS("platform:twl4030_pwrbutton");
>  MODULE_DESCRIPTION("Triton2 Power Button");
> 


-- 
Péter
--
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

* Re: [PATCHv6 1/3] Input: twl4030-pwrbutton - add device tree support
From: Florian Vaussard @ 2013-10-25 12:40 UTC (permalink / raw)
  To: Sebastian Reichel, Sebastian Reichel, Dmitry Torokhov
  Cc: Grant Likely, Rob Herring, Peter Ujfalusi, Sachin Kamat,
	linux-input, linux-kernel, devicetree
In-Reply-To: <1382626126-12565-2-git-send-email-sre@debian.org>

Hello,

On 10/24/2013 04:48 PM, Sebastian Reichel wrote:
> Add device tree support for twl4030 power button driver.
> 
> Signed-off-by: Sebastian Reichel <sre@debian.org>

I tested on my OMAP3 board, the button event works as expected. So for
the DT boot, feel free to add my:

Tested-by: Florian Vaussard <florian.vaussard@epfl.ch>

Best regards,

Florian

^ permalink raw reply

* Re: [PATCH] HID: hid-multitouch: add support for SiS panels
From: Forest Bond @ 2013-10-25 11:56 UTC (permalink / raw)
  To: Jiri Kosina; +Cc: linux-input
In-Reply-To: <alpine.LNX.2.00.1310250947340.30926@pobox.suse.cz>

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

Hi Jiri,

On Fri, Oct 25, 2013 at 09:47:55AM +0100, Jiri Kosina wrote:
> On Mon, 21 Oct 2013, Forest Bond wrote:
> 
> > From: Forest Bond <forest.bond@rapidrollout.com>
> > 
> > Signed-off-by: Forest Bond <forest.bond@rapidrollout.com>
> 
> Added changelog (I don't like commits without changelog) and applied.

Noted.  Thanks!

-Forest
-- 
Forest Bond
http://www.forestbond.com/
http://www.rapidrollout.com/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ 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