Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH 18/26] ARM: omap3-beagle.dts: add display information
From: Javier Martinez Canillas @ 2013-12-09 12:16 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <52A5B248.5050303@ti.com>

Hi Tomi,

On Mon, Dec 9, 2013 at 1:06 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 2013-12-06 10:41, Javier Martinez Canillas wrote:
>> Hi Tomi,
>>
>> On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>  arch/arm/boot/dts/omap3-beagle.dts | 67 ++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 67 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
>>> index fa532aaacc68..1ca1932d02aa 100644
>>> --- a/arch/arm/boot/dts/omap3-beagle.dts
>>> +++ b/arch/arm/boot/dts/omap3-beagle.dts
>>> @@ -178,3 +178,70 @@
>>>         mode = <3>;
>>>         power = <50>;
>>>  };
>>> +
>>> +&dpi {
>>> +       vdds_dsi-supply = <&vpll2>;
>>> +
>>> +       dpi_out: endpoint {
>>> +               remote-endpoint = <&tfp410_in>;
>>> +               data-lines = <24>;
>>> +       };
>>> +};
>>> +
>>> +&venc {
>>> +       vdda_dac-supply = <&vdac>;
>>> +
>>> +       venc_out: endpoint {
>>> +               remote-endpoint = <&tv_connector_in>;
>>> +       };
>>> +};
>>> +
>>> +/ {
>>> +       aliases {
>>> +               display0 = &dvi0;
>>> +               display1 = &tv0;
>>> +       };
>>> +
>>> +       tfp410: encoder@0 {
>>> +               compatible = "ti,tfp410";
>>> +               gpios = <&gpio5 10 0>;  /* 170, power-down */
>>> +
>>
>> Shouldn't this be gpio6 instead since OMAP GPIO banks start counting
>> from 1 so gpio5 + 10 is GPIO 138.
>
> Yes, you're right. Good catch.
>
>>> +               ports {
>>> +                       #address-cells = <1>;
>>> +                       #size-cells = <0>;
>>> +
>>> +                       port@0 {
>>> +                               reg = <0>;
>>> +
>>> +                               tfp410_in: endpoint@0 {
>>> +                                       remote-endpoint = <&dpi_out>;
>>> +                               };
>>> +                       };
>>> +
>>> +                       port@1 {
>>> +                               reg = <1>;
>>> +
>>> +                               tfp410_out: endpoint@1 {
>>> +                                       remote-endpoint = <&dvi_connector_in>;
>>> +                               };
>>> +                       };
>>> +               };
>>> +       };
>>> +
>>> +       dvi0: connector@0 {
>>> +               compatible = "ti,dvi_connector";
>>> +               i2c-bus = <&i2c3>;
>>> +
>>> +               dvi_connector_in: endpoint {
>>> +                       remote-endpoint = <&tfp410_out>;
>>> +               };
>>> +       };
>>> +
>>> +       tv0: connector@1 {
>>> +               compatible = "ti,svideo_connector";
>>> +
>>> +               tv_connector_in: endpoint {
>>> +                       remote-endpoint = <&venc_out>;
>>> +               };
>>> +       };
>>> +};
>>> --
>>> 1.8.3.2
>>>
>>
>> Also I don't see the DSS pinmux set for this board. I guess you need
>> something like the following on top:
>>
>> 0x0a4 (PIN_OUTPUT | MUX_MODE0)   /* dss_pclk.dss_pclk */
>> 0x0a6 (PIN_OUTPUT | MUX_MODE0)   /* dss_hsync.dss_hsync */
>> 0x0a8 (PIN_OUTPUT | MUX_MODE0)   /* dss_vsync.dss_vsync */
>> 0x0aa (PIN_OUTPUT | MUX_MODE0)   /* dss_acbias.dss_acbias */
>> 0x0ac (PIN_OUTPUT | MUX_MODE0)   /* dss_data0.dss_data0 */
>> 0x0ae (PIN_OUTPUT | MUX_MODE0)   /* dss_data1.dss_data1 */
>> 0x0b0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data2.dss_data2 */
>> 0x0b2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data3.dss_data3 */
>> 0x0b4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data4.dss_data4 */
>> 0x0b6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data5.dss_data5 */
>> 0x0b8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data6.dss_data6 */
>> 0x0ba (PIN_OUTPUT | MUX_MODE0)   /* dss_data7.dss_data7 */
>> 0x0bc (PIN_OUTPUT | MUX_MODE0)   /* dss_data8.dss_data8 */
>> 0x0be (PIN_OUTPUT | MUX_MODE0)   /* dss_data9.dss_data9 */
>> 0x0c0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data10.dss_data10 */
>> 0x0c2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data11.dss_data11 */
>> 0x0c4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data12.dss_data12 */
>> 0x0c6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data13.dss_data13 */
>> 0x0c8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data14.dss_data14 */
>> 0x0ca (PIN_OUTPUT | MUX_MODE0)   /* dss_data15.dss_data15 */
>> 0x0cc (PIN_OUTPUT | MUX_MODE0)   /* dss_data16.dss_data16 */
>> 0x0ce (PIN_OUTPUT | MUX_MODE0)   /* dss_data17.dss_data17 */
>> 0x0d0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data18.dss_data18 */
>> 0x0d2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data19.dss_data19 */
>> 0x0d4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data20.dss_data20 */
>> 0x0d6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data21.dss_data21 */
>> 0x0d8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data22.dss_data22 */
>> 0x0da (PIN_OUTPUT | MUX_MODE0)   /* dss_data23.dss_data23 */
>
> True. I need to add something like this to all the dts files.
>
> You didn't have muxing in the omap3-igep0020 patch you gave. Is the
> muxing already set in some other patch?
>

Your patch-set is based on v3.13-rc1 but I sent a patch-set [0] to fix
a few regressions for IGEPv2 when we moved to DT-only boot. One of
these patches added the needed DSS pinmuxing and these were included
in v3.13-rc2.

So to test your branch I cherry-picked:

d526daeb ("ARM: dts: omap3-igep0020: Add pinmux setup for i2c devices")
50592dc3 ("ARM: dts: omap3-igep0020: Add pinmuxing for DVI output")

Also, for 3.13 I added a DT hack to make dpi_init_regulator() succeed
and find the VDDS_DSI regulator. That patch is mainline comit

2f2befd82 ("ARM: dts: omap3-igep0020: name twl4030 VPLL2 regulator as vdds_dsi")

Now that I think about it, you may want to add a revert for that patch
on your series too since this hack is not needed anymore with your DSS
bindings.

>  Tomi
>

Thanks a lot and best regards,
Javier
>

[0]: http://www.spinics.net/lists/linux-omap/msg99970.html

^ permalink raw reply

* Re: [PATCH 18/26] ARM: omap3-beagle.dts: add display information
From: Tomi Valkeinen @ 2013-12-09 12:06 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <CABxcv=mgQM6oHf7=-EbryAcmgJm3vf+xXqHW6BCdW8ovS29O0g@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 4875 bytes --]

On 2013-12-06 10:41, Javier Martinez Canillas wrote:
> Hi Tomi,
> 
> On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  arch/arm/boot/dts/omap3-beagle.dts | 67 ++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 67 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
>> index fa532aaacc68..1ca1932d02aa 100644
>> --- a/arch/arm/boot/dts/omap3-beagle.dts
>> +++ b/arch/arm/boot/dts/omap3-beagle.dts
>> @@ -178,3 +178,70 @@
>>         mode = <3>;
>>         power = <50>;
>>  };
>> +
>> +&dpi {
>> +       vdds_dsi-supply = <&vpll2>;
>> +
>> +       dpi_out: endpoint {
>> +               remote-endpoint = <&tfp410_in>;
>> +               data-lines = <24>;
>> +       };
>> +};
>> +
>> +&venc {
>> +       vdda_dac-supply = <&vdac>;
>> +
>> +       venc_out: endpoint {
>> +               remote-endpoint = <&tv_connector_in>;
>> +       };
>> +};
>> +
>> +/ {
>> +       aliases {
>> +               display0 = &dvi0;
>> +               display1 = &tv0;
>> +       };
>> +
>> +       tfp410: encoder@0 {
>> +               compatible = "ti,tfp410";
>> +               gpios = <&gpio5 10 0>;  /* 170, power-down */
>> +
> 
> Shouldn't this be gpio6 instead since OMAP GPIO banks start counting
> from 1 so gpio5 + 10 is GPIO 138.

Yes, you're right. Good catch.

>> +               ports {
>> +                       #address-cells = <1>;
>> +                       #size-cells = <0>;
>> +
>> +                       port@0 {
>> +                               reg = <0>;
>> +
>> +                               tfp410_in: endpoint@0 {
>> +                                       remote-endpoint = <&dpi_out>;
>> +                               };
>> +                       };
>> +
>> +                       port@1 {
>> +                               reg = <1>;
>> +
>> +                               tfp410_out: endpoint@1 {
>> +                                       remote-endpoint = <&dvi_connector_in>;
>> +                               };
>> +                       };
>> +               };
>> +       };
>> +
>> +       dvi0: connector@0 {
>> +               compatible = "ti,dvi_connector";
>> +               i2c-bus = <&i2c3>;
>> +
>> +               dvi_connector_in: endpoint {
>> +                       remote-endpoint = <&tfp410_out>;
>> +               };
>> +       };
>> +
>> +       tv0: connector@1 {
>> +               compatible = "ti,svideo_connector";
>> +
>> +               tv_connector_in: endpoint {
>> +                       remote-endpoint = <&venc_out>;
>> +               };
>> +       };
>> +};
>> --
>> 1.8.3.2
>>
> 
> Also I don't see the DSS pinmux set for this board. I guess you need
> something like the following on top:
> 
> 0x0a4 (PIN_OUTPUT | MUX_MODE0)   /* dss_pclk.dss_pclk */
> 0x0a6 (PIN_OUTPUT | MUX_MODE0)   /* dss_hsync.dss_hsync */
> 0x0a8 (PIN_OUTPUT | MUX_MODE0)   /* dss_vsync.dss_vsync */
> 0x0aa (PIN_OUTPUT | MUX_MODE0)   /* dss_acbias.dss_acbias */
> 0x0ac (PIN_OUTPUT | MUX_MODE0)   /* dss_data0.dss_data0 */
> 0x0ae (PIN_OUTPUT | MUX_MODE0)   /* dss_data1.dss_data1 */
> 0x0b0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data2.dss_data2 */
> 0x0b2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data3.dss_data3 */
> 0x0b4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data4.dss_data4 */
> 0x0b6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data5.dss_data5 */
> 0x0b8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data6.dss_data6 */
> 0x0ba (PIN_OUTPUT | MUX_MODE0)   /* dss_data7.dss_data7 */
> 0x0bc (PIN_OUTPUT | MUX_MODE0)   /* dss_data8.dss_data8 */
> 0x0be (PIN_OUTPUT | MUX_MODE0)   /* dss_data9.dss_data9 */
> 0x0c0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data10.dss_data10 */
> 0x0c2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data11.dss_data11 */
> 0x0c4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data12.dss_data12 */
> 0x0c6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data13.dss_data13 */
> 0x0c8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data14.dss_data14 */
> 0x0ca (PIN_OUTPUT | MUX_MODE0)   /* dss_data15.dss_data15 */
> 0x0cc (PIN_OUTPUT | MUX_MODE0)   /* dss_data16.dss_data16 */
> 0x0ce (PIN_OUTPUT | MUX_MODE0)   /* dss_data17.dss_data17 */
> 0x0d0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data18.dss_data18 */
> 0x0d2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data19.dss_data19 */
> 0x0d4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data20.dss_data20 */
> 0x0d6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data21.dss_data21 */
> 0x0d8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data22.dss_data22 */
> 0x0da (PIN_OUTPUT | MUX_MODE0)   /* dss_data23.dss_data23 */

True. I need to add something like this to all the dts files.

You didn't have muxing in the omap3-igep0020 patch you gave. Is the
muxing already set in some other patch?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Tomi Valkeinen @ 2013-12-09 12:01 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren, Laurent Pinchart, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <52A2A3EF.1030306@collabora.co.uk>

[-- Attachment #1: Type: text/plain, Size: 1056 bytes --]

On 2013-12-07 06:28, Javier Martinez Canillas wrote:

> 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:

Thanks, I'll add this and the omap3-igep0020 support to my DT branch.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Javier Martinez Canillas @ 2013-12-07  4:28 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren, Laurent Pinchart, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <52A29A82.106@collabora.co.uk>

On 12/07/2013 04:48 AM, Javier Martinez Canillas wrote:
> Hi Tomi,
> 
> On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> 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 <javier.martinez@collabora.co.uk>
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 <javier.martinez@collabora.co.uk>
---
 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


^ permalink raw reply related

* Re: [PATCH 00/26] OMAPDSS: DT support (Christmas edition)
From: Javier Martinez Canillas @ 2013-12-07  3:48 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren, Laurent Pinchart, Stefan Roese, Sebastian Reichel,
	Robert Nelson, Dr . H . Nikolaus Schaller, Marek Belisko
In-Reply-To: <1386160133-24026-1-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> 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.

Also, would you mind to include [0] on your patch-set so display continue
working for IGEPv2 after you remove the pdata-quirks and dss-common.c hacks?

Thanks a lot and best regards,
Javier

[0]:
From a9af54a3b63bccade241e056d153d8bf04a6a5d5 Mon Sep 17 00:00:00 2001
From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
Date: Fri, 6 Dec 2013 02:53:38 +0100
Subject: [PATCH 1/1] ARM: dts: omap3-igep0020: add display information

The IGEPv2 has a TFP410 DPI-to-DVI encoder attached
to OMAP's Display SubSystem (DSS). Add DeviceTree
nodes for these devices.

Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
---
 arch/arm/boot/dts/omap3-igep0020.dts | 51 ++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/arch/arm/boot/dts/omap3-igep0020.dts
b/arch/arm/boot/dts/omap3-igep0020.dts
index 76509de..2569d60 100644
--- a/arch/arm/boot/dts/omap3-igep0020.dts
+++ b/arch/arm/boot/dts/omap3-igep0020.dts
@@ -215,3 +215,54 @@
 &usbhsehci {
 	phys = <&hsusb1_phy>;
 };
+
+&dpi {
+	vdds_dsi-supply = <&vpll2>;
+
+	dpi_out: endpoint {
+		remote-endpoint = <&tfp410_in>;
+		data-lines = <24>;
+	};
+};
+
+
+/ {
+	aliases {
+		display0 = &dvi0;
+	};
+
+	tfp410: encoder@0 {
+		compatible = "ti,tfp410";
+		gpios = <&gpio6 10 GPIO_ACTIVE_HIGH>; /* 170, power-down */
+
+		ports {
+			#address-cells = <1>;
+			#size-cells = <0>;
+
+			port@0 {
+				reg = <0>;
+
+				tfp410_in: endpoint@0 {
+					remote-endpoint = <&dpi_out>;
+				};
+			};
+
+			port@1 {
+				reg = <1>;
+
+				tfp410_out: endpoint@1 {
+					remote-endpoint = <&dvi_connector_in>;
+				};
+			};
+		};
+	};
+
+	dvi0: connector@0 {
+		compatible = "ti,dvi-connector";
+		i2c-bus = <&i2c3>;
+
+		dvi_connector_in: endpoint {
+			remote-endpoint = <&tfp410_out>;
+		};
+	};
+};
-- 
1.8.4.2

^ permalink raw reply related

* [PATCH] ARM: OMAPFB: panel-sony-acx565akm: fix missing unlock in acx565akm_panel_power_on()
From: Wei Yongjun @ 2013-12-06 12:55 UTC (permalink / raw)
  To: tomi.valkeinen, plagnioj, aaro.koskinen
  Cc: yongjun_wei, linux-omap, linux-fbdev
In-Reply-To: <1383773070-15114-1-git-send-email-aaro.koskinen@iki.fi>

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Add the missing unlock before return from function
acx565akm_panel_power_on() in the error handling case.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/video/omap2/displays-new/panel-sony-acx565akm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
index d94f35d..69aa4a1 100644
--- a/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
+++ b/drivers/video/omap2/displays-new/panel-sony-acx565akm.c
@@ -536,7 +536,7 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
 	r = in->ops.sdi->enable(in);
 	if (r) {
 		pr_err("%s sdi enable failed\n", __func__);
-		return r;
+		goto out;
 	}
 
 	/*FIXME tweak me */
@@ -547,7 +547,8 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
 
 	if (ddata->enabled) {
 		dev_dbg(&ddata->spi->dev, "panel already enabled\n");
-		return 0;
+		r = 0;
+		goto out;
 	}
 
 	/*
@@ -571,6 +572,10 @@ static int acx565akm_panel_power_on(struct omap_dss_device *dssdev)
 	mutex_unlock(&ddata->mutex);
 
 	return acx565akm_bl_update_status(ddata->bl_dev);
+
+out:
+	mutex_unlock(&ddata->mutex);
+	return r;
 }
 
 static void acx565akm_panel_power_off(struct omap_dss_device *dssdev)


^ permalink raw reply related

* Re: [PATCH 17/26] ARM: omap3-tobi.dts: add lcd (TEST)
From: Florian Vaussard @ 2013-12-06 10:18 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-omap, linux-fbdev, devicetree
  Cc: Archit Taneja, Darren Etheridge, Tony Lindgren
In-Reply-To: <1386160133-24026-18-git-send-email-tomi.valkeinen@ti.com>

Hello Tomi,

On 12/04/2013 01:28 PM, Tomi Valkeinen wrote:
> This is a test for Overo with Palo43 expansion, _not_ Tobi. Palo43
> doesn't have a dts, but seems to work ok with omap3-tobi.dts, so I used
> it as a test.
> 

Looking at the schematics, both seem pretty similar indeed. The main
difference seems to be the LCD on the palo43, and the HDMI on the Tobi
(TFP410, as in your patch 21).

I only have a Tobi on my desk, so this is the reason for not supporting
other boards... We could make a .dtsi with common stuff, then add the
LCD to the Palo43 and HDMI to the Tobi. I was going to add some more
support to these boards in the coming days, so I can add this too, at
least for testing purpose.

Cheers,

Florian

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add p =?UTF-8?B?d21yIGZvciB0aGUgZ
From: Alexander Shiyan @ 2013-12-06  9:40 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131206093017.GB24559@pengutronix.de>

PiA+ID4gT24gRnJpLCBEZWMgMDYsIDIwMTMgYXQgMTI6MDM6NTRQTSArMDQwMCwgQWxleGFuZGVy
IFNoaXlhbiB3cm90ZToKPiA+IC4uLgo+ID4gPiA+ID4gPiAgLSBmc2wsZG1hY3I6IERNQSBDb250
cm9sIFJlZ2lzdGVyIHZhbHVlLiBUaGlzIGlzIG9wdGlvbmFsLiBCeSBkZWZhdWx0LCB0aGUKPiA+
ID4gPiA+ID4gIAlyZWdpc3RlciBpcyBub3QgbW9kaWZpZWQgYXMgcmVjb21tZW5kZWQgYnkgdGhl
IGRhdGFzaGVldC4KPiA+ID4gPiA+ID4gKy0gZnNsLHB3bXI6ICBMQ0RDIFBXTSBDb250cmFzdCBD
b250cm9sIFJlZ2lzdGVyIHZhbHVlLiBUaGF0IHByb3BlcnR5IGlzCj4gPiA+ID4gPiA+ICsJb3B0
aW9uYWwsIGJ1dCBkZWZpbmluZyBpdCBpcyBuZWNlc3NhcnkgdG8gZ2V0IHRoZSBiYWNrbGlnaHQg
d29ya2luZy4gSWYgdGhhdAo+ID4gPiA+ID4gPiArCXByb3BlcnR5IGlzIG9tbWl0ZWQsIHRoZSBy
ZWdpc3RlciBpcyB6ZXJvZWQuCj4gPiA+ID4gPiAKPiA+ID4gPiA+IFdoeSBpc24ndCB0aGlzIGlt
cGxlbWVudGVkIGFzIGEgYmFja2xpZ2h0IGRyaXZlcj8gU3RhdGljIGRldmljZXRyZWUKPiA+ID4g
PiA+IHByb3ZpZGVkIHZhbHVlcyBpcyB2ZXJ5IGxpbWl0aW5nLgo+ID4gPiA+IAo+ID4gPiA+IExl
dCdzIHVuZGVyc3RhbmQgdGhlIHRlcm1pbm9sb2d5Lgo+ID4gPiA+IFRoaXMgcmVnaXN0ZXIgc2hv
dWxkIGJlIHJlbmFtZWQgYWNjb3JkaW5nIHRvIHRoZSBkYXRhc2hlZXQsIGkuZS4gTFBDQ1IuCj4g
PiA+ID4gQXMgSSBwb2ludGVkIG91dCBlYXJsaWVyLCBpdCBpcyBOT1QgY29udHJvbCB0aGUgYmFj
a2xpZ2h0LCB0aGlzIGlzIGEgY29udHJhc3QgY29udHJvbC4KPiA+ID4gPiBZZXMsIGl0IHdvcmtz
IGFzIFBXTSwgYnV0IG5vdGhpbmcgZG8gd2l0aCB0aGUgYmFja2xpZ2h0IHN1YnN5c3RlbS4KPiA+
ID4gPiBZZXMsIHdlIGNhbiBtYWtlIGEgZHJpdmVyIGZvciB0aGlzIFBXTSwgYnV0IGhvdyBhcmUg
d2UgZ29pbmcgdG8gY29udHJvbCBpdD8KPiA+ID4gPiBJIG1pc3VuZGVyc3Rvb2Qgc29tZXRoaW5n
Pwo+ID4gPiAKPiA+ID4gSSBzdHVtYmxlZCB1cG9uICdnZXQgdGhlIGJhY2tsaWdodCB3b3JraW5n
JyB3aGljaCBpbXBsaWVkIGZvciBtZSB0aGF0IGl0Cj4gPiA+IHNob3VsZCBiZSBhIGJhY2tsaWdo
dCBkcml2ZXIuIEJ1dCB5b3UncmUgcmlnaHQgYW5kIG5vdyBJIHJlbWVtYmVyIHdlCj4gPiA+IHRh
bGtlZCBhYm91dCB0aGlzIGFscmVhZHkuCj4gPiAKPiA+IEhhbGxlbHVqYWguCj4gPiAKPiA+ID4g
SSBzdGlsbCB0aGluayB0aGlzIHNob3VsZCBiZSBzb21ldGhpbmcgYWRqdXN0YWJsZSwgbm90IHN0
YXRpYyBkYXRhLgo+ID4gPiBNYXliZSB3ZSBjb3VsZCBjaGFuZ2UgdGhlIHdvcmRpbmcgdG8gc29t
ZXRoaW5nIGxpa2UgIlRoaXMgcHJvcGVydHkKPiA+ID4gcHJvdmlkZXMgdGhlIGRlZmF1bHQgdmFs
dWUgZm9yIHRoZSBjb250cmFzdCBjb250cm9sIHJlZ2lzdGVyIiBzaW5jZSBldmVuCj4gPiA+IGlm
IHdlIGFkZCBkcml2ZXIgc3VwcG9ydCBmb3IgY29udHJvbGxpbmcgdGhlIGNvbnRyYXN0IHdlIHN0
aWxsIHdhbnQKPiA+ID4gdG8gaGF2ZSBhIHNhbmUgZGVmYXVsdC4KPiA+IAo+ID4gU291bmRzIGdv
b2QuCj4gPiAKPiA+ID4gQlRXIHRoZSBjb250cmFzdCBjb3VsZCBiZSBjb250cm9sbGVkIHdpdGgg
YSBsY2RfZGV2aWNlIChzZWUKPiA+ID4gbGNkX2RldmljZV9yZWdpc3Rlcikgd2hpY2ggc2VlbXMg
dG8gYmUgdmVyeSBlYXN5IHRvIGltcGxlbWVudC4KPiA+IAo+ID4gQWRkcmVzcyBvZiByZWdpc3Rl
ciBpcyBwbGFjZWQgd2l0aGluIExDRCBhcmVhLCBzbyB3ZSBjYW5ub3QgdXNlIHRoaXMKPiA+IG1l
bW9yeSByZWdpb24sIEkgdGhpbmsgaXMgbm8gc28gZWFzeSBhcyB5b3Ugc2F5Li4uLgo+IAo+IFdl
IGRvIG5vdCBuZWVkIGEgc2VwYXJhdGUgZHJpdmVyIGZvciB0aGlzLiBMb29rIGZvciBleGFtcGxl
IGF0Cj4gZHJpdmVycy92aWRlby9iZjUzNy1scTAzNS5jLCBpdCBqdXN0IGNhbGxzIGxjZF9kZXZp
Y2VfcmVnaXN0ZXIoKQo+IGluIGl0cyBwcm9iZSBmdW5jdGlvbi4KCk5pY2UuIFNlZW1zIHRoaXMg
ZXhhbXBsZSBldmVuIGNhbiBoYW5kbGUgTENEIHBvd2VyCnJlZ3VsYXRvciBmcm9tICJbMS80XSB2
aWRlbzogaW14ZmI6IEludHJvZHVjZSByZWd1bGF0b3Igc3VwcG9ydC4iCgotLS0K

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add pwmr for the device tree.
From: Sascha Hauer @ 2013-12-06  9:30 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386318910.581671226@f322.i.mail.ru>

On Fri, Dec 06, 2013 at 12:35:10PM +0400, Alexander Shiyan wrote:
> > On Fri, Dec 06, 2013 at 12:03:54PM +0400, Alexander Shiyan wrote:
> ...
> > > > >  - fsl,dmacr: DMA Control Register value. This is optional. By default, the
> > > > >  	register is not modified as recommended by the datasheet.
> > > > > +- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
> > > > > +	optional, but defining it is necessary to get the backlight working. If that
> > > > > +	property is ommited, the register is zeroed.
> > > > 
> > > > Why isn't this implemented as a backlight driver? Static devicetree
> > > > provided values is very limiting.
> > > 
> > > Let's understand the terminology.
> > > This register should be renamed according to the datasheet, i.e. LPCCR.
> > > As I pointed out earlier, it is NOT control the backlight, this is a contrast control.
> > > Yes, it works as PWM, but nothing do with the backlight subsystem.
> > > Yes, we can make a driver for this PWM, but how are we going to control it?
> > > I misunderstood something?
> > 
> > I stumbled upon 'get the backlight working' which implied for me that it
> > should be a backlight driver. But you're right and now I remember we
> > talked about this already.
> 
> Hallelujah.
> 
> > I still think this should be something adjustable, not static data.
> > Maybe we could change the wording to something like "This property
> > provides the default value for the contrast control register" since even
> > if we add driver support for controlling the contrast we still want
> > to have a sane default.
> 
> Sounds good.
> 
> > BTW the contrast could be controlled with a lcd_device (see
> > lcd_device_register) which seems to be very easy to implement.
> 
> Address of register is placed within LCD area, so we cannot use this
> memory region, I think is no so easy as you say....

We do not need a separate driver for this. Look for example at
drivers/video/bf537-lq035.c, it just calls lcd_device_register()
in its probe function.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 15/26] ARM: omap4-panda.dts: add display information
From: Javier Martinez Canillas @ 2013-12-06  8:57 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <1386160133-24026-16-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/boot/dts/omap4-panda-common.dtsi | 102 ++++++++++++++++++++++++++++++
>  1 file changed, 102 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap4-panda-common.dtsi b/arch/arm/boot/dts/omap4-panda-common.dtsi
> index 298e85020e1b..51a79756e67e 100644
> --- a/arch/arm/boot/dts/omap4-panda-common.dtsi
> +++ b/arch/arm/boot/dts/omap4-panda-common.dtsi
> @@ -409,3 +409,105 @@
>  &usbhsehci {
>         phys = <&hsusb1_phy>;
>  };
> +
> +&dpi {
> +       dpi_out: endpoint {
> +               remote-endpoint = <&tfp410_in>;
> +               data-lines = <24>;
> +       };
> +};
> +
> +&dsi1 {
> +       vdds_dsi-supply = <&vcxio>;
> +};
> +
> +&dsi2 {
> +       vdds_dsi-supply = <&vcxio>;
> +};
> +
> +&hdmi {
> +       vdda_hdmi_dac-supply = <&vdac>;
> +
> +       hdmi_out: endpoint {
> +               remote-endpoint = <&tpd12s015_in>;
> +       };
> +};
> +
> +/ {
> +       aliases {
> +               display0 = &dvi0;
> +               display1 = &hdmi0;
> +       };
> +
> +       tfp410: encoder@0 {
> +               compatible = "ti,tfp410";
> +               gpios = <&gpio1 0 0>;   /* 0, power-down */
> +

Please use the constants from include/dt-bindings/ instead of magic
numbers, i.e:

gpios = <&gpio1 0 GPIO_ACTIVE_HIGH>;   /* 0, power-down */

> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       port@0 {
> +                               reg = <0>;
> +
> +                               tfp410_in: endpoint@0 {
> +                                       remote-endpoint = <&dpi_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +
> +                               tfp410_out: endpoint@1 {
> +                                       remote-endpoint = <&dvi_connector_in>;
> +                               };
> +                       };
> +               };
> +       };
> +
> +       dvi0: connector@0 {
> +               compatible = "ti,dvi-connector";
> +               i2c-bus = <&i2c3>;
> +
> +               dvi_connector_in: endpoint {
> +                       remote-endpoint = <&tfp410_out>;
> +               };
> +       };
> +
> +       tpd12s015: encoder@1 {
> +               compatible = "ti,tpd12s015";
> +
> +               gpios = <&gpio2 28 0>,  /* 60, CT CP HPD */
> +                       <&gpio2 9 0>,   /* 41, LS OE */
> +                       <&gpio2 31 0>;  /* 63, HPD */
> +
> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       port@0 {
> +                               reg = <0>;
> +
> +                               tpd12s015_in: endpoint@0 {
> +                                       remote-endpoint = <&hdmi_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +
> +                               tpd12s015_out: endpoint@1 {
> +                                       remote-endpoint = <&hdmi_connector_in>;
> +                               };
> +                       };
> +               };
> +       };
> +
> +       hdmi0: connector@1 {
> +               compatible = "ti,hdmi-connector";
> +
> +               hdmi_connector_in: endpoint {
> +                       remote-endpoint = <&tpd12s015_out>;
> +               };
> +       };
> +};
> --
> 1.8.3.2
>

