* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 16:04 UTC (permalink / raw)
To: Matthew Garrett
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328090053.GA10597@srcf.ucam.org>
On Fri, Mar 28, 2014 at 09:00:53AM +0000, Matthew Garrett wrote:
> On Fri, Mar 28, 2014 at 01:52:07AM -0700, Dmitry Torokhov wrote:
> >
> > Are we even certain that they will be consistent in use of these special
> > PNP ID's? Maybe you should really do DMI match...
>
> The Windows drivers bind on PNP IDs, not DMI.
OK, fair enough.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 06/10] regulator: AXP20x: Add support for regulators subsystem
From: Mark Brown @ 2014-03-28 13:39 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1395955764-18103-7-git-send-email-carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 675 bytes --]
On Thu, Mar 27, 2014 at 10:29:20PM +0100, Carlo Caione wrote:
> +static int axp20x_set_suspend_voltage(struct regulator_dev *rdev, int uV)
> +{
> + int sel = regulator_map_voltage_iterate(rdev, uV, uV);
> +
> + if (sel < 0)
> + return sel;
> +
> + return regulator_set_voltage_sel_regmap(rdev, sel);
> +}
This is fairly obviously broken - it's overwriting the normal runtime
value, this will disrupt the running system if we want the value we use
on suspend is different to the value we want at runtime.
Think about it - if this was a sane thing to do the core would just do
it without needing driver specific code, we already know how to set the
voltage for the device.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 07/10] ARM: sunxi: dt: Add x-powers-axp209.dtsi file
From: Mark Brown @ 2014-03-28 13:34 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1395955764-18103-8-git-send-email-carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1745 bytes --]
On Thu, Mar 27, 2014 at 10:29:21PM +0100, Carlo Caione wrote:
> This dtsi describes the axp209 PMIC, and is to be included from inside
> the i2c controller node to which the axp209 is connected.
> arch/arm/boot/dts/x-powers-axp209.dtsi | 54 ++++++++++++++++++++++++++++++++++
Something is wrong here. Either the changelog is inaccurate or this is
broken, either way this should be cleaned up because this looks like
very bad practice. If this is a common include file for boards derived
from some reference design then the changelog is misleading and the DT
should be clearer about the board tie ins. Otherwise the .dtsi is
defining what should be board specific parameters.
The fact that the DT names everything after the regulator on the PMIC
and not after the supply names on the board is especially suspicious and
glancing at a couple of the regulators it looks like the constraints
here are the maximum ranges the PMIC supports rather than anything to do
with any board.
Please take a step back and think about what the regulator constraints
are intended to do - they're about matching the regulator capabilities
to the board since it is rare for boards to be able to do everything the
regulator can support. Duplicating the basic device capabilities into
the DT would be a pointless waste of time.
> + axp_dcdc2: dcdc2 {
> + regulator-min-microvolt = <700000>;
> + regulator-max-microvolt = <2275000>;
> + regulator-always-on;
> + };
What is this configuration actually for - what guarantee do we have that
the above is safe for a given board using this regulator?
In general I'd expect to see anything specifying variability for a
regulator to also include the relevant consumer node.
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
From: Mark Brown @ 2014-03-28 13:12 UTC (permalink / raw)
To: Maxime Ripard
Cc: Carlo Caione, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140328125412.GT6120@lukather>
[-- Attachment #1: Type: text/plain, Size: 335 bytes --]
On Fri, Mar 28, 2014 at 01:54:12PM +0100, Maxime Ripard wrote:
> On Fri, Mar 28, 2014 at 11:38:39AM +0000, Mark Brown wrote:
> > Hang on, what is an include file setting up regulators for?
> Look at patch 7. To share the regulators declaration across all boards
> and avoid duplication.
Oh, wonderful. I'll go and reply to that...
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
From: Maxime Ripard @ 2014-03-28 12:54 UTC (permalink / raw)
To: Mark Brown
Cc: Carlo Caione, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140328113839.GB30768-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 601 bytes --]
On Fri, Mar 28, 2014 at 11:38:39AM +0000, Mark Brown wrote:
> On Fri, Mar 28, 2014 at 11:11:46AM +0100, Maxime Ripard wrote:
>
> > Here, you would just include the dtsi at the top of the file, and in
> > the DTSI, you would have something like:
>
> > &axp209 {
> > regulators {
> > ...
> > }
> > }
>
> Hang on, what is an include file setting up regulators for?
Look at patch 7. To share the regulators declaration across all boards
and avoid duplication.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
From: Mark Brown @ 2014-03-28 11:38 UTC (permalink / raw)
To: Maxime Ripard
Cc: Carlo Caione, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <20140328101146.GP6120@lukather>
[-- Attachment #1: Type: text/plain, Size: 293 bytes --]
On Fri, Mar 28, 2014 at 11:11:46AM +0100, Maxime Ripard wrote:
> Here, you would just include the dtsi at the top of the file, and in
> the DTSI, you would have something like:
> &axp209 {
> regulators {
> ...
> }
> }
Hang on, what is an include file setting up regulators for?
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 08/10] ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
From: Maxime Ripard @ 2014-03-28 10:11 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1395955764-18103-9-git-send-email-carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2203 bytes --]
On Thu, Mar 27, 2014 at 10:29:22PM +0100, Carlo Caione wrote:
> Signed-off-by: Hans de Goede <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> ---
> arch/arm/boot/dts/sun4i-a10-a1000.dts | 13 +++++++++++++
> arch/arm/boot/dts/sun4i-a10-cubieboard.dts | 13 +++++++++++++
> arch/arm/boot/dts/sun4i-a10-hackberry.dts | 19 +++++++++++++++++++
> arch/arm/boot/dts/sun4i-a10-inet97fv2.dts | 13 +++++++++++++
> arch/arm/boot/dts/sun4i-a10-mini-xplus.dts | 19 +++++++++++++++++++
> arch/arm/boot/dts/sun4i-a10-olinuxino-lime.dts | 19 +++++++++++++++++++
> arch/arm/boot/dts/sun4i-a10-pcduino.dts | 13 +++++++++++++
> arch/arm/boot/dts/sun7i-a20-cubieboard2.dts | 14 ++++++++++++++
> arch/arm/boot/dts/sun7i-a20-cubietruck.dts | 15 +++++++++++++++
> arch/arm/boot/dts/sun7i-a20-olinuxino-micro.dts | 14 ++++++++++++++
> 10 files changed, 152 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> index fa746aea..cf18c4d 100644
> --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts
> +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts
> @@ -88,6 +88,19 @@
> pinctrl-names = "default";
> pinctrl-0 = <&i2c0_pins_a>;
> status = "okay";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + axp: axp20x@34 {
> + reg = <0x34>;
> + interrupts = <0>;
> +
> + compatible = "x-powers,axp209";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> +
> + /include/ "x-powers-axp209.dtsi"
> + };
> };
> };
Actually, you can make the whole thing much more robust by using the
&label syntax.
Here, you would just include the dtsi at the top of the file, and in
the DTSI, you would have something like:
&axp209 {
regulators {
...
}
}
(Given that the node in your DTS is axp209: pmic@something like
Chen-Yu suggested)
You don't rely any more on the position of the include, and you use a
standard DT syntax.
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/10] mfd: AXP20x: Add mfd driver for AXP20x PMIC
From: Lee Jones @ 2014-03-28 9:21 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw,
maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8,
hdegoede-H+wXaHxf7aLQT0dZR+AlfA, emilio-0Z03zUJReD5OxF6Tv1QG9Q,
wens-jdAy2FN1RRM, sameo-VuQAYsv1563Yd54FQh9/CA,
dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
linux-input-u79uwXL29TY76Z2rM5mHXA,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
lgirdwood-Re5JQEeQqe8AvxtiuMwx3w, broonie-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1395955764-18103-2-git-send-email-carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> This patch introduces the preliminary support for PMICs X-Powers AXP202
> and AXP209. The AXP209 and AXP202 are the PMUs (Power Management Unit)
> used by A10, A13 and A20 SoCs and developed by X-Powers, a sister company
> of Allwinner.
>
> The core enables support for two subsystems:
> - PEK (Power Enable Key)
> - Regulators
>
> Signed-off-by: Carlo Caione <carlo-KA+7E9HrN00dnm+yROfE0A@public.gmane.org>
> ---
> drivers/mfd/Kconfig | 12 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/axp20x.c | 240 +++++++++++++++++++++++++++++++++++++++++++++
> include/linux/mfd/axp20x.h | 180 ++++++++++++++++++++++++++++++++++
> 4 files changed, 433 insertions(+)
> create mode 100644 drivers/mfd/axp20x.c
> create mode 100644 include/linux/mfd/axp20x.h
>
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 49bb445..24ba61a 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -59,6 +59,18 @@ config MFD_AAT2870_CORE
> additional drivers must be enabled in order to use the
> functionality of the device.
>
> +config MFD_AXP20X
> + bool "X-Powers AXP20X"
> + select MFD_CORE
> + select REGMAP_I2C
> + select REGMAP_IRQ
> + depends on I2C=y
> + help
> + If you say Y here you get support for the AXP20X.
> + This driver provides common support for accessing the device,
> + additional drivers must be enabled in order to use the
> + functionality of the device.
Please tell us what this device is and what sub-devices are available?
[...]
> diff --git a/drivers/mfd/axp20x.c b/drivers/mfd/axp20x.c
> new file mode 100644
> index 0000000..f77a663
> --- /dev/null
> +++ b/drivers/mfd/axp20x.c
> @@ -0,0 +1,240 @@
> +/*
> + * axp20x.c - MFD core driver for the X-Powers AXP202 and AXP209
... which consist of ...
[...]
> +static struct resource axp20x_pek_resources[] = {
> + {
> + .name = "PEK_DBR",
> + .start = AXP20X_IRQ_PEK_RIS_EDGE,
> + .end = AXP20X_IRQ_PEK_RIS_EDGE,
> + .flags = IORESOURCE_IRQ,
> + }, {
> + .name = "PEK_DBF",
> + .start = AXP20X_IRQ_PEK_FAL_EDGE,
> + .end = AXP20X_IRQ_PEK_FAL_EDGE,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
Have you considered doing this in the Device Tree? It's a lot less
code/overhead.
> +static const struct i2c_device_id axp20x_i2c_id[] = {
> + { },
> +};
> +MODULE_DEVICE_TABLE(i2c, axp20x_i2c_id);
We really should consider changing the I2C subsystem!
Can you add a comment here describing why we have to add this
seemingly pointless empty struct please?
> +static struct mfd_cell axp20x_cells[] = {
> + {
> + .name = "axp20x-pek",
> + .num_resources = ARRAY_SIZE(axp20x_pek_resources),
> + .resources = axp20x_pek_resources,
> + }, {
> + .name = "axp20x-regulator",
> + },
> +};
Do these drivers don't look inside the DTB at all?
[...]
> + of_id = of_match_device(axp20x_of_match, &i2c->dev);
> + if (!of_id) {
> + dev_err(&i2c->dev, "Unable to setup AXP20X data\n");
> + return -ENODEV;
> + }
> + axp20x->variant = (int) of_id->data;
'variant' needs to be a (unsigned?) long or it will break on 64bit
architectures.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Hans de Goede @ 2014-03-28 9:05 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328085207.GN22093@core.coreip.homeip.net>
Hi,
On 03/28/2014 09:52 AM, Dmitry Torokhov wrote:
> On Fri, Mar 28, 2014 at 09:29:50AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 03/28/2014 09:17 AM, Dmitry Torokhov wrote:
>>> On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote:
>>>> Hi,
>>>>
>>>> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote:
>>>>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote:
>>>>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote:
>>>>>>
>>>>>>> Which in the end turns out to be much nicer too, since it gets rid of needing
>>>>>>> a udev-helper too. After this much too long introduction I'll let the patches
>>>>>>> speak for themselves.
>>>>>>
>>>>>> Yeah, I was coming to the conclusion that this was probably the best we
>>>>>> could do. It's unfortunate that "id" is already in use - we'd be able to
>>>>>> get away without any X server modifications otherwise.
>>>>>>
>>>>>> Long term we probably still want to tie serio devices to the ACPI
>>>>>> devices in case the vendor provides power management calls there, but we
>>>>>> can leave that until there's an actual example.
>>>>>
>>>>> I am still unsure if we shoudl be adding these new IDs to serio core...
>>>>> Can't the X driver take a peek at ACPI devices on it's own?
>>>>
>>>> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx
>>>> device is the serio bus host.
>>>
>>> Practically speaking you should not care - there is only one touchpad in
>>> Lenovos.
>>
>> So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking
>> for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution,
>> which reminds me of the good old days of random poking io-ports to probe stuff.
>>
>> We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a
>> system, assuming that if it contains a pnp-id we're interested in it happens to
>> belong to the input device we're enumerating at that time.
>
> Are we even certain that they will be consistent in use of these special
> PNP ID's? Maybe you should really do DMI match...
They are more consistent with PNP ids then with their DMI strings, that is if they
re-use the exact same touchpad the PNP id stays the same. So PNP ids give us a
better chance of things just working without the user needing to wait for a distro
update which has the new DMI strings in there.
And yes so far the PNP ids use we want to-do is Lenovo only, as we've just discovered
that they are actually storing some useful info in there. If we need quirks for other
vendor laptops in the future, chances are we may want to do pnp-id matching there too.
I've deliberately tried to write this patch-set so that it adds a generic way to export
extra info the platform specific code may have. I could have added a pnp-id pointer to
the serio struct, but I deliberately went with a free-form string so as to make this
as generic as possible. As Matthew has already said we may want to also export extra
info from devicetree through this mechanism for example.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Matthew Garrett @ 2014-03-28 9:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328085207.GN22093@core.coreip.homeip.net>
On Fri, Mar 28, 2014 at 01:52:07AM -0700, Dmitry Torokhov wrote:
>
> Are we even certain that they will be consistent in use of these special
> PNP ID's? Maybe you should really do DMI match...
The Windows drivers bind on PNP IDs, not DMI.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 8:52 UTC (permalink / raw)
To: Hans de Goede
Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <533532FE.2000304@redhat.com>
On Fri, Mar 28, 2014 at 09:29:50AM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/28/2014 09:17 AM, Dmitry Torokhov wrote:
> > On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote:
> >> Hi,
> >>
> >> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote:
> >>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote:
> >>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote:
> >>>>
> >>>>> Which in the end turns out to be much nicer too, since it gets rid of needing
> >>>>> a udev-helper too. After this much too long introduction I'll let the patches
> >>>>> speak for themselves.
> >>>>
> >>>> Yeah, I was coming to the conclusion that this was probably the best we
> >>>> could do. It's unfortunate that "id" is already in use - we'd be able to
> >>>> get away without any X server modifications otherwise.
> >>>>
> >>>> Long term we probably still want to tie serio devices to the ACPI
> >>>> devices in case the vendor provides power management calls there, but we
> >>>> can leave that until there's an actual example.
> >>>
> >>> I am still unsure if we shoudl be adding these new IDs to serio core...
> >>> Can't the X driver take a peek at ACPI devices on it's own?
> >>
> >> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx
> >> device is the serio bus host.
> >
> > Practically speaking you should not care - there is only one touchpad in
> > Lenovos.
>
> So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking
> for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution,
> which reminds me of the good old days of random poking io-ports to probe stuff.
>
> We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a
> system, assuming that if it contains a pnp-id we're interested in it happens to
> belong to the input device we're enumerating at that time.
Are we even certain that they will be consistent in use of these special
PNP ID's? Maybe you should really do DMI match...
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 8:50 UTC (permalink / raw)
To: Matthew Garrett
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328082704.GC6801@srcf.ucam.org>
On Fri, Mar 28, 2014 at 08:27:04AM +0000, Matthew Garrett wrote:
> On Fri, Mar 28, 2014 at 01:24:52AM -0700, Dmitry Torokhov wrote:
> > On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote:
> > > On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote:
> > >
> > > > I am still unsure if we shoudl be adding these new IDs to serio core...
> > > > Can't the X driver take a peek at ACPI devices on it's own?
> > >
> > > In the (admittedly unlikely) event of multiple PS/2 trackpads, how do
> > > you know which one corresponds to which ACPI device?
> >
> > So far I have not seen a single instance of a laptop with 2 touchpads
> > and I doubt external PS/2 ones will ever make come back.
>
> Right, we can hack around it based on what we've seen so far. But it
> seems more attractive to fix it in such a way that we won't behave
> inappropriately even if someone does do something utterly unexpected in
> future. For instance, some ARM devices have i8042-like serio - if the
> underlying device is exposed in device tree, it'd be nice to be able to
> expose that to userspace without having to modify the core X code.
I wonder if on ARM they have the same split description for AUX and KBD
ports or of they have proper i8042 parent. ACPI way of describing PS/2
devices is ugly - keyboard gets ports and mice/touchpads are portless
and everyone knows that they should use the same as keyboard.
I am unconvinced that just storing a string as firmware ID would solve
anything other than quirky Lenovos.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: gpio-beeper: Simplify GPIO handling
From: Dmitry Torokhov @ 2014-03-28 8:38 UTC (permalink / raw)
To: Alexander Shiyan; +Cc: linux-input
In-Reply-To: <1395164746-14395-1-git-send-email-shc_work@mail.ru>
Hi Alexander,
On Tue, Mar 18, 2014 at 09:45:46PM +0400, Alexander Shiyan wrote:
> This patch simplifies GPIO handling in the driver by using GPIO
> functions based on descriptors. As a result this driver now can
> be used for boards without DT support.
>
> Signed-off-by: Alexander Shiyan <shc_work@mail.ru>
> ---
> drivers/input/misc/Kconfig | 2 +-
> drivers/input/misc/gpio-beeper.c | 37 ++++++++++++++-----------------------
> 2 files changed, 15 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
> index 762e6d2..0623e99 100644
> --- a/drivers/input/misc/Kconfig
> +++ b/drivers/input/misc/Kconfig
> @@ -224,7 +224,7 @@ config INPUT_GP2A
>
> config INPUT_GPIO_BEEPER
> tristate "Generic GPIO Beeper support"
> - depends on OF_GPIO
> + depends on GPIOLIB
> help
> Say Y here if you have a beeper connected to a GPIO pin.
>
> diff --git a/drivers/input/misc/gpio-beeper.c b/drivers/input/misc/gpio-beeper.c
> index b757435..a57f15e 100644
> --- a/drivers/input/misc/gpio-beeper.c
> +++ b/drivers/input/misc/gpio-beeper.c
> @@ -1,7 +1,7 @@
> /*
> * Generic GPIO beeper driver
> *
> - * Copyright (C) 2013 Alexander Shiyan <shc_work@mail.ru>
> + * Copyright (C) 2014 Alexander Shiyan <shc_work@mail.ru>
> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -11,7 +11,7 @@
>
> #include <linux/input.h>
> #include <linux/module.h>
> -#include <linux/of_gpio.h>
> +#include <linux/gpio.h>
> #include <linux/workqueue.h>
> #include <linux/platform_device.h>
>
> @@ -19,21 +19,15 @@
>
> struct gpio_beeper {
> struct work_struct work;
> - int gpio;
> - bool active_low;
> - bool beeping;
> + struct gpio_desc *desc;
> + int beeping;
> };
>
> -static void gpio_beeper_toggle(struct gpio_beeper *beep, bool on)
> -{
> - gpio_set_value_cansleep(beep->gpio, on ^ beep->active_low);
> -}
> -
> static void gpio_beeper_work(struct work_struct *work)
> {
> struct gpio_beeper *beep = container_of(work, struct gpio_beeper, work);
>
> - gpio_beeper_toggle(beep, beep->beeping);
> + gpiod_set_value_cansleep(beep->desc, beep->beeping);
> }
>
> static int gpio_beeper_event(struct input_dev *dev, unsigned int type,
> @@ -59,24 +53,24 @@ static void gpio_beeper_close(struct input_dev *input)
> struct gpio_beeper *beep = input_get_drvdata(input);
>
> cancel_work_sync(&beep->work);
> - gpio_beeper_toggle(beep, false);
> + gpiod_set_value_cansleep(beep->desc, 0);
These chunks do not seem to fit patch description and I would argue not
make driver simpler. Please split out.
Thanks!
> }
>
> static int gpio_beeper_probe(struct platform_device *pdev)
> {
> struct gpio_beeper *beep;
> - enum of_gpio_flags flags;
> struct input_dev *input;
> - unsigned long gflags;
> int err;
>
> beep = devm_kzalloc(&pdev->dev, sizeof(*beep), GFP_KERNEL);
> if (!beep)
> return -ENOMEM;
>
> - beep->gpio = of_get_gpio_flags(pdev->dev.of_node, 0, &flags);
> - if (!gpio_is_valid(beep->gpio))
> - return beep->gpio;
> + beep->desc = devm_gpiod_get(&pdev->dev, NULL);
> + if (!beep->desc)
> + return -EINVAL;
> + if (IS_ERR(beep->desc))
> + return PTR_ERR(beep->desc);
>
> input = devm_input_allocate_device(&pdev->dev);
> if (!input)
> @@ -94,10 +88,7 @@ static int gpio_beeper_probe(struct platform_device *pdev)
>
> input_set_capability(input, EV_SND, SND_BELL);
>
> - beep->active_low = flags & OF_GPIO_ACTIVE_LOW;
> - gflags = beep->active_low ? GPIOF_OUT_INIT_HIGH : GPIOF_OUT_INIT_LOW;
> -
> - err = devm_gpio_request_one(&pdev->dev, beep->gpio, gflags, pdev->name);
> + err = gpiod_direction_output(beep->desc, 0);
> if (err)
> return err;
>
> @@ -106,7 +97,7 @@ static int gpio_beeper_probe(struct platform_device *pdev)
> return input_register_device(input);
> }
>
> -static struct of_device_id gpio_beeper_of_match[] = {
> +static struct of_device_id __maybe_unused gpio_beeper_of_match[] = {
> { .compatible = BEEPER_MODNAME, },
> { }
> };
> @@ -116,7 +107,7 @@ static struct platform_driver gpio_beeper_platform_driver = {
> .driver = {
> .name = BEEPER_MODNAME,
> .owner = THIS_MODULE,
> - .of_match_table = gpio_beeper_of_match,
> + .of_match_table = of_match_ptr(gpio_beeper_of_match),
> },
> .probe = gpio_beeper_probe,
> };
> --
> 1.8.3.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/3] input: remove obsolete tnetv107x drivers
From: Dmitry Torokhov @ 2014-03-28 8:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arm-kernel, linux-kernel, davinci-linux-open-source,
Sekhar Nori, linux-input
In-Reply-To: <1395154561-1199121-4-git-send-email-arnd@arndb.de>
On Tue, Mar 18, 2014 at 03:56:01PM +0100, Arnd Bergmann wrote:
> The tnetv107x platform is getting removed, so the touchscreen
> and keypad drivers for this platform will no longer be needed
> either.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: Sekhar Nori <nsekhar@ti.com>
> Acked-by: Kevin Hilman <khilman@linaro.org>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: linux-input@vger.kernel.org
Applied, thank you.
> ---
> drivers/input/keyboard/Kconfig | 10 -
> drivers/input/keyboard/Makefile | 1 -
> drivers/input/keyboard/tnetv107x-keypad.c | 329 -------------------------
> drivers/input/touchscreen/Kconfig | 9 -
> drivers/input/touchscreen/Makefile | 1 -
> drivers/input/touchscreen/tnetv107x-ts.c | 384 ------------------------------
> 6 files changed, 734 deletions(-)
> delete mode 100644 drivers/input/keyboard/tnetv107x-keypad.c
> delete mode 100644 drivers/input/touchscreen/tnetv107x-ts.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index a673c9f..935dcaf 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -595,16 +595,6 @@ config KEYBOARD_TC3589X
> To compile this driver as a module, choose M here: the
> module will be called tc3589x-keypad.
>
> -config KEYBOARD_TNETV107X
> - tristate "TI TNETV107X keypad support"
> - depends on ARCH_DAVINCI_TNETV107X
> - select INPUT_MATRIXKMAP
> - help
> - Say Y here if you want to use the TNETV107X keypad.
> -
> - To compile this driver as a module, choose M here: the
> - module will be called tnetv107x-keypad.
> -
> config KEYBOARD_TWL4030
> tristate "TI TWL4030/TWL5030/TPS659x0 keypad support"
> depends on TWL4030_CORE
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index a699b61..81014d9 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -53,7 +53,6 @@ obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
> obj-$(CONFIG_KEYBOARD_SUNKBD) += sunkbd.o
> obj-$(CONFIG_KEYBOARD_TC3589X) += tc3589x-keypad.o
> obj-$(CONFIG_KEYBOARD_TEGRA) += tegra-kbc.o
> -obj-$(CONFIG_KEYBOARD_TNETV107X) += tnetv107x-keypad.o
> obj-$(CONFIG_KEYBOARD_TWL4030) += twl4030_keypad.o
> obj-$(CONFIG_KEYBOARD_XTKBD) += xtkbd.o
> obj-$(CONFIG_KEYBOARD_W90P910) += w90p910_keypad.o
> diff --git a/drivers/input/keyboard/tnetv107x-keypad.c b/drivers/input/keyboard/tnetv107x-keypad.c
> deleted file mode 100644
> index 086511c..0000000
> --- a/drivers/input/keyboard/tnetv107x-keypad.c
> +++ /dev/null
> @@ -1,329 +0,0 @@
> -/*
> - * Texas Instruments TNETV107X Keypad Driver
> - *
> - * Copyright (C) 2010 Texas Instruments
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation version 2.
> - *
> - * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> - * kind, whether express or implied; without even the implied warranty
> - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/err.h>
> -#include <linux/errno.h>
> -#include <linux/input.h>
> -#include <linux/platform_device.h>
> -#include <linux/interrupt.h>
> -#include <linux/slab.h>
> -#include <linux/delay.h>
> -#include <linux/io.h>
> -#include <linux/clk.h>
> -#include <linux/input/matrix_keypad.h>
> -#include <linux/module.h>
> -
> -#define BITS(x) (BIT(x) - 1)
> -
> -#define KEYPAD_ROWS 9
> -#define KEYPAD_COLS 9
> -
> -#define DEBOUNCE_MIN 0x400ul
> -#define DEBOUNCE_MAX 0x3ffffffful
> -
> -struct keypad_regs {
> - u32 rev;
> - u32 mode;
> - u32 mask;
> - u32 pol;
> - u32 dclock;
> - u32 rclock;
> - u32 stable_cnt;
> - u32 in_en;
> - u32 out;
> - u32 out_en;
> - u32 in;
> - u32 lock;
> - u32 pres[3];
> -};
> -
> -#define keypad_read(kp, reg) __raw_readl(&(kp)->regs->reg)
> -#define keypad_write(kp, reg, val) __raw_writel(val, &(kp)->regs->reg)
> -
> -struct keypad_data {
> - struct input_dev *input_dev;
> - struct resource *res;
> - struct keypad_regs __iomem *regs;
> - struct clk *clk;
> - struct device *dev;
> - spinlock_t lock;
> - int irq_press;
> - int irq_release;
> - int rows, cols, row_shift;
> - int debounce_ms, active_low;
> - u32 prev_keys[3];
> - unsigned short keycodes[];
> -};
> -
> -static irqreturn_t keypad_irq(int irq, void *data)
> -{
> - struct keypad_data *kp = data;
> - int i, bit, val, row, col, code;
> - unsigned long flags;
> - u32 curr_keys[3];
> - u32 change;
> -
> - spin_lock_irqsave(&kp->lock, flags);
> -
> - memset(curr_keys, 0, sizeof(curr_keys));
> - if (irq == kp->irq_press)
> - for (i = 0; i < 3; i++)
> - curr_keys[i] = keypad_read(kp, pres[i]);
> -
> - for (i = 0; i < 3; i++) {
> - change = curr_keys[i] ^ kp->prev_keys[i];
> -
> - while (change) {
> - bit = fls(change) - 1;
> - change ^= BIT(bit);
> - val = curr_keys[i] & BIT(bit);
> - bit += i * 32;
> - row = bit / KEYPAD_COLS;
> - col = bit % KEYPAD_COLS;
> -
> - code = MATRIX_SCAN_CODE(row, col, kp->row_shift);
> - input_event(kp->input_dev, EV_MSC, MSC_SCAN, code);
> - input_report_key(kp->input_dev, kp->keycodes[code],
> - val);
> - }
> - }
> - input_sync(kp->input_dev);
> - memcpy(kp->prev_keys, curr_keys, sizeof(curr_keys));
> -
> - if (irq == kp->irq_press)
> - keypad_write(kp, lock, 0); /* Allow hardware updates */
> -
> - spin_unlock_irqrestore(&kp->lock, flags);
> -
> - return IRQ_HANDLED;
> -}
> -
> -static int keypad_start(struct input_dev *dev)
> -{
> - struct keypad_data *kp = input_get_drvdata(dev);
> - unsigned long mask, debounce, clk_rate_khz;
> - unsigned long flags;
> -
> - clk_enable(kp->clk);
> - clk_rate_khz = clk_get_rate(kp->clk) / 1000;
> -
> - spin_lock_irqsave(&kp->lock, flags);
> -
> - /* Initialize device registers */
> - keypad_write(kp, mode, 0);
> -
> - mask = BITS(kp->rows) << KEYPAD_COLS;
> - mask |= BITS(kp->cols);
> - keypad_write(kp, mask, ~mask);
> -
> - keypad_write(kp, pol, kp->active_low ? 0 : 0x3ffff);
> - keypad_write(kp, stable_cnt, 3);
> -
> - debounce = kp->debounce_ms * clk_rate_khz;
> - debounce = clamp(debounce, DEBOUNCE_MIN, DEBOUNCE_MAX);
> - keypad_write(kp, dclock, debounce);
> - keypad_write(kp, rclock, 4 * debounce);
> -
> - keypad_write(kp, in_en, 1);
> -
> - spin_unlock_irqrestore(&kp->lock, flags);
> -
> - return 0;
> -}
> -
> -static void keypad_stop(struct input_dev *dev)
> -{
> - struct keypad_data *kp = input_get_drvdata(dev);
> -
> - synchronize_irq(kp->irq_press);
> - synchronize_irq(kp->irq_release);
> - clk_disable(kp->clk);
> -}
> -
> -static int keypad_probe(struct platform_device *pdev)
> -{
> - const struct matrix_keypad_platform_data *pdata;
> - const struct matrix_keymap_data *keymap_data;
> - struct device *dev = &pdev->dev;
> - struct keypad_data *kp;
> - int error = 0, sz, row_shift;
> - u32 rev = 0;
> -
> - pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> - dev_err(dev, "cannot find device data\n");
> - return -EINVAL;
> - }
> -
> - keymap_data = pdata->keymap_data;
> - if (!keymap_data) {
> - dev_err(dev, "cannot find keymap data\n");
> - return -EINVAL;
> - }
> -
> - row_shift = get_count_order(pdata->num_col_gpios);
> - sz = offsetof(struct keypad_data, keycodes);
> - sz += (pdata->num_row_gpios << row_shift) * sizeof(kp->keycodes[0]);
> - kp = kzalloc(sz, GFP_KERNEL);
> - if (!kp) {
> - dev_err(dev, "cannot allocate device info\n");
> - return -ENOMEM;
> - }
> -
> - kp->dev = dev;
> - kp->rows = pdata->num_row_gpios;
> - kp->cols = pdata->num_col_gpios;
> - kp->row_shift = row_shift;
> - platform_set_drvdata(pdev, kp);
> - spin_lock_init(&kp->lock);
> -
> - kp->irq_press = platform_get_irq_byname(pdev, "press");
> - kp->irq_release = platform_get_irq_byname(pdev, "release");
> - if (kp->irq_press < 0 || kp->irq_release < 0) {
> - dev_err(dev, "cannot determine device interrupts\n");
> - error = -ENODEV;
> - goto error_res;
> - }
> -
> - kp->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!kp->res) {
> - dev_err(dev, "cannot determine register area\n");
> - error = -ENODEV;
> - goto error_res;
> - }
> -
> - if (!request_mem_region(kp->res->start, resource_size(kp->res),
> - pdev->name)) {
> - dev_err(dev, "cannot claim register memory\n");
> - kp->res = NULL;
> - error = -EINVAL;
> - goto error_res;
> - }
> -
> - kp->regs = ioremap(kp->res->start, resource_size(kp->res));
> - if (!kp->regs) {
> - dev_err(dev, "cannot map register memory\n");
> - error = -ENOMEM;
> - goto error_map;
> - }
> -
> - kp->clk = clk_get(dev, NULL);
> - if (IS_ERR(kp->clk)) {
> - dev_err(dev, "cannot claim device clock\n");
> - error = PTR_ERR(kp->clk);
> - goto error_clk;
> - }
> -
> - error = request_threaded_irq(kp->irq_press, NULL, keypad_irq,
> - IRQF_ONESHOT, dev_name(dev), kp);
> - if (error < 0) {
> - dev_err(kp->dev, "Could not allocate keypad press key irq\n");
> - goto error_irq_press;
> - }
> -
> - error = request_threaded_irq(kp->irq_release, NULL, keypad_irq,
> - IRQF_ONESHOT, dev_name(dev), kp);
> - if (error < 0) {
> - dev_err(kp->dev, "Could not allocate keypad release key irq\n");
> - goto error_irq_release;
> - }
> -
> - kp->input_dev = input_allocate_device();
> - if (!kp->input_dev) {
> - dev_err(dev, "cannot allocate input device\n");
> - error = -ENOMEM;
> - goto error_input;
> - }
> -
> - kp->input_dev->name = pdev->name;
> - kp->input_dev->dev.parent = &pdev->dev;
> - kp->input_dev->open = keypad_start;
> - kp->input_dev->close = keypad_stop;
> -
> - clk_enable(kp->clk);
> - rev = keypad_read(kp, rev);
> - kp->input_dev->id.bustype = BUS_HOST;
> - kp->input_dev->id.product = ((rev >> 8) & 0x07);
> - kp->input_dev->id.version = ((rev >> 16) & 0xfff);
> - clk_disable(kp->clk);
> -
> - error = matrix_keypad_build_keymap(keymap_data, NULL,
> - kp->rows, kp->cols,
> - kp->keycodes, kp->input_dev);
> - if (error) {
> - dev_err(dev, "Failed to build keymap\n");
> - goto error_reg;
> - }
> -
> - if (!pdata->no_autorepeat)
> - kp->input_dev->evbit[0] |= BIT_MASK(EV_REP);
> - input_set_capability(kp->input_dev, EV_MSC, MSC_SCAN);
> -
> - input_set_drvdata(kp->input_dev, kp);
> -
> - error = input_register_device(kp->input_dev);
> - if (error < 0) {
> - dev_err(dev, "Could not register input device\n");
> - goto error_reg;
> - }
> -
> - return 0;
> -
> -
> -error_reg:
> - input_free_device(kp->input_dev);
> -error_input:
> - free_irq(kp->irq_release, kp);
> -error_irq_release:
> - free_irq(kp->irq_press, kp);
> -error_irq_press:
> - clk_put(kp->clk);
> -error_clk:
> - iounmap(kp->regs);
> -error_map:
> - release_mem_region(kp->res->start, resource_size(kp->res));
> -error_res:
> - kfree(kp);
> - return error;
> -}
> -
> -static int keypad_remove(struct platform_device *pdev)
> -{
> - struct keypad_data *kp = platform_get_drvdata(pdev);
> -
> - free_irq(kp->irq_press, kp);
> - free_irq(kp->irq_release, kp);
> - input_unregister_device(kp->input_dev);
> - clk_put(kp->clk);
> - iounmap(kp->regs);
> - release_mem_region(kp->res->start, resource_size(kp->res));
> - kfree(kp);
> -
> - return 0;
> -}
> -
> -static struct platform_driver keypad_driver = {
> - .probe = keypad_probe,
> - .remove = keypad_remove,
> - .driver.name = "tnetv107x-keypad",
> - .driver.owner = THIS_MODULE,
> -};
> -module_platform_driver(keypad_driver);
> -
> -MODULE_AUTHOR("Cyril Chemparathy");
> -MODULE_DESCRIPTION("TNETV107X Keypad Driver");
> -MODULE_ALIAS("platform:tnetv107x-keypad");
> -MODULE_LICENSE("GPL");
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 07e9e82..68edc9d 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -514,15 +514,6 @@ config TOUCHSCREEN_MIGOR
> To compile this driver as a module, choose M here: the
> module will be called migor_ts.
>
> -config TOUCHSCREEN_TNETV107X
> - tristate "TI TNETV107X touchscreen support"
> - depends on ARCH_DAVINCI_TNETV107X
> - help
> - Say Y here if you want to use the TNETV107X touchscreen.
> -
> - To compile this driver as a module, choose M here: the
> - module will be called tnetv107x-ts.
> -
> config TOUCHSCREEN_TOUCHRIGHT
> tristate "Touchright serial touchscreen"
> select SERIO
> diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
> index 62801f2..4bc954b 100644
> --- a/drivers/input/touchscreen/Makefile
> +++ b/drivers/input/touchscreen/Makefile
> @@ -56,7 +56,6 @@ obj-$(CONFIG_TOUCHSCREEN_ST1232) += st1232.o
> obj-$(CONFIG_TOUCHSCREEN_STMPE) += stmpe-ts.o
> obj-$(CONFIG_TOUCHSCREEN_SUR40) += sur40.o
> obj-$(CONFIG_TOUCHSCREEN_TI_AM335X_TSC) += ti_am335x_tsc.o
> -obj-$(CONFIG_TOUCHSCREEN_TNETV107X) += tnetv107x-ts.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHIT213) += touchit213.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHRIGHT) += touchright.o
> obj-$(CONFIG_TOUCHSCREEN_TOUCHWIN) += touchwin.o
> diff --git a/drivers/input/touchscreen/tnetv107x-ts.c b/drivers/input/touchscreen/tnetv107x-ts.c
> deleted file mode 100644
> index c47827a..0000000
> --- a/drivers/input/touchscreen/tnetv107x-ts.c
> +++ /dev/null
> @@ -1,384 +0,0 @@
> -/*
> - * Texas Instruments TNETV107X Touchscreen Driver
> - *
> - * Copyright (C) 2010 Texas Instruments
> - *
> - * This program is free software; you can redistribute it and/or
> - * modify it under the terms of the GNU General Public License as
> - * published by the Free Software Foundation version 2.
> - *
> - * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> - * kind, whether express or implied; without even the implied warranty
> - * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> - * GNU General Public License for more details.
> - */
> -
> -#include <linux/module.h>
> -#include <linux/kernel.h>
> -#include <linux/err.h>
> -#include <linux/errno.h>
> -#include <linux/input.h>
> -#include <linux/platform_device.h>
> -#include <linux/interrupt.h>
> -#include <linux/slab.h>
> -#include <linux/delay.h>
> -#include <linux/ctype.h>
> -#include <linux/io.h>
> -#include <linux/clk.h>
> -
> -#include <mach/tnetv107x.h>
> -
> -#define TSC_PENUP_POLL (HZ / 5)
> -#define IDLE_TIMEOUT 100 /* msec */
> -
> -/*
> - * The first and last samples of a touch interval are usually garbage and need
> - * to be filtered out with these devices. The following definitions control
> - * the number of samples skipped.
> - */
> -#define TSC_HEAD_SKIP 1
> -#define TSC_TAIL_SKIP 1
> -#define TSC_SKIP (TSC_HEAD_SKIP + TSC_TAIL_SKIP + 1)
> -#define TSC_SAMPLES (TSC_SKIP + 1)
> -
> -/* Register Offsets */
> -struct tsc_regs {
> - u32 rev;
> - u32 tscm;
> - u32 bwcm;
> - u32 swc;
> - u32 adcchnl;
> - u32 adcdata;
> - u32 chval[4];
> -};
> -
> -/* TSC Mode Configuration Register (tscm) bits */
> -#define WMODE BIT(0)
> -#define TSKIND BIT(1)
> -#define ZMEASURE_EN BIT(2)
> -#define IDLE BIT(3)
> -#define TSC_EN BIT(4)
> -#define STOP BIT(5)
> -#define ONE_SHOT BIT(6)
> -#define SINGLE BIT(7)
> -#define AVG BIT(8)
> -#define AVGNUM(x) (((x) & 0x03) << 9)
> -#define PVSTC(x) (((x) & 0x07) << 11)
> -#define PON BIT(14)
> -#define PONBG BIT(15)
> -#define AFERST BIT(16)
> -
> -/* ADC DATA Capture Register bits */
> -#define DATA_VALID BIT(16)
> -
> -/* Register Access Macros */
> -#define tsc_read(ts, reg) __raw_readl(&(ts)->regs->reg)
> -#define tsc_write(ts, reg, val) __raw_writel(val, &(ts)->regs->reg);
> -#define tsc_set_bits(ts, reg, val) \
> - tsc_write(ts, reg, tsc_read(ts, reg) | (val))
> -#define tsc_clr_bits(ts, reg, val) \
> - tsc_write(ts, reg, tsc_read(ts, reg) & ~(val))
> -
> -struct sample {
> - int x, y, p;
> -};
> -
> -struct tsc_data {
> - struct input_dev *input_dev;
> - struct resource *res;
> - struct tsc_regs __iomem *regs;
> - struct timer_list timer;
> - spinlock_t lock;
> - struct clk *clk;
> - struct device *dev;
> - int sample_count;
> - struct sample samples[TSC_SAMPLES];
> - int tsc_irq;
> -};
> -
> -static int tsc_read_sample(struct tsc_data *ts, struct sample* sample)
> -{
> - int x, y, z1, z2, t, p = 0;
> - u32 val;
> -
> - val = tsc_read(ts, chval[0]);
> - if (val & DATA_VALID)
> - x = val & 0xffff;
> - else
> - return -EINVAL;
> -
> - y = tsc_read(ts, chval[1]) & 0xffff;
> - z1 = tsc_read(ts, chval[2]) & 0xffff;
> - z2 = tsc_read(ts, chval[3]) & 0xffff;
> -
> - if (z1) {
> - t = ((600 * x) * (z2 - z1));
> - p = t / (u32) (z1 << 12);
> - if (p < 0)
> - p = 0;
> - }
> -
> - sample->x = x;
> - sample->y = y;
> - sample->p = p;
> -
> - return 0;
> -}
> -
> -static void tsc_poll(unsigned long data)
> -{
> - struct tsc_data *ts = (struct tsc_data *)data;
> - unsigned long flags;
> - int i, val, x, y, p;
> -
> - spin_lock_irqsave(&ts->lock, flags);
> -
> - if (ts->sample_count >= TSC_SKIP) {
> - input_report_abs(ts->input_dev, ABS_PRESSURE, 0);
> - input_report_key(ts->input_dev, BTN_TOUCH, 0);
> - input_sync(ts->input_dev);
> - } else if (ts->sample_count > 0) {
> - /*
> - * A touch event lasted less than our skip count. Salvage and
> - * report anyway.
> - */
> - for (i = 0, val = 0; i < ts->sample_count; i++)
> - val += ts->samples[i].x;
> - x = val / ts->sample_count;
> -
> - for (i = 0, val = 0; i < ts->sample_count; i++)
> - val += ts->samples[i].y;
> - y = val / ts->sample_count;
> -
> - for (i = 0, val = 0; i < ts->sample_count; i++)
> - val += ts->samples[i].p;
> - p = val / ts->sample_count;
> -
> - input_report_abs(ts->input_dev, ABS_X, x);
> - input_report_abs(ts->input_dev, ABS_Y, y);
> - input_report_abs(ts->input_dev, ABS_PRESSURE, p);
> - input_report_key(ts->input_dev, BTN_TOUCH, 1);
> - input_sync(ts->input_dev);
> - }
> -
> - ts->sample_count = 0;
> -
> - spin_unlock_irqrestore(&ts->lock, flags);
> -}
> -
> -static irqreturn_t tsc_irq(int irq, void *dev_id)
> -{
> - struct tsc_data *ts = (struct tsc_data *)dev_id;
> - struct sample *sample;
> - int index;
> -
> - spin_lock(&ts->lock);
> -
> - index = ts->sample_count % TSC_SAMPLES;
> - sample = &ts->samples[index];
> - if (tsc_read_sample(ts, sample) < 0)
> - goto out;
> -
> - if (++ts->sample_count >= TSC_SKIP) {
> - index = (ts->sample_count - TSC_TAIL_SKIP - 1) % TSC_SAMPLES;
> - sample = &ts->samples[index];
> -
> - input_report_abs(ts->input_dev, ABS_X, sample->x);
> - input_report_abs(ts->input_dev, ABS_Y, sample->y);
> - input_report_abs(ts->input_dev, ABS_PRESSURE, sample->p);
> - if (ts->sample_count == TSC_SKIP)
> - input_report_key(ts->input_dev, BTN_TOUCH, 1);
> - input_sync(ts->input_dev);
> - }
> - mod_timer(&ts->timer, jiffies + TSC_PENUP_POLL);
> -out:
> - spin_unlock(&ts->lock);
> - return IRQ_HANDLED;
> -}
> -
> -static int tsc_start(struct input_dev *dev)
> -{
> - struct tsc_data *ts = input_get_drvdata(dev);
> - unsigned long timeout = jiffies + msecs_to_jiffies(IDLE_TIMEOUT);
> - u32 val;
> -
> - clk_enable(ts->clk);
> -
> - /* Go to idle mode, before any initialization */
> - while (time_after(timeout, jiffies)) {
> - if (tsc_read(ts, tscm) & IDLE)
> - break;
> - }
> -
> - if (time_before(timeout, jiffies)) {
> - dev_warn(ts->dev, "timeout waiting for idle\n");
> - clk_disable(ts->clk);
> - return -EIO;
> - }
> -
> - /* Configure TSC Control register*/
> - val = (PONBG | PON | PVSTC(4) | ONE_SHOT | ZMEASURE_EN);
> - tsc_write(ts, tscm, val);
> -
> - /* Bring TSC out of reset: Clear AFE reset bit */
> - val &= ~(AFERST);
> - tsc_write(ts, tscm, val);
> -
> - /* Configure all pins for hardware control*/
> - tsc_write(ts, bwcm, 0);
> -
> - /* Finally enable the TSC */
> - tsc_set_bits(ts, tscm, TSC_EN);
> -
> - return 0;
> -}
> -
> -static void tsc_stop(struct input_dev *dev)
> -{
> - struct tsc_data *ts = input_get_drvdata(dev);
> -
> - tsc_clr_bits(ts, tscm, TSC_EN);
> - synchronize_irq(ts->tsc_irq);
> - del_timer_sync(&ts->timer);
> - clk_disable(ts->clk);
> -}
> -
> -static int tsc_probe(struct platform_device *pdev)
> -{
> - struct device *dev = &pdev->dev;
> - struct tsc_data *ts;
> - int error = 0;
> - u32 rev = 0;
> -
> - ts = kzalloc(sizeof(struct tsc_data), GFP_KERNEL);
> - if (!ts) {
> - dev_err(dev, "cannot allocate device info\n");
> - return -ENOMEM;
> - }
> -
> - ts->dev = dev;
> - spin_lock_init(&ts->lock);
> - setup_timer(&ts->timer, tsc_poll, (unsigned long)ts);
> - platform_set_drvdata(pdev, ts);
> -
> - ts->tsc_irq = platform_get_irq(pdev, 0);
> - if (ts->tsc_irq < 0) {
> - dev_err(dev, "cannot determine device interrupt\n");
> - error = -ENODEV;
> - goto error_res;
> - }
> -
> - ts->res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - if (!ts->res) {
> - dev_err(dev, "cannot determine register area\n");
> - error = -ENODEV;
> - goto error_res;
> - }
> -
> - if (!request_mem_region(ts->res->start, resource_size(ts->res),
> - pdev->name)) {
> - dev_err(dev, "cannot claim register memory\n");
> - ts->res = NULL;
> - error = -EINVAL;
> - goto error_res;
> - }
> -
> - ts->regs = ioremap(ts->res->start, resource_size(ts->res));
> - if (!ts->regs) {
> - dev_err(dev, "cannot map register memory\n");
> - error = -ENOMEM;
> - goto error_map;
> - }
> -
> - ts->clk = clk_get(dev, NULL);
> - if (IS_ERR(ts->clk)) {
> - dev_err(dev, "cannot claim device clock\n");
> - error = PTR_ERR(ts->clk);
> - goto error_clk;
> - }
> -
> - error = request_threaded_irq(ts->tsc_irq, NULL, tsc_irq, IRQF_ONESHOT,
> - dev_name(dev), ts);
> - if (error < 0) {
> - dev_err(ts->dev, "Could not allocate ts irq\n");
> - goto error_irq;
> - }
> -
> - ts->input_dev = input_allocate_device();
> - if (!ts->input_dev) {
> - dev_err(dev, "cannot allocate input device\n");
> - error = -ENOMEM;
> - goto error_input;
> - }
> - input_set_drvdata(ts->input_dev, ts);
> -
> - ts->input_dev->name = pdev->name;
> - ts->input_dev->id.bustype = BUS_HOST;
> - ts->input_dev->dev.parent = &pdev->dev;
> - ts->input_dev->open = tsc_start;
> - ts->input_dev->close = tsc_stop;
> -
> - clk_enable(ts->clk);
> - rev = tsc_read(ts, rev);
> - ts->input_dev->id.product = ((rev >> 8) & 0x07);
> - ts->input_dev->id.version = ((rev >> 16) & 0xfff);
> - clk_disable(ts->clk);
> -
> - __set_bit(EV_KEY, ts->input_dev->evbit);
> - __set_bit(EV_ABS, ts->input_dev->evbit);
> - __set_bit(BTN_TOUCH, ts->input_dev->keybit);
> -
> - input_set_abs_params(ts->input_dev, ABS_X, 0, 0xffff, 5, 0);
> - input_set_abs_params(ts->input_dev, ABS_Y, 0, 0xffff, 5, 0);
> - input_set_abs_params(ts->input_dev, ABS_PRESSURE, 0, 4095, 128, 0);
> -
> - error = input_register_device(ts->input_dev);
> - if (error < 0) {
> - dev_err(dev, "failed input device registration\n");
> - goto error_reg;
> - }
> -
> - return 0;
> -
> -error_reg:
> - input_free_device(ts->input_dev);
> -error_input:
> - free_irq(ts->tsc_irq, ts);
> -error_irq:
> - clk_put(ts->clk);
> -error_clk:
> - iounmap(ts->regs);
> -error_map:
> - release_mem_region(ts->res->start, resource_size(ts->res));
> -error_res:
> - kfree(ts);
> -
> - return error;
> -}
> -
> -static int tsc_remove(struct platform_device *pdev)
> -{
> - struct tsc_data *ts = platform_get_drvdata(pdev);
> -
> - input_unregister_device(ts->input_dev);
> - free_irq(ts->tsc_irq, ts);
> - clk_put(ts->clk);
> - iounmap(ts->regs);
> - release_mem_region(ts->res->start, resource_size(ts->res));
> - kfree(ts);
> -
> - return 0;
> -}
> -
> -static struct platform_driver tsc_driver = {
> - .probe = tsc_probe,
> - .remove = tsc_remove,
> - .driver.name = "tnetv107x-ts",
> - .driver.owner = THIS_MODULE,
> -};
> -module_platform_driver(tsc_driver);
> -
> -MODULE_AUTHOR("Cyril Chemparathy");
> -MODULE_DESCRIPTION("TNETV107X Touchscreen Driver");
> -MODULE_ALIAS("platform:tnetv107x-ts");
> -MODULE_LICENSE("GPL");
> --
> 1.8.3.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: synaptics add manual min/max quirk
From: Dmitry Torokhov @ 2014-03-28 8:29 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Christopher Heiny, Andrew Duggan, linux-input, linux-kernel,
Peter Hutterer, Stephen Chandler Paul, Hans de Goede
In-Reply-To: <1394207364-4235-1-git-send-email-benjamin.tissoires@redhat.com>
On Fri, Mar 07, 2014 at 10:49:24AM -0500, Benjamin Tissoires wrote:
> The new Lenovo Haswell series (-40's) contains a new Synaptics touchpad.
> However, these new Synaptics devices report bad axis ranges.
> Under Windows, it is not a problem because the Windows driver uses RMI4
> over SMBus to talk to the device. Under Linux, we are using the PS/2
> fallback interface and it occurs the reported ranges are wrong.
>
> Of course, it would be too easy to have only one range for the whole
> series, each touchpad seems to be calibrated in a different way.
>
> We can not use SMBus to get the actual range because I suspect the firmware
> will switch into the SMBus mode and stop talking through PS/2 (this is the
> case for hybrid HID over I2C / PS/2 Synaptics touchpads).
>
> So as a temporary solution (until RMI4 land into upstream), start a new
> list of quirks with the min/max manually set.
>
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> CC: stable@vger.kernel.org
Applied, thank you.
> ---
>
> Hi Dmitry,
>
> Well, this work is part of a (big) attempt to support the new touchpads
> Lenovo put in its latest series.
> Those new laptops have lost the buttons associated to the trackstick and are
> what we call "clickpad".
> Things would be easy if:
> 1. the PS/2 firmware did not lied about the actual range of the axis
> 2. on some install (mine) the DMI matching in udev would not have been broken
> 3. we did not have to fix a lot of stuff in Xorg / libinput / wayland
>
> This patch fixes 1.
>
> Matthew Garrett fixed 2. -> https://patchwork.kernel.org/patch/3704401/
>
> 3. is heavily working as shown by this tracker bug: https://bugs.freedesktop.org/show_bug.cgi?id=73158
>
> I put the 'stable' marker, feel free to remove it if you don't think it should
> be there, but I really think this should also be backported to have something
> working in current distros.
>
> Cheers,
> Benjamin
>
> drivers/input/mouse/synaptics.c | 43 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index 26386f9..ff6a4cf 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -67,6 +67,8 @@
> #define X_MAX_POSITIVE 8176
> #define Y_MAX_POSITIVE 8176
>
> +static int *quirk_min_max = NULL;
> +
> /*****************************************************************************
> * Stuff we need even when we do not want native Synaptics support
> ****************************************************************************/
> @@ -302,6 +304,13 @@ static int synaptics_resolution(struct psmouse *psmouse)
> }
> }
>
> + if (quirk_min_max) {
> + priv->x_min = quirk_min_max[0];
> + priv->x_max = quirk_min_max[1];
> + priv->y_min = quirk_min_max[2];
> + priv->y_max = quirk_min_max[3];
> + }
> +
> return 0;
> }
>
> @@ -1485,10 +1494,44 @@ static const struct dmi_system_id olpc_dmi_table[] __initconst = {
> { }
> };
>
> +static const struct dmi_system_id min_max_dmi_table[] __initconst = {
> +#if defined(CONFIG_DMI)
> + {
> + /* Lenovo ThinkPad Helix */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad Helix"),
> + },
> + .driver_data = (int []){1024, 5052, 2258, 4832},
> + },
> + {
> + /* Lenovo ThinkPad T440s */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T440"),
> + },
> + .driver_data = (int []){1024, 5112, 2024, 4832},
> + },
> + {
> + /* Lenovo ThinkPad T540p */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad T540"),
> + },
> + .driver_data = (int []){1024, 5056, 2058, 4832},
> + },
> +#endif
> + { }
> +};
> +
> void __init synaptics_module_init(void)
> {
> + const struct dmi_system_id *min_max_dmi;
> impaired_toshiba_kbc = dmi_check_system(toshiba_dmi_table);
> broken_olpc_ec = dmi_check_system(olpc_dmi_table);
> + min_max_dmi = dmi_first_match(min_max_dmi_table);
> + if (min_max_dmi)
> + quirk_min_max = (int *)min_max_dmi->driver_data;
> }
>
> static int __synaptics_init(struct psmouse *psmouse, bool absolute_mode)
> --
> 1.8.5.3
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Hans de Goede @ 2014-03-28 8:29 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328081739.GG22093@core.coreip.homeip.net>
Hi,
On 03/28/2014 09:17 AM, Dmitry Torokhov wrote:
> On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote:
>> Hi,
>>
>> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote:
>>> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote:
>>>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote:
>>>>
>>>>> Which in the end turns out to be much nicer too, since it gets rid of needing
>>>>> a udev-helper too. After this much too long introduction I'll let the patches
>>>>> speak for themselves.
>>>>
>>>> Yeah, I was coming to the conclusion that this was probably the best we
>>>> could do. It's unfortunate that "id" is already in use - we'd be able to
>>>> get away without any X server modifications otherwise.
>>>>
>>>> Long term we probably still want to tie serio devices to the ACPI
>>>> devices in case the vendor provides power management calls there, but we
>>>> can leave that until there's an actual example.
>>>
>>> I am still unsure if we shoudl be adding these new IDs to serio core...
>>> Can't the X driver take a peek at ACPI devices on it's own?
>>
>> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx
>> device is the serio bus host.
>
> Practically speaking you should not care - there is only one touchpad in
> Lenovos.
So are you suggesting we simply go over all /sys/devices/pnp0/00:xx devices looking
for a pnp-id we're interested in ? I'm sorry but that is just a non-sense solution,
which reminds me of the good old days of random poking io-ports to probe stuff.
We're not blindly going to read every /sys/devices/pnp0/00:xx/id attribute on a
system, assuming that if it contains a pnp-id we're interested in it happens to
belong to the input device we're enumerating at that time.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH] input: synaptics add manual min/max quirk for ThinkPad X240
From: Dmitry Torokhov @ 2014-03-28 8:29 UTC (permalink / raw)
To: Hans de Goede
Cc: Benjamin Tissoires, Christopher Heiny, Andrew Duggan, linux-input,
Peter Hutterer, Stephen Chandler Paul, stable
In-Reply-To: <1394278521-6870-2-git-send-email-hdegoede@redhat.com>
On Sat, Mar 08, 2014 at 12:35:21PM +0100, Hans de Goede wrote:
> This extends Benjamin Tissoires manual min/max quirk table with support for
> the ThinkPad X240.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
> ---
> drivers/input/mouse/synaptics.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
> index ff6a4cf..b522bdc 100644
> --- a/drivers/input/mouse/synaptics.c
> +++ b/drivers/input/mouse/synaptics.c
> @@ -1505,6 +1505,14 @@ static const struct dmi_system_id min_max_dmi_table[] __initconst = {
> .driver_data = (int []){1024, 5052, 2258, 4832},
> },
> {
> + /* Lenovo ThinkPad X240 */
> + .matches = {
> + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> + DMI_MATCH(DMI_PRODUCT_VERSION, "ThinkPad X240"),
> + },
> + .driver_data = (int []){1232, 5710, 1156, 4696},
> + },
> + {
> /* Lenovo ThinkPad T440s */
> .matches = {
> DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"),
> --
> 1.8.4.2
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Matthew Garrett @ 2014-03-28 8:27 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328082452.GH22093@core.coreip.homeip.net>
On Fri, Mar 28, 2014 at 01:24:52AM -0700, Dmitry Torokhov wrote:
> On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote:
> > On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote:
> >
> > > I am still unsure if we shoudl be adding these new IDs to serio core...
> > > Can't the X driver take a peek at ACPI devices on it's own?
> >
> > In the (admittedly unlikely) event of multiple PS/2 trackpads, how do
> > you know which one corresponds to which ACPI device?
>
> So far I have not seen a single instance of a laptop with 2 touchpads
> and I doubt external PS/2 ones will ever make come back.
Right, we can hack around it based on what we've seen so far. But it
seems more attractive to fix it in such a way that we won't behave
inappropriately even if someone does do something utterly unexpected in
future. For instance, some ARM devices have i8042-like serio - if the
underlying device is exposed in device tree, it'd be nice to be able to
expose that to userspace without having to modify the core X code.
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 8:24 UTC (permalink / raw)
To: Matthew Garrett
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328082004.GA6801@srcf.ucam.org>
On Fri, Mar 28, 2014 at 08:20:04AM +0000, Matthew Garrett wrote:
> On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote:
>
> > I am still unsure if we shoudl be adding these new IDs to serio core...
> > Can't the X driver take a peek at ACPI devices on it's own?
>
> In the (admittedly unlikely) event of multiple PS/2 trackpads, how do
> you know which one corresponds to which ACPI device?
So far I have not seen a single instance of a laptop with 2 touchpads
and I doubt external PS/2 ones will ever make come back.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Matthew Garrett @ 2014-03-28 8:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Hans de Goede, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <20140328075655.GE22093@core.coreip.homeip.net>
On Fri, Mar 28, 2014 at 12:56:55AM -0700, Dmitry Torokhov wrote:
> I am still unsure if we shoudl be adding these new IDs to serio core...
> Can't the X driver take a peek at ACPI devices on it's own?
In the (admittedly unlikely) event of multiple PS/2 trackpads, how do
you know which one corresponds to which ACPI device?
--
Matthew Garrett | mjg59@srcf.ucam.org
^ permalink raw reply
* Re: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Dmitry Torokhov @ 2014-03-28 8:17 UTC (permalink / raw)
To: Hans de Goede
Cc: Matthew Garrett, Benjamin Tissoires, Peter Hutterer,
platform-driver-x86, linux-input
In-Reply-To: <53352EFB.8000309@redhat.com>
On Fri, Mar 28, 2014 at 09:12:43AM +0100, Hans de Goede wrote:
> Hi,
>
> On 03/28/2014 08:56 AM, Dmitry Torokhov wrote:
> > On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote:
> >> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote:
> >>
> >>> Which in the end turns out to be much nicer too, since it gets rid of needing
> >>> a udev-helper too. After this much too long introduction I'll let the patches
> >>> speak for themselves.
> >>
> >> Yeah, I was coming to the conclusion that this was probably the best we
> >> could do. It's unfortunate that "id" is already in use - we'd be able to
> >> get away without any X server modifications otherwise.
> >>
> >> Long term we probably still want to tie serio devices to the ACPI
> >> devices in case the vendor provides power management calls there, but we
> >> can leave that until there's an actual example.
> >
> > I am still unsure if we shoudl be adding these new IDs to serio core...
> > Can't the X driver take a peek at ACPI devices on it's own?
>
> The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx
> device is the serio bus host.
Practically speaking you should not care - there is only one touchpad in
Lenovos.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: 8 months to review a patch (was Re: [PATCH] Route kbd LEDs through the generic LEDs layer)
From: Samuel Thibault @ 2014-03-28 8:08 UTC (permalink / raw)
To: Pavel Machek
Cc: Pali Rohár, dtor, linux-input, jkosina, Dmitry Torokhov,
Andrew Morton, jslaby, Richard Purdie, LKML, Evan Broder,
Arnaud Patard, Greg KH, Linus Torvalds
In-Reply-To: <20140328070136.GA15953@amd.pavel.ucw.cz>
Pavel Machek, le Fri 28 Mar 2014 08:01:36 +0100, a écrit :
> On Thu 2014-03-27 02:08:17, Pali Rohár wrote:
> > 2014-03-16 11:19 GMT+01:00 Samuel Thibault <samuel.thibault@ens-lyon.org>:
> > > Pali Rohár, le Sun 16 Mar 2014 11:16:25 +0100, a écrit :
> > >> Hello, what happened with this patch? Is there any problem with accepting it?
> > >
> > > Dmitry finding time to review it, I guess.
>
> > Dmitry, can you look and review this patch?
>
> Dmitry, what happened to you, are you there? This patch is like 8
> months old,
Well, the patch was written in Feb 2010 actually, it got some reviews
overs years, the version submitted in July 2013 is the same as the one
submitted in Dec 2013.
Samuel
--
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: [PATCH 0/2] input/serio: Add a firmware_id sysfs attribute
From: Hans de Goede @ 2014-03-28 8:12 UTC (permalink / raw)
To: Dmitry Torokhov, Matthew Garrett
Cc: Benjamin Tissoires, Peter Hutterer, platform-driver-x86,
linux-input
In-Reply-To: <20140328075655.GE22093@core.coreip.homeip.net>
Hi,
On 03/28/2014 08:56 AM, Dmitry Torokhov wrote:
> On Thu, Mar 20, 2014 at 05:21:59PM +0000, Matthew Garrett wrote:
>> On Thu, Mar 20, 2014 at 11:12:08AM +0100, Hans de Goede wrote:
>>
>>> Which in the end turns out to be much nicer too, since it gets rid of needing
>>> a udev-helper too. After this much too long introduction I'll let the patches
>>> speak for themselves.
>>
>> Yeah, I was coming to the conclusion that this was probably the best we
>> could do. It's unfortunate that "id" is already in use - we'd be able to
>> get away without any X server modifications otherwise.
>>
>> Long term we probably still want to tie serio devices to the ACPI
>> devices in case the vendor provides power management calls there, but we
>> can leave that until there's an actual example.
>
> I am still unsure if we shoudl be adding these new IDs to serio core...
> Can't the X driver take a peek at ACPI devices on it's own?
The problem is there is no way for userspace to know which /sys/devices/pnp0/00:xx
device is the serio bus host.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v3 00/10] mfd: AXP20x: Add support for AXP202 and AXP209
From: Lee Jones @ 2014-03-28 8:01 UTC (permalink / raw)
To: Carlo Caione
Cc: linux-arm-kernel, linux-sunxi, maxime.ripard, hdegoede, emilio,
wens, sameo, dmitry.torokhov, linux-input, linux-doc, lgirdwood,
broonie
In-Reply-To: <1395955764-18103-1-git-send-email-carlo@caione.org>
> AXP209 and AXP202 are the PMUs (Power Management Unit) used by A10, A13
> and A20 SoCs and developed by X-Powers, a sister company of Allwinner.
> AXP20x comprises an adaptive USB-Compatible PWM charger, 2 BUCK DC-DC
> converters, 5 LDOs, multiple 12-bit ADCs of voltage, current and temperature
> as well as 4 configurable GPIOs.
[...]
> Carlo Caione (10):
> mfd: AXP20x: Add mfd driver for AXP20x PMIC
> dt-bindings: add vendor-prefix for X-Powers
> mfd: AXP20x: Add bindings documentation
> input: misc: Add driver for AXP20x Power Enable Key
> input: misc: Add ABI docs for AXP20x PEK
> regulator: AXP20x: Add support for regulators subsystem
> ARM: sunxi: dt: Add x-powers-axp209.dtsi file
> ARM: sun7i/sun4i: dt: Add AXP209 support to various boards
> ARM: sunxi: Add AXP20x support in defconfig
> ARM: sunxi: Add AXP20x support multi_v7_defconfig
I'm only CC'ed on two of these patches. In order to gain a full
appreciation what what the set is trying to achieve, can you CC me on
the entire set please?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
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: [PATCH] input: cypress_ps2: Don't report the cypress PS/2 trackpads as a button pad
From: Dmitry Torokhov @ 2014-03-28 8:01 UTC (permalink / raw)
To: Hans de Goede
Cc: Benjamin Tissoires, linux-input, Adam Williamson, Peter Hutterer
In-Reply-To: <1395668975-10588-1-git-send-email-hdegoede@redhat.com>
On Mon, Mar 24, 2014 at 02:49:35PM +0100, Hans de Goede wrote:
> The cypress PS/2 trackpad models supported by the cypress_ps2 driver emulate
> BTN_RIGHT events in firmware based on the finger position, as part of this
> no motion events are sent when the finger is in the button area.
>
> The INPUT_PROP_BUTTONPAD property is there to indicate to userspace that
> BTN_RIGHT events should be emulated in userspace, which is not necessary
> in this case.
>
> When INPUT_PROP_BUTTONPAD is advertised userspace will wait for a motion event
> before propagating the button event higher up the stack, as it needs current
> abs x + y data for its BTN_RIGHT emulation. Since in the cypress_ps2 pads
> don't report motion events in the button area, this means that clicks in the
> button area end up being ignored, so INPUT_PROP_BUTTONPAD actually causes
> problems for these touchpads, and removing it fixes:
>
> https://bugs.freedesktop.org/show_bug.cgi?id=76341
>
> Reported-by: Adam Williamson <awilliam@redhat.com>
> Tested-by: Adam Williamson <awilliam@redhat.com>
> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
> Cc: Adam Williamson <awilliam@redhat.com>
> Cc: Peter Hutterer <peter.hutterer@who-t.net>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Applied, thank you.
> ---
> drivers/input/mouse/cypress_ps2.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/input/mouse/cypress_ps2.c b/drivers/input/mouse/cypress_ps2.c
> index 87095e2..8af34ff 100644
> --- a/drivers/input/mouse/cypress_ps2.c
> +++ b/drivers/input/mouse/cypress_ps2.c
> @@ -409,7 +409,6 @@ static int cypress_set_input_params(struct input_dev *input,
> __clear_bit(REL_X, input->relbit);
> __clear_bit(REL_Y, input->relbit);
>
> - __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> __set_bit(EV_KEY, input->evbit);
> __set_bit(BTN_LEFT, input->keybit);
> __set_bit(BTN_RIGHT, input->keybit);
> --
> 1.9.0
>
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox