early platform drivers vs resource lookup by name

early platform drivers vs resource lookup by name

[Mike Frysinger]

ive been toying with the new early platform driver framework and
noticed a not-so-friendly feature wrt resource handling.  the
platform_device_add() function will take care of making sure that
every resource's name field is not NULL:
int platform_device_add(struct platform_device *pdev)
{
...
        if (r->name == NULL)
            r->name = dev_name(&pdev->dev);
...

unfortunately, the early_platform_add_devices() function does not have
similar code.  this leads to NULL pointer crashes if you attempt to
use any of the "byname" resource functions (e.g.
platform_get_resource_byname) and the resources in question arent
fully named.

is the simple fix here to add a resource loop to
early_platform_add_devices() that makes sure all the name fields are
non-NULL ?
-mike

Re: early platform drivers vs resource lookup by name

[Greg KH]

On Mon, Jul 27, 2009 at 02:20:06AM -0400, Mike Frysinger wrote:
> ive been toying with the new early platform driver framework and
> noticed a not-so-friendly feature wrt resource handling.  the
> platform_device_add() function will take care of making sure that
> every resource's name field is not NULL:
> int platform_device_add(struct platform_device *pdev)
> {
> ...
>         if (r->name == NULL)
>             r->name = dev_name(&pdev->dev);
> ...
> 
> unfortunately, the early_platform_add_devices() function does not have
> similar code.  this leads to NULL pointer crashes if you attempt to
> use any of the "byname" resource functions (e.g.
> platform_get_resource_byname) and the resources in question arent
> fully named.
> 
> is the simple fix here to add a resource loop to
> early_platform_add_devices() that makes sure all the name fields are
> non-NULL ?

I really do not know, what do you think would resolve this best for you?

thanks,

greg k-h

Re: early platform drivers vs resource lookup by name

[Mike Frysinger]

On Mon, Jul 27, 2009 at 13:48, Greg KH wrote:
> On Mon, Jul 27, 2009 at 02:20:06AM -0400, Mike Frysinger wrote:
>> ive been toying with the new early platform driver framework and
>> noticed a not-so-friendly feature wrt resource handling.  the
>> platform_device_add() function will take care of making sure that
>> every resource's name field is not NULL:
>> int platform_device_add(struct platform_device *pdev)
>> {
>> ...
>>         if (r->name == NULL)
>>             r->name = dev_name(&pdev->dev);
>> ...
>>
>> unfortunately, the early_platform_add_devices() function does not have
>> similar code.  this leads to NULL pointer crashes if you attempt to
>> use any of the "byname" resource functions (e.g.
>> platform_get_resource_byname) and the resources in question arent
>> fully named.
>>
>> is the simple fix here to add a resource loop to
>> early_platform_add_devices() that makes sure all the name fields are
>> non-NULL ?
>
> I really do not know, what do you think would resolve this best for you?

i think the r->name setting is a little hacky, but i assume it was
added in the first place for a reason, so having the early code follow
suite would probably be the least painful

otherwise platform_get_resource_byname() could be fixed by doing:
        if (r->name && type == resource_type(r) && !strcmp(r->name, name))
            return r;

but who knows what other functions are lurking that assuming r->name
is always set ...
-mike

Re: early platform drivers vs resource lookup by name

[Magnus Damm]

Hi Mike,

On Mon, Jul 27, 2009 at 3:20 PM, Mike Frysinger<vapier.adi@gmail.com> wrote:
> ive been toying with the new early platform driver framework and
> noticed a not-so-friendly feature wrt resource handling.  the
> platform_device_add() function will take care of making sure that
> every resource's name field is not NULL:
> int platform_device_add(struct platform_device *pdev)
> {
> ...
>        if (r->name == NULL)
>            r->name = dev_name(&pdev->dev);
> ...
>
> unfortunately, the early_platform_add_devices() function does not have
> similar code.  this leads to NULL pointer crashes if you attempt to
> use any of the "byname" resource functions (e.g.
> platform_get_resource_byname) and the resources in question arent
> fully named.
>
> is the simple fix here to add a resource loop to
> early_platform_add_devices() that makes sure all the name fields are
> non-NULL ?

I'm not aware of any available fix for this. I'm all for improving the
code as long as we don't add too many dependencies that prevents the
code from working very early on during the boot. Making it depend on
kmalloc() is for instance a bad idea. The early platform SuperH
drivers that I've hacked up do not use "byname" so this has not been
an issue for me so far

Please CC me on this address for future patches. =)

Cheers,

/ magnus