Best regards,
Javier

^ permalink raw reply

* Re: [PATCH 18/26] ARM: omap3-beagle.dts: add display information
From: Javier Martinez Canillas @ 2013-12-06  8:41 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap@vger.kernel.org, linux-fbdev,
	devicetree@vger.kernel.org, Archit Taneja, Darren Etheridge,
	Tony Lindgren
In-Reply-To: <1386160133-24026-19-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

On Wed, Dec 4, 2013 at 1:28 PM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/boot/dts/omap3-beagle.dts | 67 ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>
> diff --git a/arch/arm/boot/dts/omap3-beagle.dts b/arch/arm/boot/dts/omap3-beagle.dts
> index fa532aaacc68..1ca1932d02aa 100644
> --- a/arch/arm/boot/dts/omap3-beagle.dts
> +++ b/arch/arm/boot/dts/omap3-beagle.dts
> @@ -178,3 +178,70 @@
>         mode = <3>;
>         power = <50>;
>  };
> +
> +&dpi {
> +       vdds_dsi-supply = <&vpll2>;
> +
> +       dpi_out: endpoint {
> +               remote-endpoint = <&tfp410_in>;
> +               data-lines = <24>;
> +       };
> +};
> +
> +&venc {
> +       vdda_dac-supply = <&vdac>;
> +
> +       venc_out: endpoint {
> +               remote-endpoint = <&tv_connector_in>;
> +       };
> +};
> +
> +/ {
> +       aliases {
> +               display0 = &dvi0;
> +               display1 = &tv0;
> +       };
> +
> +       tfp410: encoder@0 {
> +               compatible = "ti,tfp410";
> +               gpios = <&gpio5 10 0>;  /* 170, power-down */
> +

Shouldn't this be gpio6 instead since OMAP GPIO banks start counting
from 1 so gpio5 + 10 is GPIO 138.

> +               ports {
> +                       #address-cells = <1>;
> +                       #size-cells = <0>;
> +
> +                       port@0 {
> +                               reg = <0>;
> +
> +                               tfp410_in: endpoint@0 {
> +                                       remote-endpoint = <&dpi_out>;
> +                               };
> +                       };
> +
> +                       port@1 {
> +                               reg = <1>;
> +
> +                               tfp410_out: endpoint@1 {
> +                                       remote-endpoint = <&dvi_connector_in>;
> +                               };
> +                       };
> +               };
> +       };
> +
> +       dvi0: connector@0 {
> +               compatible = "ti,dvi_connector";
> +               i2c-bus = <&i2c3>;
> +
> +               dvi_connector_in: endpoint {
> +                       remote-endpoint = <&tfp410_out>;
> +               };
> +       };
> +
> +       tv0: connector@1 {
> +               compatible = "ti,svideo_connector";
> +
> +               tv_connector_in: endpoint {
> +                       remote-endpoint = <&venc_out>;
> +               };
> +       };
> +};
> --
> 1.8.3.2
>

