Skip to contentSkip to conversation

Haptic class support (v2)

PATCH In review Kyungmin Park 7 participants 13 messages Oct 6, 2009 – Oct 20, 2009 HID

lore
Share & tools
lore

Kyungmin Park

author
#0 lore raw

PATCH 1/1 Haptic class support (v2)

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/Kconfig                     |    2
 drivers/Makefile                    |    1
 drivers/haptic/Kconfig              |   31 ++
 drivers/haptic/Makefile             |    8
 drivers/haptic/haptic-class.c       |  256 ++++++++++++++++++++++
 drivers/haptic/haptic-samsung-pwm.c |  377 ++++++++++++++++++++++++++++++++
 drivers/haptic/haptic.h             |   35 +++
 drivers/haptic/isa1200.c            |  413 ++++++++++++++++++++++++++++++++++++
 include/linux/haptic.h              |   85 +++++++
 9 files changed, 1208 insertions(+)
drivers/Kconfig +2 -0
index 48bbdbe..d44a601 100644
@@ -62,6 +62,8 @@ source "drivers/power/Kconfig"
 
 source "drivers/hwmon/Kconfig"
 
+source "drivers/haptic/Kconfig"
+
 source "drivers/thermal/Kconfig"
 
 source "drivers/watchdog/Kconfig"
drivers/Makefile +1 -0
index 6ee53c7..16b8f67 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_PPS)		+= pps/
 obj-$(CONFIG_W1)		+= w1/
 obj-$(CONFIG_POWER_SUPPLY)	+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/
+obj-$(CONFIG_HAPTIC)		+= haptic/
 obj-$(CONFIG_THERMAL)		+= thermal/
 obj-$(CONFIG_WATCHDOG)		+= watchdog/
 obj-$(CONFIG_PHONE)		+= telephony/
drivers/haptic/Kconfig new file +31 -0
new file mode 100644
index 0000000..9acb02a
@@ -0,0 +1,31 @@
+menuconfig HAPTIC
+	bool "HAPTIC support"
+	help
+	  Say Y to enalbe haptic support. It enables the haptic and controlled
+	  from both userspace and kernel
+
+if HAPTIC
+
+config HAPTIC_CLASS
+	tristate "Haptic Class Support"
+	help
+	  This option enables the haptic sysfs class in /sys/class/haptic.
+
+comment "Haptic drivers"
+
+config HAPTIC_SAMSUNG_PWM
+	tristate "Haptic Support for SAMSUNG PWM-controlled motor (ISA1000)"
+	depends on HAPTIC_CLASS && (ARCH_S3C64XX || ARCH_S5PC1XX)
+	help
+	  This options enables support for haptic connected to GPIO lines
+	  controlled by a PWM timer on SAMSUNG CPUs.
+
+comment "Haptic chips"
+
+config HAPTIC_ISA1200
+	tristate "Haptic Motor"
+	depends on HAPTIC_CLASS && I2C
+	help
+	  The ISA1200 is a high performance enhanced haptic motor driver
+
+endif	# HAPTIC
drivers/haptic/Makefile new file +8 -0
new file mode 100644
index 0000000..be20d4f
@@ -0,0 +1,8 @@
+# Haptic Core
+obj-$(CONFIG_HAPTIC_CLASS)		+= haptic-class.o
+
+# Drivers
+obj-$(CONFIG_HAPTIC_SAMSUNG_PWM)	+= haptic-samsung-pwm.o
+
+# Chips
+obj-$(CONFIG_HAPTIC_ISA1200)		+= isa1200.o
drivers/haptic/haptic-class.c new file +256 -0

256 lines not on this page · raw message

drivers/haptic/haptic-samsung-pwm.c new file +377 -0

377 lines not on this page · raw message

