* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Geert Uytterhoeven @ 2014-11-13 8:58 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141113085238.GE20972@lukather>
Hi Maxime,
On Thu, Nov 13, 2014 at 9:52 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
>> -module_init(simplefb_init);
>> +/*
>> + * While this can be a module, if builtin it's most likely the console
>> + * So let's leave module_exit but move module_init to an earlier place
>> + */
>
> Not really related to this patch itself, but do we want to support
> simplefb as a module? It seems like it's going to be most of the time
> broken.
If it depends on clocks, it won't work as a module, as CCF will have disabled
all unused clocks at that point.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [alsa-devel] [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Jean-Francois Moine @ 2014-11-13 9:17 UTC (permalink / raw)
To: Tomi Valkeinen
Cc: Mark Brown, Jyri Sarha, peter.ujfalusi, liam.r.girdwood,
alsa-devel, linux-omap, linux-fbdev
In-Reply-To: <54646648.5030807@ti.com>
On Thu, 13 Nov 2014 10:05:28 +0200
Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
[snip]
> I don't have much knowledge of the asoc architecture, so I probably
> can't comment much on the sound/ side design. For me the most important
> things are that 1) it works 2) I can easily unload/load the modules
> (which was broken in some of the earlier versions).
>
> As a more general discussion item, I'm still wondering why it feels like
> we (OMAP) are doing something totally new here. I'd imagine that almost
> every device with HDMI would need both video and audio side support, and
> those sides need to work together. And the audio side would need to get
> notified of things like cable disconnect (i.e. the video stream is
> stopped -> audio must be stopped also). But if I've understood right,
> there was no similar existing code to be found.
I recently posted a patch on the HDMI CODEC to interface a HDMI
transmitter
http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/082745.html
and I saw only a few dependencies between the 2 subsystems:
- the CODEC must know the transmitter parameters (DAIs - static -,
audio constraints - dynamic -),
- the CODEC must alert the transmitter on audio start and stop.
I don't think that stopping audio streaming on HDMI disconnection is
useful. I even let audio streaming start when the HDMI cable is
disconnected.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Hans de Goede @ 2014-11-13 9:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141113085238.GE20972@lukather>
Hi,
On 11/13/2014 09:52 AM, Maxime Ripard wrote:
> On Wed, Nov 12, 2014 at 11:08:43PM +0100, Hans de Goede wrote:
>> One of the reasons for having the simplefb nodes in /chosen, and doing
>> explicit enumeration of the nodes there, is too allow enumerating them sooner,
>> so that we get a console earlier on.
>>
>> Doing this earlier then fs_initcall is not useful, since the fb only turns into
>> a console when fbcon intializes, which is a fs_initcall too.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/video/fbdev/simplefb.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>> index be7d288..8c0c972 100644
>> --- a/drivers/video/fbdev/simplefb.c
>> +++ b/drivers/video/fbdev/simplefb.c
>> @@ -415,7 +415,11 @@ static void __exit simplefb_exit(void)
>> platform_driver_unregister(&simplefb_driver);
>> }
>>
>> -module_init(simplefb_init);
>> +/*
>> + * While this can be a module, if builtin it's most likely the console
>> + * So let's leave module_exit but move module_init to an earlier place
>> + */
>
> Not really related to this patch itself, but do we want to support
> simplefb as a module? It seems like it's going to be most of the time
> broken.
A valid point, my mean reasoning here is that some may see not being able to
use it as a module as a regression, so I just kept things as is, but I do
agree that it is advisable to just build it in.
Regards,
Hans
^ permalink raw reply
* [PATCH 1/7] ARM: dts: sunxi: Add regulator-boot-on property to ahci-5v regulator
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
This avoids it getting briefly turned off between when the regulator getting
registered and the ahci driver turning it back on, thus avoiding the disk
going into emergency head park mode.
Cc: Bruno Prémont <bonbons@linux-vserver.org>
Reported-and-tested-by: Bruno Prémont <bonbons@linux-vserver.org>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sunxi-common-regulators.dtsi | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/boot/dts/sunxi-common-regulators.dtsi b/arch/arm/boot/dts/sunxi-common-regulators.dtsi
index c9c5b10..a950671 100644
--- a/arch/arm/boot/dts/sunxi-common-regulators.dtsi
+++ b/arch/arm/boot/dts/sunxi-common-regulators.dtsi
@@ -44,6 +44,7 @@
regulator-name = "ahci-5v";
regulator-min-microvolt = <5000000>;
regulator-max-microvolt = <5000000>;
+ regulator-boot-on;
enable-active-high;
gpio = <&pio 1 8 0>;
status = "disabled";
--
2.1.0
^ permalink raw reply related
* [PATCH 2/7] ARM: dts: sun4i: Add simplefb node
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
Add a simplefb template node for u-boot to further fill and activate.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun4i-a10.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun4i-a10.dtsi b/arch/arm/boot/dts/sun4i-a10.dtsi
index 1ef7d57..686d8ef 100644
--- a/arch/arm/boot/dts/sun4i-a10.dtsi
+++ b/arch/arm/boot/dts/sun4i-a10.dtsi
@@ -27,6 +27,18 @@
serial7 = &uart7;
};
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer0 {
+ compatible = "simple-framebuffer";
+ clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
+ status = "disabled";
+ };
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.1.0
^ permalink raw reply related
* [PATCH 3/7] ARM: dts: sun5i: Add simplefb node
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
Add a simplefb template node for u-boot to further fill and activate.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun5i-a10s.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index 7089284..dcea088 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -24,6 +24,18 @@
serial3 = &uart3;
};
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer0 {
+ compatible = "simple-framebuffer";
+ clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
+ status = "disabled";
+ };
+ };
+
cpus {
cpu@0 {
compatible = "arm,cortex-a8";
--
2.1.0
^ permalink raw reply related
* [PATCH 4/7] ARM: dts: sun6i: Add simplefb node
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
Add a simplefb template node for u-boot to further fill and activate.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun6i-a31.dtsi | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi
index 543f895..438952e 100644
--- a/arch/arm/boot/dts/sun6i-a31.dtsi
+++ b/arch/arm/boot/dts/sun6i-a31.dtsi
@@ -62,6 +62,16 @@
ethernet0 = &gmac;
};
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer0 {
+ compatible = "simple-framebuffer";
+ status = "disabled";
+ };
+ };
cpus {
enable-method = "allwinner,sun6i-a31";
--
2.1.0
^ permalink raw reply related
* [PATCH 5/7] ARM: dts: sun7i: Add simplefb node
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
Add a simplefb template node for u-boot to further fill and activate.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun7i-a20.dtsi | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index 81d4e55..3ef078f 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -64,6 +64,18 @@
serial7 = &uart7;
};
+ chosen {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ framebuffer0 {
+ compatible = "simple-framebuffer";
+ clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
+ status = "disabled";
+ };
+ };
+
cpus {
#address-cells = <1>;
#size-cells = <0>;
--
2.1.0
^ permalink raw reply related
* [PATCH 6/7] ARM: dts: sun6i: Add EHCI support for the M9 board
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
The Mele M9 / A1000G quad uses both usb-ports, one goes to an internal
usb wifi card, the other to a build-in usb-hub, so neither need their
OHCI companion controller to be enabled since the are always connected at
USB-2 speeds.
The controller which is attached to the wifi also does not need a vbus
regulator.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun6i-a31-m9.dts | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-m9.dts b/arch/arm/boot/dts/sun6i-a31-m9.dts
index bc6115d..c85247e 100644
--- a/arch/arm/boot/dts/sun6i-a31-m9.dts
+++ b/arch/arm/boot/dts/sun6i-a31-m9.dts
@@ -32,6 +32,19 @@
status = "okay";
};
+ usbphy: phy@01c19400 {
+ usb1_vbus-supply = <®_usb1_vbus>;
+ status = "okay";
+ };
+
+ ehci0: usb@01c1a000 {
+ status = "okay";
+ };
+
+ ehci1: usb@01c1b000 {
+ status = "okay";
+ };
+
pio: pinctrl@01c20800 {
mmc0_cd_pin_m9: mmc0_cd_pin@0 {
allwinner,pins = "PH22";
@@ -39,6 +52,12 @@
allwinner,drive = <0>;
allwinner,pull = <1>;
};
+ usb1_vbus_pin_m9: usb1_vbus_pin@0 {
+ allwinner,pins = "PC27";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
};
uart0: serial@01c28000 {
@@ -47,4 +66,11 @@
status = "okay";
};
};
+
+ reg_usb1_vbus: usb1-vbus {
+ pinctrl-names = "default";
+ pinctrl-0 = <&usb1_vbus_pin_m9>;
+ gpio = <&pio 2 27 0>;
+ status = "okay";
+ };
};
--
2.1.0
^ permalink raw reply related
* [PATCH 7/7] ARM: dts: sun6i: Add support for the status led
From: Hans de Goede @ 2014-11-13 9:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415871109-28332-1-git-send-email-hdegoede@redhat.com>
The Mele M9 / A1000G quad has a blue status led, add support for this.
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
arch/arm/boot/dts/sun6i-a31-m9.dts | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/arch/arm/boot/dts/sun6i-a31-m9.dts b/arch/arm/boot/dts/sun6i-a31-m9.dts
index c85247e..43f8f18 100644
--- a/arch/arm/boot/dts/sun6i-a31-m9.dts
+++ b/arch/arm/boot/dts/sun6i-a31-m9.dts
@@ -46,6 +46,12 @@
};
pio: pinctrl@01c20800 {
+ led_pins_m9: led_pins@0 {
+ allwinner,pins = "PH13";
+ allwinner,function = "gpio_out";
+ allwinner,drive = <0>;
+ allwinner,pull = <0>;
+ };
mmc0_cd_pin_m9: mmc0_cd_pin@0 {
allwinner,pins = "PH22";
allwinner,function = "gpio_in";
@@ -67,6 +73,17 @@
};
};
+ leds {
+ compatible = "gpio-leds";
+ pinctrl-names = "default";
+ pinctrl-0 = <&led_pins_m9>;
+
+ blue {
+ label = "m9:blue:usr";
+ gpios = <&pio 7 13 0>;
+ };
+ };
+
reg_usb1_vbus: usb1-vbus {
pinctrl-names = "default";
pinctrl-0 = <&usb1_vbus_pin_m9>;
--
2.1.0
^ permalink raw reply related
* Re: [alsa-devel] [PATCH v7.1 00/19] Rework OMAP4+ HDMI audio support
From: Tomi Valkeinen @ 2014-11-13 10:00 UTC (permalink / raw)
To: Jean-Francois Moine
Cc: liam.r.girdwood, alsa-devel, linux-fbdev, Jyri Sarha,
peter.ujfalusi, Mark Brown, linux-omap
In-Reply-To: <20141113101738.6338bb1c@armhf>
[-- Attachment #1: Type: text/plain, Size: 4617 bytes --]
Hi,
On 13/11/14 11:17, Jean-Francois Moine wrote:
> On Thu, 13 Nov 2014 10:05:28 +0200
> Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> [snip]
>> I don't have much knowledge of the asoc architecture, so I probably
>> can't comment much on the sound/ side design. For me the most important
>> things are that 1) it works 2) I can easily unload/load the modules
>> (which was broken in some of the earlier versions).
>>
>> As a more general discussion item, I'm still wondering why it feels like
>> we (OMAP) are doing something totally new here. I'd imagine that almost
>> every device with HDMI would need both video and audio side support, and
>> those sides need to work together. And the audio side would need to get
>> notified of things like cable disconnect (i.e. the video stream is
>> stopped -> audio must be stopped also). But if I've understood right,
>> there was no similar existing code to be found.
>
> I recently posted a patch on the HDMI CODEC to interface a HDMI
> transmitter
> http://mailman.alsa-project.org/pipermail/alsa-devel/2014-October/082745.html
Jyri or Peter knows this better, but I think one difference with OMAP
HDMI case and tda998x is that tda998x is an external encoder, and you
transfer audio data to it via i2s or spdif, whereas OMAP HDMI is inside
the SoC, and the HDMI IP gets the audio data via system DMA.
The system DMA transfers are triggered by the HDMI IP, and the HDMI IP
has to be enabled and properly configured for the DMA ports to function.
So, correct me if I'm wrong, but I think this is the fundamental difference:
In your case, the actual audio playback happens with the i2s or spdif
side. You can enable the playback at any time, no matter what is the
status of tda998x. If tda998x is not operational, the audio data will
just go to /dev/null.
In our case, the actual audio playback happens inside the HDMI block. We
need the whole HDMI block to be operational and correctly configured for
(fake or real) playback to be possible.
> and I saw only a few dependencies between the 2 subsystems:
>
> - the CODEC must know the transmitter parameters (DAIs - static -,
> audio constraints - dynamic -),
The video mode (i.e. availability of audio) or the EDID (i.e. the
supported audio parameters) may change at any time, so I presume in your
series the video side will inform the codec of these changes at any point?
> - the CODEC must alert the transmitter on audio start and stop.
>
> I don't think that stopping audio streaming on HDMI disconnection is
And you allow audio playback also if the monitor does not support audio,
or the video mode does not supprot audio?
> useful. I even let audio streaming start when the HDMI cable is
> disconnected.
Ah, this is actually unclear thing to me: what should the audio side
behavior be when the HDMI cable is disconnected or the video is blanked?
I believe the options are:
a) Always keep the audio device operational, no matter what is the
status of the video side. How should this work if the HDMI videomode or
the HDMI monitor does not support audio? Is it desirable that the
userspace has no idea that the audio is not actually going anywhere?
b) Remove the audio device when audio support is not available. This
kind of makes sense, as, well, there's no possibility for audio output.
But maybe this is not how linux sound devices are supposed to behave?
c) Return errors when playback is attempted when audio support is not
available. Again, this kind of makes sense, as audio playback is not
possible. But maybe this is also not how audio devices generally work.
Jyri, were there some other options we discussed?
Currently, the OMAP HDMI version does c). It's the easiest solution for us.
Option a) is a bit problematic for us, as it requires the HDMI IP side
to be fully operational, with the video output configured and enabled,
as that is what drives the audio DMA. If we stop the video, I believe
the audio DMA will freeze, and it'll lead to timeouts on the audio side.
I haven't tried, but I believe we could program the HDMI IP to some safe
default video mode if the cable is disconnected, and turn off the actual
HDMI PHY, so that the audio side could work even if the HDMI stream is
not going anywhere. I think it would be quite big change to how the HDMI
driver works at the moment.
But then, if the cable _is_ connected and the video mode is such that it
cannot not support audio, I wonder if we can still fake the audio
playback or will the HDMI IP get confused...
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [linux-sunxi] [PATCH 2/4] simplefb: Add support for enumerating simplefb dt nodes in /chosen
From: Grant Likely @ 2014-11-13 10:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGRGNgW_Q6oghpe5Mw-8aYPVVj1YZ6R3iaqOQzCE9dSD1nYSyA@mail.gmail.com>
On Wed, Nov 12, 2014 at 11:39 PM, Julian Calaby <julian.calaby@gmail.com> wrote:
> Hi Hans,
>
> On Thu, Nov 13, 2014 at 9:08 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Update simplefb to support the new preferred location for simplefb dt nodes
>> under /chosen.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> ---
>> drivers/video/fbdev/simplefb.c | 33 ++++++++++++++++++++++++++++++++-
>> 1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>> index cd96edd..be7d288 100644
>> --- a/drivers/video/fbdev/simplefb.c
>> +++ b/drivers/video/fbdev/simplefb.c
>> @@ -27,6 +27,7 @@
>> #include <linux/platform_data/simplefb.h>
>> #include <linux/platform_device.h>
>> #include <linux/clk-provider.h>
>> +#include <linux/of_platform.h>
>>
>> static struct fb_fix_screeninfo simplefb_fix = {
>> .id = "simple",
>> @@ -385,7 +386,37 @@ static struct platform_driver simplefb_driver = {
>> .probe = simplefb_probe,
>> .remove = simplefb_remove,
>> };
>> -module_platform_driver(simplefb_driver);
>> +
>> +static int __init simplefb_init(void)
>> +{
>> + int i, ret;
>> + char name[16];
>> + struct device_node *np;
>> +
>> + ret = platform_driver_register(&simplefb_driver);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; ; i++) {
>> + snprintf(name, sizeof(name), "framebuffer%d", i);
>
> This smells like an infinite loop: we can be pretty sure that no
> hardware will ever exist with more than 9999 (I think?) framebuffers,
> however if that ever happens this'll loop until it runs out of RAM.
> Maybe add a suitably high limit to the for loop?
Unlikely, but the loop is wrong anyway. The loop should be:
for_each_child_of_node(of_chosen, child)
if (of_device_is_compatible(child, "simple-framebuffer");
of_platform_device_create(np, NULL, NULL);
Then make the probe hook choose an appropriate FB number. It looks
like you structured the code the way you did to get the framebuffers
to register in a particular order, and therefore implicitly get the
right numbers, but that is a fragile way to go about it.
Using /aliases really is the right way to get specific framebuffer
numbers. We already use it for UARTs, so we should do the same here.
Use of_alias_get_id(, "framebuffer") to get the framebuffer number.
g.
^ permalink raw reply
* Re: [PATCH v2 2/4] simplefb: Add support for enumerating simplefb dt nodes in /chosen
From: Grant Likely @ 2014-11-13 10:27 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415868610-13297-2-git-send-email-hdegoede@redhat.com>
On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Update simplefb to support the new preferred location for simplefb dt nodes
> under /chosen.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> --
> Changes in v2:
> -Make name array larger in case we ever encounter more then 10000 framebuffers
I just replied on v1, but for completeness I'm replying here. Nak on
this method. for_each_child_of_node() should be used instead.
g.
> ---
> drivers/video/fbdev/simplefb.c | 33 ++++++++++++++++++++++++++++++++-
> 1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
> index cd96edd..2705af8 100644
> --- a/drivers/video/fbdev/simplefb.c
> +++ b/drivers/video/fbdev/simplefb.c
> @@ -27,6 +27,7 @@
> #include <linux/platform_data/simplefb.h>
> #include <linux/platform_device.h>
> #include <linux/clk-provider.h>
> +#include <linux/of_platform.h>
>
> static struct fb_fix_screeninfo simplefb_fix = {
> .id = "simple",
> @@ -385,7 +386,37 @@ static struct platform_driver simplefb_driver = {
> .probe = simplefb_probe,
> .remove = simplefb_remove,
> };
> -module_platform_driver(simplefb_driver);
> +
> +static int __init simplefb_init(void)
> +{
> + int i, ret;
> + char name[32];
> + struct device_node *np;
> +
> + ret = platform_driver_register(&simplefb_driver);
> + if (ret)
> + return ret;
> +
> + for (i = 0; ; i++) {
> + snprintf(name, sizeof(name), "framebuffer%d", i);
> + np = of_find_node_by_name(of_chosen, name);
> + if (!np)
> + break;
> +
> + /* of_platform_device_create will check status for us */
> + of_platform_device_create(np, NULL, NULL);
> + }
> +
> + return 0;
> +}
> +
> +static void __exit simplefb_exit(void)
> +{
> + platform_driver_unregister(&simplefb_driver);
> +}
> +
> +module_init(simplefb_init);
> +module_exit(simplefb_exit);
>
> MODULE_AUTHOR("Stephen Warren <swarren@wwwdotorg.org>");
> MODULE_DESCRIPTION("Simple framebuffer driver");
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Grant Likely @ 2014-11-13 10:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <546478B5.2040002@redhat.com>
On Thu, Nov 13, 2014 at 9:24 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 11/13/2014 09:52 AM, Maxime Ripard wrote:
>> On Wed, Nov 12, 2014 at 11:08:43PM +0100, Hans de Goede wrote:
>>> One of the reasons for having the simplefb nodes in /chosen, and doing
>>> explicit enumeration of the nodes there, is too allow enumerating them sooner,
>>> so that we get a console earlier on.
>>>
>>> Doing this earlier then fs_initcall is not useful, since the fb only turns into
>>> a console when fbcon intializes, which is a fs_initcall too.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> drivers/video/fbdev/simplefb.c | 6 +++++-
>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>>> index be7d288..8c0c972 100644
>>> --- a/drivers/video/fbdev/simplefb.c
>>> +++ b/drivers/video/fbdev/simplefb.c
>>> @@ -415,7 +415,11 @@ static void __exit simplefb_exit(void)
>>> platform_driver_unregister(&simplefb_driver);
>>> }
>>>
>>> -module_init(simplefb_init);
>>> +/*
>>> + * While this can be a module, if builtin it's most likely the console
>>> + * So let's leave module_exit but move module_init to an earlier place
>>> + */
>>
>> Not really related to this patch itself, but do we want to support
>> simplefb as a module? It seems like it's going to be most of the time
>> broken.
>
> A valid point, my mean reasoning here is that some may see not being able to
> use it as a module as a regression, so I just kept things as is, but I do
> agree that it is advisable to just build it in.
Like a lot of things, if it is made a modules, and it breaks for the
user, the user gets to keep the pieces. There are potentially some
valid scenarios where it is fine to have it as a module. I don't
recommend changing this unless is actually starts causing problems.
g.
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Grant Likely @ 2014-11-13 10:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6u8B854PEWw8XzwhyX8joMWwOsHOOXtmBmC1TgCGjGtdg@mail.gmail.com>
On Thu, Nov 13, 2014 at 10:29 AM, Grant Likely <grant.likely@linaro.org> wrote:
> On Thu, Nov 13, 2014 at 9:24 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Hi,
>>
>> On 11/13/2014 09:52 AM, Maxime Ripard wrote:
>>> On Wed, Nov 12, 2014 at 11:08:43PM +0100, Hans de Goede wrote:
>>>> One of the reasons for having the simplefb nodes in /chosen, and doing
>>>> explicit enumeration of the nodes there, is too allow enumerating them sooner,
>>>> so that we get a console earlier on.
>>>>
>>>> Doing this earlier then fs_initcall is not useful, since the fb only turns into
>>>> a console when fbcon intializes, which is a fs_initcall too.
>>>>
>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>> ---
>>>> drivers/video/fbdev/simplefb.c | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>>>> index be7d288..8c0c972 100644
>>>> --- a/drivers/video/fbdev/simplefb.c
>>>> +++ b/drivers/video/fbdev/simplefb.c
>>>> @@ -415,7 +415,11 @@ static void __exit simplefb_exit(void)
>>>> platform_driver_unregister(&simplefb_driver);
>>>> }
>>>>
>>>> -module_init(simplefb_init);
>>>> +/*
>>>> + * While this can be a module, if builtin it's most likely the console
>>>> + * So let's leave module_exit but move module_init to an earlier place
>>>> + */
>>>
>>> Not really related to this patch itself, but do we want to support
>>> simplefb as a module? It seems like it's going to be most of the time
>>> broken.
>>
>> A valid point, my mean reasoning here is that some may see not being able to
>> use it as a module as a regression, so I just kept things as is, but I do
>> agree that it is advisable to just build it in.
>
> Like a lot of things, if it is made a modules, and it breaks for the
> user, the user gets to keep the pieces. There are potentially some
> valid scenarios where it is fine to have it as a module. I don't
> recommend changing this unless is actually starts causing problems.
I assume that you've tested this and it actually makes a difference,
correct? If so,
Acked-by: Grant Likely <grant.likely@linaro.org>
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Maxime Ripard @ 2014-11-13 10:42 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6u8B854PEWw8XzwhyX8joMWwOsHOOXtmBmC1TgCGjGtdg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1643 bytes --]
On Thu, Nov 13, 2014 at 10:29:52AM +0000, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 9:24 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> > On 11/13/2014 09:52 AM, Maxime Ripard wrote:
> >>> +/*
> >>> + * While this can be a module, if builtin it's most likely the console
> >>> + * So let's leave module_exit but move module_init to an earlier place
> >>> + */
> >>
> >> Not really related to this patch itself, but do we want to support
> >> simplefb as a module? It seems like it's going to be most of the time
> >> broken.
> >
> > A valid point, my mean reasoning here is that some may see not being able to
> > use it as a module as a regression, so I just kept things as is, but I do
> > agree that it is advisable to just build it in.
>
> Like a lot of things, if it is made a modules, and it breaks for the
> user, the user gets to keep the pieces. There are potentially some
> valid scenarios where it is fine to have it as a module. I don't
> recommend changing this unless is actually starts causing problems.
I don't really agree here. If it's broken because the clocks, reset,
memory, or whatever resource has been reclaimed by the kernel before
the module even had a chance to probe, the only thing that the user
will get is that there's no chance it's ever going to work, and that
it's just unreliable.
If we know that it's going to break, and that there's no way it can be
reliable (as in on all the SoCs reliable), keeping it as a module is
just asking for trouble.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Maxime Ripard @ 2014-11-13 10:45 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415868610-13297-1-git-send-email-hdegoede@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 973 bytes --]
On Thu, Nov 13, 2014 at 09:50:07AM +0100, Hans de Goede wrote:
> Since simplefb nodes do not relate directly to hw typically they have been
> placed in the root of the devicetree. As the represent runtime information
> having them as sub-nodes of /chosen is more logical, specify this.
>
> Also specify when to set the chosen stdout-path property to a simplefb node.
>
> For reliable handover to a hardware specific driver, that driver needs to
> know which simplefb to unregister when taking over, specify how the hw driver
> can find the matching simplefb node.
>
> Last add some advice on how to fill and use simplefb nodes from a firmware
> pov.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Grant Likely @ 2014-11-13 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415868610-13297-1-git-send-email-hdegoede@redhat.com>
On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
> Since simplefb nodes do not relate directly to hw typically they have been
> placed in the root of the devicetree. As the represent runtime information
> having them as sub-nodes of /chosen is more logical, specify this.
>
> Also specify when to set the chosen stdout-path property to a simplefb node.
>
> For reliable handover to a hardware specific driver, that driver needs to
> know which simplefb to unregister when taking over, specify how the hw driver
> can find the matching simplefb node.
>
> Last add some advice on how to fill and use simplefb nodes from a firmware
> pov.
>
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
> --
> Changes in v2:
> -Add stdout-path to the example code
> ---
> .../bindings/video/simple-framebuffer.txt | 38 +++++++++++++++++++++-
> 1 file changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> index 8f35718..8b7ecf6 100644
> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
> @@ -4,6 +4,26 @@ A simple frame-buffer describes a frame-buffer setup by firmware or
> the bootloader, with the assumption that the display hardware has already
> been set up to scan out from the memory pointed to by the reg property.
>
> +Since simplefb nodes represent runtime information they must be sub-nodes of
> +the chosen node (*). The primary display node must be named framebuffer0,
> +additional nodes must be called framebuffer1, etc.
Thinking more about our conversation yesterday, the preferred name
should still be framebuffer@<address>. There is no reason to break
with established convention, and as mentioned in my reply on patch #2,
using the name is not the preferred way to identify a device node. So,
my recommendation is to prefer the name "framebuffer@<addr>", but if
it is inconvenient because the address isn't known, then
"framebuffer#" is acceptable. Then use /aliases for actual enumeration
which has the advantage of also working for framebuffers that aren't
in /chosen.
> +
> +If a simplefb node represents the preferred console for user interaction,
> +then the chosen node's stdout-path property must point to it.
> +
> +If the devicetree contains nodes for the display hardware used by a simplefb,
> +then one of the hw nodes must have a property called "framebuffer" pointing to
> +the framebuffer# node in chosen, so that the operating system knows which
> +simplefb to disable when handing over control to a driver for the real
> +hardware. The bindings for the hw nodes must specify which node contains the
> +framebuffer property.
I've also been thinking about this. When we talked yesterday I said I
didn't have a preference as to which direction the link between the
framebuffer and the display hardware pointed. Thinking about it now,
between the two nodes, the HW node is pretty much static, but the FB
node is dynamic and in most scenarios will be enabled by firmware, and
then disabled by the operating system. If the framebuffer property is
in the HW node, then that means the firmware needs to modify two nodes
to set up the linkage correctly. It needs to enable the framebuffer,
and then add the link. If one of several possible FB nodes was enabled
by the firmware, it need to modify the display controller node to
match. And, when the framebuffer is disabled, the HW node is left with
a dangling link that kexec won't necessarily know how to fix up.
If instead the link goes the other way, as a property in the
framebuffer node pointing to display controller, all of the
functionality still stays the same, but the display controller node
never needs to change when framebuffers are
enabled/disabled/added/removed. Since the location and/or compatible
value of framebuffers is always well known, (aside from the fact that
the simplefb driver binds to them), we can have a single API that will
return all the framebuffers associated to a given display controller
node. The real HW driver will be able to use that to find its
framebuffers and coordinate handover between them.
g.
> +
> +It is advised that devicetree files contain pre-filled, disabled framebuffer#
> +nodes, so that the firmware only needs to update the mode information and
> +enable them. This way if e.g. later on support for more display clocks get
> +added, the simplefb nodes will already contain this info and the firmware
> +does not need to be updated.
> +
> Required properties:
> - compatible: "simple-framebuffer"
> - reg: Should contain the location and size of the framebuffer memory.
> @@ -22,11 +42,27 @@ Optional properties:
>
> Example:
>
> - framebuffer {
> +chosen {
> + framebuffer0 {
> compatible = "simple-framebuffer";
> reg = <0x1d385000 (1600 * 1200 * 2)>;
> width = <1600>;
> height = <1200>;
> stride = <(1600 * 2)>;
> format = "r5g6b5";
> + clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
> };
> + stdout-path = &framebuffer0;
> +};
> +
> +soc@01c00000 {
> + lcdc0: lcdc@1c0c000 {
> + compatible = "allwinner,sun4i-a10-lcdc";
> + framebuffer = <&framebuffer0>;
> + };
> +};
> +
> +
> +*) Older devicetree files may have a compatible = "simple-framebuffer" node
> +in a different place, operating systems must first enumerate any framebuffer#
> +nodes found under chosen and then check for other compatible nodes.
> --
> 2.1.0
>
^ permalink raw reply
* Re: [PATCH 4/4] arm: dts: omap3-gta04: Add static configuration for devconf1 register
From: Tomi Valkeinen @ 2014-11-13 11:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20141112150210.GC26481@atomide.com>
[-- Attachment #1: Type: text/plain, Size: 728 bytes --]
On 12/11/14 17:02, Tony Lindgren wrote:
>> And, with a quick grep, I see CONTROL_DEVCONF1 touched in multiple
>> places in the kernel. I wonder if adding a pinmux entry for it could
>> cause some rather odd problems.
>
> They can all use pinctrl-single no problem.
Can, but don't. That's my worry. If we touch the DEVCONF1 via pinmux,
and we have code in mach-omap2 that also touch DEVCONF1, without any
knowledge (and locking) between those...
So _maybe_ that's not an issue, as the pinmux config we have here is
fixed, and done once at boot time, and maybe the code in mach-omap2 that
touch DEVCONF1 is also ran just once and not at the same time as the
pinmux. But I don't know if that's so.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH v2 1/5] video: omapdss: Add opa362 driver
From: Tomi Valkeinen @ 2014-11-13 11:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1415830247-31633-2-git-send-email-marek@goldelico.com>
[-- Attachment #1: Type: text/plain, Size: 10111 bytes --]
On 13/11/14 00:10, Marek Belisko wrote:
> opa362 is amplifier for video and can be connected to the tvout pads
> of the OMAP3. It has one gpio control for enable/disable of the output
> (high impedance).
>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/video/fbdev/omap2/displays-new/Kconfig | 6 +
> drivers/video/fbdev/omap2/displays-new/Makefile | 1 +
> .../fbdev/omap2/displays-new/amplifier-opa362.c | 343 +++++++++++++++++++++
I think it would be better to rename this to encoder-opa362.c. It's not
encoder as such, but it falls into the same category.
> include/video/omap-panel-data.h | 12 +
> 4 files changed, 362 insertions(+)
> create mode 100644 drivers/video/fbdev/omap2/displays-new/amplifier-opa362.c
>
> diff --git a/drivers/video/fbdev/omap2/displays-new/Kconfig b/drivers/video/fbdev/omap2/displays-new/Kconfig
> index e6cfc38..211b3ec 100644
> --- a/drivers/video/fbdev/omap2/displays-new/Kconfig
> +++ b/drivers/video/fbdev/omap2/displays-new/Kconfig
> @@ -1,6 +1,12 @@
> menu "OMAP Display Device Drivers (new device model)"
> depends on OMAP2_DSS
>
> +config DISPLAY_AMPLIFIER_OPA362
Here also use ENCODER instead.
> + tristate "external analog amplifier with output disable/high-Z (e.g. OPA362)"
> + help
> + Driver to enable an external analog TV amplifier (e.g. OPA362)
> + through a GPIO.
The indentation above seems funny.
The text looks a bit odd. So is this a driver for OPA362, or is this a
generic driver for any similar devices? Most of the names and code makes
me think this is a driver for OPA362, but the text above quite clearly
gives the impression that this is a driver for any analog video amp,
with single enable gpio.
> +
> config DISPLAY_ENCODER_TFP410
> tristate "TFP410 DPI to DVI Encoder"
> help
> diff --git a/drivers/video/fbdev/omap2/displays-new/Makefile b/drivers/video/fbdev/omap2/displays-new/Makefile
> index 0323a8a..b311542 100644
> --- a/drivers/video/fbdev/omap2/displays-new/Makefile
> +++ b/drivers/video/fbdev/omap2/displays-new/Makefile
> @@ -1,3 +1,4 @@
> +obj-$(CONFIG_DISPLAY_AMPLIFIER_OPA362) += amplifier-opa362.o
> obj-$(CONFIG_DISPLAY_ENCODER_TFP410) += encoder-tfp410.o
> obj-$(CONFIG_DISPLAY_ENCODER_TPD12S015) += encoder-tpd12s015.o
> obj-$(CONFIG_DISPLAY_CONNECTOR_DVI) += connector-dvi.o
> diff --git a/drivers/video/fbdev/omap2/displays-new/amplifier-opa362.c b/drivers/video/fbdev/omap2/displays-new/amplifier-opa362.c
> new file mode 100644
> index 0000000..8065a28
> --- /dev/null
> +++ b/drivers/video/fbdev/omap2/displays-new/amplifier-opa362.c
> @@ -0,0 +1,343 @@
> +/*
> + * OPA362 analog video amplifier with output/power control
> + *
> + * Copyright (C) 2014 Golden Delicious Computers
> + * Author: H. Nikolaus Schaller <hns@goldelico.com>
> + *
> + * based on encoder-tfp410
> + *
> + * Copyright (C) 2013 Texas Instruments
> + * Author: Tomi Valkeinen <tomi.valkeinen@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/gpio.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <linux/of_gpio.h>
> +
> +#include <video/omapdss.h>
> +#include <video/omap-panel-data.h>
> +
> +struct panel_drv_data {
> + struct omap_dss_device dssdev;
> + struct omap_dss_device *in;
> +
> + int enable_gpio;
> +
> + struct omap_video_timings timings;
> +};
> +
> +#define to_panel_data(x) container_of(x, struct panel_drv_data, dssdev)
> +
> +static int opa362_connect(struct omap_dss_device *dssdev,
> + struct omap_dss_device *dst)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> + int r;
> +
> + dev_dbg(dssdev->dev, "connect\n");
> +
> + if (omapdss_device_is_connected(dssdev))
> + return -EBUSY;
> +
> + r = in->ops.atv->connect(in, dssdev);
> + if (r)
> + return r;
> +
> + dst->src = dssdev;
> + dssdev->dst = dst;
> +
> + return 0;
> +}
> +
> +static void opa362_disconnect(struct omap_dss_device *dssdev,
> + struct omap_dss_device *dst)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> +
> + dev_dbg(dssdev->dev, "disconnect\n");
> +
> + WARN_ON(!omapdss_device_is_connected(dssdev));
> + if (!omapdss_device_is_connected(dssdev))
> + return;
> +
> + WARN_ON(dst != dssdev->dst);
> + if (dst != dssdev->dst)
> + return;
> +
> + dst->src = NULL;
> + dssdev->dst = NULL;
> +
> + in->ops.atv->disconnect(in, &ddata->dssdev);
> +}
> +
> +static int opa362_enable(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> + int r;
> +
> + dev_dbg(dssdev->dev, "enable\n");
> +
> + if (!omapdss_device_is_connected(dssdev))
> + return -ENODEV;
> +
> + if (omapdss_device_is_enabled(dssdev))
> + return 0;
> +
> + in->ops.atv->set_timings(in, &ddata->timings);
> + /* fixme: should we receive the invert from our consumer, i.e. the connector? */
> + in->ops.atv->invert_vid_out_polarity(in, true);
Well this does seem to be broken. I don't know what the answer to the
question above is, but the code doesn't work properly.
In the opa362_invert_vid_out_polarity function below, you get the invert
boolean from the connector. This you pass to the OMAP venc. However,
above you always override that value in venc with true.
So, either the invert_vid_out_polarity value has to be always true or
false, because _OPA362_ requires it to be true or false, OR you need use
the value from the connector.
Seeing the comment in opa362_invert_vid_out_polarity, my guess is the
latter, and the call above should be removed.
> +
> + r = in->ops.atv->enable(in);
> + if (r)
> + return r;
> +
> + if (gpio_is_valid(ddata->enable_gpio))
> + gpio_set_value_cansleep(ddata->enable_gpio, 1);
> +
> + dssdev->state = OMAP_DSS_DISPLAY_ACTIVE;
> +
> + return 0;
> +}
> +
> +static void opa362_disable(struct omap_dss_device *dssdev)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> +
> + dev_dbg(dssdev->dev, "disable\n");
> +
> + if (!omapdss_device_is_enabled(dssdev))
> + return;
> +
> + if (gpio_is_valid(ddata->enable_gpio))
> + gpio_set_value_cansleep(ddata->enable_gpio, 0);
> +
> + in->ops.atv->disable(in);
> +
> + dssdev->state = OMAP_DSS_DISPLAY_DISABLED;
> +}
> +
> +static void opa362_set_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> +
> + dev_dbg(dssdev->dev, "set_timings\n");
> +
> + ddata->timings = *timings;
> + dssdev->panel.timings = *timings;
> +
> + in->ops.atv->set_timings(in, timings);
> +}
> +
> +static void opa362_get_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> +
> + dev_dbg(dssdev->dev, "get_timings\n");
> +
> + *timings = ddata->timings;
> +}
> +
> +static int opa362_check_timings(struct omap_dss_device *dssdev,
> + struct omap_video_timings *timings)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> +
> + dev_dbg(dssdev->dev, "check_timings\n");
> +
> + return in->ops.atv->check_timings(in, timings);
> +}
> +
> +static void opa362_set_type(struct omap_dss_device *dssdev,
> + enum omap_dss_venc_type type)
> +{
> + /* we can only drive a COMPOSITE output */
> + WARN_ON(type != OMAP_DSS_VENC_TYPE_COMPOSITE);
> +
> +}
> +
> +static void opa362_invert_vid_out_polarity(struct omap_dss_device *dssdev,
> + bool invert_polarity)
> +{
> + struct panel_drv_data *ddata = to_panel_data(dssdev);
> + struct omap_dss_device *in = ddata->in;
> +
> + /* OPA362 inverts the polarity */
> + in->ops.atv->invert_vid_out_polarity(in, !invert_polarity);
> +}
> +
> +static const struct omapdss_atv_ops opa362_atv_ops = {
> + .connect = opa362_connect,
> + .disconnect = opa362_disconnect,
> +
> + .enable = opa362_enable,
> + .disable = opa362_disable,
> +
> + .check_timings = opa362_check_timings,
> + .set_timings = opa362_set_timings,
> + .get_timings = opa362_get_timings,
> +
> + .set_type = opa362_set_type,
> + .invert_vid_out_polarity = opa362_invert_vid_out_polarity,
> +};
> +
> +static int opa362_probe_pdata(struct platform_device *pdev)
> +{
> + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct amplifier_opa362_platform_data *pdata;
> + struct omap_dss_device *dssdev, *in;
> +
> + pdata = dev_get_platdata(&pdev->dev);
> +
> + ddata->enable_gpio = pdata->enable_gpio;
> +
> + in = omap_dss_find_output(pdata->source);
> + if (in == NULL) {
> + dev_err(&pdev->dev, "Failed to find video source\n");
> + return -ENODEV;
> + }
> +
> + ddata->in = in;
> +
> + dssdev = &ddata->dssdev;
> + dssdev->name = pdata->name;
> +
> + return 0;
> +}
We are going to support only DT boot at some point. Thus I think the
whole platform data code should be left out.
> +
> +static int opa362_probe_of(struct platform_device *pdev)
> +{
> + struct panel_drv_data *ddata = platform_get_drvdata(pdev);
> + struct device_node *node = pdev->dev.of_node;
> + struct omap_dss_device *in;
> + int gpio;
> +
> + gpio = of_get_gpio(node, 0);
> +
> + if (gpio_is_valid(gpio) || gpio == -ENOENT) {
> + ddata->enable_gpio = gpio;
> + } else {
> + dev_err(&pdev->dev, "failed to parse enable gpio\n");
> + return gpio;
> + }
You should use the new GPIO API to get full support from the DT data.
For an example, see panel-dpi.c's enable_gpio.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 3/4] simplefb: Change simplefb_init from module_init to fs_initcall
From: Hans de Goede @ 2014-11-13 12:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6sYD-Ow2wcwAKuXpR3A9VZozo2sgz=eEcWXi5ayKd2jWQ@mail.gmail.com>
Hi,
On 11/13/2014 11:31 AM, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 10:29 AM, Grant Likely <grant.likely@linaro.org> wrote:
>> On Thu, Nov 13, 2014 at 9:24 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Hi,
>>>
>>> On 11/13/2014 09:52 AM, Maxime Ripard wrote:
>>>> On Wed, Nov 12, 2014 at 11:08:43PM +0100, Hans de Goede wrote:
>>>>> One of the reasons for having the simplefb nodes in /chosen, and doing
>>>>> explicit enumeration of the nodes there, is too allow enumerating them sooner,
>>>>> so that we get a console earlier on.
>>>>>
>>>>> Doing this earlier then fs_initcall is not useful, since the fb only turns into
>>>>> a console when fbcon intializes, which is a fs_initcall too.
>>>>>
>>>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>>>> ---
>>>>> drivers/video/fbdev/simplefb.c | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>>>>> index be7d288..8c0c972 100644
>>>>> --- a/drivers/video/fbdev/simplefb.c
>>>>> +++ b/drivers/video/fbdev/simplefb.c
>>>>> @@ -415,7 +415,11 @@ static void __exit simplefb_exit(void)
>>>>> platform_driver_unregister(&simplefb_driver);
>>>>> }
>>>>>
>>>>> -module_init(simplefb_init);
>>>>> +/*
>>>>> + * While this can be a module, if builtin it's most likely the console
>>>>> + * So let's leave module_exit but move module_init to an earlier place
>>>>> + */
>>>>
>>>> Not really related to this patch itself, but do we want to support
>>>> simplefb as a module? It seems like it's going to be most of the time
>>>> broken.
>>>
>>> A valid point, my mean reasoning here is that some may see not being able to
>>> use it as a module as a regression, so I just kept things as is, but I do
>>> agree that it is advisable to just build it in.
>>
>> Like a lot of things, if it is made a modules, and it breaks for the
>> user, the user gets to keep the pieces. There are potentially some
>> valid scenarios where it is fine to have it as a module. I don't
>> recommend changing this unless is actually starts causing problems.
>
> I assume that you've tested this and it actually makes a difference,
> correct? If so,
>
> Acked-by: Grant Likely <grant.likely@linaro.org>
Yes I've tested the entire set on 4 boards / 4 SoCs (A10, A10s, A20 and A31),
and yes it makes a difference.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Grant Likely @ 2014-11-13 12:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6uX7V-TcZfM0vP4SvjxKx3T_RGByX8wFqTdFo4LEzP2Yg@mail.gmail.com>
On Thu, Nov 13, 2014 at 11:02 AM, Grant Likely <grant.likely@linaro.org> wrote:
> On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Since simplefb nodes do not relate directly to hw typically they have been
>> placed in the root of the devicetree. As the represent runtime information
>> having them as sub-nodes of /chosen is more logical, specify this.
>>
>> Also specify when to set the chosen stdout-path property to a simplefb node.
>>
>> For reliable handover to a hardware specific driver, that driver needs to
>> know which simplefb to unregister when taking over, specify how the hw driver
>> can find the matching simplefb node.
>>
>> Last add some advice on how to fill and use simplefb nodes from a firmware
>> pov.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> --
>> Changes in v2:
>> -Add stdout-path to the example code
>> ---
>> .../bindings/video/simple-framebuffer.txt | 38 +++++++++++++++++++++-
>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> index 8f35718..8b7ecf6 100644
>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> @@ -4,6 +4,26 @@ A simple frame-buffer describes a frame-buffer setup by firmware or
>> the bootloader, with the assumption that the display hardware has already
>> been set up to scan out from the memory pointed to by the reg property.
>>
>> +Since simplefb nodes represent runtime information they must be sub-nodes of
>> +the chosen node (*). The primary display node must be named framebuffer0,
>> +additional nodes must be called framebuffer1, etc.
>
> Thinking more about our conversation yesterday, the preferred name
> should still be framebuffer@<address>. There is no reason to break
> with established convention, and as mentioned in my reply on patch #2,
> using the name is not the preferred way to identify a device node. So,
> my recommendation is to prefer the name "framebuffer@<addr>", but if
> it is inconvenient because the address isn't known, then
> "framebuffer#" is acceptable. Then use /aliases for actual enumeration
> which has the advantage of also working for framebuffers that aren't
> in /chosen.
Some more thoughts about aliases.
There is a natural tension between enumerate framebuffers or
enumerating displays. There is precedence for displays to be named
'display...' and to use /aliases/display* to enumerate them. What do
we do for framebuffers? Does it make sense to have a separate
/aliases/framebuffer* enumeration, or would it be better to have a
single 'display' namespace so that the same name is used right
through?
Right now I'm leaning towards using /aliases/display* for everything,
and making simplefb understand that /aliases/display# could either
point directly to the framebuffer when there is not node for the
hardware, or point to the display node. This would make it easy to
have consistent names for display. For example, with the kexec
scenario, if /aliases/display0 points at the display node then the
alias won't need to be changed between first boot when firmware sets
up a framebuffer, and kexec boot after the kernel has torn down the
original framebuffer. If /aliases/display0 points to the framebuffer
node directly, then the linkage from /aliases/display0 to the HW
display node will be lost when the framebuffer gets torn down.
Doing it this way does make two assumptions though. It assumes that
each display will have its own node somewhere so that no two
framebuffers will point to the same node. Second, it assumes that we
have a mechanism to handoff a display name (/dev/fb*)? from simplefb
to another device. I don't know enough about the fbdev subsystem to
know whether or not this will be a problem. Or if there are any
namespace conflicts between fbdev and drm.
Implementation would be simple to do. If the simple framebuffer node
has a 'display' property, then it will call of_alias_get_id() on the
target of that phandle. Otherwise it will call of_alias_get_id() on
itself.
Thoughts?
g.
>
>> +
>> +If a simplefb node represents the preferred console for user interaction,
>> +then the chosen node's stdout-path property must point to it.
>> +
>> +If the devicetree contains nodes for the display hardware used by a simplefb,
>> +then one of the hw nodes must have a property called "framebuffer" pointing to
>> +the framebuffer# node in chosen, so that the operating system knows which
>> +simplefb to disable when handing over control to a driver for the real
>> +hardware. The bindings for the hw nodes must specify which node contains the
>> +framebuffer property.
>
> I've also been thinking about this. When we talked yesterday I said I
> didn't have a preference as to which direction the link between the
> framebuffer and the display hardware pointed. Thinking about it now,
> between the two nodes, the HW node is pretty much static, but the FB
> node is dynamic and in most scenarios will be enabled by firmware, and
> then disabled by the operating system. If the framebuffer property is
> in the HW node, then that means the firmware needs to modify two nodes
> to set up the linkage correctly. It needs to enable the framebuffer,
> and then add the link. If one of several possible FB nodes was enabled
> by the firmware, it need to modify the display controller node to
> match. And, when the framebuffer is disabled, the HW node is left with
> a dangling link that kexec won't necessarily know how to fix up.
>
> If instead the link goes the other way, as a property in the
> framebuffer node pointing to display controller, all of the
> functionality still stays the same, but the display controller node
> never needs to change when framebuffers are
> enabled/disabled/added/removed. Since the location and/or compatible
> value of framebuffers is always well known, (aside from the fact that
> the simplefb driver binds to them), we can have a single API that will
> return all the framebuffers associated to a given display controller
> node. The real HW driver will be able to use that to find its
> framebuffers and coordinate handover between them.
>
> g.
>
>> +
>> +It is advised that devicetree files contain pre-filled, disabled framebuffer#
>> +nodes, so that the firmware only needs to update the mode information and
>> +enable them. This way if e.g. later on support for more display clocks get
>> +added, the simplefb nodes will already contain this info and the firmware
>> +does not need to be updated.
>> +
>> Required properties:
>> - compatible: "simple-framebuffer"
>> - reg: Should contain the location and size of the framebuffer memory.
>> @@ -22,11 +42,27 @@ Optional properties:
>>
>> Example:
>>
>> - framebuffer {
>> +chosen {
>> + framebuffer0 {
>> compatible = "simple-framebuffer";
>> reg = <0x1d385000 (1600 * 1200 * 2)>;
>> width = <1600>;
>> height = <1200>;
>> stride = <(1600 * 2)>;
>> format = "r5g6b5";
>> + clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
>> };
>> + stdout-path = &framebuffer0;
>> +};
>> +
>> +soc@01c00000 {
>> + lcdc0: lcdc@1c0c000 {
>> + compatible = "allwinner,sun4i-a10-lcdc";
>> + framebuffer = <&framebuffer0>;
>> + };
>> +};
>> +
>> +
>> +*) Older devicetree files may have a compatible = "simple-framebuffer" node
>> +in a different place, operating systems must first enumerate any framebuffer#
>> +nodes found under chosen and then check for other compatible nodes.
>> --
>> 2.1.0
>>
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Hans de Goede @ 2014-11-13 12:14 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6uX7V-TcZfM0vP4SvjxKx3T_RGByX8wFqTdFo4LEzP2Yg@mail.gmail.com>
Hi,
On 11/13/2014 12:02 PM, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>> Since simplefb nodes do not relate directly to hw typically they have been
>> placed in the root of the devicetree. As the represent runtime information
>> having them as sub-nodes of /chosen is more logical, specify this.
>>
>> Also specify when to set the chosen stdout-path property to a simplefb node.
>>
>> For reliable handover to a hardware specific driver, that driver needs to
>> know which simplefb to unregister when taking over, specify how the hw driver
>> can find the matching simplefb node.
>>
>> Last add some advice on how to fill and use simplefb nodes from a firmware
>> pov.
>>
>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>> --
>> Changes in v2:
>> -Add stdout-path to the example code
>> ---
>> .../bindings/video/simple-framebuffer.txt | 38 +++++++++++++++++++++-
>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> index 8f35718..8b7ecf6 100644
>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>> @@ -4,6 +4,26 @@ A simple frame-buffer describes a frame-buffer setup by firmware or
>> the bootloader, with the assumption that the display hardware has already
>> been set up to scan out from the memory pointed to by the reg property.
>>
>> +Since simplefb nodes represent runtime information they must be sub-nodes of
>> +the chosen node (*). The primary display node must be named framebuffer0,
>> +additional nodes must be called framebuffer1, etc.
>
> Thinking more about our conversation yesterday, the preferred name
> should still be framebuffer@<address>. There is no reason to break
> with established convention, and as mentioned in my reply on patch #2,
> using the name is not the preferred way to identify a device node. So,
> my recommendation is to prefer the name "framebuffer@<addr>", but if
> it is inconvenient because the address isn't known, then
> "framebuffer#" is acceptable.
Since we want to use pre-filled simplefb nodes already present in the devicetree,
and u-boot is the one chosing the framebuffer address we don't know the
address to put in the dts file beforehand, so framebuffer# it is.
> Then use /aliases for actual enumeration
> which has the advantage of also working for framebuffers that aren't
> in /chosen.
I'll discuss the aliases in my reply to your next mail which goes into
more details about this.
>
>> +
>> +If a simplefb node represents the preferred console for user interaction,
>> +then the chosen node's stdout-path property must point to it.
>> +
>> +If the devicetree contains nodes for the display hardware used by a simplefb,
>> +then one of the hw nodes must have a property called "framebuffer" pointing to
>> +the framebuffer# node in chosen, so that the operating system knows which
>> +simplefb to disable when handing over control to a driver for the real
>> +hardware. The bindings for the hw nodes must specify which node contains the
>> +framebuffer property.
>
> I've also been thinking about this. When we talked yesterday I said I
> didn't have a preference as to which direction the link between the
> framebuffer and the display hardware pointed. Thinking about it now,
> between the two nodes, the HW node is pretty much static, but the FB
> node is dynamic and in most scenarios will be enabled by firmware, and
> then disabled by the operating system. If the framebuffer property is
> in the HW node, then that means the firmware needs to modify two nodes
> to set up the linkage correctly. It needs to enable the framebuffer,
> and then add the link. If one of several possible FB nodes was enabled
> by the firmware, it need to modify the display controller node to
> match. And, when the framebuffer is disabled, the HW node is left with
> a dangling link that kexec won't necessarily know how to fix up.
>
> If instead the link goes the other way, as a property in the
> framebuffer node pointing to display controller, all of the
> functionality still stays the same, but the display controller node
> never needs to change when framebuffers are
> enabled/disabled/added/removed. Since the location and/or compatible
> value of framebuffers is always well known, (aside from the fact that
> the simplefb driver binds to them), we can have a single API that will
> return all the framebuffers associated to a given display controller
> node. The real HW driver will be able to use that to find its
> framebuffers and coordinate handover between them.
Ok, I'll change the bindings so that the link goes the other way, note that
he firmware should not be touching the link at all, we've a pre-populated
simplefb node in the devicetree for the firmware to further fill in
(and enable), and that should already contain the link.
Regards,
Hans
>
> g.
>
>> +
>> +It is advised that devicetree files contain pre-filled, disabled framebuffer#
>> +nodes, so that the firmware only needs to update the mode information and
>> +enable them. This way if e.g. later on support for more display clocks get
>> +added, the simplefb nodes will already contain this info and the firmware
>> +does not need to be updated.
>> +
>> Required properties:
>> - compatible: "simple-framebuffer"
>> - reg: Should contain the location and size of the framebuffer memory.
>> @@ -22,11 +42,27 @@ Optional properties:
>>
>> Example:
>>
>> - framebuffer {
>> +chosen {
>> + framebuffer0 {
>> compatible = "simple-framebuffer";
>> reg = <0x1d385000 (1600 * 1200 * 2)>;
>> width = <1600>;
>> height = <1200>;
>> stride = <(1600 * 2)>;
>> format = "r5g6b5";
>> + clocks = <&ahb_gates 36>, <&ahb_gates 43>, <&ahb_gates 44>;
>> };
>> + stdout-path = &framebuffer0;
>> +};
>> +
>> +soc@01c00000 {
>> + lcdc0: lcdc@1c0c000 {
>> + compatible = "allwinner,sun4i-a10-lcdc";
>> + framebuffer = <&framebuffer0>;
>> + };
>> +};
>> +
>> +
>> +*) Older devicetree files may have a compatible = "simple-framebuffer" node
>> +in a different place, operating systems must first enumerate any framebuffer#
>> +nodes found under chosen and then check for other compatible nodes.
>> --
>> 2.1.0
>>
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Hans de Goede @ 2014-11-13 12:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CACxGe6uPKcZ4XqdiPQtia-0D+BnqjDE3eW-oDCR+byUrVtNpSg@mail.gmail.com>
Hi,
On 11/13/2014 01:03 PM, Grant Likely wrote:
> On Thu, Nov 13, 2014 at 11:02 AM, Grant Likely <grant.likely@linaro.org> wrote:
>> On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Since simplefb nodes do not relate directly to hw typically they have been
>>> placed in the root of the devicetree. As the represent runtime information
>>> having them as sub-nodes of /chosen is more logical, specify this.
>>>
>>> Also specify when to set the chosen stdout-path property to a simplefb node.
>>>
>>> For reliable handover to a hardware specific driver, that driver needs to
>>> know which simplefb to unregister when taking over, specify how the hw driver
>>> can find the matching simplefb node.
>>>
>>> Last add some advice on how to fill and use simplefb nodes from a firmware
>>> pov.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> --
>>> Changes in v2:
>>> -Add stdout-path to the example code
>>> ---
>>> .../bindings/video/simple-framebuffer.txt | 38 +++++++++++++++++++++-
>>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> index 8f35718..8b7ecf6 100644
>>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> @@ -4,6 +4,26 @@ A simple frame-buffer describes a frame-buffer setup by firmware or
>>> the bootloader, with the assumption that the display hardware has already
>>> been set up to scan out from the memory pointed to by the reg property.
>>>
>>> +Since simplefb nodes represent runtime information they must be sub-nodes of
>>> +the chosen node (*). The primary display node must be named framebuffer0,
>>> +additional nodes must be called framebuffer1, etc.
>>
>> Thinking more about our conversation yesterday, the preferred name
>> should still be framebuffer@<address>. There is no reason to break
>> with established convention, and as mentioned in my reply on patch #2,
>> using the name is not the preferred way to identify a device node. So,
>> my recommendation is to prefer the name "framebuffer@<addr>", but if
>> it is inconvenient because the address isn't known, then
>> "framebuffer#" is acceptable. Then use /aliases for actual enumeration
>> which has the advantage of also working for framebuffers that aren't
>> in /chosen.
>
> Some more thoughts about aliases.
>
> There is a natural tension between enumerate framebuffers or
> enumerating displays. There is precedence for displays to be named
> 'display...' and to use /aliases/display* to enumerate them. What do
> we do for framebuffers? Does it make sense to have a separate
> /aliases/framebuffer* enumeration, or would it be better to have a
> single 'display' namespace so that the same name is used right
> through?
>
> Right now I'm leaning towards using /aliases/display* for everything,
> and making simplefb understand that /aliases/display# could either
> point directly to the framebuffer when there is not node for the
> hardware, or point to the display node. This would make it easy to
> have consistent names for display. For example, with the kexec
> scenario, if /aliases/display0 points at the display node then the
> alias won't need to be changed between first boot when firmware sets
> up a framebuffer, and kexec boot after the kernel has torn down the
> original framebuffer. If /aliases/display0 points to the framebuffer
> node directly, then the linkage from /aliases/display0 to the HW
> display node will be lost when the framebuffer gets torn down.
>
> Doing it this way does make two assumptions though. It assumes that
> each display will have its own node somewhere so that no two
> framebuffers will point to the same node. Second, it assumes that we
> have a mechanism to handoff a display name (/dev/fb*)? from simplefb
> to another device. I don't know enough about the fbdev subsystem to
> know whether or not this will be a problem. Or if there are any
> namespace conflicts between fbdev and drm.
>
> Implementation would be simple to do. If the simple framebuffer node
> has a 'display' property, then it will call of_alias_get_id() on the
> target of that phandle. Otherwise it will call of_alias_get_id() on
> itself.
>
> Thoughts?
Re-using display# aliases, and specifying that if the simplefb node has
a display property, that then the id of the node the display property
points to should be used, and otherwise the id of the simplefb node itself
sounds like a good idea.
I'll go work on a v3 and put this in (and update the example for this
as well).
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: simplefb: Specify node location and handoff related properties
From: Grant Likely @ 2014-11-13 12:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5464A0A2.1080503@redhat.com>
On Thu, Nov 13, 2014 at 12:14 PM, Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 11/13/2014 12:02 PM, Grant Likely wrote:
>> On Thu, Nov 13, 2014 at 8:50 AM, Hans de Goede <hdegoede@redhat.com> wrote:
>>> Since simplefb nodes do not relate directly to hw typically they have been
>>> placed in the root of the devicetree. As the represent runtime information
>>> having them as sub-nodes of /chosen is more logical, specify this.
>>>
>>> Also specify when to set the chosen stdout-path property to a simplefb node.
>>>
>>> For reliable handover to a hardware specific driver, that driver needs to
>>> know which simplefb to unregister when taking over, specify how the hw driver
>>> can find the matching simplefb node.
>>>
>>> Last add some advice on how to fill and use simplefb nodes from a firmware
>>> pov.
>>>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>
>>> --
>>> Changes in v2:
>>> -Add stdout-path to the example code
>>> ---
>>> .../bindings/video/simple-framebuffer.txt | 38 +++++++++++++++++++++-
>>> 1 file changed, 37 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/video/simple-framebuffer.txt b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> index 8f35718..8b7ecf6 100644
>>> --- a/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> +++ b/Documentation/devicetree/bindings/video/simple-framebuffer.txt
>>> @@ -4,6 +4,26 @@ A simple frame-buffer describes a frame-buffer setup by firmware or
>>> the bootloader, with the assumption that the display hardware has already
>>> been set up to scan out from the memory pointed to by the reg property.
>>>
>>> +Since simplefb nodes represent runtime information they must be sub-nodes of
>>> +the chosen node (*). The primary display node must be named framebuffer0,
>>> +additional nodes must be called framebuffer1, etc.
>>
>> Thinking more about our conversation yesterday, the preferred name
>> should still be framebuffer@<address>. There is no reason to break
>> with established convention, and as mentioned in my reply on patch #2,
>> using the name is not the preferred way to identify a device node. So,
>> my recommendation is to prefer the name "framebuffer@<addr>", but if
>> it is inconvenient because the address isn't known, then
>> "framebuffer#" is acceptable.
>
> Since we want to use pre-filled simplefb nodes already present in the devicetree,
> and u-boot is the one chosing the framebuffer address we don't know the
> address to put in the dts file beforehand, so framebuffer# it is.
fdt_set_name() is available to U-Boot to rename a node. :-)
Prepopulating with /chosen/framebuffer#, and then renaming to
/chosen/framebuffer@<addr> is perfectly reasonable.
g.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox