| Message ID | 1376687156-6737-15-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
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.
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.
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 --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;
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(-)