Also I don't see the DSS pinmux set for this board. I guess you need
something like the following on top:

0x0a4 (PIN_OUTPUT | MUX_MODE0)   /* dss_pclk.dss_pclk */
0x0a6 (PIN_OUTPUT | MUX_MODE0)   /* dss_hsync.dss_hsync */
0x0a8 (PIN_OUTPUT | MUX_MODE0)   /* dss_vsync.dss_vsync */
0x0aa (PIN_OUTPUT | MUX_MODE0)   /* dss_acbias.dss_acbias */
0x0ac (PIN_OUTPUT | MUX_MODE0)   /* dss_data0.dss_data0 */
0x0ae (PIN_OUTPUT | MUX_MODE0)   /* dss_data1.dss_data1 */
0x0b0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data2.dss_data2 */
0x0b2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data3.dss_data3 */
0x0b4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data4.dss_data4 */
0x0b6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data5.dss_data5 */
0x0b8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data6.dss_data6 */
0x0ba (PIN_OUTPUT | MUX_MODE0)   /* dss_data7.dss_data7 */
0x0bc (PIN_OUTPUT | MUX_MODE0)   /* dss_data8.dss_data8 */
0x0be (PIN_OUTPUT | MUX_MODE0)   /* dss_data9.dss_data9 */
0x0c0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data10.dss_data10 */
0x0c2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data11.dss_data11 */
0x0c4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data12.dss_data12 */
0x0c6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data13.dss_data13 */
0x0c8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data14.dss_data14 */
0x0ca (PIN_OUTPUT | MUX_MODE0)   /* dss_data15.dss_data15 */
0x0cc (PIN_OUTPUT | MUX_MODE0)   /* dss_data16.dss_data16 */
0x0ce (PIN_OUTPUT | MUX_MODE0)   /* dss_data17.dss_data17 */
0x0d0 (PIN_OUTPUT | MUX_MODE0)   /* dss_data18.dss_data18 */
0x0d2 (PIN_OUTPUT | MUX_MODE0)   /* dss_data19.dss_data19 */
0x0d4 (PIN_OUTPUT | MUX_MODE0)   /* dss_data20.dss_data20 */
0x0d6 (PIN_OUTPUT | MUX_MODE0)   /* dss_data21.dss_data21 */
0x0d8 (PIN_OUTPUT | MUX_MODE0)   /* dss_data22.dss_data22 */
0x0da (PIN_OUTPUT | MUX_MODE0)   /* dss_data23.dss_data23 */

Best regards,
Javier

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add p =?UTF-8?B?d21yIGZvciB0aGUgZ
From: Alexander Shiyan @ 2013-12-06  8:35 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131206081640.GZ24559@pengutronix.de>

