On Wed, Nov 12, 2014 at 11:39 PM, Julian Calaby <julian.calaby@xxxxxxxxx> wrote:
> Hi Hans,
>
> On Thu, Nov 13, 2014 at 9:08 AM, Hans de Goede <hdegoede@xxxxxxxxxx> wrote:
>> Update simplefb to support the new preferred location for simplefb dt nodes
>> under /chosen.
>>
>> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
>> ---
>> drivers/video/fbdev/simplefb.c | 33 ++++++++++++++++++++++++++++++++-
>> 1 file changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/video/fbdev/simplefb.c b/drivers/video/fbdev/simplefb.c
>> index cd96edd..be7d288 100644
>> --- a/drivers/video/fbdev/simplefb.c
>> +++ b/drivers/video/fbdev/simplefb.c
>> @@ -27,6 +27,7 @@
>> #include <linux/platform_data/simplefb.h>
>> #include <linux/platform_device.h>
>> #include <linux/clk-provider.h>
>> +#include <linux/of_platform.h>
>>
>> static struct fb_fix_screeninfo simplefb_fix = {
>> .id = "simple",
>> @@ -385,7 +386,37 @@ static struct platform_driver simplefb_driver = {
>> .probe = simplefb_probe,
>> .remove = simplefb_remove,
>> };
>> -module_platform_driver(simplefb_driver);
>> +
>> +static int __init simplefb_init(void)
>> +{
>> + int i, ret;
>> + char name[16];
>> + struct device_node *np;
>> +
>> + ret = platform_driver_register(&simplefb_driver);
>> + if (ret)
>> + return ret;
>> +
>> + for (i = 0; ; i++) {
>> + snprintf(name, sizeof(name), "framebuffer%d", i);
>
> This smells like an infinite loop: we can be pretty sure that no
> hardware will ever exist with more than 9999 (I think?) framebuffers,
> however if that ever happens this'll loop until it runs out of RAM.
> Maybe add a suitably high limit to the for loop?
Unlikely, but the loop is wrong anyway. The loop should be:
for_each_child_of_node(of_chosen, child)
if (of_device_is_compatible(child, "simple-framebuffer");
of_platform_device_create(np, NULL, NULL);
Then make the probe hook choose an appropriate FB number. It looks
like you structured the code the way you did to get the framebuffers
to register in a particular order, and therefore implicitly get the
right numbers, but that is a fragile way to go about it.
Using /aliases really is the right way to get specific framebuffer
numbers. We already use it for UARTs, so we should do the same here.
Use of_alias_get_id(, "framebuffer") to get the framebuffer number.
g.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
