Re: [PATCH v2 02/23] OMAPDSS: outputs: Create and register output instances

[Archit Taneja <a0393947@ti.com>]

On Friday 31 August 2012 05:27 PM, Tomi Valkeinen wrote:
> On Thu, 2012-08-30 at 17:10 +0530, Archit Taneja wrote:
>> Add output structs to output driver's private data. Register output instances by
>> having an init function in the probes of the platform device drivers for
>> different outputs. The *_init_output for each output registers the output and
>> fill up the output's plaform device, type and id fields.
>>
>> In the probe of each interface driver, the output entities are initialized
>> before the *_probe_pdata() functions intentionally. This is done to ensure that
>> the output entity is prepared before the panels connected to the output are
>> registered. We need the output entities to be ready because OMAPDSS will try
>> to make connections between overlays, managers, outputs and devices during the
>> panel's probe.
>>
>> Signed-off-by: Archit Taneja <archit@ti.com>
>> ---
>>   drivers/video/omap2/dss/dpi.c  |   15 +++++++++++++++
>>   drivers/video/omap2/dss/dsi.c  |   18 ++++++++++++++++++
>>   drivers/video/omap2/dss/hdmi.c |   15 +++++++++++++++
>>   drivers/video/omap2/dss/rfbi.c |   17 +++++++++++++++++
>>   drivers/video/omap2/dss/sdi.c  |   15 +++++++++++++++
>>   drivers/video/omap2/dss/venc.c |   15 +++++++++++++++
>>   6 files changed, 95 insertions(+)
>>
>> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c
>> index 25fb895..9a7aee5 100644
>> --- a/drivers/video/omap2/dss/dpi.c
>> +++ b/drivers/video/omap2/dss/dpi.c
>> @@ -45,6 +45,8 @@ static struct {
>>   	struct omap_video_timings timings;
>>   	struct dss_lcd_mgr_config mgr_config;
>>   	int data_lines;
>> +
>> +	struct omap_dss_output output;
>>   } dpi;
>>
>>   static struct platform_device *dpi_get_dsidev(enum omap_dss_clk_source clk)
>> @@ -410,10 +412,23 @@ static void __init dpi_probe_pdata(struct platform_device *pdev)
>>   	}
>>   }
>>
>> +static void __init dpi_init_output(struct platform_device *pdev)
>> +{
>> +	struct omap_dss_output *out = &dpi.output;
>> +
>> +	dss_register_output(out);
>> +
>> +	out->pdev = pdev;
>> +	out->id = OMAP_DSS_OUTPUT_DPI;
>> +	out->type = OMAP_DISPLAY_TYPE_DPI;
>> +}
>
> I think you should first initialize the output, and then register it.
> Not the other way around.

Ok.

>
> I believe you need to implement unregister also. Normally unregister
> won't be done, but the probe of an output driver can fail after the
> output has been registered, and thus the output needs to be unregistered
> at cleanup.

Ah, right.

>
> And it doesn't harm to unregister at the driver's remove.

I'll fix these, thanks.

Archit