PiBPbiBGcmksIERlYyAwNiwgMjAxMyBhdCAxMjowMzo1NFBNICswNDAwLCBBbGV4YW5kZXIgU2hp
eWFuIHdyb3RlOgouLi4KPiA+ID4gPiAgLSBmc2wsZG1hY3I6IERNQSBDb250cm9sIFJlZ2lzdGVy
IHZhbHVlLiBUaGlzIGlzIG9wdGlvbmFsLiBCeSBkZWZhdWx0LCB0aGUKPiA+ID4gPiAgCXJlZ2lz
dGVyIGlzIG5vdCBtb2RpZmllZCBhcyByZWNvbW1lbmRlZCBieSB0aGUgZGF0YXNoZWV0Lgo+ID4g
PiA+ICstIGZzbCxwd21yOiAgTENEQyBQV00gQ29udHJhc3QgQ29udHJvbCBSZWdpc3RlciB2YWx1
ZS4gVGhhdCBwcm9wZXJ0eSBpcwo+ID4gPiA+ICsJb3B0aW9uYWwsIGJ1dCBkZWZpbmluZyBpdCBp
cyBuZWNlc3NhcnkgdG8gZ2V0IHRoZSBiYWNrbGlnaHQgd29ya2luZy4gSWYgdGhhdAo+ID4gPiA+
ICsJcHJvcGVydHkgaXMgb21taXRlZCwgdGhlIHJlZ2lzdGVyIGlzIHplcm9lZC4KPiA+ID4gCj4g
PiA+IFdoeSBpc24ndCB0aGlzIGltcGxlbWVudGVkIGFzIGEgYmFja2xpZ2h0IGRyaXZlcj8gU3Rh
dGljIGRldmljZXRyZWUKPiA+ID4gcHJvdmlkZWQgdmFsdWVzIGlzIHZlcnkgbGltaXRpbmcuCj4g
PiAKPiA+IExldCdzIHVuZGVyc3RhbmQgdGhlIHRlcm1pbm9sb2d5Lgo+ID4gVGhpcyByZWdpc3Rl
ciBzaG91bGQgYmUgcmVuYW1lZCBhY2NvcmRpbmcgdG8gdGhlIGRhdGFzaGVldCwgaS5lLiBMUEND
Ui4KPiA+IEFzIEkgcG9pbnRlZCBvdXQgZWFybGllciwgaXQgaXMgTk9UIGNvbnRyb2wgdGhlIGJh
Y2tsaWdodCwgdGhpcyBpcyBhIGNvbnRyYXN0IGNvbnRyb2wuCj4gPiBZZXMsIGl0IHdvcmtzIGFz
IFBXTSwgYnV0IG5vdGhpbmcgZG8gd2l0aCB0aGUgYmFja2xpZ2h0IHN1YnN5c3RlbS4KPiA+IFll
cywgd2UgY2FuIG1ha2UgYSBkcml2ZXIgZm9yIHRoaXMgUFdNLCBidXQgaG93IGFyZSB3ZSBnb2lu
ZyB0byBjb250cm9sIGl0Pwo+ID4gSSBtaXN1bmRlcnN0b29kIHNvbWV0aGluZz8KPiAKPiBJIHN0
dW1ibGVkIHVwb24gJ2dldCB0aGUgYmFja2xpZ2h0IHdvcmtpbmcnIHdoaWNoIGltcGxpZWQgZm9y
IG1lIHRoYXQgaXQKPiBzaG91bGQgYmUgYSBiYWNrbGlnaHQgZHJpdmVyLiBCdXQgeW91J3JlIHJp
Z2h0IGFuZCBub3cgSSByZW1lbWJlciB3ZQo+IHRhbGtlZCBhYm91dCB0aGlzIGFscmVhZHkuCgpI
YWxsZWx1amFoLgoKPiBJIHN0aWxsIHRoaW5rIHRoaXMgc2hvdWxkIGJlIHNvbWV0aGluZyBhZGp1
c3RhYmxlLCBub3Qgc3RhdGljIGRhdGEuCj4gTWF5YmUgd2UgY291bGQgY2hhbmdlIHRoZSB3b3Jk
aW5nIHRvIHNvbWV0aGluZyBsaWtlICJUaGlzIHByb3BlcnR5Cj4gcHJvdmlkZXMgdGhlIGRlZmF1
bHQgdmFsdWUgZm9yIHRoZSBjb250cmFzdCBjb250cm9sIHJlZ2lzdGVyIiBzaW5jZSBldmVuCj4g
aWYgd2UgYWRkIGRyaXZlciBzdXBwb3J0IGZvciBjb250cm9sbGluZyB0aGUgY29udHJhc3Qgd2Ug
c3RpbGwgd2FudAo+IHRvIGhhdmUgYSBzYW5lIGRlZmF1bHQuCgpTb3VuZHMgZ29vZC4KCj4gQlRX
IHRoZSBjb250cmFzdCBjb3VsZCBiZSBjb250cm9sbGVkIHdpdGggYSBsY2RfZGV2aWNlIChzZWUK
PiBsY2RfZGV2aWNlX3JlZ2lzdGVyKSB3aGljaCBzZWVtcyB0byBiZSB2ZXJ5IGVhc3kgdG8gaW1w
bGVtZW50LgoKQWRkcmVzcyBvZiByZWdpc3RlciBpcyBwbGFjZWQgd2l0aGluIExDRCBhcmVhLCBz
byB3ZSBjYW5ub3QgdXNlIHRoaXMKbWVtb3J5IHJlZ2lvbiwgSSB0aGluayBpcyBubyBzbyBlYXN5
IGFzIHlvdSBzYXkuLi4uCgotLS0K

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add pwmr for the device tree.
From: Sascha Hauer @ 2013-12-06  8:16 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386317034.190941796@f354.i.mail.ru>

On Fri, Dec 06, 2013 at 12:03:54PM +0400, Alexander Shiyan wrote:
> > >  .../devicetree/bindings/video/fsl,imx-fb.txt       |    3 +++
> > >  drivers/video/imxfb.c                              |    2 ++
> > >  2 files changed, 5 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > index 46da08d..ac457ae 100644
> > > --- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> > > @@ -17,6 +17,9 @@ Required nodes:
> > >  Optional properties:
> > >  - fsl,dmacr: DMA Control Register value. This is optional. By default, the
> > >  	register is not modified as recommended by the datasheet.
> > > +- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
> > > +	optional, but defining it is necessary to get the backlight working. If that
> > > +	property is ommited, the register is zeroed.
> > 
> > Why isn't this implemented as a backlight driver? Static devicetree
> > provided values is very limiting.
> 
> Let's understand the terminology.
> This register should be renamed according to the datasheet, i.e. LPCCR.
> As I pointed out earlier, it is NOT control the backlight, this is a contrast control.
> Yes, it works as PWM, but nothing do with the backlight subsystem.
> Yes, we can make a driver for this PWM, but how are we going to control it?
> I misunderstood something?

I stumbled upon 'get the backlight working' which implied for me that it
should be a backlight driver. But you're right and now I remember we
talked about this already.

I still think this should be something adjustable, not static data.
Maybe we could change the wording to something like "This property
provides the default value for the contrast control register" since even
if we add driver support for controlling the contrast we still want
to have a sane default.

BTW the contrast could be controlled with a lcd_device (see
lcd_device_register) which seems to be very easy to implement.

SaschaMaybe we could change the wording to something like "This property
provides the default value for the contrast control register" since even
if we add driver support for controlling the contrast we still want
to have a sane default.

BTW the contrast could be controlled with a lcd_device (see
lcd_device_register) which seems to be very easy to implement.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add p =?UTF-8?B?d21yIGZvciB0aGUgZ
From: Alexander Shiyan @ 2013-12-06  8:03 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <20131206070525.GV24559@pengutronix.de>

PiBPbiBUaHUsIERlYyAwNSwgMjAxMyBhdCAwNDo0MzozN1BNICswMTAwLCBEZW5pcyBDYXJpa2xp
IHdyb3RlOgo+ID4gcHdtciBoYXMgdG8gYmUgc2V0IHRvIGdldCB0aGUgaW14ZmIgYmFja2xpZ2h0
IHdvcmssCj4gPiB0aG91Z2ggcHdtciB3YXMgb25seSBjb25maWd1cmFibGUgdHJvdWdoIHRoZSBw
bGF0Zm9ybSBkYXRhLgo+ID4gCj4gPiBDYzogUm9iIEhlcnJpbmcgPHJvYi5oZXJyaW5nQGNhbHhl
ZGEuY29tPgo+ID4gQ2M6IFBhd2VsIE1vbGwgPHBhd2VsLm1vbGxAYXJtLmNvbT4KPiA+IENjOiBN
YXJrIFJ1dGxhbmQgPG1hcmsucnV0bGFuZEBhcm0uY29tPgo+ID4gQ2M6IFN0ZXBoZW4gV2FycmVu
IDxzd2FycmVuQHd3d2RvdG9yZy5vcmc+Cj4gPiBDYzogSWFuIENhbXBiZWxsIDxpamMrZGV2aWNl
dHJlZUBoZWxsaW9uLm9yZy51az4KPiA+IENjOiBkZXZpY2V0cmVlQHZnZXIua2VybmVsLm9yZwo+
ID4gQ2M6IEplYW4tQ2hyaXN0b3BoZSBQbGFnbmlvbC1WaWxsYXJkIDxwbGFnbmlvakBqY3Jvc29m
dC5jb20+Cj4gPiBDYzogVG9taSBWYWxrZWluZW4gPHRvbWkudmFsa2VpbmVuQHRpLmNvbT4KPiA+
IENjOiBsaW51eC1mYmRldkB2Z2VyLmtlcm5lbC5vcmcKPiA+IENjOiBTYXNjaGEgSGF1ZXIgPGtl
cm5lbEBwZW5ndXRyb25peC5kZT4KPiA+IENjOiBsaW51eC1hcm0ta2VybmVsQGxpc3RzLmluZnJh
ZGVhZC5vcmcKPiA+IENjOiBFcmljIELDqW5hcmQgPGVyaWNAZXVrcmVhLmNvbT4KPiA+IFNpZ25l
ZC1vZmYtYnk6IERlbmlzIENhcmlrbGkgPGRlbmlzQGV1a3JlYS5jb20+Cj4gPiBBY2tlZC1ieTog
SmVhbi1DaHJpc3RvcGhlIFBMQUdOSU9MLVZJTExBUkQgPHBsYWduaW9qQGpjcm9zb2Z0LmNvbT4K
PiA+IEFja2VkLWJ5OiBHcmFudCBMaWtlbHkgPGdyYW50Lmxpa2VseUBsaW5hcm8ub3JnPgo+ID4g
LS0tCj4gPiAgLi4uL2RldmljZXRyZWUvYmluZGluZ3MvdmlkZW8vZnNsLGlteC1mYi50eHQgICAg
ICAgfCAgICAzICsrKwo+ID4gIGRyaXZlcnMvdmlkZW8vaW14ZmIuYyAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIHwgICAgMiArKwo+ID4gIDIgZmlsZXMgY2hhbmdlZCwgNSBpbnNlcnRpb25z
KCspCj4gPiAKPiA+IGRpZmYgLS1naXQgYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu
Z3MvdmlkZW8vZnNsLGlteC1mYi50eHQgYi9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGlu
Z3MvdmlkZW8vZnNsLGlteC1mYi50eHQKPiA+IGluZGV4IDQ2ZGEwOGQuLmFjNDU3YWUgMTAwNjQ0
Cj4gPiAtLS0gYS9Eb2N1bWVudGF0aW9uL2RldmljZXRyZWUvYmluZGluZ3MvdmlkZW8vZnNsLGlt
eC1mYi50eHQKPiA+ICsrKyBiL0RvY3VtZW50YXRpb24vZGV2aWNldHJlZS9iaW5kaW5ncy92aWRl
by9mc2wsaW14LWZiLnR4dAo+ID4gQEAgLTE3LDYgKzE3LDkgQEAgUmVxdWlyZWQgbm9kZXM6Cj4g
PiAgT3B0aW9uYWwgcHJvcGVydGllczoKPiA+ICAtIGZzbCxkbWFjcjogRE1BIENvbnRyb2wgUmVn
aXN0ZXIgdmFsdWUuIFRoaXMgaXMgb3B0aW9uYWwuIEJ5IGRlZmF1bHQsIHRoZQo+ID4gIAlyZWdp
c3RlciBpcyBub3QgbW9kaWZpZWQgYXMgcmVjb21tZW5kZWQgYnkgdGhlIGRhdGFzaGVldC4KPiA+
ICstIGZzbCxwd21yOiAgTENEQyBQV00gQ29udHJhc3QgQ29udHJvbCBSZWdpc3RlciB2YWx1ZS4g
VGhhdCBwcm9wZXJ0eSBpcwo+ID4gKwlvcHRpb25hbCwgYnV0IGRlZmluaW5nIGl0IGlzIG5lY2Vz
c2FyeSB0byBnZXQgdGhlIGJhY2tsaWdodCB3b3JraW5nLiBJZiB0aGF0Cj4gPiArCXByb3BlcnR5
IGlzIG9tbWl0ZWQsIHRoZSByZWdpc3RlciBpcyB6ZXJvZWQuCj4gCj4gV2h5IGlzbid0IHRoaXMg
aW1wbGVtZW50ZWQgYXMgYSBiYWNrbGlnaHQgZHJpdmVyPyBTdGF0aWMgZGV2aWNldHJlZQo+IHBy
b3ZpZGVkIHZhbHVlcyBpcyB2ZXJ5IGxpbWl0aW5nLgoKTGV0J3MgdW5kZXJzdGFuZCB0aGUgdGVy
bWlub2xvZ3kuClRoaXMgcmVnaXN0ZXIgc2hvdWxkIGJlIHJlbmFtZWQgYWNjb3JkaW5nIHRvIHRo
ZSBkYXRhc2hlZXQsIGkuZS4gTFBDQ1IuCkFzIEkgcG9pbnRlZCBvdXQgZWFybGllciwgaXQgaXMg
Tk9UIGNvbnRyb2wgdGhlIGJhY2tsaWdodCwgdGhpcyBpcyBhIGNvbnRyYXN0IGNvbnRyb2wuClll
cywgaXQgd29ya3MgYXMgUFdNLCBidXQgbm90aGluZyBkbyB3aXRoIHRoZSBiYWNrbGlnaHQgc3Vi
c3lzdGVtLgpZZXMsIHdlIGNhbiBtYWtlIGEgZHJpdmVyIGZvciB0aGlzIFBXTSwgYnV0IGhvdyBh
cmUgd2UgZ29pbmcgdG8gY29udHJvbCBpdD8KSSBtaXN1bmRlcnN0b29kIHNvbWV0aGluZz8KCi0t
LQo

