Re: [PATCH 2/2 v2] of/platform: Use platform_device interface

[Rob Herring]

On Tue, Apr 21, 2015 at 4:09 PM, Ricardo Ribalda Delgado
<ricardo.ribalda@xxxxxxxxx> wrote:
> Hello Rob
>
> On Tue, Apr 21, 2015 at 10:13 PM, Rob Herring <robherring2@xxxxxxxxx> wrote:
>> On Tue, Apr 21, 2015 at 3:25 AM, Ricardo Ribalda Delgado
>> <ricardo.ribalda@xxxxxxxxx> wrote:
>>> of_platform_device_create_pdata() was using of_device_add() to create
>>> the devices, but of_platform_device_destroy was using
>>> of_platform_device_destroy().
>>>
>>> of_device_add(), do not call insert_resource(), which initializes the
>>> parent field of the resource structure, needed by release_resource(),
>>> called by of_platform_device_destroy().
>>
>> This is because some DTs have overlapping resources and doing this
>> would break things. If you look at the git history, this was fixed and
>> then reverted by Grant.
>
> I cannot find that commit sorry, could you give me the hash or a link
> to the mailing list?
>
> ricardo@pilix:~/linux$ git shortlog drivers/of/platform.c | grep -i Revert
>       Revert "drivers: of: add initialization code for dma reserved memory"

commit 02bbde7849e68e193cefaa1885fe0df0f03c9fcd
Author: Grant Likely <grant.likely@xxxxxxxxxxxx>
Date:   Sun Feb 17 20:03:27 2013 +0000

    Revert "of: use platform_device_add"

    This reverts commit aac73f34542bc7ae4317928d2eabfeb21d247323. That
    commit causes two kinds of breakage; it breaks registration of AMBA
    devices when one of the parent nodes already contains overlapping
    resource regions, and it breaks calls to request_region() by device
    drivers in certain conditions where there are overlapping memory
    regions. Both of these problems can probably be fixed, but it is better
    to back out the commit and get a proper fix designed before trying again.

    Signed-off-by: Grant Likely <grant.likely@xxxxxxxxxxxx>

>
>
> To give a litte context to this patch, the issue started with this
> conversaion with Bjorn:
> https://lkml.org/lkml/2015/4/20/435
>
>
> What we have today is also wrong, it leads to a null pointer deference
> (and therefore a whole crash).
>
> If we cannot use platform_device_add, then we cannot use
> platform_device_destroy :)
>
> Shall I prepare a patch replacing platform_device_destroy()?

Perhaps we make inserting resource failure non-fatal so by default we
can have resources inserted but not break the cases Grant mentioned.
Ideally we want to not have new platforms with overlapping resources
in the DT.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/