drivers/haptic/haptic.h new file +35 -0
new file mode 100644
index 0000000..888aaa3
@@ -0,0 +1,35 @@
+/*
+ *  Haptic Core
+ *
+ *  Copyright (C) 2008 Samsung Electronics
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
+#ifndef __HAPTIC_H_INCLUDED
+#define __HAPTIC_H_INCLUDED
+
+#include <linux/device.h>
+#include <linux/rwsem.h>
+#include <linux/haptic.h>
+
+static inline void haptic_set_value(struct haptic_classdev *haptic_cdev,
+					enum haptic_value value)
+{
+	if (value > HAPTIC_FULL)
+		value = HAPTIC_FULL;
+	haptic_cdev->value = value;
+	if (!(haptic_cdev->flags & HAPTIC_SUSPENDED))
+		haptic_cdev->set(haptic_cdev, value);
+}
+
+static inline int haptic_get_value(struct haptic_classdev *haptic_cdev)
+{
+	return haptic_cdev->value;
+}
+
+#endif	/* __HAPTIC_H_INCLUDED */
drivers/haptic/isa1200.c new file +413 -0

413 lines not on this page · raw message

include/linux/haptic.h new file +85 -0

85 lines not on this page · raw message

#1 lore raw
Quoted 53 lines · On Tue, 06 Oct 2009 16:45:33 +0900 Kyungmin Park wrote:

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Signed-off-by Kyungmin Park <[email protected]>
---
 drivers/Kconfig                     |    2
 drivers/Makefile                    |    1
 drivers/haptic/Kconfig              |   31 ++
 drivers/haptic/Makefile             |    8
 drivers/haptic/haptic-class.c       |  256 ++++++++++++++++++++++
 drivers/haptic/haptic-samsung-pwm.c |  377 ++++++++++++++++++++++++++++++++
 drivers/haptic/haptic.h             |   35 +++
 drivers/haptic/isa1200.c            |  413 ++++++++++++++++++++++++++++++++++++
 include/linux/haptic.h              |   85 +++++++
 9 files changed, 1208 insertions(+)
drivers/Kconfig +2 -0
index 48bbdbe..d44a601 100644
@@ -62,6 +62,8 @@ source "drivers/power/Kconfig"
 
 source "drivers/hwmon/Kconfig"
 
+source "drivers/haptic/Kconfig"
+
 source "drivers/thermal/Kconfig"
 
 source "drivers/watchdog/Kconfig"
drivers/Makefile +1 -0
index 6ee53c7..16b8f67 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_PPS)		+= pps/
 obj-$(CONFIG_W1)		+= w1/
 obj-$(CONFIG_POWER_SUPPLY)	+= power/
 obj-$(CONFIG_HWMON)		+= hwmon/
+obj-$(CONFIG_HAPTIC)		+= haptic/
 obj-$(CONFIG_THERMAL)		+= thermal/
 obj-$(CONFIG_WATCHDOG)		+= watchdog/
 obj-$(CONFIG_PHONE)		+= telephony/
drivers/haptic/Kconfig new file +0 -0
new file mode 100644
index 0000000..9acb02a

drivers/haptic/Kconfig line 4

Quoted 4 lines
drivers/haptic/Kconfig new file +4 -0
@@ -0,0 +1,31 @@
+menuconfig HAPTIC
+	bool "HAPTIC support"
+	help
+	  Say Y to enalbe haptic support. It enables the haptic and controlled

