diff mbox

[27/33] OMAPDSS: n8x0 panel: handle gpio data in panel driver

Message ID 1360765345-19312-28-git-send-email-archit@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

archit taneja Feb. 13, 2013, 2:22 p.m. UTC
The n8x0 panel driver leaves gpio configurations to the platform_enable and
disable calls in the platform's board file. These should happen in the panel
driver itself.

A platform data struct called panel_n8x0_data already exists to hold gpio
numbers and other platform data. However, the gpio requests are expected to be
done in the board file and not the panel driver.

Request all the gpios in the panel driver so that the board files which use
the the panel don't need to do it. This will help in removing the need for the
panel drivers to have platform related callbacks.

Signed-off-by: Archit Taneja <archit@ti.com>
---
 drivers/video/omap2/displays/panel-n8x0.c |   15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Aaro Koskinen Feb. 13, 2013, 5:35 p.m. UTC | #1
Hi,

On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
> @@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
>  	dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
>  	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
>  
> +	if (gpio_is_valid(bdata->panel_reset)) {
> +		r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
> +				GPIOF_OUT_INIT_LOW, "PANEL RESET");
> +		if (r)
> +			return r;
> +	}
> +
> +	if (gpio_is_valid(bdata->ctrl_pwrdown)) {
> +		r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
> +				GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
> +		if (r)
> +			return r;
> +	}
> +

In the error case, the other GPIO is not freed. Also maybe you should
free them on module removal, because now the module owns the GPIOs.

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
archit taneja Feb. 14, 2013, 6:34 a.m. UTC | #2
Hi,

On Wednesday 13 February 2013 11:05 PM, Aaro Koskinen wrote:
> Hi,
>
> On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
>> @@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
>>   	dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
>>   	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
>>
>> +	if (gpio_is_valid(bdata->panel_reset)) {
>> +		r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
>> +				GPIOF_OUT_INIT_LOW, "PANEL RESET");
>> +		if (r)
>> +			return r;
>> +	}
>> +
>> +	if (gpio_is_valid(bdata->ctrl_pwrdown)) {
>> +		r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
>> +				GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
>> +		if (r)
>> +			return r;
>> +	}
>> +
>
> In the error case, the other GPIO is not freed. Also maybe you should
> free them on module removal, because now the module owns the GPIOs.

Wouldn't the usage of devm_* functions take care of this? If the device 
isn't registered successfully, then all allocations/requests done using 
devm_* functions will be free'd automatically.

Archit

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Aaro Koskinen Feb. 14, 2013, 12:45 p.m. UTC | #3
On Thu, Feb 14, 2013 at 12:04:32PM +0530, Archit Taneja wrote:
> On Wednesday 13 February 2013 11:05 PM, Aaro Koskinen wrote:
> >On Wed, Feb 13, 2013 at 07:52:19PM +0530, Archit Taneja wrote:
> >>@@ -444,6 +445,20 @@ static int n8x0_panel_probe(struct omap_dss_device *dssdev)
> >>  	dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
> >>  	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
> >>
> >>+	if (gpio_is_valid(bdata->panel_reset)) {
> >>+		r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
> >>+				GPIOF_OUT_INIT_LOW, "PANEL RESET");
> >>+		if (r)
> >>+			return r;
> >>+	}
> >>+
> >>+	if (gpio_is_valid(bdata->ctrl_pwrdown)) {
> >>+		r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
> >>+				GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
> >>+		if (r)
> >>+			return r;
> >>+	}
> >>+
> >
> >In the error case, the other GPIO is not freed. Also maybe you should
> >free them on module removal, because now the module owns the GPIOs.
> 
> Wouldn't the usage of devm_* functions take care of this? If the
> device isn't registered successfully, then all allocations/requests
> done using devm_* functions will be free'd automatically.

Sorry, I didn't realized they are devm_* now. You are right.

A.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/video/omap2/displays/panel-n8x0.c b/drivers/video/omap2/displays/panel-n8x0.c
index 9c55c91..c146a3d 100644
--- a/drivers/video/omap2/displays/panel-n8x0.c
+++ b/drivers/video/omap2/displays/panel-n8x0.c
@@ -426,6 +426,7 @@  static int n8x0_panel_probe(struct omap_dss_device *dssdev)
 {
 	struct panel_n8x0_data *bdata = get_board_data(dssdev);
 	struct panel_drv_data *ddata;
+	int r;
 
 	dev_dbg(&dssdev->dev, "probe\n");
 
@@ -444,6 +445,20 @@  static int n8x0_panel_probe(struct omap_dss_device *dssdev)
 	dssdev->ctrl.rfbi_timings = n8x0_panel_timings;
 	dssdev->caps = OMAP_DSS_DISPLAY_CAP_MANUAL_UPDATE;
 
+	if (gpio_is_valid(bdata->panel_reset)) {
+		r = devm_gpio_request_one(&dssdev->dev, bdata->panel_reset,
+				GPIOF_OUT_INIT_LOW, "PANEL RESET");
+		if (r)
+			return r;
+	}
+
+	if (gpio_is_valid(bdata->ctrl_pwrdown)) {
+		r = devm_gpio_request_one(&dssdev->dev, bdata->ctrl_pwrdown,
+				GPIOF_OUT_INIT_LOW, "PANEL PWRDOWN");
+		if (r)
+			return r;
+	}
+
 	return 0;
 }