Hi,

On 11/20/2014 10:22 AM, Ian Campbell wrote:
> On Wed, 2014-11-19 at 14:32 +0100, Hans de Goede wrote:
>> From: Luc Verhaegen <[email protected]>
>>
>> Add simplefb support, note this depends on the kernel having support for
>> the clocks property which has recently been added to the simplefb devicetree
>> binding.
>>
>> Signed-off-by: Luc Verhaegen <[email protected]>
>> [[email protected]: Use pre-populated simplefb node under /chosen as
>>  disussed on the devicetree list]
>> Signed-off-by: Hans de Goede <[email protected]>
> 
> Acked-by: Ian Campbell <[email protected]>.

Thanks, any chance you could also review v2 of the CONFIG_USB_KEYBOARD patch ?

> One non-blocking queries:
> 
>> +    /* Find a framebuffer node, with pipeline == "de_be0-lcd0-hdmi" */
>> +    offset = fdt_node_offset_by_compatible(blob, -1,
>> +                                           "allwinner,simple-framebuffer");
>> +    while (offset >= 0) {
>> +            ret = fdt_find_string(blob, offset, "allwinner,pipeline",
>> +                                  "de_be0-lcd0-hdmi");
>> +            if (ret == 0)
>> +                    break;
>> +            offset = fdt_node_offset_by_compatible(blob, offset,
>> +                                           "allwinner,simple-framebuffer");
>> +    }
> 
> Is this variant non-conformant with coding style?:
> 
>       int offset = -1;
>       while ( (offset = fdt_node_offset_by_compatible(blob, offset,
>                                                         
> "allwinner,simple-framebuffer") ) {
>               LOOP BODY
>       }
> 
> I expect it is because of the assignment within the while condition,
> which is a shame, since this is one case where it is IMHO leads to
> clearer code.

AFAIK this indeed would go against the coding style, and TBH I'm also not
sure if I agree it would be cleaner (mainly because indentation would become
(even more) messy due to the 80 columns limit).

Regards,

Hans
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to