Hello Paul,
Paul Mundt wrote:
> On Tue, Jan 25, 2011 at 08:20:31AM +0100, Heiko Schocher wrote:
>> @@ -1934,7 +1943,29 @@ static int __devinit sm501fb_probe(struct
>> platform_device *pdev)
>> }
>>
>> if (info->pdata == NULL) {
>> - dev_info(dev, "using default configuration data\n");
>> + int found = 0;
>> +#if defined(CONFIG_OF)
>> + struct device_node *np = pdev->dev.parent->of_node;
>> + const u8 *prop;
>> + const char *cp;
>> + int len;
>> +
>> + info->pdata = &sm501fb_def_pdata;
>> + if (np) {
>> + /* Get EDID */
>> + cp = of_get_property(np, "mode", &len);
>> + if (cp)
>> + strcpy(fb_mode, cp);
>> + prop = of_get_property(np, "edid", &len);
>> + if (prop && len == EDID_LENGTH) {
>> + info->edid_data = kmemdup(prop, EDID_LENGTH,
>> + GFP_KERNEL);
>> + found = 1;
>> + }
>> + }
>> +#endif
>> + if (!found)
>> + dev_info(dev, "using default configuration data\n");
>> }
>>
>> /* probe for the presence of each panel */
>
> Starting to get a bit pedantic.. but kmemdup() tries to do a kmalloc(),
No problem!
> and so can fail. Your other patches handle the info->edid_data == NULL
> case, in addition to the kfree(), but you're probably going to want to
> chomp that found assignment incase of the allocation failing and falling
> back on the default mode.
>
> You also don't really have any need to keep the EDID block around after
> probe as far as I can tell, so you should be able to rework this in to
> something more like:
>
> info->edid_data = kmemdup(..);
> ...
> if (info->edid_data) {
> fb_edid_to_monspecs(..);
> kfree(info->edid_data);
> fb_videomode_to_modelist(..);
> }
>
> ...
Ok, rework this part, thanks!
bye,
Heiko
--
DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