^ permalink raw reply

* Re: [PATCHv13][ 1/4] video: imxfb: Introduce regulator support.
From: Sascha Hauer @ 2013-12-06  7:12 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-1-git-send-email-denis@eukrea.com>

On Thu, Dec 05, 2013 at 04:43:36PM +0100, Denis Carikli wrote:
> This commit is based on the following commit by Fabio Estevam:
> @@ -1020,6 +1045,12 @@ static int imxfb_probe(struct platform_device *pdev)
>  		goto failed_register;
>  	}
>  
> +	fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
> +	if (IS_ERR(fbi->reg_lcd)) {
> +		dev_info(&pdev->dev, "No lcd regulator used.\n");

Please remove this log spam or at least reduce it to dev_dbg. That's the
typical message to which our customers keep asking us whether that's
something they have to care about.

Also, you should check for -EPROBE_DEFER here and return it in this
case.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCHv13][ 3/4] video: Kconfig: Allow more broad selection of the imxfb framebuffer driver.
From: Sascha Hauer @ 2013-12-06  7:06 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-3-git-send-email-denis@eukrea.com>

On Thu, Dec 05, 2013 at 04:43:38PM +0100, Denis Carikli wrote:
> Without that patch, a user can't select the imxfb driver when the i.MX25 and/or
>   the i.MX27 device tree board are selected and that no boards that selects
>   IMX_HAVE_PLATFORM_IMX_FB are compiled in.
> 
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>


Acked-by: Sascha Hauer <s.hauer@pengutronix.de>

Sascha

> ---
> ChangeLog v10->v11:
> - moved my signed-off-by.
> 
> ChangeLog v8->v9:
> - Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
> ---
>  drivers/video/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 4f2e1b3..22adaee 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -363,7 +363,7 @@ config FB_SA1100
>  
>  config FB_IMX
>  	tristate "Freescale i.MX1/21/25/27 LCD support"
> -	depends on FB && IMX_HAVE_PLATFORM_IMX_FB
> +	depends on FB && ARCH_MXC
>  	select FB_CFB_FILLRECT
>  	select FB_CFB_COPYAREA
>  	select FB_CFB_IMAGEBLIT
> -- 
> 1.7.9.5
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add pwmr for the device tree.
From: Sascha Hauer @ 2013-12-06  7:05 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-2-git-send-email-denis@eukrea.com>

On Thu, Dec 05, 2013 at 04:43:37PM +0100, Denis Carikli wrote:
> pwmr has to be set to get the imxfb backlight work,
> though pwmr was only configurable trough the platform data.
> 
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
> Cc: devicetree@vger.kernel.org
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Sascha Hauer <kernel@pengutronix.de>
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: Eric Bénard <eric@eukrea.com>
> Signed-off-by: Denis Carikli <denis@eukrea.com>
> Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> Acked-by: Grant Likely <grant.likely@linaro.org>
> ---
>  .../devicetree/bindings/video/fsl,imx-fb.txt       |    3 +++
>  drivers/video/imxfb.c                              |    2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> index 46da08d..ac457ae 100644
> --- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> +++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
> @@ -17,6 +17,9 @@ Required nodes:
>  Optional properties:
>  - fsl,dmacr: DMA Control Register value. This is optional. By default, the
>  	register is not modified as recommended by the datasheet.
> +- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
> +	optional, but defining it is necessary to get the backlight working. If that
> +	property is ommited, the register is zeroed.

Why isn't this implemented as a backlight driver? Static devicetree
provided values is very limiting.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply

* Re: [PATCH 13/26] ARM: omap3.dtsi: add omapdss information
From: Tony Lindgren @ 2013-12-05 17:05 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: linux-omap, linux-fbdev, devicetree, Archit Taneja,
	Darren Etheridge
In-Reply-To: <1386160133-24026-14-git-send-email-tomi.valkeinen@ti.com>

* Tomi Valkeinen <tomi.valkeinen@ti.com> [131204 04:31]:

Description missing.. But other than that can you please check that
the latest patch I posted in thread "[PATCH] ARM: OMAP2+: Fix populating
the hwmod data from device" works with this?

The test to do is to remove the related reg, interrupt and dma entries
from omap_hwmod_*_data.c, and make sure the related hwmod data is initialized
from DT properly.

I don't know if it makes sense to have them as children of dss_core, they
really all seem to be completely independent devices?

BTW, for v3.15, I'm hoping to do patches where we deprecate ti,hwmods
property and do the lookup based on the compatible property instead ;)
So from that point of view we need to get the device mapping right in
the .dtsi files, and don't want to start mixing up separate devices into
single .dtsi entry.

Regards,

Tony

> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  arch/arm/boot/dts/omap3.dtsi | 43 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 43 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/omap3.dtsi b/arch/arm/boot/dts/omap3.dtsi
> index f3a0c26ed0c2..6fc163201cbd 100644
> --- a/arch/arm/boot/dts/omap3.dtsi
> +++ b/arch/arm/boot/dts/omap3.dtsi
> @@ -588,5 +588,48 @@
>  			num-eps = <16>;
>  			ram-bits = <12>;
>  		};
> +
> +		dss@48050000 {
> +			compatible = "ti,omap3-dss", "simple-bus";
> +			reg = <0x48050000 0x200>;
> +			ti,hwmods = "dss_core";
> +			#address-cells = <1>;
> +			#size-cells = <1>;
> +			ranges;
> +
> +			dispc@48050400 {
> +				compatible = "ti,omap3-dispc";
> +				reg = <0x48050400 0x400>;
> +				interrupts = <25>;
> +				ti,hwmods = "dss_dispc";
> +			};
> +
> +			dpi: encoder@0 {
> +				compatible = "ti,omap3-dpi";
> +			};
> +
> +			sdi: encoder@1 {
> +				compatible = "ti,omap3-sdi";
> +			};
> +
> +			dsi: encoder@4804fc00 {
> +				compatible = "ti,omap3-dsi";
> +				reg = <0x4804fc00 0x400>;
> +				interrupts = <25>;
> +				ti,hwmods = "dss_dsi1";
> +			};
> +
> +			rfbi: encoder@48050800 {
> +				compatible = "ti,omap3-rfbi";
> +				reg = <0x48050800 0x100>;
> +				ti,hwmods = "dss_rfbi";
> +			};
> +
> +			venc: encoder@48050c00 {
> +				compatible = "ti,omap3-venc";
> +				reg = <0x48050c00 0x100>;
> +				ti,hwmods = "dss_venc";
> +			};
> +		};
>  	};
>  };
> -- 
> 1.8.3.2
> 

^ permalink raw reply

* Re: [PATCHv13][ 2/4] video: imxfb: Also add p =?UTF-8?B?d21yIGZvciB0aGUgZ
From: Alexander Shiyan @ 2013-12-05 15:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-2-git-send-email-denis@eukrea.com>

