On 03/14/2012 05:46 PM, Dmitry Torokhov wrote:

Hi Chanwoo,

On Mon, Mar 12, 2012 at 05:31:21PM +0900, Chanwoo Choi wrote:

quoted

+
+static int __devexit max8997_haptic_remove(struct platform_device *pdev)
+{
+	struct max8997_haptic *chip = platform_get_drvdata(pdev);
+
+	destroy_work_on_stack(&chip->work);

This does not make sense because:

- chip->work is not on stack
- even if you "destroy" work nothing stops play effect to schedule a new
  one - you need to cancel work in close method.

Agreed, I will fix it.

quoted

+	input_unregister_device(chip->input_dev);
+	regulator_put(chip->regulator);
+
+	if (chip->mode == MAX8997_EXTERNAL_MODE)
+		pwm_free(chip->pwm);
+
+	kfree(chip);
+
+	return 0;
+}
+
+static int max8997_haptic_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct max8997_haptic *chip = platform_get_drvdata(pdev);
+	struct input_dev *input_dev = chip->input_dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&input_dev->event_lock, flags);
+	max8997_haptic_enable(chip, false);
+	spin_unlock_irqrestore(&input_dev->event_lock, flags);
+

This is not proper locking between playing effect and suspend. Now that
you are using workqueue you need to synchronize access with it, not with
play_effect method.

Does the following patch work for you?

You're right. As you said, I face with the kernel panic when enter
suspend state
while testing max8997-haptic by using fftest at once.

Previously, I  tested for each operation of max8997-haptic and
suspend/resume.
I will fix it according to your comment.

Thank you for your comment.

Best Regards,
Chanwoo Choi