simplefb: add clock handling code
Subsystems (3)
v4 cover simplefb: add clock handling code
Hi Tomi,
Here is v4 of the patch-set to add clocks support to simplefb.
Changes in v2:
-Added "simplefb: Add simplefb MAINTAINERS entry" patch
Changes in v3:
-Improved description of simplefb binding
Changes in v4:
-Change clocks in simplefb from a linked-list to a (simpler) dynamically
allocated array
v2 got some objections from some people because clocks are hardware description and as such do not belong in a virtual device node as simplefb is. As explained in the following thread the use of clocks in simplefb has nothing to do with clock topology (so hardware description), but is about which clocks of a number of possible clocks where actually chosen by the firmware to bring of the fb, as such this clearly is state description.
AFAIK at the end of the thread no one was disagreeing with this anymore (I did not get any replies to my last mails explaining this).
Some of us also got together at the Plumbers conference in Dusseldorf to discuss this, present were: David Herrmann, Maxime Ripard, Geert Uytterhoeven and Hans de Goede (me), conclusions:
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
-simplefb/simpledrm -> native-kms driver handoff will keep using the
linear framebuffer address to identify which simplefb platform device to destroy during handoff, so that if their are multiple kms devices involved, the handover happens at the right moment and we don't loose console output anywhere (and we should be able to do a flickerfree handover)
Since we seem to have broad agreement on how to move forward with this, I would like to ask you to please merge this patch-set for 3.19.
I know 3.19 is still somewhat ar away, but I would like to submit the u-boot side of this to upstream ASAP, so can you please let me know if you plan to take this patch-set for 3.19 soon ?
Regards,
Hans
PATCH v4 1/5 simplefb: Add simplefb MAINTAINERS entry
During the discussion about adding clock handling code to simplefb, it became clear that simplefb currently does not have an active maintainer.
I've discussed this with Stephen Warren <[email protected]>, the original author of simplefb, and with his permisson I'm picking up maintainership of simplefb.
--- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+)
MAINTAINERS +8 -0
@@ -8416,6 +8416,14 @@ F: drivers/media/usb/siano/ F: drivers/media/usb/siano/ F: drivers/media/mmc/siano/ +SIMPLEFB FB DRIVER +M: Hans de Goede <[email protected]> +L: linux-fbdev at vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/video/simple-framebuffer.txt +F: drivers/video/fbdev/simplefb.c +F: include/linux/platform_data/simplefb.h + SH_VEU V4L2 MEM2MEM DRIVER L: linux-media at vger.kernel.org S: Orphan
Signature · 1 line
2.1.0
PATCH v4 1/5 simplefb: Add simplefb MAINTAINERS entry
Quoted 7 lines · Hans de Goede
During the discussion about adding clock handling code to simplefb, it became clear that simplefb currently does not have an active maintainer.
I've discussed this with Stephen Warren <[email protected]>, the original author of simplefb, and with his permisson I'm picking up maintainership of simplefb.
Quoted 6 lines
Cc Stephen Warren <[email protected]>Signed-off-by Hans de Goede <[email protected]>Acked-by Stephen Warren <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Reviewed-by David Herrmann <[email protected]>
Quoted 26 lines
--- MAINTAINERS | 8 ++++++++ 1 file changed, 8 insertions(+)MAINTAINERS +8
-0@@ -8416,6 +8416,14 @@ F: drivers/media/usb/siano/ F: drivers/media/usb/siano/ F: drivers/media/mmc/siano/ +SIMPLEFB FB DRIVER +M: Hans de Goede <[email protected]> +L: linux-fbdev at vger.kernel.org +S: Maintained +F: Documentation/devicetree/bindings/video/simple-framebuffer.txt +F: drivers/video/fbdev/simplefb.c +F: include/linux/platform_data/simplefb.h + SH_VEU V4L2 MEM2MEM DRIVER L: linux-media at vger.kernel.org S: OrphanSignature · 1 line
2.1.0
PATCH v4 2/5 dt-bindings: Add a clocks property to the simple-framebuffer binding
In reply to Hans de GoedeA simple-framebuffer node represents a framebuffer setup by the firmware / bootloader. Such a framebuffer may have a number of clocks in use, add a property to communicate this to the OS.
--
Changes in v2:
-Added Reviewed-by: Mike Turquette <[email protected]> Changes in v3:
-Updated description to make clear simplefb deals with more then just memory
--- Documentation/devicetree/bindings/video/simple-framebuffer.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/video/simple-framebuffer.txt +6 -3
@@ -1,8 +1,8 @@ Simple Framebuffer -A simple frame-buffer describes a raw memory region that may be rendered to, -with the assumption that the display hardware has already been set up to scan -out from that buffer. +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. Required properties: - compatible: "simple-framebuffer"
@@ -14,6 +14,9 @@ Required properties: - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). +Optional properties: +- clocks : List of clocks used by the framebuffer + Example: framebuffer {
Signature · 1 line
2.1.0
PATCH v4 2/5 dt-bindings: Add a clocks property to the simple-framebuffer binding
Quoted 35 lines · Hans de Goede
A simple-framebuffer node represents a framebuffer setup by the firmware / bootloader. Such a framebuffer may have a number of clocks in use, add a property to communicate this to the OS.
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>--
Changes in v2:
-Added Reviewed-by: Mike Turquette <[email protected]> Changes in v3:(quoted diff) +0
-2-Updated description to make clear simplefb deals with more then just memory --- Documentation/devicetree/bindings/video/simple-framebuffer.txt | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/video/simple-framebuffer.txt +3
-3@@ -1,8 +1,8 @@ Simple Framebuffer -A simple frame-buffer describes a raw memory region that may be rendered to, -with the assumption that the display hardware has already been set up to scan -out from that buffer. +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. Required properties: - compatible: "simple-framebuffer"
Documentation/devicetree/bindings/video/simple-framebuffer.txt Required properties: · line 18
Quoted 5 lines
Documentation/devicetree/bindings/video/simple-framebuffer.txt +2
-0@@ -14,6 +14,9 @@ Required properties: - r5g6b5 (16-bit pixels, d[15:11]=r, d[10:5]=g, d[4:0]=b). - a8b8g8r8 (32-bit pixels, d[31:24]=a, d[23:16]=b, d[15:8]=g, d[7:0]=r). +Optional properties: +- clocks : List of clocks used by the framebuffer
Please change to the following:
- clocks : List of clocks used by the framebuffer. Clocks listed here are expected to already be configured correctly. The OS must ensure these clocks are not modified or disabled while the simple framebuffer remains active.
g.
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v4 2/5 dt-bindings: Add a clocks property to the simple-framebuffer binding
[Replying on-list and with original ccs]
Quoted 11 lines · Hans de Goede
Hi,
Quoted 4 lines · Grant Likely
- clocks : List of clocks used by the framebuffer. Clocks listed here are expected to already be configured correctly. The OS must ensure these clocks are not modified or disabled while the simple framebuffer remains active.
Updated in my local tree, where I've also added proper commit messages to the 2 patches missing them. Shall I send a v5 of the set, and may I add your Acked-by to this patch when sending v5?
For this whole series, please add my Acked-by.
I'm okay with the bindings, and I'm okay with adding the clock dependency to a simplefb node. I'm happy with the conversation we had today on IRC, and I'm expecting to see a binding that covers expected firmware/OS behaviour and association with the real hardware posted to the list shortly.
g.
PATCH v4 2/5 dt-bindings: Add a clocks property to the simple-framebuffer binding
Hi,
Quoted 11 lines · Grant Likely
[Replying on-list and with original ccs]
Quoted 8 lines · Hans de Goede
Hi,
Quoted 4 lines · Grant Likely
- clocks : List of clocks used by the framebuffer. Clocks listed here are expected to already be configured correctly. The OS must ensure these clocks are not modified or disabled while the simple framebuffer remains active.
Quoted 7 lines
Quoted 3 lines
Updated in my local tree, where I've also added proper commit messages to the 2 patches missing them. Shall I send a v5 of the set, and may I add your Acked-by to this patch when sending v5?
For this whole series, please add my Acked-by.
Acked-by Grant Likely <[email protected]>
Thanks, given the above update to the bindings text, and also some updated commit messages, I'll post a v5 with all these added soon.
Regards,
Hans
A message that is not in the archive (sent off-list or not yet received): CACxGe6v15EjpALit4ZJLXDgCkwEYM4t1mryjoQpa3=wpKmXWEg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Re: [PATCH v4 2/5] dt-bindings: Add a clocks property to the simple-framebuffer binding
Rob, seeing as we disagree on patch #5, do you have time to discuss this evening, or tomorrow?
g.
Quoted 28 lines · Grant Likely
[Replying on-list and with original ccs]
Quoted 11 lines · Hans de Goede
Hi,
Quoted 4 lines · Grant Likely
- clocks : List of clocks used by the framebuffer. Clocks listed here are expected to already be configured correctly. The OS must ensure these clocks are not modified or disabled while the simple framebuffer remains active.
Updated in my local tree, where I've also added proper commit messages to the 2 patches missing them. Shall I send a v5 of the set, and may I add your Acked-by to this patch when sending v5?
For this whole series, please add my Acked-by.
Acked-by Grant Likely <[email protected]>I'm okay with the bindings, and I'm okay with adding the clock dependency to a simplefb node. I'm happy with the conversation we had today on IRC, and I'm expecting to see a binding that covers expected
firmware/OSbehaviour and association with the real hardware posted to the list shortly.g.
Signature · 3 lines
You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit https://groups.google.com/d/optout.
PATCH v4 2/5 dt-bindings: Add a clocks property to the simple-framebuffer binding
Quoted 2 lines · Grant Likely
Rob, seeing as we disagree on patch #5, do you have time to discuss this evening, or tomorrow?
Yes, either is fine.
Rob
Quoted 32 lines
g.
Quoted 28 lines · Grant Likely
[Replying on-list and with original ccs]
Quoted 11 lines · Hans de Goede
Hi,
Quoted 4 lines · Grant Likely
- clocks : List of clocks used by the framebuffer. Clocks listed here are expected to already be configured correctly. The OS must ensure these clocks are not modified or disabled while the simple framebuffer remains active.
Updated in my local tree, where I've also added proper commit messages to the 2 patches missing them. Shall I send a v5 of the set, and may I add your Acked-by to this patch when sending v5?
For this whole series, please add my Acked-by.
Acked-by Grant Likely <[email protected]>I'm okay with the bindings, and I'm okay with adding the clock dependency to a simplefb node. I'm happy with the conversation we had today on IRC, and I'm expecting to see a binding that covers expected
firmware/OSbehaviour and association with the real hardware posted to the list shortly.g.
PATCH v4 3/5 simplefb: formalize pseudo palette handling
In reply to Hans de GoedeFrom: Luc Verhaegen <[email protected]>
--- drivers/video/fbdev/simplefb.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
drivers/video/fbdev/simplefb.c +12 -3
@@ -41,6 +41,8 @@ static struct fb_var_screeninfo simplefb_var = { .vmode = FB_VMODE_NONINTERLACED, }; +#define PSEUDO_PALETTE_SIZE 16 +static int simplefb_setcolreg (u_int regno,u_int red,u_int green,u_int blue,u_int transp,struct fb_info *info) {
@@ -50,7 +52,7 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,u32 cb = blue >> (16 - info->var.blue.length);u32 value; -if (regno >=16 ) +if (regno >=PSEUDO_PALETTE_SIZE )return -EINVAL ; value = (cr << info->var.red.offset) |
@@ -163,11 +165,16 @@ static int simplefb_parse_pd(struct platform_device *pdev,return 0 ; } +struct simplefb_par { +u32 palette[PSEUDO_PALETTE_SIZE ]; +}; +static int simplefb_probe (struct platform_device *pdev) {int ret;struct simplefb_params params;struct fb_info *info; +struct simplefb_par *par;struct resource *mem;if (fb_get_options ("simplefb" ,NULL ))
@@ -188,11 +195,13 @@ static int simplefb_probe(struct platform_device *pdev)return -EINVAL ; } - info =framebuffer_alloc (sizeof (u32) *16 , &pdev->dev); + info =framebuffer_alloc (sizeof (struct simplefb_par ), &pdev->dev);if (!info)return -ENOMEM ;platform_set_drvdata (pdev, info); + par = info->par; + info->fix = simplefb_fix; info->fix.smem_start = mem->start; info->fix.smem_len =resource_size (mem);
@@ -225,7 +234,7 @@ static int simplefb_probe(struct platform_device *pdev)framebuffer_release (info);return -ENODEV ; } - info->pseudo_palette = (void *)(info +1 ); + info->pseudo_palette = par->palette; dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n", info->fix.smem_start, info->fix.smem_len,
Signature · 1 line
2.1.0
PATCH v4 3/5 simplefb: formalize pseudo palette handling
Quoted 2 lines · Hans de Goede
From: Luc Verhaegen <[email protected]>
Patch description? Write a proper changelog before merging please. Describe why the change is necessary and what was done. You need to give future readers some information about this patch. Heck, you're listed in MAINTAINERS, you should be *enforcing* proper changelogs.
The code looks fine though.
Quoted 78 lines
Signed-off-by Luc Verhaegen <[email protected]>Acked-by Stephen Warren <[email protected]>[hdegoede at redhat.com: drop unnecessary void * cast]Reviewed-by Hans de Goede <[email protected]>Signed-off-by Hans de Goede <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Reviewed-by David Herrmann <[email protected]>--- drivers/video/fbdev/simplefb.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)drivers/video/fbdev/simplefb.c +12
-3@@ -41,6 +41,8 @@ static struct fb_var_screeninfo simplefb_var = { .vmode = FB_VMODE_NONINTERLACED, }; +#define PSEUDO_PALETTE_SIZE 16 +static int simplefb_setcolreg (u_int regno,u_int red,u_int green,u_int blue,u_int transp,struct fb_info *info) {@@ -50,7 +52,7 @@ static int simplefb_setcolreg(u_int regno, u_int red, u_int green, u_int blue,u32 cb = blue >> (16 - info->var.blue.length);u32 value; -if (regno >=16 ) +if (regno >=PSEUDO_PALETTE_SIZE )return -EINVAL ; value = (cr << info->var.red.offset) |@@ -163,11 +165,16 @@ static int simplefb_parse_pd(struct platform_device *pdev,return 0 ; } +struct simplefb_par { +u32 palette[PSEUDO_PALETTE_SIZE ]; +}; +static int simplefb_probe (struct platform_device *pdev) {int ret;struct simplefb_params params;struct fb_info *info; +struct simplefb_par *par;struct resource *mem;if (fb_get_options ("simplefb" ,NULL ))@@ -188,11 +195,13 @@ static int simplefb_probe(struct platform_device *pdev)return -EINVAL ; } - info =framebuffer_alloc (sizeof (u32) *16 , &pdev->dev); + info =framebuffer_alloc (sizeof (struct simplefb_par ), &pdev->dev);if (!info)return -ENOMEM ;platform_set_drvdata (pdev, info); + par = info->par; + info->fix = simplefb_fix; info->fix.smem_start = mem->start; info->fix.smem_len =resource_size (mem);@@ -225,7 +234,7 @@ static int simplefb_probe(struct platform_device *pdev)framebuffer_release (info);return -ENODEV ; } - info->pseudo_palette = (void *)(info +1 ); + info->pseudo_palette = par->palette; dev_info(&pdev->dev, "framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n", info->fix.smem_start, info->fix.smem_len,Signature · 1 line
2.1.0
PATCH v4 4/5 simplefb: add goto error path to probe
In reply to Hans de GoedeFrom: Luc Verhaegen <[email protected]>
While at it update ioremap_wc error return from ENODEV to ENOMEM.
--- drivers/video/fbdev/simplefb.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
drivers/video/fbdev/simplefb.c +11 -7
@@ -220,8 +220,8 @@ static int simplefb_probe(struct platform_device *pdev) info->apertures =alloc_apertures (1 );if (!info->apertures) { -framebuffer_release (info); -return -ENOMEM ; + ret = -ENOMEM ; + goto error_fb_release; } info->apertures->ranges[0 ].base = info->fix.smem_start; info->apertures->ranges[0 ].size = info->fix.smem_len;
@@ -231,8 +231,8 @@ static int simplefb_probe(struct platform_device *pdev) info->screen_base =ioremap_wc (info->fix.smem_start, info->fix.smem_len);if (!info->screen_base) { -framebuffer_release (info); -return -ENODEV ; + ret = -ENOMEM ; + goto error_fb_release; } info->pseudo_palette = par->palette;
@@ -247,14 +247,18 @@ static int simplefb_probe(struct platform_device *pdev) ret =register_framebuffer (info);if (ret <0 ) {dev_err (&pdev->dev,"Unable to register simplefb: %d\n" , ret); -iounmap (info->screen_base); -framebuffer_release (info); -return ret; + goto error_unmap; }dev_info (&pdev->dev,"fb%d: simplefb registered!\n" , info->node);return 0 ; + +error_unmap: +iounmap (info->screen_base); +error_fb_release: +framebuffer_release (info); +return ret; }static int simplefb_remove (struct platform_device *pdev)
Signature · 1 line
2.1.0
PATCH v4 5/5 simplefb: add clock handling code
In reply to Hans de GoedeFrom: Luc Verhaegen <[email protected]>
This claims and enables clocks listed in the simple framebuffer dt node. This is needed so that the display engine, in case the required clocks are known by the kernel code and are described in the dt, will remain properly enabled.
--
Changes in v4:
-change clks from linkedlist to dynamic allocated array -propagate EPROBE_DEFER up from simplefb_clocks_init to simplefb_probe
--- drivers/video/fbdev/simplefb.c | 101 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 1 deletion(-)
drivers/video/fbdev/simplefb.c +100 -1
@@ -26,6 +26,7 @@#include <linux/module.h> #include <linux/platform_data/simplefb.h> #include <linux/platform_device.h> +#include <linux/clk-provider.h> static struct fb_fix_screeninfo simplefb_fix = { .id ="simple" ,
@@ -167,8 +168,98 @@ static int simplefb_parse_pd(struct platform_device *pdev,struct simplefb_par {u32 palette[PSEUDO_PALETTE_SIZE ]; +int clk_count; +struct clk **clks; }; +/* +* Clock handling code. +* +* Here we handle the clocks property of our "simple-framebuffer" dt node. +* This is necessary so that we can make sure that any clocks needed by +* the display engine that the bootloader set up for us (and for which it +* provided a simplefb dt node), stay up, for the life of the simplefb +* driver. +* +* When the driver unloads, we cleanly disable, and then release the clocks. +* +* We only complain about errors here, no action is taken as the most likely +* error can only happen due to a mismatch between the bootloader which set +* up simplefb, and the clock definitions in the device tree. Chances are +* that there are no adverse effects, and if there are, a clean teardown of +* the fb probe will not help us much either. So just complain and carry on, +* and hope that the user actually gets a working fb at the end of things. +*/ +static int +simplefb_clocks_init (struct simplefb_par *par,struct platform_device *pdev) +{ +struct device_node *np = pdev->dev.of_node; +struct clk *clock; +int i, ret; + +if (dev_get_platdata (&pdev->dev) || !np) +return 0 ; + + par->clk_count =of_clk_get_parent_count (np); +if (par->clk_count <=0 ) +return 0 ; + + par->clks =kcalloc (par->clk_count,sizeof (struct clk *),GFP_KERNEL ); +if (!par->clks) +return -ENOMEM ; + +for (i =0 ; i < par->clk_count; i++) { + clock =of_clk_get (np, i); +if (IS_ERR (clock)) { +if (PTR_ERR (clock) == -EPROBE_DEFER ) { +while (--i >=0 ) { +if (par->clks[i]) +clk_put (par->clks[i]); + } +kfree (par->clks); +return -EPROBE_DEFER ; + } +dev_err (&pdev->dev,"%s: clock %d not found: %ld\n" , + __func__, i,PTR_ERR (clock)); +continue ; + } + par->clks[i] = clock; + } + +for (i =0 ; i < par->clk_count; i++) { +if (par->clks[i]) { + ret =clk_prepare_enable (par->clks[i]); +if (ret) { +dev_err (&pdev->dev, +"%s: failed to enable clock %d: %d\n" , + __func__, i, ret); +clk_put (par->clks[i]); + par->clks[i] =NULL ; + } + } + } + +return 0 ; +} + +static void +simplefb_clocks_destroy (struct simplefb_par *par) +{ +int i; + +if (!par->clks) +return ; + +for (i =0 ; i < par->clk_count; i++) { +if (par->clks[i]) { +clk_disable_unprepare (par->clks[i]); +clk_put (par->clks[i]); + } + } + +kfree (par->clks); +} +static int simplefb_probe (struct platform_device *pdev) {int ret;
@@ -236,6 +327,10 @@ static int simplefb_probe(struct platform_device *pdev) } info->pseudo_palette = par->palette; + ret =simplefb_clocks_init (par, pdev); +if (ret <0 ) + goto error_unmap; +dev_info (&pdev->dev,"framebuffer at 0x%lx, 0x%x bytes, mapped to 0x%p\n" , info->fix.smem_start, info->fix.smem_len, info->screen_base);
@@ -247,13 +342,15 @@ static int simplefb_probe(struct platform_device *pdev) ret =register_framebuffer (info);if (ret <0 ) {dev_err (&pdev->dev,"Unable to register simplefb: %d\n" , ret); - goto error_unmap; + goto error_clocks; }dev_info (&pdev->dev,"fb%d: simplefb registered!\n" , info->node);return 0 ; +error_clocks: +simplefb_clocks_destroy (par); error_unmap:iounmap (info->screen_base); error_fb_release:
@@ -264,8 +361,10 @@ error_fb_release:static int simplefb_remove (struct platform_device *pdev) {struct fb_info *info =platform_get_drvdata (pdev); +struct simplefb_par *par = info->par;unregister_framebuffer (info); +simplefb_clocks_destroy (par);framebuffer_release (info);return 0 ;
Signature · 1 line
2.1.0
v4 cover simplefb: add clock handling code
In reply to Hans de GoedeQuoted 6 lines · Hans de Goede
Since we seem to have broad agreement on how to move forward with this, I would like to ask you to please merge this patch-set for 3.19.
I know 3.19 is still somewhat ar away, but I would like to submit the u-boot side of this to upstream ASAP, so can you please let me know if you plan to take this patch-set for 3.19 soon ?
This series looks fine to me, except patch 3 is missing a description, and patch 4 makes the desc a continuation of the subject. Each patch should have a description (independent from subject).
No need to resend, I can cook up something for those, or I can edit the descs if you provide the text.
So if there are no strong objections, I'll queue this for 3.19.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141028/41aa0d04/attachment-0001.sig>
v4 cover simplefb: add clock handling code
Hi,
Quoted 10 lines · Tomi Valkeinen
Quoted 6 lines · Hans de Goede
Since we seem to have broad agreement on how to move forward with this, I would like to ask you to please merge this patch-set for 3.19.
I know 3.19 is still somewhat ar away, but I would like to submit the u-boot side of this to upstream ASAP, so can you please let me know if you plan to take this patch-set for 3.19 soon ?
This series looks fine to me, except patch 3 is missing a description,
Description for patch 3:
"Add a proper struct describing simplefb private data, with the palette in there, instead of directly storing the palette in the fb_info->par pointer."
Quoted 1 line
and patch 4 makes the desc a continuation of the subject.
Description for patch 4:
"Use the usual kernel style of goto error_foo to free resources on probe error. This is a preparation patch for adding clocks support.
While at it update ioremap_wc error return from ENODEV to ENOMEM."
Quoted 1 line
Each patch should have a description (independent from subject).
Ok, I'll remember that for future patches.
Quoted 2 lines
No need to resend, I can cook up something for those, or I can edit the descs if you provide the text.
Descriptions provided above, thanks!
Quoted 1 line
So if there are no strong objections, I'll queue this for 3.19.
Regards,
Hans
v4 cover simplefb: add clock handling code
In reply to Tomi ValkeinenQuoted 9 lines · Tomi Valkeinen
Quoted 6 lines · Hans de Goede
Since we seem to have broad agreement on how to move forward with this, I would like to ask you to please merge this patch-set for 3.19.
I know 3.19 is still somewhat ar away, but I would like to submit the u-boot side of this to upstream ASAP, so can you please let me know if you plan to take this patch-set for 3.19 soon ?
Quoted 8 lines
This series looks fine to me, except patch 3 is missing a description, and patch 4 makes the desc a continuation of the subject. Each patch should have a description (independent from subject).
No need to resend, I can cook up something for those, or I can edit the descs if you provide the text.
So if there are no strong objections, I'll queue this for 3.19.
Yes, I object to the binding still as it has not changed from what was previously posted.
Rob
Hans de Goede
Reviewed-by Mike Turquette Acked-by Geert Uytterhoeven Reviewed-by Maxime Ripard on the v4 seriesHi,
Quoted 14 lines · Rob Herring
Quoted 13 lines · Tomi Valkeinen
Quoted 6 lines · Hans de Goede
Since we seem to have broad agreement on how to move forward with this, I would like to ask you to please merge this patch-set for 3.19.
I know 3.19 is still somewhat ar away, but I would like to submit the u-boot side of this to upstream ASAP, so can you please let me know if you plan to take this patch-set for 3.19 soon ?
This series looks fine to me, except patch 3 is missing a description, and patch 4 makes the desc a continuation of the subject. Each patch should have a description (independent from subject).
Quoted 7 lines
Quoted 4 lines
No need to resend, I can cook up something for those, or I can edit the descs if you provide the text.
So if there are no strong objections, I'll queue this for 3.19.
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."
I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
Regards,
Hans
Hi Hans, Rob,
Quoted 32 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>
Quoted 7 lines
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141029/41c130bf/attachment.sig>
Quoted 45 lines · Tomi Valkeinen
Hi Hans, Rob,
Quoted 32 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Quoted 7 lines
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
Quoted 7 lines
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed. I'm not saying this can't be in DT, only that DT is not the only solution here. This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
As regulators were also mentioned, they already have a "regulator-boot-on" property defined. Perhaps this is suitable to be mirrored for clocks. If it is not, then I'm wondering why we have it. A key difference here is that the property is part of the provider and can be dealt with in the clock driver rather than requiring a temporary driver.
Rob
Hi,
Quoted 54 lines · Rob Herring
Quoted 45 lines · Tomi Valkeinen
Hi Hans, Rob,
Quoted 32 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Quoted 7 lines
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
Quoted 7 lines
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Quoted 4 lines
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed.
This is not just about being unmanaged, clocks can be shared, and another driver can actively claim the clock and turn it off, we need to protect against this too.
Quoted 2 lines
I'm not saying this can't be in DT, only that DT is not the only solution here.
Basically the problem is that we must keep the clock enabled for the lifetime of the simplefb device. We've been over this in 2 very long threads already, and many alternatives have been evaluated and found wanting.
The only 100% safe way to ensure clocks are not turned off is to tie there lifetime to that of the simplefb device, and the only reliable way to do that is through a clocks property.
It really is that simple, and I don't understand why people keep insisting there must be another way, while they fail to offer a (reliable, working) other way.
Quoted 6 lines
This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
Quoted 6 lines
As regulators were also mentioned, they already have a "regulator-boot-on" property defined. Perhaps this is suitable to be mirrored for clocks. If it is not, then I'm wondering why we have it. A key difference here is that the property is part of the provider and can be dealt with in the clock driver rather than requiring a temporary driver.
"regulator-boot-on" tells the regulator framework to enable a regulator at boot, but it does not protect it from getting turned of by another driver later (it does not increase use_count).
So this is the same none solution as not turning off unmanaged clocks, what if another driver who shares the clock, enables it during probe, then disables it while done probing (to put things in low power state until userspace uses the device).
This is not about not turning the clock off, this is about keeping the clock on for the lifetime of the simplefb device, which is a different problem, cause we must not remove a single calling point of clock_off, put we must block all attempts to turn the clock off, which is done by increasing its use-count, which is done by claiming + enabling it, which needs a phandle to the clock ...
Regards,
Hans
A message that is not in the archive (sent off-list or not yet received): CAL_JsqKWqq+H5ymUW3n8tkmLJHEUGaKDLX5+orJmGUQ3ECQwUQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 68 lines · Rob Herring
Quoted 12 lines · Tomi Valkeinen
Hi Hans, Rob,
Quoted 9 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simplyinaccurate."
Quoted 2 lines
Quoted 2 lines
I then explained that this is not hardware description, but runtime
state
Quoted 1 line
Quoted 1 line
information, as it tells the kernel which clocks were chosen to drive
the
Quoted 6 lines
Quoted 6 lines
display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then
pop
Quoted 1 line
Quoted 1 line
up again when a new version is posted to say "I object" yet another
time,
Quoted 1 line
Quoted 1 line
you've had your chance to make your arguments last time, and chose to
stay
Quoted 1 line
Quoted 1 line
quiet after I explained in detail that this is not hardware description
but
Quoted 3 lines
Quoted 3 lines
state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested
people
Quoted 1 line
Quoted 1 line
present, and the conclusion was that this really is the best way to
handle this,
Quoted 8 lines
Quoted 8 lines
so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>And David Herrman who is working on simpledrm, which will be merged
soon, which
Quoted 1 line
Quoted 1 line
will also use the simplefb bindings also agrees. So we have the
simplefb maintainer,
Quoted 1 line
Quoted 1 line
simpledrm maintainer, and the clk subsystem maintainer + 2 other
maintainers all
Quoted 1 line
Quoted 1 line
agreeing on a way forward, the time for bikeshedding now really really
really is
Quoted 6 lines
Quoted 3 lines
over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
Quoted 7 lines
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Quoted 11 lines
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed. I'm not saying this can't be in DT, only that DT is not the only solution here. This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
I am in agreement this point of view. This is a problem in the Linux kernel. It is not a generic problem.
The Linux problem is that during boot device drivers are loaded in two phases - built-in and loadable modules. The clock-regulator clean up is happening too early. It happens after the built-in drivers load and before the loadable modules have a chance to load. That's simply the wrong place for that clean up happen. A simple alternative would be to make a tiny loadable module that triggers the clean up. Then adjust your boot scripts to load this module after the other ones have loaded. But instead of fixing this simple timing flaw in the Linux boot process a complex mechanism is being created to work around it.
Simplefb is also being developed as a way of protecting the BIOS setup of the framebuffer past the boot process and out into use as a normal user space console. I in no way support this use. We have experienced decades of problems on the x86 with VGA and BIOSes that I do not wish to repeat in the ARM world. Write a correct, device specific framebuffer driver for this piece of hardware. That device specific driver will load before the clock/regulator cleanup and claim all of the correct resources.
Quoted 17 lines
As regulators were also mentioned, they already have a "regulator-boot-on" property defined. Perhaps this is suitable to be mirrored for clocks. If it is not, then I'm wondering why we have it. A key difference here is that the property is part of the provider and can be dealt with in the clock driver rather than requiring a temporary driver.
Rob
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit https://groups.google.com/d/optout.
Signature · 7 lines
Jon Smirl [email protected] -- You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit https://groups.google.com/d/optout.
Quoted 55 lines · Rob Herring
Quoted 45 lines · Tomi Valkeinen
Hi Hans, Rob,
Quoted 32 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Quoted 7 lines
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
Quoted 7 lines
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Quoted 10 lines
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed. I'm not saying this can't be in DT, only that DT is not the only solution here. This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
I am in agreement this point of view. This is a problem in the Linux kernel. It is not a generic problem.
The Linux problem is that during boot device drivers are loaded in two phases - built-in and loadable modules. The clock-regulator clean up is happening too early. It happens after the built-in drivers load and before the loadable modules have a chance to load. That's simply the wrong place for that clean up happen. A simple alternative would be to make a tiny loadable module that triggers the clean up. Then adjust your boot scripts to load this module after the other ones have loaded. But instead of fixing this simple timing flaw in the Linux boot process a complex mechanism is being created to work around it.
Simplefb is also being developed as a way of protecting the BIOS setup of the framebuffer past the boot process and out into use as a normal user space console. I in no way support this use. We have experienced decades of problems on the x86 with VGA and BIOSes that I do not wish to repeat in the ARM world. Write a correct, device specific framebuffer driver for this piece of hardware. That device specific driver will load before the clock/regulator cleanup and claim all of the correct resources.
Quoted 15 lines
As regulators were also mentioned, they already have a "regulator-boot-on" property defined. Perhaps this is suitable to be mirrored for clocks. If it is not, then I'm wondering why we have it. A key difference here is that the property is part of the provider and can be dealt with in the clock driver rather than requiring a temporary driver.
Rob
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe at googlegroups.com. For more options, visit https://groups.google.com/d/optout.
Signature · 2 lines
Jon Smirl jonsmirl at gmail.com
A message that is not in the archive (sent off-list or not yet received): CAKON4Oyw46zjq8QJNAFi9qWA9-6MpXtGavzVmXQT3SPqH=NpjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org
Quoted 2 lines · On Nov 2, 2014 7:28 AM, "[email protected]"…
On Fri, Oct 31, 2014 at 4:47 PM, Rob Herring <[email protected]>
Quoted 2 lines · wrote:
Quoted 2 lines
On Wed, Oct 29, 2014 at 7:08 PM, Tomi Valkeinen <[email protected]>
Quoted 8 lines · wrote:
Quoted 8 lines
Quoted 8 lines
Hi Hans, Rob,
Quoted 5 lines · Hans de Goede
Hi,
Quoted 1 line · Rob Herring
Yes, I object to the binding still as it has not changed from what
was
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
previously posted.
It would be helpful if you could explain why you object. Last time
you
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
said: " You are mixing in a hardware description that is simply
inaccurate."
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
I then explained that this is not hardware description, but runtime
state
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
information, as it tells the kernel which clocks were chosen to
drive the
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
display (out of typically a list of possible options, depending on
which
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
output is used, etc.). Just like which memory address the bootloader
has
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is
too
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
late. You cannot simply go quiet halfway through a discussion and
then pop
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
up again when a new version is posted to say "I object" yet another
time,
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
you've had your chance to make your arguments last time, and chose
to stay
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
quiet after I explained in detail that this is not hardware
description but
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various
interested people
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
present, and the conclusion was that this really is the best way to
handle this,
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>And David Herrman who is working on simpledrm, which will be merged
soon, which
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
will also use the simplefb bindings also agrees. So we have the
simplefb maintainer,
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
simpledrm maintainer, and the clk subsystem maintainer + 2 other
maintainers all
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
agreeing on a way forward, the time for bikeshedding now really
really really is
Quoted 10 lines
Quoted 10 lines
Quoted 10 lines
Quoted 3 lines
over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get
your
Quoted 29 lines
Quoted 4 lines
Quoted 3 lines
ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Quoted 10 lines
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed. I'm not saying this can't be in DT, only that DT is not the only solution here. This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
I am in agreement this point of view. This is a problem in the Linux kernel. It is not a generic problem.
The Linux problem is that during boot device drivers are loaded in two phases - built-in and loadable modules. The clock-regulator clean up is happening too early. It happens after the built-in drivers load and before the loadable modules have a chance to load. That's simply the wrong place for that clean up happen. A simple alternative would be to make a tiny loadable module that triggers the clean up. Then adjust your boot scripts to load this module after the other ones have loaded. But instead of fixing this simple timing flaw in the Linux boot process a complex mechanism is being created to work around it.
Quoted 17 lines
Simplefb is also being developed as a way of protecting the BIOS setup of the framebuffer past the boot process and out into use as a normal user space console. I in no way support this use. We have experienced decades of problems on the x86 with VGA and BIOSes that I do not wish to repeat in the ARM world. Write a correct, device specific framebuffer driver for this piece of hardware. That device specific driver will load before the clock/regulator cleanup and claim all of the correct resources.
Quoted 8 lines
As regulators were also mentioned, they already have a "regulator-boot-on" property defined. Perhaps this is suitable to be mirrored for clocks. If it is not, then I'm wondering why we have it. A key difference here is that the property is part of the provider and can be dealt with in the clock driver rather than requiring a temporary driver.
What if the hardware specific 'real' driver was a new driver that is basically a wrapper of the simplefb but adding the clock. This seems to be the only sunxi specific addition.
This would stand in until the 'real' driver is created. As far as I can tell nothing about this setup would be wrong, so even if the real driver never came in it wouldn't be a problem.
Is this a workable alternate solution?
Quoted 5 lines
Quoted 5 lines
Rob
--
You received this message because you are subscribed to the Google
Groups "linux-sunxi" group.
Quoted 1 line
Quoted 1 line
To unsubscribe from this group and stop receiving emails from it, send
an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
Quoted 11 lines
Quoted 1 line
For more options, visit https://groups.google.com/d/optout.
--
Jon Smirl
[email protected]--
You received this message because you are subscribed to the Google Groups
"linux-sunxi" group.
Quoted 1 line
To unsubscribe from this group and stop receiving emails from it, send an
email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected]
Quoted 1 line
For more options, visit https://groups.google.com/d/optout.
Signature · 3 lines
You received this message because you are subscribed to the Google Groups "linux-sunxi" group. To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/[email protected] For more options, visit https://groups.google.com/d/optout.
v4 cover simplefb: add clock handling code
In reply to jonsmirl at gmail.comQuoted 6 lines · jonsmirl at gmail.com
Simplefb is also being developed as a way of protecting the BIOS setup of the framebuffer past the boot process and out into use as a normal user space console. I in no way support this use. We have experienced decades of problems on the x86 with VGA and BIOSes that I do not wish to repeat in the ARM world.
Oh wow.
Luc Verhaegen, aka, the guy who made BIOS-free display drivers possible.
Quoted 54 lines · Rob Herring
Quoted 45 lines · Tomi Valkeinen
Hi Hans, Rob,
Quoted 32 lines · Hans de Goede
Hi,
Quoted 2 lines · Rob Herring
Yes, I object to the binding still as it has not changed from what was previously posted.
It would be helpful if you could explain why you object. Last time you
said: " You are mixing in a hardware description that is simply inaccurate."I then explained that this is not hardware description, but runtime state information, as it tells the kernel which clocks were chosen to drive the display (out of typically a list of possible options, depending on which output is used, etc.). Just like which memory address the bootloader has chosen to scan out the video image from.
Then you got quiet, so sorry, but this time your objection really is too late. You cannot simply go quiet halfway through a discussion and then pop up again when a new version is posted to say "I object" yet another time, you've had your chance to make your arguments last time, and chose to stay quiet after I explained in detail that this is not hardware description but state information, so now it is simply too late.
These bindings have been discussed at Plumbers with various interested people present, and the conclusion was that this really is the best way to handle this, so this patch is:
Signed-off-by Hans de Goede <[email protected]>Reviewed-by Mike Turquette <[email protected]>Reviewed-by Maxime Ripard <[email protected]>Quoted 7 lines
And David Herrman who is working on simpledrm, which will be merged soon, which will also use the simplefb bindings also agrees. So we have the simplefb maintainer, simpledrm maintainer, and the clk subsystem maintainer + 2 other maintainers all agreeing on a way forward, the time for bikeshedding now really really really is over.
Tomi, can you please let us know how you plan to proceed with this ?
I won't merge DT bindings via fbdev tree, if a DT maintainer says no.
Quoted 7 lines
I took Rob's silence to the earlier series as a silent ack for your explanation. Obviously that was not the case.
Rob, please advice asap what should be done to the bindings to get your ack. As Hans explained above, this discussion has been going on for a long time, and afaik this series is the best way forward of all the options discussed.
Quoted 10 lines
I still think for the most part this is a kernel problem. It is a kernel policy to turn off unused clocks. The clock framework could just as easily decide that any clocks enabled at boot and left un-managed (i.e. w/o a driver) are kept on until they are managed. I'm not saying this can't be in DT, only that DT is not the only solution here. This problem is not unique to simplefb. A serial console could stop working if no serial driver is loaded before unused clocks are disabled. CPU core clocks have a similar issue as well (often enabled in platform code). I want to see this solved in a generic way for any clock.
I've spent most of today rereading the entire thread and thinking about this issue. Given that the clock binding is a well established core binding at this point, it is entirely reasonable for a generic driver to have part of the binding specify the clock resources that it depends on to keep working. The binding needs to be very clear that it is a mere dependency, and that the OS must not make any attempt to modify the clock settings, but it is reasonable indicate to the OS that the clocks are used by the firmware framebuffer, and turning them off will cause the framebuffer will break. The best place to provide that guidance is directly in the node.
If we were talking about a resource that didn't have a well established binding, ie, the video path configuration, then I'd still say no way because there is no way to do it generically. Claiming clocks on the other hand is straight forward and simple.
Aside from wanting the documentation to be more explicit about what the driver does with the clocks, I don't have a problem with doing it.
However, I am concerned about handover. I've lost track over the entire thread on whether the handover mechanism has been resolved, and I would really like to have a proposed solution to this documented in the binding. The fact that there is nothing tying the simple framebuffer to the actual hardware backing the framebuffer is concerning. It means the kernel needs to guess which graphics device is associated with the framebuffer.
The best solution in my opinion is to embed the simplefb properties directly into the graphics device node (Yes, I know some have argued that the graphics subsystem is composed of multiple devices; That doesn't matter. Just choose one of the device nodes; the device that actually DMAs the video buffer is probably the correct choice). That makes it crystal clear which graphics controller owns the framebuffer. There is precedence in DT bindings for putting current state into the tree. offb.c is trying to solve the exact same problem. It also means that we can use the driver model to provide mutual exclusion between probing simplefb and a full-featured driver.
That approach would require some modifications of the binding to make sure properties don't conflict with the device specific properties. The easiest way is to allow the properties to live in a sub node so that the reg property doesn't conflict.
However, I'm not stuck on this approach. The solution must be unambiguious, and the handover mechanism must be thought through, but it doesn't /have/ to be what I suggested above if a proposal that can be well argued as better.
Hans, let's talk about this tomorrow. #devicetree on freenode? It looks like I'm committed to volunteer at my daughters school tomorrow morning, but the afternoon looks good.
g.
Hi Grant,
Quoted 7 lines · Grant Likely
However, I am concerned about handover. I've lost track over the entire thread on whether the handover mechanism has been resolved, and I would really like to have a proposed solution to this documented in the binding. The fact that there is nothing tying the simple framebuffer to the actual hardware backing the framebuffer is concerning. It means the kernel needs to guess which graphics device is associated with the framebuffer.
We did discuss handover in D?sseldorf, and concluded that the simplefb's regs property can be used for this.
While on a modern system with unified memory this association cannot be derived in a generic way, a device-specific driver for the graphics hardware can if the regs property of the simplefb node matches the address the CRTC engine is configured for.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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
v4 cover simplefb: add clock handling code
Quoted 12 lines · Geert Uytterhoeven
Hi Grant,
Quoted 7 lines · Grant Likely
However, I am concerned about handover. I've lost track over the entire thread on whether the handover mechanism has been resolved, and I would really like to have a proposed solution to this documented in the binding. The fact that there is nothing tying the simple framebuffer to the actual hardware backing the framebuffer is concerning. It means the kernel needs to guess which graphics device is associated with the framebuffer.
Quoted 7 lines
We did discuss handover in D?sseldorf, and concluded that the simplefb's regs property can be used for this.
While on a modern system with unified memory this association cannot be derived in a generic way, a device-specific driver for the graphics hardware can if the regs property of the simplefb node matches the address the CRTC engine is configured for.
???
Right, I'm going to be blunt here: That's just dumb. All the capability needed is there in the DT to associate a simple FB to a display controller, and the solution chosen is to use a heuristic?
The association needs to be explicit. I strongly prefer putting the simple FB directly into the display controller node, but I would consider phandle linkage also.
g.
Quoted 11 lines
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.orgIn 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
v4 cover simplefb: add clock handling code
Hi Grant,
Quoted 21 lines · Grant Likely
Quoted 9 lines · Geert Uytterhoeven
Quoted 7 lines · Grant Likely
However, I am concerned about handover. I've lost track over the entire thread on whether the handover mechanism has been resolved, and I would really like to have a proposed solution to this documented in the binding. The fact that there is nothing tying the simple framebuffer to the actual hardware backing the framebuffer is concerning. It means the kernel needs to guess which graphics device is associated with the framebuffer.
Quoted 7 lines
We did discuss handover in D?sseldorf, and concluded that the simplefb's regs property can be used for this.
While on a modern system with unified memory this association cannot be derived in a generic way, a device-specific driver for the graphics hardware can if the regs property of the simplefb node matches the address the CRTC engine is configured for.
???
Quoted 7 lines
Right, I'm going to be blunt here: That's just dumb. All the capability needed is there in the DT to associate a simple FB to a display controller, and the solution chosen is to use a heuristic?
The association needs to be explicit. I strongly prefer putting the simple FB directly into the display controller node, but I would consider phandle linkage also.
IFF there's a display controller node, you can put it there.
I actually proposed to have a minimal/preliminary display controller node, but people countered that for various reasons (too many components with multiple nodes on many systems, bindings not yet defined, etc.).
And if there's no graphics driver/bindings yet at the time the bootloader is written, it doesn't know how to link simplefb with it in DT. Hence the heuristic to match regs... Does that make sense?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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
v4 cover simplefb: add clock handling code
Quoted 32 lines · Geert Uytterhoeven
Hi Grant,
Quoted 21 lines · Grant Likely
Quoted 9 lines · Geert Uytterhoeven
Quoted 7 lines · Grant Likely
However, I am concerned about handover. I've lost track over the entire thread on whether the handover mechanism has been resolved, and I would really like to have a proposed solution to this documented in the binding. The fact that there is nothing tying the simple framebuffer to the actual hardware backing the framebuffer is concerning. It means the kernel needs to guess which graphics device is associated with the framebuffer.
Quoted 7 lines
We did discuss handover in D?sseldorf, and concluded that the simplefb's regs property can be used for this.
While on a modern system with unified memory this association cannot be derived in a generic way, a device-specific driver for the graphics hardware can if the regs property of the simplefb node matches the address the CRTC engine is configured for.
???
Quoted 7 lines
Right, I'm going to be blunt here: That's just dumb. All the capability needed is there in the DT to associate a simple FB to a display controller, and the solution chosen is to use a heuristic?
The association needs to be explicit. I strongly prefer putting the simple FB directly into the display controller node, but I would consider phandle linkage also.
Quoted 8 lines
IFF there's a display controller node, you can put it there.
I actually proposed to have a minimal/preliminary display controller node, but people countered that for various reasons (too many components with multiple nodes on many systems, bindings not yet defined, etc.).And if there's no graphics driver/bindings yet at the time the bootloader is written, it doesn't know how to link simplefb with it in DT. Hence the heuristic to match regs... Does that make sense?
No, not really. The simplefb binding /should/ make it clear that handover to the real display controller driver is an expected use case; therefore the simplefb binding should itemize exactly how that linkage is supposed to be expressed. When the display controller binding is written, it should go hand in glove with the simplefb binding. We want to be sure that generic code can be used to resolve a display controller with (one of) the simple framebuffers.
It also sounds like some are making the assumption that firmware should generate the simplefb node from scratch, hence all the hand wringing about firmware not knowing what extra stuff to put into the simplefb node. I don't think this is necessary, or even a good approach. A disabled simplefb node can be put into the dts file with any dependencies it needs to care about and linkage to the real display controller. Firmware should be able to locate that node, update the parameters as needed, and enable it. That gets out of the cycle of updating firmware in lockstep.
g.
v4 cover simplefb: add clock handling code
In reply to Hans de GoedeHi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
Perhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141031/d8c71f16/attachment.sig>
Hi Tomi,
Quoted 9 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
Quoted 6 lines
Perhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Yes this has been discussed before, AFAIK gpio / pinctrls should not be spontaneously reset by the kernels. Regulators will be a problem, but there we can pretty much follow whatever solution we end up with for clocks, since they work very much alike (e.g. we could add a supply property to the simplefb node).
Note I've no intention to add regulator support atm, we can do that when the need arises, so that we can actually test the code.
Regards,
Hans
Hello Hans,
Quoted 15 lines · Hans de Goede
Hi Tomi,
Quoted 12 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early consolePerhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
Managing clocks is definitely not enough. For example, adding some regulators to the Snow Chromebook DTS broke simplefb [0]. This has been mentioned on at least one of the many simplefb threads.
Quoted 10 lines
Quoted 3 lines
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Yes this has been discussed before, AFAIK gpio / pinctrls should not be spontaneously reset by the kernels. Regulators will be a problem, but there we can pretty much follow whatever solution we end up with for clocks, since they work very much alike (e.g. we could add a supply property to the simplefb node).
I think someone already said this but with regulators is not that easy since you don't have an equivalent of the "clocks" property.
You need a "<name>-supply" property for each regulator that is an input supply. Some boards may need different regulators for their display (e.g: one to power the LCD and another to power the backlight) so adding a single supply property will not be enough.
Quoted 3 lines
Note I've no intention to add regulator support atm, we can do that when the need arises, so that we can actually test the code.
AFAIU that's exactly what Thierry was complaining about. There seems to be only a focus on solving the clocks issue since that is needed for sunxi but ignoring how the problem for other resources should be solved.
The assumptions made by simplefb are quite fragile and only adding support for managing clocks is just a partial solution that will give a false sense of robustness since it could break later due to other resources.
Best regards,
Javier
Hi,
Quoted 15 lines · Javier Martinez Canillas
Hello Hans,
Quoted 12 lines · Hans de Goede
Hi Tomi,
Quoted 9 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
Quoted 7 lines
Quoted 3 lines
Quoted 3 lines
Perhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
Managing clocks is definitely not enough. For example, adding some regulators to the Snow Chromebook DTS broke simplefb [0]. This has been mentioned on at least one of the many simplefb threads.
Ok.
Quoted 11 lines
Quoted 10 lines
Quoted 3 lines
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Yes this has been discussed before, AFAIK gpio / pinctrls should not be spontaneously reset by the kernels. Regulators will be a problem, but there we can pretty much follow whatever solution we end up with for clocks, since they work very much alike (e.g. we could add a supply property to the simplefb node).
Quoted 7 lines
I think someone already said this but with regulators is not that easy since you don't have an equivalent of the "clocks" property.
You need a "<name>-supply" property for each regulator that is an input supply. Some boards may need different regulators for their display (e.g: one to power the LCD and another to power the backlight) so adding a single supply property will not be enough.
Right so we can simply insert simplefbvdd1-supply, simplefbvdd2-supply, etc. regulator nodes into the simplefb node, and have the simplefb kernel code look for supplies with a name of simplefb_vdd%d in a loop until it fails to find one.
Note that the foo-supply names are local to the simplefb node, these do not need to be the same as the ones in the nodes describing the actual hardware blocks. Not having an array is somewhat inconvenient, but easily worked around and once worked around this really is just the same problem as with clocks.
Quoted 8 lines
Quoted 3 lines
Note I've no intention to add regulator support atm, we can do that when the need arises, so that we can actually test the code.
AFAIU that's exactly what Thierry was complaining about. There seems to be only a focus on solving the clocks issue since that is needed for sunxi but ignoring how the problem for other resources should be solved.
That is simply not true, I'm fully aware that we may eventually need other resources, e.g. regulators. I just want to take this one step at a time, I really believe that once we have this figured out for one resource, it becomes a lot easier to also figure out for other resources.
Quoted 4 lines
The assumptions made by simplefb are quite fragile and only adding support for managing clocks is just a partial solution that will give a false sense of robustness since it could break later due to other resources.
The key word here being resources. We need to figure out how to do resource management for simplefb, once we have that figured out for one resource, the others can follow the same "design-pattern". As the long thread about the clocks support has shown, getting consensus on this with only one resource in play already is hard enough, so baby steps is the way to go.
Regards,
Hans
Hello Hans,
Thanks for your answers.
Quoted 19 lines · Hans de Goede
Quoted 11 lines
Quoted 10 lines
Quoted 3 lines
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Yes this has been discussed before, AFAIK gpio / pinctrls should not be spontaneously reset by the kernels. Regulators will be a problem, but there we can pretty much follow whatever solution we end up with for clocks, since they work very much alike (e.g. we could add a supply property to the simplefb node).
Quoted 7 lines
I think someone already said this but with regulators is not that easy since you don't have an equivalent of the "clocks" property.
You need a "<name>-supply" property for each regulator that is an input supply. Some boards may need different regulators for their display (e.g: one to power the LCD and another to power the backlight) so adding a single supply property will not be enough.
Quoted 9 lines
Right so we can simply insert simplefbvdd1-supply, simplefbvdd2-supply, etc. regulator nodes into the simplefb node, and have the simplefb kernel code look for supplies with a name of simplefb_vdd%d in a loop until it fails to find one.
Note that the foo-supply names are local to the simplefb node, these do not need to be the same as the ones in the nodes describing the actual hardware blocks. Not having an array is somewhat inconvenient, but easily worked around and once worked around this really is just the same problem as with clocks.
Yes, I guess the question is how many regulators the driver should support but I expect most displays don't need more than a couple.
Quoted 9 lines
Quoted 8 lines
Quoted 3 lines
Note I've no intention to add regulator support atm, we can do that when the need arises, so that we can actually test the code.
AFAIU that's exactly what Thierry was complaining about. There seems to be only a focus on solving the clocks issue since that is needed for sunxi but ignoring how the problem for other resources should be solved.
Quoted 5 lines
That is simply not true, I'm fully aware that we may eventually need other resources, e.g. regulators. I just want to take this one step at a time, I really believe that once we have this figured out for one resource, it becomes a lot easier to also figure out for other resources.
Great, I'm glad to hear that you are willing to add support for regulators (and other resources).
Quoted 5 lines
Quoted 4 lines
The assumptions made by simplefb are quite fragile and only adding support for managing clocks is just a partial solution that will give a false sense of robustness since it could break later due to other resources.
Quoted 7 lines
The key word here being resources. We need to figure out how to do resource management for simplefb, once we have that figured out for one resource, the others can follow the same "design-pattern". As the long thread about the clocks support has shown, getting consensus on this with only one resource in play already is hard enough, so baby steps is the way to go.
I'm certainly not against adding clock management if that will make simplefb more robust. I was just worried that other resources could be ignored. But I'm ok of doing baby steps if there is a long term plan.
As I mentioned in another thread, I believe Doug's suggestion is the best approach. That is to have the simplefb node with all its resources in the DTS but with status = "disabled". The bootloader should only fill the information about the framebuffer memory region (location and size, width, height, format, etc) and change the simplefb node status to "okay".
If a FDT does not have a simplefb node then the bootloader can create one (like is made for the /choosen node in most bootloaders).
That will better document which resources are really used when someone reads the DTS and also avoid duplicating code in the bootloaders to fill all the information about resources but is just my humble opinion and I really don't want to bikesheed more on this.
Best regards,
Javier
v4 cover simplefb: add clock handling code
In reply to Tomi ValkeinenQuoted 9 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
Quoted 6 lines
Perhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Hans, ping. Have these been already discussed?
Tomi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141106/2975ed95/attachment.sig>
Hi,
Quoted 10 lines · Tomi Valkeinen
Quoted 9 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early console
Quoted 8 lines
Quoted 6 lines
Perhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
Hans, ping. Have these been already discussed?
Yes, and I already responded to your original mail on this on Oct. 31th, for some reason my mail client drops you from the CC, when I do a reply-to-all to one of your mails (I've added you manually this time).
My replies did go to simple-fb at vger.kernel.org .I'll forward my 2 mails on this to you.
Regards,
Hans
Quoted 19 lines · Hans de Goede
Hi,
Quoted 16 lines · Tomi Valkeinen
Quoted 15 lines · Tomi Valkeinen
Hi Hans,
Quoted 4 lines · Hans de Goede
-We will add a clocks property to the simplefb devicetree bindings, so
that u-boot setup framebuffers passed to the kernel (for early console support) can properly list the clocks used, and simplefb can claim them to avoid them getting turned off, thereby breaking the early consolePerhaps this has been discussed earlier, but I started to wonder if managing clocks is enough.
What about regulators (to power the LCD), gpios (to keep the LCD enabled) and pinctrl (for video signals)? I guess any those could be reset by the kernel if no driver uses them.
I'm okay with extending the binding for each of these dependencies /providing/ it can be argued that the way simplefb uses it is merely to inhibit the kernel from making any changes to the resource, and then releasing the resource when it is done. Each resource type will need to be taken on a case-by-case basis since each has it's own behaviour. I'll want to see that the simplefb driver doesn't need to do anything special per device, and that the resource binding is well established and generic.
g.