PiBwd21yIGhhcyB0byBiZSBzZXQgdG8gZ2V0IHRoZSBpbXhmYiBiYWNrbGlnaHQgd29yaywKPiB0
aG91Z2ggcHdtciB3YXMgb25seSBjb25maWd1cmFibGUgdHJvdWdoIHRoZSBwbGF0Zm9ybSBkYXRh
LgoKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvcGlwZXJtYWlsL2xpbnV4LWFybS1rZXJuZWwv
MjAxMy1KdWx5LzE4NTAyMi5odG1sCgouLi4KLS0tCg=

^ permalink raw reply

* [PATCHv13][ 4/4] ARM: dts: imx25: mbimxsd25: Add displays support.
From: Denis Carikli @ 2013-12-05 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-1-git-send-email-denis@eukrea.com>

The CMO-QVGA(With backlight), DVI-VGA and DVI-SVGA displays
were added.

Cc: Shawn Guo <shawn.guo@linaro.org>
Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v10->v13:
- This patch is the display part splitted out from the patch adding
  support for the cpuimx25(and its baseboard).
- Shawn Guo was added to the Cc list.
- The regulator part was updated to match the current style.
- The new GPIO defines are now used in the dts(i).
---
 .../imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts  |   72 ++++++++++++++++++++
 .../imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts  |   45 ++++++++++++
 .../imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts   |   45 ++++++++++++
 3 files changed, 162 insertions(+)
 create mode 100644 arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
 create mode 100644 arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts
 create mode 100644 arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts

diff --git a/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
new file mode 100644
index 0000000..0df7e9e
--- /dev/null
+++ b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-cmo-qvga.dts
@@ -0,0 +1,72 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx25-eukrea-mbimxsd25-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD25 with the CMO-QVGA Display";
+	compatible = "eukrea,mbimxsd25-baseboard-cmo-qvga", "eukrea,mbimxsd25-baseboard", "eukrea,cpuimx25", "fsl,imx25";
+
+	cmo_qvga: display {
+		model = "CMO-QVGA";
+		bits-per-pixel = <16>;
+		fsl,pcr = <0xcad08b80>;
+		bus-width = <18>;
+		native-mode = <&qvga_timings>;
+		display-timings {
+			qvga_timings: 320x240 {
+				clock-frequency = <6500000>;
+				hactive = <320>;
+				vactive = <240>;
+				hback-porch = <30>;
+				hfront-porch = <38>;
+				vback-porch = <20>;
+				vfront-porch = <3>;
+				hsync-len = <15>;
+				vsync-len = <4>;
+			};
+		};
+	};
+
+	regulators {
+		compatible = "simple-bus";
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		reg_lcd_3v3: regulator@0 {
+			compatible = "regulator-fixed";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_reg_lcd_3v3>;
+			regulator-name = "lcd-3v3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
+			gpio = <&gpio1 26 GPIO_ACTIVE_HIGH>;
+			enable-active-high;
+		};
+	};
+};
+
+&iomuxc {
+	imx25-eukrea-mbimxsd25-baseboard-cmo-qvga {
+		pinctrl_reg_lcd_3v3: reg_lcd_3v3 {
+			fsl,pins = <MX25_PAD_PWM__GPIO_1_26 0x80000000>;
+		};
+	};
+};
+
+&lcdc {
+	display = <&cmo_qvga>;
+	fsl,pwmr = <0x00a903ff>;
+	lcd-supply = <&reg_lcd_3v3>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts
new file mode 100644
index 0000000..8eee2f6
--- /dev/null
+++ b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-svga.dts
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx25-eukrea-mbimxsd25-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD25 with the DVI-SVGA Display";
+	compatible = "eukrea,mbimxsd25-baseboard-dvi-svga", "eukrea,mbimxsd25-baseboard", "eukrea,cpuimx25", "fsl,imx25";
+
+	dvi_svga: display {
+		model = "DVI-SVGA";
+		bits-per-pixel = <16>;
+		fsl,pcr = <0xfa208b80>;
+		bus-width = <18>;
+		native-mode = <&dvi_svga_timings>;
+		display-timings {
+			dvi_svga_timings: 800x600 {
+				clock-frequency = <40000000>;
+				hactive = <800>;
+				vactive = <600>;
+				hback-porch = <75>;
+				hfront-porch = <75>;
+				vback-porch = <7>;
+				vfront-porch = <75>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	display = <&dvi_svga>;
+	status = "okay";
+};
diff --git a/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts
new file mode 100644
index 0000000..447da62
--- /dev/null
+++ b/arch/arm/boot/dts/imx25-eukrea-mbimxsd25-baseboard-dvi-vga.dts
@@ -0,0 +1,45 @@
+/*
+ * Copyright 2013 Eukréa Electromatique <denis@eukrea.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "imx25-eukrea-mbimxsd25-baseboard.dts"
+
+/ {
+	model = "Eukrea MBIMXSD25 with the DVI-VGA Display";
+	compatible = "eukrea,mbimxsd25-baseboard-dvi-vga", "eukrea,mbimxsd25-baseboard", "eukrea,cpuimx25", "fsl,imx25";
+
+	dvi_vga: display {
+		model = "DVI-VGA";
+		bits-per-pixel = <16>;
+		fsl,pcr = <0xfa208b80>;
+		bus-width = <18>;
+		native-mode = <&dvi_vga_timings>;
+		display-timings {
+			dvi_vga_timings: 640x480 {
+				clock-frequency = <31250000>;
+				hactive = <640>;
+				vactive = <480>;
+				hback-porch = <100>;
+				hfront-porch = <100>;
+				vback-porch = <7>;
+				vfront-porch = <100>;
+				hsync-len = <7>;
+				vsync-len = <7>;
+			};
+		};
+	};
+};
+
+&lcdc {
+	display = <&dvi_vga>;
+	status = "okay";
+};
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv13][ 3/4] video: Kconfig: Allow more broad selection of the imxfb framebuffer driver.
From: Denis Carikli @ 2013-12-05 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-1-git-send-email-denis@eukrea.com>

Without that patch, a user can't select the imxfb driver when the i.MX25 and/or
  the i.MX27 device tree board are selected and that no boards that selects
  IMX_HAVE_PLATFORM_IMX_FB are compiled in.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
ChangeLog v10->v11:
- moved my signed-off-by.

ChangeLog v8->v9:
- Added Jean-Christophe PLAGNIOL-VILLARD's ACK.
---
 drivers/video/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 4f2e1b3..22adaee 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -363,7 +363,7 @@ config FB_SA1100
 
 config FB_IMX
 	tristate "Freescale i.MX1/21/25/27 LCD support"
-	depends on FB && IMX_HAVE_PLATFORM_IMX_FB
+	depends on FB && ARCH_MXC
 	select FB_CFB_FILLRECT
 	select FB_CFB_COPYAREA
 	select FB_CFB_IMAGEBLIT
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv13][ 2/4] video: imxfb: Also add pwmr for the device tree.
From: Denis Carikli @ 2013-12-05 15:43 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <1386258219-26437-1-git-send-email-denis@eukrea.com>

pwmr has to be set to get the imxfb backlight work,
though pwmr was only configurable trough the platform data.

Cc: Rob Herring <rob.herring@calxeda.com>
Cc: Pawel Moll <pawel.moll@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Ian Campbell <ijc+devicetree@hellion.org.uk>
Cc: devicetree@vger.kernel.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Grant Likely <grant.likely@linaro.org>
---
 .../devicetree/bindings/video/fsl,imx-fb.txt       |    3 +++
 drivers/video/imxfb.c                              |    2 ++
 2 files changed, 5 insertions(+)

diff --git a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
index 46da08d..ac457ae 100644
--- a/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
+++ b/Documentation/devicetree/bindings/video/fsl,imx-fb.txt
@@ -17,6 +17,9 @@ Required nodes:
 Optional properties:
 - fsl,dmacr: DMA Control Register value. This is optional. By default, the
 	register is not modified as recommended by the datasheet.
+- fsl,pwmr:  LCDC PWM Contrast Control Register value. That property is
+	optional, but defining it is necessary to get the backlight working. If that
+	property is ommited, the register is zeroed.
 - fsl,lscr1: LCDC Sharp Configuration Register value.
 
 Example:
diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 57d3b81..17a46a7 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -835,6 +835,8 @@ static int imxfb_init_fbinfo(struct platform_device *pdev)
 
 		of_property_read_u32(np, "fsl,dmacr", &fbi->dmacr);
 
+		of_property_read_u32(np, "fsl,pwmr", &fbi->pwmr);
+
 		/* These two function pointers could be used by some specific
 		 * platforms. */
 		fbi->lcd_power = NULL;
-- 
1.7.9.5


^ permalink raw reply related

* [PATCHv13][ 1/4] video: imxfb: Introduce regulator support.
From: Denis Carikli @ 2013-12-05 15:43 UTC (permalink / raw)
  To: linux-arm-kernel

This commit is based on the following commit by Fabio Estevam:
  4344429 video: mxsfb: Introduce regulator support

Cc: Fabio Estevam <fabio.estevam@freescale.com>
Cc: Sascha Hauer <kernel@pengutronix.de>
Cc: linux-arm-kernel@lists.infradead.org
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Cc: Eric Bénard <eric@eukrea.com>
Signed-off-by: Denis Carikli <denis@eukrea.com>
---
ChangeLog v9->v10:
- Added a return 0; at the end of imxfb_disable_controller.

ChangeLog v8->v9:
- return an error if regulator_{enable,disable} fails in
  imxfb_{enable,disable}_controller, and use it.
---
 drivers/video/imxfb.c |   55 ++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/video/imxfb.c b/drivers/video/imxfb.c
index 44ee678..57d3b81 100644
--- a/drivers/video/imxfb.c
+++ b/drivers/video/imxfb.c
@@ -28,6 +28,7 @@
 #include <linux/cpufreq.h>
 #include <linux/clk.h>
 #include <linux/platform_device.h>
+#include <linux/regulator/consumer.h>
 #include <linux/dma-mapping.h>
 #include <linux/io.h>
 #include <linux/math64.h>
@@ -145,6 +146,7 @@ struct imxfb_info {
 	struct clk		*clk_ipg;
 	struct clk		*clk_ahb;
 	struct clk		*clk_per;
+	struct regulator	*reg_lcd;
 	enum imxfb_type		devtype;
 	bool			enabled;
 
@@ -561,14 +563,25 @@ static void imxfb_exit_backlight(struct imxfb_info *fbi)
 }
 #endif
 
-static void imxfb_enable_controller(struct imxfb_info *fbi)
+static int imxfb_enable_controller(struct imxfb_info *fbi)
 {
+	int ret;
 
 	if (fbi->enabled)
-		return;
+		return 0;
 
 	pr_debug("Enabling LCD controller\n");
 
+	if (fbi->reg_lcd) {
+		ret = regulator_enable(fbi->reg_lcd);
+		if (ret) {
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator enable failed with error: %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	writel(fbi->screen_dma, fbi->regs + LCDC_SSA);
 
 	/* panning offset 0 (0 pixel offset)        */
@@ -593,12 +606,16 @@ static void imxfb_enable_controller(struct imxfb_info *fbi)
 		fbi->backlight_power(1);
 	if (fbi->lcd_power)
 		fbi->lcd_power(1);
+
+	return 0;
 }
 
-static void imxfb_disable_controller(struct imxfb_info *fbi)
+static int imxfb_disable_controller(struct imxfb_info *fbi)
 {
+	int ret;
+
 	if (!fbi->enabled)
-		return;
+		return 0;
 
 	pr_debug("Disabling LCD controller\n");
 
@@ -613,6 +630,17 @@ static void imxfb_disable_controller(struct imxfb_info *fbi)
 	fbi->enabled = false;
 
 	writel(0, fbi->regs + LCDC_RMCR);
+
+	if (fbi->reg_lcd) {
+		ret = regulator_disable(fbi->reg_lcd);
+		if (ret)
+			dev_err(&fbi->pdev->dev,
+				"lcd regulator disable failed with error: %d\n",
+				ret);
+			return ret;
+	}
+
+	return 0;
 }
 
 static int imxfb_blank(int blank, struct fb_info *info)
@@ -626,13 +654,12 @@ static int imxfb_blank(int blank, struct fb_info *info)
 	case FB_BLANK_VSYNC_SUSPEND:
 	case FB_BLANK_HSYNC_SUSPEND:
 	case FB_BLANK_NORMAL:
-		imxfb_disable_controller(fbi);
-		break;
+		return imxfb_disable_controller(fbi);
 
 	case FB_BLANK_UNBLANK:
-		imxfb_enable_controller(fbi);
-		break;
+		return imxfb_enable_controller(fbi);
 	}
+
 	return 0;
 }
 
@@ -734,8 +761,7 @@ static int imxfb_suspend(struct platform_device *dev, pm_message_t state)
 
 	pr_debug("%s\n", __func__);
 
-	imxfb_disable_controller(fbi);
-	return 0;
+	return imxfb_disable_controller(fbi);
 }
 
 static int imxfb_resume(struct platform_device *dev)
@@ -745,8 +771,7 @@ static int imxfb_resume(struct platform_device *dev)
 
 	pr_debug("%s\n", __func__);
 
-	imxfb_enable_controller(fbi);
-	return 0;
+	return imxfb_enable_controller(fbi);
 }
 #else
 #define imxfb_suspend	NULL
@@ -1020,6 +1045,12 @@ static int imxfb_probe(struct platform_device *pdev)
 		goto failed_register;
 	}
 