enable
The next sentence is incomplete. Maybe it should be (but I don't know):

It enables haptic devices and controls

drivers/haptic/Kconfig line 5

Quoted 1 line
drivers/haptic/Kconfig new file +1 -0
+	  from both userspace and kernel

and kernel.

Quoted 14 lines
drivers/haptic/Kconfig new file +14 -0
+
+if HAPTIC
+
+config HAPTIC_CLASS
+	tristate "Haptic Class Support"
+	help
+	  This option enables the haptic sysfs class in /sys/class/haptic.
+
+comment "Haptic drivers"
+
+config HAPTIC_SAMSUNG_PWM
+	tristate "Haptic Support for SAMSUNG PWM-controlled motor (ISA1000)"
+	depends on HAPTIC_CLASS && (ARCH_S3C64XX || ARCH_S5PC1XX)
+	help

drivers/haptic/Kconfig line 29

Quoted 10 lines
drivers/haptic/Kconfig new file +10 -0
+	  This options enables support for haptic connected to GPIO lines
+	  controlled by a PWM timer on SAMSUNG CPUs.
+
+comment "Haptic chips"
+
+config HAPTIC_ISA1200
+	tristate "Haptic Motor"
+	depends on HAPTIC_CLASS && I2C
+	help
+	  The ISA1200 is a high performance enhanced haptic motor driver

end sentence with period ('.')

Quoted 14 lines
drivers/haptic/Kconfig new file +2 -0
+
+endif	# HAPTIC
drivers/haptic/haptic-class.c new file +5 -0
new file mode 100644
index 0000000..b93e8e0
@@ -0,0 +1,256 @@
+/*
+ *  Haptic Class Core
+ *
+ *  Copyright (C) 2008 Samsung Electronics
+ *  Kyungmin Park <[email protected]>

drivers/haptic/haptic-class.c

Quoted 10 lines
drivers/haptic/haptic-class.c new file +9 -0
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ */
+
 
+/**
+ * haptic_classdev_unregister - unregisters a object of haptic_properties class.

s/haptic_properties/haptic_classdev/ ??

Quoted 55 lines

+ * @haptic_cdev: the haptic device to unregister
+ *
+ * Unregisters a previously registered via haptic_classdev_register object.
+ */

drivers/haptic/haptic-class.c new file +15 -0
+void haptic_classdev_unregister(struct haptic_classdev *haptic_cdev)
+{
+	class_remove_file(haptic_class, &class_attr_enable);
+	class_remove_file(haptic_class, &class_attr_oneshot);
+	class_remove_file(haptic_class, &class_attr_level);
+	class_remove_file(haptic_class, &class_attr_level_max);
+	class_remove_file(haptic_class, &class_attr_value);
+
+	device_unregister(haptic_cdev->dev);
+
+	down_write(&haptic_list_lock);
+	list_del(&haptic_cdev->node);
+	up_write(&haptic_list_lock);
+}
+EXPORT_SYMBOL_GPL(haptic_classdev_unregister);
drivers/haptic/haptic-samsung-pwm.c new file +11 -0
new file mode 100644
index 0000000..0fc1093
@@ -0,0 +1,377 @@
+/*
+ *  drivers/haptic/haptic-samsung-pwm.c
+ *
+ *  Copyright (C) 2008 Samsung Electronics
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
drivers/haptic/isa1200.c new file +10 -0
new file mode 100644
index 0000000..19a3801
@@ -0,0 +1,413 @@
+/*
+ *  isa1200.c - Haptic Motor
+ *
+ *  Copyright (C) 2009 Samsung Electronics
+ *  Kyungmin Park <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */

drivers/haptic/isa1200.c line 20

Quoted 10 lines
drivers/haptic/isa1200.c new file +10 -0
+
+struct isa1200_chip {
+	struct i2c_client *client;
+	struct pwm_device *pwm;
+	struct haptic_classdev cdev;
+	struct work_struct work;
+	struct timer_list timer;
+
+	unsigned int	len;		/* LDO enable */
+	unsigned int	hen;		/* Haptic haptic enable */

Drop one "haptic" ?

drivers/haptic/isa1200.c line 42

Quoted 9 lines
drivers/haptic/isa1200.c new file +9 -0
+
+	int enable;
+	int powered;
+
+	int level;
+	int level_max;
+
+	int ldo_level;
+};

---
~Randy

#2 lore raw
In reply to Kyungmin Park

Hi Kyungmin,

Quoted 10 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Signed-off-by Kyungmin Park <[email protected]>

Many thanks for the posting the update and example drivers. Here are my suggestions to make it mainlined fast:

1. split these patches - probably in following order would be good
a) haptics class
b) haptics samsung pwm driver
c) isa1200 haptics driver
d) add maintainers entry in MAINTAINERS file under your name for haptics :)
d) please add short documentation under Documentation/haptics ?
directory describe what haptics class is all about, and atleast userspace interfaces which you are providing using sysfs file. We can use led class documentation as example here. Short documentation is fine to begin with.
e) if possible create your own git haptics project tree at
kernel.org and add it to linux-next, but I would also suggest that as no. of updates to these framework will be not so frequent, so we can also follow -mm route and let Andrew pick up your patches and get them cooked under -mm.

