Re: [PATCH 2/5 v2] Input: bu21013_ts - Move GPIO init and exit functions into the driver

On Mon, 26 Nov 2012, Dmitry Torokhov wrote:

Hi Lee,

On Mon, Nov 26, 2012 at 12:01:43PM +0000, Lee Jones wrote:

quoted

 /**
+ * bu21013_gpio_board_init() - configures the touch panel
+ * @reset_pin: reset pin number
+ *
+ * This function is used to configure the voltage and
+ * reset the touch panel controller.
+ */
+static int bu21013_gpio_board_init(int reset_pin)
+{
+	int retval = 0;
+
+	retval = gpio_request_one(reset_pin, GPIOF_INIT_HIGH, "touchp_reset");

Also need to specify dircetion (even though it defaults to output).

You do? Okay, I'll do that.

quoted

+	if (retval) {
+		printk(KERN_ERR "Unable to request gpio reset_pin");
+		return retval;
+	}
+
+	return retval;
+}
+
+/**
+ * bu21013_gpio_board_exit() - deconfigures the touch panel controller
+ * @reset_pin: reset pin number
+ *
+ * This function is used to deconfigure the chip selection
+ * for touch panel controller.
+ */
+static int bu21013_gpio_board_exit(int reset_pin)
+{
+	int retval = 0;
+
+	retval = gpio_direction_output(reset_pin, 0);
+	if (retval < 0) {
+		printk(KERN_ERR "%s: gpio direction failed\n",
+		       __func__);
+		return retval;

You should not return here, as gpio has to be freed even if you unable
to toggle it.

You're right.

quoted

+	}
+	gpio_set_value(reset_pin, 0);
+	gpio_free(reset_pin);
+
+	return retval;
+}
+
+/**
  * bu21013_init_chip() - power on sequence for the bu21013 controller
  * @data: device structure pointer
  *

@@ -449,6 +493,8 @@ static int __devinit bu21013_probe(struct i2c_client *client,
 		return -EINVAL;
 	}
 
+	pdata->irq = gpio_to_irq(pdata->touch_pin);
+

No, you still can not do it since pdata is a const pointer. Does not
your compiler throw a warning here?

I already sent you a version of the patch that does not have these
issues, is there a particular reason why you needed to roll your own
instead of simply trying out the one I sent?

I didn't see the other patch, sorry.

I'll check my mail and apply it.

-- 
Lee Jones
Linaro ST-Ericsson 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