On 12/07/2013 04:48 AM, Javier Martinez Canillas wrote:
> Hi Tomi,
>
> On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <[email protected]> wrote:
>> Hi,
>>
>> Here's a new version for DT support to OMAP Display Subsystem. See
>> http://article.gmane.org/gmane.linux.ports.arm.omap/102689 for the intro of
>> the
>> previous version, which contains thoughts about the related problems.
>>
>> The major change in this version is the use of V4L2 and CDF style
>> port/endpoint
>> style in the DT data. However, note that even if the DT data contains proper
>> port & endpoint data, the drivers only use the first endpoint. This is to
>> simplify the patches, as adding full support for the ports and endpoints to
>> the
>> drivers will be a big task. This approach still works with more or less all
>> the
>> boards, as the only cases where the simpler model is an issue are the boards
>> with multiple display devices connected to a single output.
>>
>> Laurent, I'd appreciate if you could have a look at the .dts changes, to see
>> if
>> there's anything that's clearly not CDF compatible.
>>
>> The patches can also be found from:
>> git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git work/dss-dt
>>
>
> I tested your branch on my DM3730 IGEPv2 board by cherry-picking the following
> commits from latest Linus tree on top of your work/dss-dt branch:
>
> d526daeb ("ARM: dts: omap3-igep0020: Add pinmux setup for i2c devices")
> 50592dc3 ("ARM: dts: omap3-igep0020: Add pinmuxing for DVI output")
>
> And adding the display information using your DSS bindings to
> omap3-igep0020.dts
> [0].
>
> But it failed for me because of a probing order. The problem is that the
> probing
> order is:
>
> omap_i2c
> pinctrl-single
> OMAP DSS
> connector-dvi
> omapfb
>
> Now DT good practices says that the pinmux setup needed for a device have to
> be
> initialized in a pin control state for the device using these pins (i.e: i2c3)
> instead of doing globally by being part of a pinctrl state of the pinmux
> device
> (i.e: omap3_pmx_core).
>
> But due this init order the omap_i2c probe is deferred due pinctrl-single not
> being initialized yet:
>
> [ 0.565246] omap_i2c 48060000.i2c: could not find pctldev for node
> /ocp/pinmu
>
>
> x@48002030/pinmux_i2c3_pins, deferring probe
>
> This is ok since eventually the pinctrl-single driver will be initialized and
> the next time the omap_i2c probe is called it will succeed. But before
> omap_i2c
> has a chance to be probed again the connector-dvi driver is probed and fails
> due
> the i2c bus not being initialized yet:
>
> [ 1.064300] OMAP DSS rev 2.0
> [ 1.073669] connector-dvi connector.12: failed to parse i2c-bus
> [ 1.073730] connector-dvi: probe of connector.12 failed with error -22
> [ 1.078216] omapfb omapfb: no displays
> [ 1.080871] omapfb omapfb: failed to setup omapfb
> [ 1.080932] platform omapfb: Driver omapfb requests probe deferral
> ..
>
> Later the omap_i2c driver probe succees since the pinctrl-single driver was
> initialized but by then it is already too late:
>
> [ 3.293914] omap_i2c 48070000.i2c: bus 0 rev4.4 at 2600 kHz
> [ 3.301940] omap_i2c 48072000.i2c: bus 1 rev4.4 at 400 kHz
> [ 3.310882] omap_i2c 48060000.i2c: bus 2 rev4.4 at 100 kHz
> [ 3.317565] omapfb omapfb: no displays
> [ 3.321716] omapfb omapfb: failed to setup omapfb
> [ 3.326751] platform omapfb: Driver omapfb requests probe deferral
> ..
> [ 3.694915] omapfb omapfb: no displays
> [ 3.699127] omapfb omapfb: failed to setup omapfb
> [ 3.704071] platform omapfb: Driver omapfb requests probe deferral
> ..
>
> If I move the i2c3 pinmux definitions from the i2c3 device node to
> omap3_pmx_core then the display works correctly.
>
> So, I think that we should add deferred probing to drivers/video/omap2/*.c too
> to avoid the scenario I described above.
>
Actually, I looked at drivers/video/omap2/connector-dvi.c and it does the right
thing for legacy platform data probing but no for DT probing:
static int dvic_probe_pdata(struct platform_device *pdev)
{
..
adapter = i2c_get_adapter(i2c_bus_num);
if (!adapter) {
dev_err(&pdev->dev,
"Failed to get I2C adapter, bus %d\n",
i2c_bus_num);
return -EPROBE_DEFER;
}
..
}
static int dvic_probe_of(struct platform_device *pdev)
{
..
adapter = of_find_i2c_adapter_by_node(adapter_node);
if (adapter == NULL) {
dev_err(&pdev->dev, "failed to parse i2c-bus\n");
omap_dss_put_device(ddata->in);
return -EINVAL;
}
..
}
The following patch solves the issue if you want to include in your patch-set:
>From c85d46b94c3d27d30218af23a6a8d61e6c7d2ae8 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <[email protected]>
Date: Sat, 7 Dec 2013 05:18:56 +0100
Subject: [PATCH 1/1] OMAPDSS: connector-dvi: add deferred probing support for
OF path
When booting with Device Trees the order in which device drivers
are probed is not defined so drivers should be able to defer its
probing if a dependency is not found (such as an i2c bus) instead
of just failing.
Signed-off-by: Javier Martinez Canillas <[email protected]>
---
drivers/video/omap2/displays-new/connector-dvi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/omap2/displays-new/connector-dvi.c
b/drivers/video/omap2/displays-new/connector-dvi.c
index 8f7e576..f94344a 100644
--- a/drivers/video/omap2/displays-new/connector-dvi.c
+++ b/drivers/video/omap2/displays-new/connector-dvi.c
@@ -299,7 +299,7 @@ static int dvic_probe_of(struct platform_device *pdev)
if (adapter == NULL) {
dev_err(&pdev->dev, "failed to parse i2c-bus\n");
omap_dss_put_device(ddata->in);
- return -EINVAL;
+ return -EPROBE_DEFER;
}
ddata->i2c_adapter = adapter;
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