Quoted 32 lines
drivers/Kconfig +0 -0
index 48bbdbe..d44a601 100644
@@ -62,6 +62,8 @@ source "drivers/power/Kconfig"

 source "drivers/hwmon/Kconfig"

drivers/Kconfig +2 -0
+source "drivers/haptic/Kconfig"
+

 source "drivers/thermal/Kconfig"

 source "drivers/watchdog/Kconfig"

drivers/Makefile +0 -0
index 6ee53c7..16b8f67 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_PPS)             += pps/

 obj-$(CONFIG_W1)               += w1/
 obj-$(CONFIG_POWER_SUPPLY)     += power/
 obj-$(CONFIG_HWMON)            += hwmon/
+obj-$(CONFIG_HAPTIC)           += haptic/
 obj-$(CONFIG_THERMAL)          += thermal/
 obj-$(CONFIG_WATCHDOG)         += watchdog/
 obj-$(CONFIG_PHONE)            += telephony/

drivers/haptic/Kconfig new file +0 -0
new file mode 100644
index 0000000..9acb02a

drivers/haptic/Kconfig line 4

Quoted 4 lines
drivers/haptic/Kconfig new file +4 -0
@@ -0,0 +1,31 @@
+menuconfig HAPTIC
+       bool "HAPTIC support"
+       help
+         Say Y to enalbe haptic support. It enables the haptic and controlled

s/enalbe/enable

drivers/haptic/Kconfig line 5

Quoted 1 line
drivers/haptic/Kconfig new file +1 -0
+         from both userspace and kernel

Please explain how from kernel? We are only providing sysfs interface for this framework, and I don't see in-kernel control of haptics driver in your patch. Also I don't see any need of in-kernel control of them though.

Quoted 14 lines
drivers/haptic/Kconfig new file +14 -0
+
+if HAPTIC
+
+config HAPTIC_CLASS
+       tristate "Haptic Class Support"
+       help
+         This option enables the haptic sysfs class in /sys/class/haptic.
+
+comment "Haptic drivers"
+
+config HAPTIC_SAMSUNG_PWM
+       tristate "Haptic Support for SAMSUNG PWM-controlled motor (ISA1000)"
+       depends on HAPTIC_CLASS && (ARCH_S3C64XX || ARCH_S5PC1XX)
+       help

drivers/haptic/Kconfig line 29

Quoted 10 lines
drivers/haptic/Kconfig new file +10 -0
+         This options enables support for haptic connected to GPIO lines
+         controlled by a PWM timer on SAMSUNG CPUs.
+
+comment "Haptic chips"
+
+config HAPTIC_ISA1200
+       tristate "Haptic Motor"
+       depends on HAPTIC_CLASS && I2C
+       help
+         The ISA1200 is a high performance enhanced haptic motor driver

This chip also seems to support external pwm mode like isa1000, so you could explicitly add that this driver is about controlling it through i2c only, but there could be a possibility of writing the same through external pwm mode also.

drivers/haptic/Kconfig

Quoted 3 lines
drivers/haptic/Kconfig new file +3 -0
+
+static int samsung_pwm_haptic_probe(struct platform_device *pdev)
+{

__devinit?

drivers/haptic/Kconfig

Quoted 11 lines

+       struct haptic_platform_data *pdata = pdev->dev.platform_data;
+       struct samsung_pwm_haptic *haptic;
+       int ret;

drivers/haptic/Kconfig new file +8 -0
+
+       haptic = kzalloc(sizeof(struct samsung_pwm_haptic), GFP_KERNEL);
+       if (!haptic) {
+               dev_err(&pdev->dev, "No memory for device\n");
+               return -ENOMEM;
+       }
+static int samsung_pwm_haptic_remove(struct platform_device *pdev)
+{

__devexit

drivers/haptic/Kconfig

Quoted 4 lines
drivers/haptic/Kconfig new file +4 -0
+
+static int __devexit isa1200_remove(struct i2c_client *client)
+{
+       return 0;

nothing to remove?

Quoted 4 lines
drivers/haptic/Kconfig new file +4 -0
+}
+
+static int isa1200_suspend(struct i2c_client *client, pm_message_t mesg)
+{

drivers/haptic/Kconfig

Quoted 10 lines
drivers/haptic/Kconfig new file +10 -0
+       struct isa1200_chip *chip = i2c_get_clientdata(client);
+       isa1200_chip_power_off(chip);
+       return 0;
+}
+
+static int isa1200_resume(struct i2c_client *client)
+{
+       isa1200_setup(client);
+       return 0;
+}

#ifdef CONFIG_PM checks please.

Signature · 7 lines
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
author
#3 lore raw

Hi,

Quoted 17 lines · Trilok Soni

Hi Kyungmin,

Quoted 10 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Signed-off-by Kyungmin Park <[email protected]>

Many thanks for the posting the update and example drivers. Here are my suggestions to make it mainlined fast:

Quoted 10 lines

1. split these patches - probably in following order would be good
  a) haptics class
  b) haptics samsung pwm driver
  c) isa1200 haptics driver
  d) add maintainers entry in MAINTAINERS file under your name for haptics :)
  d) please add short documentation under Documentation/haptics ? directory describe what haptics class is all about, and atleast userspace interfaces which you are providing using sysfs file. We can use led class documentation as example here. Short documentation is fine to begin with.

Thank you for suggestion. I will split the patches. and resend.

Quoted 5 lines

  e) if possible create your own git haptics project tree at kernel.org and add it to linux-next, but I would also suggest that as no. of updates to these framework will be not so frequent, so we can also follow -mm route and let Andrew pick up your patches and get them cooked under -mm.

I think no need to create new haptic tree. it's simple class to handle haptic drivers.

Quoted 31 lines
Quoted 31 lines
drivers/Kconfig +0 -0
index 48bbdbe..d44a601 100644
@@ -62,6 +62,8 @@ source "drivers/power/Kconfig"

 source "drivers/hwmon/Kconfig"

drivers/Kconfig +2 -0
+source "drivers/haptic/Kconfig"
+

 source "drivers/thermal/Kconfig"

 source "drivers/watchdog/Kconfig"

drivers/Makefile +0 -0
index 6ee53c7..16b8f67 100644
@@ -77,6 +77,7 @@ obj-$(CONFIG_PPS)             += pps/

 obj-$(CONFIG_W1)               += w1/
 obj-$(CONFIG_POWER_SUPPLY)     += power/
 obj-$(CONFIG_HWMON)            += hwmon/
+obj-$(CONFIG_HAPTIC)           += haptic/
 obj-$(CONFIG_THERMAL)          += thermal/
 obj-$(CONFIG_WATCHDOG)         += watchdog/
 obj-$(CONFIG_PHONE)            += telephony/

drivers/haptic/Kconfig new file +0 -1
new file mode 100644
index 0000000..9acb02a
--- /dev/null
Quoted 8 lines

drivers/haptic/Kconfig line 4

Quoted 6 lines

+++ b/drivers/haptic/Kconfig

drivers/haptic/Kconfig new file +4 -0
@@ -0,0 +1,31 @@
+menuconfig HAPTIC
+       bool "HAPTIC support"
+       help
+         Say Y to enalbe haptic support. It enables the haptic and controlled

s/enalbe/enable

Fixed.

Quoted 7 lines

drivers/haptic/Kconfig line 5

Quoted 2 lines
drivers/haptic/Kconfig new file +1 -0
+         from both userspace and kernel

Please explain how from kernel? We are only providing sysfs interface for this framework, and I don't see in-kernel control of haptics driver in your patch. Also I don't see any need of in-kernel control of them though.

It's wrong description. It's handled from userspace not kernel.

Quoted 22 lines
Quoted 22 lines
drivers/haptic/Kconfig new file +21 -0
+
+if HAPTIC
+
+config HAPTIC_CLASS
+       tristate "Haptic Class Support"
+       help
+         This option enables the haptic sysfs class in /sys/class/haptic.
+
+comment "Haptic drivers"
+
+config HAPTIC_SAMSUNG_PWM
+       tristate "Haptic Support for SAMSUNG PWM-controlled motor (ISA1000)"
+       depends on HAPTIC_CLASS && (ARCH_S3C64XX || ARCH_S5PC1XX)
+       help
+         This options enables support for haptic connected to GPIO lines
+         controlled by a PWM timer on SAMSUNG CPUs.
+
+comment "Haptic chips"
+
+config HAPTIC_ISA1200
+       tristate "Haptic Motor"
Quoted 8 lines

drivers/haptic/Kconfig line 29

Quoted 3 lines
drivers/haptic/Kconfig new file +3 -0
+       depends on HAPTIC_CLASS && I2C
+       help
+         The ISA1200 is a high performance enhanced haptic motor driver

This chip also seems to support external pwm mode like isa1000, so you could explicitly add that this driver is about controlling it through i2c only, but there could be a possibility of writing the same through external pwm mode also.

Actually it supports both internal and external. but in my hardware it connects with external.
So I implement it as previous isa1000 does.
Someone can implement it as internal.

Quoted 6 lines

drivers/haptic/Kconfig

