[U-Boot] [PATCH v5 1/3] fdt_support: Add a fdt_setup_simplefb_node helper function

[Hans de Goede]

Hi,

On 11/20/2014 06:46 PM, Simon Glass wrote:
> Hi Hans,
> 
> On 19 November 2014 13:31, 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>
> 
> Acked-by: Simon Glass <sjg at chromium.org>
> 
> See below if you respin.

We agreed on getting this in via the sunxi custodian tree right ? Then I
can fixup your comments (and re-test things on a sunxi board) before sending a
pull-req.

>> ---
>>  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.
>> + */
> 
> Better to put the comment in the header IMO.

I followed the example of other functions in fdt_support, there currently are
no doxygen comments in fdt_support.h . Still I can move this over if you want.


> 
>> +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]));
> 
> How about fdt_setprop_u32() instead?

Yeah thats much better, will fix.

> 
>> +       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
>> --
>> 2.1.0
> 
> Regards,
> Simon
> 

Regards,

Hans