| Message ID | 1416239334-29802-2-git-send-email-hdegoede@redhat.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Campbell |
| Headers | show |
Hi Hans, On 17 November 2014 15:48, Hans de Goede <hdegoede@redhat.com> wrote: > Add a generic helper to fill and enable simplefb nodes. > > The first user of this will be the sunxi display code. > > lcd_dt_simplefb_configure_node is also a good candidate to be converted > to use this, but that requires someone to run some tests first, as > lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, > but simply assumes 1 and 1 for both. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ > include/fdt_support.h | 3 +++ > 2 files changed, 68 insertions(+) > > diff --git a/common/fdt_support.c b/common/fdt_support.c > index 3f64156..0ffa711 100644 > --- a/common/fdt_support.c > +++ b/common/fdt_support.c > @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr, > > return 0; > } > + > +/** > + * fdt_setup_simplefb_node - Fill and enable a simplefb node > + * > + * @fdt: ptr to device tree > + * @node: offset of the simplefb node > + * @base_address: framebuffer base address > + * @width: width in pixels > + * @height: height in pixels > + * @stride: bytes per line > + * @format: pixel format string > + * > + * Convenience function to fill and enable a simplefb node. > + */ > +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, > + u32 height, u32 stride, const char *format) Please see lcd_dt_simplefb_configure_node() which seems similar. Regards, Simon
Hi Simon, On 11/17/2014 07:32 PM, Simon Glass wrote: > Hi Hans, > > On 17 November 2014 15:48, Hans de Goede <hdegoede@redhat.com> wrote: >> Add a generic helper to fill and enable simplefb nodes. >> >> The first user of this will be the sunxi display code. >> >> lcd_dt_simplefb_configure_node is also a good candidate to be converted >> to use this, but that requires someone to run some tests first, as >> lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, >> but simply assumes 1 and 1 for both. >> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/fdt_support.h | 3 +++ >> 2 files changed, 68 insertions(+) >> >> diff --git a/common/fdt_support.c b/common/fdt_support.c >> index 3f64156..0ffa711 100644 >> --- a/common/fdt_support.c >> +++ b/common/fdt_support.c >> @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr, >> >> return 0; >> } >> + >> +/** >> + * fdt_setup_simplefb_node - Fill and enable a simplefb node >> + * >> + * @fdt: ptr to device tree >> + * @node: offset of the simplefb node >> + * @base_address: framebuffer base address >> + * @width: width in pixels >> + * @height: height in pixels >> + * @stride: bytes per line >> + * @format: pixel format string >> + * >> + * Convenience function to fill and enable a simplefb node. >> + */ >> +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, >> + u32 height, u32 stride, const char *format) > > Please see lcd_dt_simplefb_configure_node() which seems similar. As mentioned in the commit message already :) lcd_dt_simplefb_configure_node() should be made to use this new helper function eventually. There are several reasons why lcd_dt_simplefb_configure_node() is not usable as a generic helper, and this new function is necessary: 1) It is lcd specific, only available if CONFIG_LCD is set 2) It does not take width / height / stride parameters, instead it reads global variables from lcd.c which are only set if the other common/lcd.c functions are used. 3) It hardcodes the format 4) It does not properly handle #address-cells and #size-cells 4) Is the main reason why I've not included a patch in my patch-set to move lcd_dt_simplefb_configure_node() over to use this new helper, as I'm afraid that may break things. of_bus_default_count_cells() will return different values then the 1/1 the current lcd code assumes when no #address-cells or #size-cells are present in the parent. So my plan is to add this new helper, use it for sunxi, and ask someone who has a board which is using lcd.c + simplefb to test moving lcd.c over to the new helper. Regards, Hans
Hi Hans, On 18 November 2014 11:18, Hans de Goede <hdegoede@redhat.com> wrote: > > Hi Simon, > > On 11/17/2014 07:32 PM, Simon Glass wrote: > > Hi Hans, > > > > On 17 November 2014 15:48, Hans de Goede <hdegoede@redhat.com> wrote: > >> Add a generic helper to fill and enable simplefb nodes. > >> > >> The first user of this will be the sunxi display code. > >> > >> lcd_dt_simplefb_configure_node is also a good candidate to be converted > >> to use this, but that requires someone to run some tests first, as > >> lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, > >> but simply assumes 1 and 1 for both. > >> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > >> --- > >> common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ > >> include/fdt_support.h | 3 +++ > >> 2 files changed, 68 insertions(+) > >> > >> diff --git a/common/fdt_support.c b/common/fdt_support.c > >> index 3f64156..0ffa711 100644 > >> --- a/common/fdt_support.c > >> +++ b/common/fdt_support.c > >> @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr, > >> > >> return 0; > >> } > >> + > >> +/** > >> + * fdt_setup_simplefb_node - Fill and enable a simplefb node > >> + * > >> + * @fdt: ptr to device tree > >> + * @node: offset of the simplefb node > >> + * @base_address: framebuffer base address > >> + * @width: width in pixels > >> + * @height: height in pixels > >> + * @stride: bytes per line > >> + * @format: pixel format string > >> + * > >> + * Convenience function to fill and enable a simplefb node. > >> + */ > >> +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, > >> + u32 height, u32 stride, const char *format) > > > > Please see lcd_dt_simplefb_configure_node() which seems similar. > > As mentioned in the commit message already :) lcd_dt_simplefb_configure_node() > should be made to use this new helper function eventually. OK I think we have established that I can't read :-) > > There are several reasons why lcd_dt_simplefb_configure_node() is not usable > as a generic helper, and this new function is necessary: > > 1) It is lcd specific, only available if CONFIG_LCD is set > 2) It does not take width / height / stride parameters, instead it reads global > variables from lcd.c which are only set if the other common/lcd.c functions are > used. > 3) It hardcodes the format > 4) It does not properly handle #address-cells and #size-cells > > 4) Is the main reason why I've not included a patch in my patch-set to move > lcd_dt_simplefb_configure_node() over to use this new helper, as I'm afraid > that may break things. of_bus_default_count_cells() will return different > values then the 1/1 the current lcd code assumes when no #address-cells or > #size-cells are present in the parent. > > So my plan is to add this new helper, use it for sunxi, and ask someone who > has a board which is using lcd.c + simplefb to test moving lcd.c over to > the new helper. I think it's only used by Raspberry Pi. If you send the patch I can test it next week. But we really shouldn't have two such similar functions. Regards, Simon
diff --git a/common/fdt_support.c b/common/fdt_support.c index 3f64156..0ffa711 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -1523,3 +1523,68 @@ int fdt_read_range(void *fdt, int node, int n, uint64_t *child_addr, return 0; } + +/** + * fdt_setup_simplefb_node - Fill and enable a simplefb node + * + * @fdt: ptr to device tree + * @node: offset of the simplefb node + * @base_address: framebuffer base address + * @width: width in pixels + * @height: height in pixels + * @stride: bytes per line + * @format: pixel format string + * + * Convenience function to fill and enable a simplefb node. + */ +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, + u32 height, u32 stride, const char *format) +{ + char name[32]; + fdt32_t cells[4]; + int i, addrc, sizec, ret; + + of_bus_default_count_cells(fdt, fdt_parent_offset(fdt, node), + &addrc, &sizec); + i = 0; + if (addrc == 2) + cells[i++] = cpu_to_fdt32(base_address >> 32); + cells[i++] = cpu_to_fdt32(base_address); + if (sizec == 2) + cells[i++] = 0; + cells[i++] = cpu_to_fdt32(height * stride); + + ret = fdt_setprop(fdt, node, "reg", cells, sizeof(cells[0]) * i); + if (ret < 0) + return ret; + + snprintf(name, sizeof(name), "framebuffer@%llx", base_address); + ret = fdt_set_name(fdt, node, name); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(width); + ret = fdt_setprop(fdt, node, "width", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(height); + ret = fdt_setprop(fdt, node, "height", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + cells[0] = cpu_to_fdt32(stride); + ret = fdt_setprop(fdt, node, "stride", cells, sizeof(cells[0])); + if (ret < 0) + return ret; + + ret = fdt_setprop_string(fdt, node, "format", format); + if (ret < 0) + return ret; + + ret = fdt_setprop_string(fdt, node, "status", "okay"); + if (ret < 0) + return ret; + + return 0; +} diff --git a/include/fdt_support.h b/include/fdt_support.h index 55cef94..d5e09e6 100644 --- a/include/fdt_support.h +++ b/include/fdt_support.h @@ -147,6 +147,9 @@ void of_bus_default_count_cells(void *blob, int parentoffset, int ft_verify_fdt(void *fdt); int arch_fixup_memory_node(void *blob); +int fdt_setup_simplefb_node(void *fdt, int node, u64 base_address, u32 width, + u32 height, u32 stride, const char *format); + #endif /* ifdef CONFIG_OF_LIBFDT */ #ifdef USE_HOSTCC
Add a generic helper to fill and enable simplefb nodes. The first user of this will be the sunxi display code. lcd_dt_simplefb_configure_node is also a good candidate to be converted to use this, but that requires someone to run some tests first, as lcd_dt_simplefb_configure_node does not honor #address-cells and #size-cells, but simply assumes 1 and 1 for both. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- common/fdt_support.c | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++ include/fdt_support.h | 3 +++ 2 files changed, 68 insertions(+)