diff mbox

[v5,05/17] input: make LoCoMo keyboard driver support both poodle and collie

Message ID 1433797008-6246-6-git-send-email-dbaryshkov@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Dmitry Baryshkov June 8, 2015, 8:56 p.m. UTC 
Keyboards on collie and poodle differ only in wiring of 'Home' key.
Instead of complicating the driver with platform data, just check for
the machine for the time being. This will be converted to DTS property
sometime in the future.

Signed-off-by: Dmitry Eremin-Solenikov <dbaryshkov@gmail.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
 drivers/input/keyboard/locomokbd.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Russell King - ARM Linux June 14, 2015, 3:11 p.m. UTC | #1 
On Mon, Jun 08, 2015 at 11:56:36PM +0300, Dmitry Eremin-Solenikov wrote:
> @@ -278,6 +281,11 @@ static int locomokbd_probe(struct platform_device *dev)
>  			locomokbd_keycode,
>  			sizeof(locomokbd->keycode));
>  
> +	if (machine_is_collie())
> +		locomokbd->keycode[18] = KEY_HOME;
> +	else
> +		locomokbd->keycode[3] = KEY_HOME;

We had decided that we weren't allowing any new machine_is_xxx() in drivers.
Why can't this difference be encoded via platform data, so it can be later
encoded in DT if sa11x0 moves in that direction?
Dmitry Baryshkov June 14, 2015, 4:26 p.m. UTC | #2 
Hello,

2015-06-14 18:11 GMT+03:00 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Jun 08, 2015 at 11:56:36PM +0300, Dmitry Eremin-Solenikov wrote:
>> @@ -278,6 +281,11 @@ static int locomokbd_probe(struct platform_device *dev)
>>                       locomokbd_keycode,
>>                       sizeof(locomokbd->keycode));
>>
>> +     if (machine_is_collie())
>> +             locomokbd->keycode[18] = KEY_HOME;
>> +     else
>> +             locomokbd->keycode[3] = KEY_HOME;
>
> We had decided that we weren't allowing any new machine_is_xxx() in drivers.
> Why can't this difference be encoded via platform data, so it can be later
> encoded in DT if sa11x0 moves in that direction?

I'd agree with you. However this looks rather long path:
* Introduce platform data with just one preference (for home key).
* Switch poodle(pxa) to dts
* Switch collie(sa1100) to dts
* Introduce special property (or use compatibility string)
* Kill platform data.

By allowing machine_is_xx() here we can forget about this temporary platform
data. The change is quite isolated. If you completely disagree with this patch,
I'd prefer to just drop it for now (as it is an added feature rather
than just refactoring).
diff mbox

Patch

diff --git a/drivers/input/keyboard/locomokbd.c b/drivers/input/keyboard/locomokbd.c
index 7645053..73d6e58 100644
--- a/drivers/input/keyboard/locomokbd.c
+++ b/drivers/input/keyboard/locomokbd.c
@@ -33,6 +33,9 @@ 
 #include <linux/slab.h>
 #include <linux/mfd/locomo.h>
 
+/* There is one minor difference between mappings on poodle and collie */
+#include <asm/mach-types.h>
+
 #define KEY_ACTIVITY		KEY_F16
 #define KEY_CONTACT		KEY_F18
 #define KEY_CENTER		KEY_F15
@@ -45,7 +48,7 @@ 
 static const unsigned char
 locomokbd_keycode[LOCOMOKBD_NUMKEYS] = {
 	0, KEY_ESC, KEY_ACTIVITY, 0, 0, 0, 0, 0, 0, 0,				/* 0 - 9 */
-	0, 0, 0, 0, 0, 0, 0, KEY_MENU, KEY_HOME, KEY_CONTACT,			/* 10 - 19 */
+	0, 0, 0, 0, 0, 0, 0, KEY_MENU, 0, KEY_CONTACT,				/* 10 - 19 */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, 0,						/* 20 - 29 */
 	0, 0, 0, KEY_CENTER, 0, KEY_MAIL, 0, 0, 0, 0,				/* 30 - 39 */
 	0, 0, 0, 0, 0, 0, 0, 0, 0, KEY_RIGHT,					/* 40 - 49 */
@@ -278,6 +281,11 @@  static int locomokbd_probe(struct platform_device *dev)
 			locomokbd_keycode,
 			sizeof(locomokbd->keycode));
 
+	if (machine_is_collie())
+		locomokbd->keycode[18] = KEY_HOME;
+	else
+		locomokbd->keycode[3] = KEY_HOME;
+
 	for (i = 0; i < LOCOMOKBD_NUMKEYS; i++)
 		input_set_capability(input_dev, EV_KEY, locomokbd->keycode[i]);
 	input_set_capability(input_dev, EV_PWR, KEY_SUSPEND);