[U-Boot] [PATCH v4 1/2] fdt_support: Add a fdt_setup_simplefb_node helper function

[Simon Glass]

Hi Hans,

On 18 November 2014 11:18, Hans de Goede <hdegoede at 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 at 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 at 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