Anton Vorontsov wrote:
>> + firmware {
>> + id = "Soft-UART";
>> + extended_modes = <0 0>;
>> + virtual_traps = <0 0 0 0 0 0 0 0>;
>
> I believe using underscores for the property name is discouraged.
Ugh, this one change would require me to repost all my patches. Oh
well. I wish someone else had noticed it before I got to version 5.
I'll post a new version on January 7, when I get back to the office.
>> +A Python program that creates firmware binaries from the header files
>> normally
>> +distributed by Freescale can be found on http://opensource.freescale.com.
>
> Hm... I didn't find it there. Could you provide more specific pointer?
I'll add those files only after my patches have been applied. The
layout may change between now and then, and I don't want to have to
repost the firmware.
>> +
>> + /*
>> + * If we haven't checked yet, and a driver hasn't uploaded a firmware
>> + * yet, then check the device tree for information.
>> + */
>> + do {
> ^^^^
> This is very unusual method of error handling. You could stick
> to gotos and lower the indentation level.
Ok, I'll change it. I wasn't crazy about it after I wrote it, anyway.
>> + qe = of_find_node_by_type(NULL, "qe");
>
> Please, add compatible "fsl,qe" matching, so this code could
> work with new device trees.
Ok, but that new design was added after I posted this patch.
>> + /* Find the 'firmware' child node */
>> + while ((fw = of_get_next_child(qe, fw)))
>> + if (strcmp(fw->name, "firmware") == 0)
>> + break;
>
> Hmm. Maybe of_find_node_by_name? Or better by compatible.
What's wrong with looking for the firmware node inside the QE node?
That's the only place it can be?
>> + iprop = of_get_property(fw, "extended_modes", NULL);
>
> Checking for size?
Ok.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