+	fbi->reg_lcd = devm_regulator_get(&pdev->dev, "lcd");
+	if (IS_ERR(fbi->reg_lcd)) {
+		dev_info(&pdev->dev, "No lcd regulator used.\n");
+		fbi->reg_lcd = NULL;
+	}
+
 	imxfb_enable_controller(fbi);
 	fbi->pdev = pdev;
 #ifdef PWMR_BACKLIGHT_AVAILABLE
-- 
1.7.9.5


^ permalink raw reply related

* [GIT PULL] fbdev fixes for 3.13
From: Tomi Valkeinen @ 2013-12-05 12:04 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-kernel, linux-fbdev, Jean-Christophe PLAGNIOL-VILLARD

[-- Attachment #1: Type: text/plain, Size: 1471 bytes --]

Hi Linus,

Please pull these minor fbdev fixes for 3.13.

 Tomi

The following changes since commit 6ce4eac1f600b34f2f7f58f9cd8f0503d79e42ae:

  Linux 3.13-rc1 (2013-11-22 11:30:55 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/tomba/linux.git tags/fbdev-fixes-3.13

for you to fetch changes up to 46ac29568e63c9da9c15804648d3ed8c25e2dc44:

  video: vt8500: fix error handling in probe() (2013-12-04 10:50:16 +0200)

----------------------------------------------------------------
Minor fbdev fixes for 3.13.

----------------------------------------------------------------
Aaro Koskinen (1):
      ARM: OMAPFB: panel-sony-acx565akm: fix bad unlock balance

Dan Carpenter (1):
      video: vt8500: fix error handling in probe()

Geert Uytterhoeven (1):
      fbdev: sh_mobile_meram: Fix defined but not used compiler warnings

Johan Hovold (1):
      atmel_lcdfb: fix module autoload

Sasha Levin (1):
      video: kyro: fix incorrect sizes when copying to userspace

 drivers/video/atmel_lcdfb.c                             |  1 +
 drivers/video/kyro/fbdev.c                              |  6 +++---
 drivers/video/omap2/displays-new/panel-sony-acx565akm.c |  5 ++---
 drivers/video/sh_mobile_meram.c                         |  2 ++
 drivers/video/vt8500lcdfb.c                             | 25 +++++++++++--------------
 5 files changed, 19 insertions(+), 20 deletions(-)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* [PATCH] video: Replace local macro with PCI standard macro
From: Yijing Wang @ 2013-12-05 11:25 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen
  Cc: linux-fbdev, linux-kernel, Yijing Wang, Hanjun Guo

Replace local macro TGA_BUS_PCI with PCI standard
marco dev_is_pci().

Signed-off-by: Yijing Wang <wangyijing@huawei.com>
---
 drivers/video/tgafb.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/video/tgafb.c b/drivers/video/tgafb.c
index f28674f..5e94c6e 100644
--- a/drivers/video/tgafb.c
+++ b/drivers/video/tgafb.c
@@ -32,12 +32,6 @@
 
 #include <video/tgafb.h>
 
-#ifdef CONFIG_PCI
-#define TGA_BUS_PCI(dev) (dev->bus = &pci_bus_type)
-#else
-#define TGA_BUS_PCI(dev) 0
-#endif
-
 #ifdef CONFIG_TC
 #define TGA_BUS_TC(dev) (dev->bus = &tc_bus_type)
 #else
@@ -236,7 +230,7 @@ tgafb_set_par(struct fb_info *info)
 	};
 
 	struct tga_par *par = (struct tga_par *) info->par;
-	int tga_bus_pci = TGA_BUS_PCI(par->dev);
+	int tga_bus_pci = dev_is_pci(par->dev);
 	int tga_bus_tc = TGA_BUS_TC(par->dev);
 	u32 htimings, vtimings, pll_freq;
 	u8 tga_type;
@@ -519,7 +513,7 @@ tgafb_setcolreg(unsigned regno, unsigned red, unsigned green, unsigned blue,
 		unsigned transp, struct fb_info *info)
 {
 	struct tga_par *par = (struct tga_par *) info->par;
-	int tga_bus_pci = TGA_BUS_PCI(par->dev);
+	int tga_bus_pci = dev_is_pci(par->dev);
 	int tga_bus_tc = TGA_BUS_TC(par->dev);
 
 	if (regno > 255)
@@ -1472,7 +1466,7 @@ static void
 tgafb_init_fix(struct fb_info *info)
 {
 	struct tga_par *par = (struct tga_par *)info->par;
-	int tga_bus_pci = TGA_BUS_PCI(par->dev);
+	int tga_bus_pci = dev_is_pci(par->dev);
 	int tga_bus_tc = TGA_BUS_TC(par->dev);
 	u8 tga_type = par->tga_type;
 	const char *tga_type_name = NULL;
@@ -1560,7 +1554,7 @@ static int tgafb_register(struct device *dev)
 	const struct fb_videomode *modedb_tga = NULL;
 	resource_size_t bar0_start = 0, bar0_len = 0;
 	const char *mode_option_tga = NULL;
-	int tga_bus_pci = TGA_BUS_PCI(dev);
+	int tga_bus_pci = dev_is_pci(dev);
 	int tga_bus_tc = TGA_BUS_TC(dev);
 	unsigned int modedbsize_tga = 0;
 	void __iomem *mem_base;
@@ -1690,7 +1684,7 @@ static int tgafb_register(struct device *dev)
 static void tgafb_unregister(struct device *dev)
 {
 	resource_size_t bar0_start = 0, bar0_len = 0;
-	int tga_bus_pci = TGA_BUS_PCI(dev);
+	int tga_bus_pci = dev_is_pci(dev);
 	int tga_bus_tc = TGA_BUS_TC(dev);
 	struct fb_info *info = NULL;
 	struct tga_par *par;
-- 
1.7.1



^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox