[U-Boot] [PATCH v5 3/3] sunxi: video: Add simplefb support

[Hans de Goede]

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 <libv at skynet.be>
>>
>> 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 <libv at skynet.be>
>> [hdegoede at redhat.com: Use pre-populated simplefb node under /chosen as
>>  disussed on the devicetree list]
>> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> 
> Acked-by: Ian Campbell <ijc at hellion.org.uk>.

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