diff mbox

[RFC,14/24] xen/video: hdlcd: Convert the driver to the new device tree API

Message ID 1376687156-6737-15-git-send-email-julien.grall@linaro.org
State Superseded, archived
Headers show

Commit Message

Julien Grall Aug. 16, 2013, 9:05 p.m. UTC
Avoid to use FDT API which will be removed soon.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/drivers/video/arm_hdlcd.c |   58 +++++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 26 deletions(-)

Comments

Ian Campbell Aug. 22, 2013, 1:28 p.m. UTC | #1
On Fri, 2013-08-16 at 22:05 +0100, Julien Grall wrote:
> Avoid to use FDT API which will be removed soon.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/drivers/video/arm_hdlcd.c |   58 +++++++++++++++++++++++------------------
>  1 file changed, 32 insertions(+), 26 deletions(-)
> 
> diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
> index 72979ea..0ff8c22 100644
> --- a/xen/drivers/video/arm_hdlcd.c
> +++ b/xen/drivers/video/arm_hdlcd.c
> @@ -25,6 +25,7 @@
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/init.h>
>  #include <xen/mm.h>
> +#include <asm/early_printk.h>
>  #include "font.h"
>  #include "lfb.h"
>  #include "modelines.h"
> @@ -96,62 +97,67 @@ static int __init get_color_masks(const char* bpp, struct color_masks **masks)
>  
>  static void __init set_pixclock(uint32_t pixclock)
>  {
> -    if ( device_tree_node_compatible(device_tree_flattened, 0, "arm,vexpress") )
> +    if ( dt_find_compatible_node(NULL, NULL, "arm,vexpress") )
>              vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC,
>                              V2M_SYS_CFG_OSC5, &pixclock);
>  }
>  
>  void __init video_init(void)
>  {
> -    int node, depth;
> -    u32 address_cells, size_cells;
>      struct lfb_prop lfbp;
>      unsigned char *lfb;
> -    paddr_t hdlcd_start, hdlcd_size;
> +    u64 hdlcd_start, hdlcd_size;

Why? These are physical addresses.

>      if ( !hdlcd_start )
>      {
> -        printk(KERN_ERR "HDLCD address missing from device tree, disabling driver\n");
> +        early_printk("hdlcd: address missing from device tree, disabling driver\n");

I suppose this (and the other instances of this) is for when the console
is on HDLCD? That's a separate fix really I think.

Why tr /A-Z/ /a-z/?

Ian.
Julien Grall Aug. 22, 2013, 2:02 p.m. UTC | #2
On 08/22/2013 02:28 PM, Ian Campbell wrote:
> On Fri, 2013-08-16 at 22:05 +0100, Julien Grall wrote:
>> Avoid to use FDT API which will be removed soon.
>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> ---
>>  xen/drivers/video/arm_hdlcd.c |   58 +++++++++++++++++++++++------------------
>>  1 file changed, 32 insertions(+), 26 deletions(-)
>>
>> diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
>> index 72979ea..0ff8c22 100644
>> --- a/xen/drivers/video/arm_hdlcd.c
>> +++ b/xen/drivers/video/arm_hdlcd.c
>> @@ -25,6 +25,7 @@
>>  #include <xen/libfdt/libfdt.h>
>>  #include <xen/init.h>
>>  #include <xen/mm.h>
>> +#include <asm/early_printk.h>
>>  #include "font.h"
>>  #include "lfb.h"
>>  #include "modelines.h"
>> @@ -96,62 +97,67 @@ static int __init get_color_masks(const char* bpp, struct color_masks **masks)
>>  
>>  static void __init set_pixclock(uint32_t pixclock)
>>  {
>> -    if ( device_tree_node_compatible(device_tree_flattened, 0, "arm,vexpress") )
>> +    if ( dt_find_compatible_node(NULL, NULL, "arm,vexpress") )
>>              vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC,
>>                              V2M_SYS_CFG_OSC5, &pixclock);
>>  }
>>  
>>  void __init video_init(void)
>>  {
>> -    int node, depth;
>> -    u32 address_cells, size_cells;
>>      struct lfb_prop lfbp;
>>      unsigned char *lfb;
>> -    paddr_t hdlcd_start, hdlcd_size;
>> +    u64 hdlcd_start, hdlcd_size;
> 
> Why? These are physical addresses.

dt_get_address take a pointer to u64 value (actually paddr_t == u64).
Perhaps dt_get_addres should take a paddr_t.

> 
>>      if ( !hdlcd_start )
>>      {
>> -        printk(KERN_ERR "HDLCD address missing from device tree, disabling driver\n");
>> +        early_printk("hdlcd: address missing from device tree, disabling driver\n");
> 
> I suppose this (and the other instances of this) is for when the console
> is on HDLCD? That's a separate fix really I think.

The console is not yet set up when the video driver is initialized.

I will divide this patch in 2 parts.

> 
> Why tr /A-Z/ /a-z/?

To keep all printks with the same format. I will stick to HDLCD then.
Ian Campbell Aug. 22, 2013, 2:11 p.m. UTC | #3
On Thu, 2013-08-22 at 15:02 +0100, Julien Grall wrote:
> On 08/22/2013 02:28 PM, Ian Campbell wrote:
> > On Fri, 2013-08-16 at 22:05 +0100, Julien Grall wrote:
> >> Avoid to use FDT API which will be removed soon.
> >>
> >> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> >> ---
> >>  xen/drivers/video/arm_hdlcd.c |   58 +++++++++++++++++++++++------------------
> >>  1 file changed, 32 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
> >> index 72979ea..0ff8c22 100644
> >> --- a/xen/drivers/video/arm_hdlcd.c
> >> +++ b/xen/drivers/video/arm_hdlcd.c
> >> @@ -25,6 +25,7 @@
> >>  #include <xen/libfdt/libfdt.h>
> >>  #include <xen/init.h>
> >>  #include <xen/mm.h>
> >> +#include <asm/early_printk.h>
> >>  #include "font.h"
> >>  #include "lfb.h"
> >>  #include "modelines.h"
> >> @@ -96,62 +97,67 @@ static int __init get_color_masks(const char* bpp, struct color_masks **masks)
> >>  
> >>  static void __init set_pixclock(uint32_t pixclock)
> >>  {
> >> -    if ( device_tree_node_compatible(device_tree_flattened, 0, "arm,vexpress") )
> >> +    if ( dt_find_compatible_node(NULL, NULL, "arm,vexpress") )
> >>              vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC,
> >>                              V2M_SYS_CFG_OSC5, &pixclock);
> >>  }
> >>  
> >>  void __init video_init(void)
> >>  {
> >> -    int node, depth;
> >> -    u32 address_cells, size_cells;
> >>      struct lfb_prop lfbp;
> >>      unsigned char *lfb;
> >> -    paddr_t hdlcd_start, hdlcd_size;
> >> +    u64 hdlcd_start, hdlcd_size;
> > 
> > Why? These are physical addresses.
> 
> dt_get_address take a pointer to u64 value (actually paddr_t == u64).
> Perhaps dt_get_addres should take a paddr_t.

Assuming dt_gt_address always deals in physical addresses then,  yes, I
think so.
> 
> > 
> >>      if ( !hdlcd_start )
> >>      {
> >> -        printk(KERN_ERR "HDLCD address missing from device tree, disabling driver\n");
> >> +        early_printk("hdlcd: address missing from device tree, disabling driver\n");
> > 
> > I suppose this (and the other instances of this) is for when the console
> > is on HDLCD? That's a separate fix really I think.
> 
> The console is not yet set up when the video driver is initialized.
> 
> I will divide this patch in 2 parts.

Thanks.

> > 
> > Why tr /A-Z/ /a-z/?
> 
> To keep all printks with the same format. I will stick to HDLCD then.

I think they were until you introduced a lower case one ;-)

Ian.
diff mbox

Patch

diff --git a/xen/drivers/video/arm_hdlcd.c b/xen/drivers/video/arm_hdlcd.c
index 72979ea..0ff8c22 100644
--- a/xen/drivers/video/arm_hdlcd.c
+++ b/xen/drivers/video/arm_hdlcd.c
@@ -25,6 +25,7 @@ 
 #include <xen/libfdt/libfdt.h>
 #include <xen/init.h>
 #include <xen/mm.h>
+#include <asm/early_printk.h>
 #include "font.h"
 #include "lfb.h"
 #include "modelines.h"
@@ -96,62 +97,67 @@  static int __init get_color_masks(const char* bpp, struct color_masks **masks)
 
 static void __init set_pixclock(uint32_t pixclock)
 {
-    if ( device_tree_node_compatible(device_tree_flattened, 0, "arm,vexpress") )
+    if ( dt_find_compatible_node(NULL, NULL, "arm,vexpress") )
             vexpress_syscfg(1, V2M_SYS_CFG_OSC_FUNC,
                             V2M_SYS_CFG_OSC5, &pixclock);
 }
 
 void __init video_init(void)
 {
-    int node, depth;
-    u32 address_cells, size_cells;
     struct lfb_prop lfbp;
     unsigned char *lfb;
-    paddr_t hdlcd_start, hdlcd_size;
+    u64 hdlcd_start, hdlcd_size;
     paddr_t framebuffer_start, framebuffer_size;
-    const struct fdt_property *prop;
-    const u32 *cell;
     const char *mode_string;
     char _mode_string[16];
     int bytes_per_pixel = 4;
     struct color_masks *c = NULL;
     struct modeline *videomode = NULL;
     int i;
+    const struct dt_device_node *dev;
+    const __be32 *cells;
+    u32 lenp;
+    int res;
 
-    if ( find_compatible_node("arm,hdlcd", &node, &depth,
-                &address_cells, &size_cells) <= 0 )
-        return;
+    dev = dt_find_compatible_node(NULL, NULL, "arm,hdlcd");
 
-    prop = fdt_get_property(device_tree_flattened, node, "reg", NULL);
-    if ( !prop )
+    if ( !dev )
+    {
+        early_printk("hdlcd: Cannot find node compatible with \"arm,hdcld\"\n");
         return;
+    }
 
-    cell = (const u32 *)prop->data;
-    device_tree_get_reg(&cell, address_cells, size_cells,
-            &hdlcd_start, &hdlcd_size);
+    res = dt_device_get_address(dev, 0, &hdlcd_start, &hdlcd_size);
+    if ( !res )
+    {
+        early_printk("hdlcd: Unable to retrieve MMIO base address\n");
+        return;
+    }
 
-    prop = fdt_get_property(device_tree_flattened, node, "framebuffer", NULL);
-    if ( !prop )
+    cells = dt_get_property(dev, "framebuffer", &lenp);
+    if ( !cells )
+    {
+        early_printk("hdlcd: Unable to retrieve framebuffer property\n");
         return;
+    }
 
-    cell = (const u32 *)prop->data;
-    device_tree_get_reg(&cell, address_cells, size_cells,
-            &framebuffer_start, &framebuffer_size);
+    framebuffer_start = dt_next_cell(dt_n_addr_cells(dev), &cells);
+    framebuffer_size = dt_next_cell(dt_n_size_cells(dev), &cells);
 
     if ( !hdlcd_start )
     {
-        printk(KERN_ERR "HDLCD address missing from device tree, disabling driver\n");
+        early_printk("hdlcd: address missing from device tree, disabling driver\n");
         return;
     }
 
     if ( !hdlcd_start || !framebuffer_start )
     {
-        printk(KERN_ERR "HDLCD: framebuffer address missing from device tree, disabling driver\n");
+        printk("hdlcd: framebuffer address missing from device tree, disabling driver\n");
         return;
     }
 
-    mode_string = fdt_getprop(device_tree_flattened, node, "mode", NULL);
-    if ( !mode_string )
+    res = dt_property_read_string(dev, "mode", &mode_string);
+    if ( res )
     {
         get_color_masks("32", &c);
         memcpy(_mode_string, "1280x1024@60", strlen("1280x1024@60") + 1);
@@ -160,14 +166,14 @@  void __init video_init(void)
     else if ( strlen(mode_string) < strlen("800x600@60") ||
             strlen(mode_string) > sizeof(_mode_string) - 1 )
     {
-        printk(KERN_ERR "HDLCD: invalid modeline=%s\n", mode_string);
+        early_printk("hdlcd: invalid modeline=%s\n", mode_string);
         return;
     } else {
         char *s = strchr(mode_string, '-');
         if ( !s )
         {
-            printk(KERN_INFO "HDLCD: bpp not found in modeline %s, assume 32 bpp\n",
-                    mode_string);
+            early_printk("hdcld: bpp not found in modeline %s, assume 32 bpp\n",
+                         mode_string);
             get_color_masks("32", &c);
             memcpy(_mode_string, mode_string, strlen(mode_string) + 1);
             bytes_per_pixel = 4;