Quoted 4 lines
drivers/haptic/Kconfig new file +3 -0
+
+static int samsung_pwm_haptic_probe(struct platform_device *pdev)
+{

__devinit?

Fixed.

Quoted 14 lines

drivers/haptic/Kconfig

Quoted 12 lines

+       struct haptic_platform_data *pdata = pdev->dev.platform_data;
+       struct samsung_pwm_haptic *haptic;
+       int ret;

drivers/haptic/Kconfig new file +8 -0
+
+       haptic = kzalloc(sizeof(struct samsung_pwm_haptic), GFP_KERNEL);
+       if (!haptic) {
+               dev_err(&pdev->dev, "No memory for device\n");
+               return -ENOMEM;
+       }
+static int samsung_pwm_haptic_remove(struct platform_device *pdev)
+{

__devexit

Fixed

Quoted 7 lines

drivers/haptic/Kconfig

Quoted 5 lines
drivers/haptic/Kconfig new file +4 -0
+
+static int __devexit isa1200_remove(struct i2c_client *client)
+{
+       return 0;

nothing to remove?

Sorry I'm not yet implemented. Will fix it.

Quoted 17 lines
Quoted 5 lines
drivers/haptic/Kconfig new file +4 -0
+}
+
+static int isa1200_suspend(struct i2c_client *client, pm_message_t mesg)
+{

drivers/haptic/Kconfig

Quoted 10 lines
drivers/haptic/Kconfig new file +10 -0
+       struct isa1200_chip *chip = i2c_get_clientdata(client);
+       isa1200_chip_power_off(chip);
+       return 0;
+}
+
+static int isa1200_resume(struct i2c_client *client)
+{
+       isa1200_setup(client);
+       return 0;
+}

#ifdef CONFIG_PM checks please.

Of course.

Thank you,
Kyungmin Park

#4 lore raw
In reply to Kyungmin Park
Quoted 8 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

New device classes should certainly be documented in Documentation/.

For example... is level 0-100? Is it valid to change level while 'oneshot' is running? Does oneshot use last level set?

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
author
#5 lore raw
Quoted 5 lines · Pavel Machek
Quoted 4 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

Quoted 8 lines
Quoted 4 lines

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

New device classes should certainly be documented in Documentation/.

For example... is level 0-100?

Basically. range from 0 to 255. also each device can set max level in case of isa1000 it's 6.

Quoted 1 line

Is it valid to change level while 'oneshot' is running?

we usually use it under 1 sec. but I think it's possible to change it but not tried.

Quoted 1 line

Does oneshot use last level set?

Right. it used last set value.

If you don't mind to update these at Documentation please wait until commit first patches merged.

Thank you,
Kyungmin Park

#6 lore raw

Hi Kyungmin,

Quoted 23 lines · Kyungmin Park
Quoted 5 lines · Pavel Machek
Quoted 4 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

Quoted 8 lines
Quoted 4 lines

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

New device classes should certainly be documented in Documentation/.

For example... is level 0-100?

Basically. range from 0 to 255. also each device can set max level in case of isa1000 it's 6.

Quoted 1 line

 Is it valid to change level while 'oneshot' is running?

we usually use it under 1 sec. but I think it's possible to change it but not tried.

Quoted 6 lines
Quoted 1 line

 Does oneshot use last level set?

Right. it used last set value.

If you don't mind to update these at Documentation please wait until commit first patches merged.

Let's see what Greg prefers. Could you please refresh your patchset with probably addressing comments given in the isa1200 and samsung_pwm driver by me?

Signature · 3 lines
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
#7 lore raw
Quoted 28 lines · Trilok Soni

Hi Kyungmin,

Quoted 25 lines · Kyungmin Park
Quoted 5 lines · Pavel Machek
Quoted 4 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

Quoted 8 lines
Quoted 4 lines

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

New device classes should certainly be documented in Documentation/.

For example... is level 0-100?

Basically. range from 0 to 255. also each device can set max level in case of isa1000 it's 6.

Quoted 1 line

 Is it valid to change level while 'oneshot' is running?

we usually use it under 1 sec. but I think it's possible to change it but not tried.

Quoted 1 line

 Does oneshot use last level set?

Quoted 8 lines
Quoted 4 lines

Right. it used last set value.

If you don't mind to update these at Documentation please wait until commit first patches merged.

Let's see what Greg prefers. Could you please refresh your patchset with probably addressing comments given in the isa1200 and samsung_pwm driver by me?

Greg wants to see patches that add sysfs files to the kernel, also contain Documentation/ABI/ updates as well.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#8 lore raw
In reply to Kyungmin Park
Quoted 11 lines · Kyungmin Park
Quoted 10 lines · Pavel Machek
Quoted 8 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Quoted 6 lines
Quoted 3 lines

New device classes should certainly be documented in Documentation/.

For example... is level 0-100?

Basically. range from 0 to 255. also each device can set max level in case of isa1000 it's 6.

How will userspace know the range?

Quoted 4 lines
Quoted 1 line

Is it valid to change level while 'oneshot' is running?

we usually use it under 1 sec. but I think it's possible to change it but not tried.

You are specifying interface here. You have to decide if it is valid or not, then document it.

Quoted 6 lines
Quoted 1 line

Does oneshot use last level set?

Right. it used last set value.

If you don't mind to update these at Documentation please wait until commit first patches merged.

I'd expect your reply to be merged into the Documentation, and that to be merged with the rest of the patches...
Pavel

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#9 lore raw
In reply to Kyungmin Park
Quoted 8 lines · Kyungmin Park

This patch includes two haptic devices, isa1000 and isa1200 ISA1000 is gpio based haptic, but isa1200 is based on I2C Both are working on Samsung SoCs and tested.

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Please never add any custom files to the top-level of a class directory. This place is reserved for devices, and not for custom files. It's a serious bug in the layout and API of sysfs that this allowed at all.

If you need these subsystem-wide contols please use a bus and not a class to stuff these files into a place where they don't mix up with the list of devices belonging to a class. Buses have all devices in a devices/ subdir so they will not conflict.

Thanks,
Kay

#10 lore raw
Quoted 11 lines · Kay Sievers
Quoted 4 lines · Kyungmin Park

To enable the haptic, echo 1 > /sys/class/haptic/${name}/enable You can adjust the level by echo ${level} > /sys/class/haptic/${name}/enable or
With oneshot feature, echo ${msec time} > /sys/class/haptic/${name}/oneshot

Please never add any custom files to the top-level of a class directory. This place is reserved for devices, and not for custom files. It's a serious bug in the layout and API of sysfs that this allowed at all.

I'm assuming that ${name} is the device name in the above in which case it should be fine from that point of view?

#11 lore raw
In reply to Kyungmin Park
Quoted 12 lines · Kyungmin Park
drivers/haptic/haptic-class.c new file +12 -0
+/**
+ * haptic_classdev_register - register a new object of haptic_classdev class.
+ * @dev: The device to register.
+ * @haptic_cdev: the haptic_classdev structure for this device.
+ */
+int haptic_classdev_register(struct device *parent,
+                               struct haptic_classdev *haptic_cdev)
+{
+       int ret;
+
+       haptic_cdev->dev = device_create(haptic_class, parent, 0,
+                               haptic_cdev, "%s", haptic_cdev->name);

drivers/haptic/haptic-class.c line 174

Quoted 10 lines
drivers/haptic/haptic-class.c new file +10 -0
+       if (IS_ERR(haptic_cdev->dev))
+               return PTR_ERR(haptic_cdev->dev);
+
+       /* register the attributes */
+       ret = class_create_file(haptic_class, &class_attr_enable);
+       if (ret) {
+               printk(KERN_ERR "%s: class_create_file(enable) failed\n",
+                                __func__);
+               return ret;
+       }

As mentioned in an earlier mail, this needs some explanation. What are you doing here? Creating a device below a class, and then add a bunch of attributes to the class itself, instead of the device you created?

All calls to class_create_file() need to go, we can not allow any new users of this broken interface. As mentioned, if you need subsystem-wide attributes you need to use a bus and not a class, classes are flat and can not handle such things properly.

Thanks,
Kay

#12 lore raw

Hi Kyungmin,

Quoted 29 lines · Kay Sievers
Quoted 12 lines · Kyungmin Park
drivers/haptic/haptic-class.c new file +12 -0
+/**
+ * haptic_classdev_register - register a new object of haptic_classdev class.
+ * @dev: The device to register.
+ * @haptic_cdev: the haptic_classdev structure for this device.
+ */
+int haptic_classdev_register(struct device *parent,
+                               struct haptic_classdev *haptic_cdev)
+{
+       int ret;
+
+       haptic_cdev->dev = device_create(haptic_class, parent, 0,
+                               haptic_cdev, "%s", haptic_cdev->name);

drivers/haptic/haptic-class.c line 174

Quoted 10 lines
drivers/haptic/haptic-class.c new file +10 -0
+       if (IS_ERR(haptic_cdev->dev))
+               return PTR_ERR(haptic_cdev->dev);
+
+       /* register the attributes */
+       ret = class_create_file(haptic_class, &class_attr_enable);
+       if (ret) {
+               printk(KERN_ERR "%s: class_create_file(enable) failed\n",
+                                __func__);
+               return ret;
+       }

As mentioned in an earlier mail, this needs some explanation. What are you doing here? Creating a device below a class, and then add a bunch of attributes to the class itself, instead of the device you created?

Quoted 8 lines

All calls to class_create_file() need to go, we can not allow any new users of this broken interface. As mentioned, if you need subsystem-wide attributes you need to use a bus and not a class, classes are flat and can not handle such things properly.

Thanks,
Kay

Any plan of refreshing these patches?

Signature · 7 lines
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html