Hi,
On Wed, Jan 25, 2012 at 02:29:40PM +0400, Sergei Shtylyov wrote:
> Hello.
>
> I'm forwarding this mail to linux-usb and the MUSB maintainer.
>
> WBR, Sergei
>
> -------- Original Message --------
> Subject: platform driver (musb/davinci.ko) and resource.c
> Date: Tue, 24 Jan 2012 23:02:59 +0200
> From: Nam Nequam <maxdw.ml@xxxxxxxxx>
> To: davinci-linux-open-source@xxxxxxxxxxxxxxxxxxxx
>
>
>
> Hello,
>
> In the recent kernel versions usb/musb/davinci has been promoted to a module
> (platform driver) instead of being part of musb_hdrc.
>
> There is a little bug if davinci.ko is compiled as loadable module. davinci.ko
> can be inserted (modprobe) once only.
> After removal, it cannot be inserted again, due to "musb_hdrc: failed to claim
> resource 0" error in platform.c:platform_device_add() or crash.
> It happens because upon davinci.ko:davinci_probe(),
> resource.c:__insert_resource() modifies the global
> mach-davinci/usb.c:usb_dev.resource variable.
> After davinci.ko removal, the global usb.c:usb_dev.resource.parent points to
> freed memory.
> So, at the next modprobe davinci.ko, if, luckily, davinci.ko allocates memory
> platfrom_device_add_resources():kmemdup at the same location,
> it gives the above error ("claim resource"), otherwise - crash (since
> resource->parent pointer is not valid)
>
> It may be a common problem for platform devices that use global resource
> array, and their loadable platform drivers.
>
> BTW: Why does resource.c build a tree from overlapping resources at multiple
> insertions, and then silently drops the whole sub-tree upon removal of the
> current root.
> Where the root is just the most recently added resource.
>
>
> Here is the exact sequence:
> 1. Upon boot /mach-davinci/usb:davinci_setup_usb registers a usb_dev
> 1.1 platform.c:platform_device_add inserts a usb_dev.resource[0] (defaulting
> its parent to iomem_resource)
> 1.2 resource.c:__insert_resource places usb_dev.resource[0] under iomem_resource
>
> 2. Upon modprobe davinci.ko:
> 2.1 davinci.c:davinci_probe kmemdup(platform_device_add_resources) the
> resource from usb_dev.resource[0] (where sb_dev.resource[0].parent ==
> iomem_resource from 1.1)
> 2.2 davinci.c:davinci_probe calls platform.c:platform_device_add, which
> calls resource.c:insert_resource
> 2.3 resource.c:__insert_resource:
> 2.3.1 finds a registered (at 1.2) usb_dev.resource[0]
> 2.3.2 replaces it by musb->resource[0] and makes usb_dev.resource[0] child of
> musb->resource[0]
> 2.3.3 so, now, the global usb_dev.resource[0].parent == musb->resource[0]
> (usb_dev.resource[0].parent->musb->resource[0].parent->iomem_resource)
>
> 3. Upon rmmod davinci.ko
> 2.1 resource.c:release_resource drops both musb->resource[0] and
> usb_dev.resource[0] (usb_dev.resource[0].parent->musb->resource[0].parent)
> 2.2 davinci.c:davinci_removes platform device
> 2.3 the global usb_dev.resource[0].parent now points to kfreed
> parent->musb->resource[0]
>
> 4. Upon the second modprobe davinci.ko: resource.c:__insert_resource
> 4.1 davinci.c:davinci_probe kmemdup(platform_device_add_resources) the
> resource from usb_dev.resource[0] (where usb_dev.resource[0].parent ==
> deleted musb->resource[0] from 2.3.3)
> 4.2 if kmemdup allocated musb->resource[0] at the same place:
> __insert_resource will reject it, because its parent points to itself
> (the same memory location from the previous reincarantion),
> otherwise
> __insert_resource will crash trying to insert musb->resource[0] under
> [usb_dev/musb].parent which points to invalid memory location (released at 2.2)
try patch below:
diff --git a/drivers/usb/musb/davinci.c b/drivers/usb/musb/davinci.c
index 7c569f5..d6774fc 100644
--- a/drivers/usb/musb/davinci.c
+++ b/drivers/usb/musb/davinci.c
@@ -511,7 +511,7 @@ static const struct musb_platform_ops davinci_ops = {
static u64 davinci_dmamask = DMA_BIT_MASK(32);
-static int __init davinci_probe(struct platform_device *pdev)
+static int __devinit davinci_probe(struct platform_device *pdev)
{
struct musb_hdrc_platform_data *pdata = pdev->dev.platform_data;
struct platform_device *musb;
@@ -594,7 +594,7 @@ err0:
return ret;
}
-static int __exit davinci_remove(struct platform_device *pdev)
+static int __devexit davinci_remove(struct platform_device *pdev)
{
struct davinci_glue *glue = platform_get_drvdata(pdev);
@@ -608,7 +608,8 @@ static int __exit davinci_remove(struct platform_device *pdev)
}
static struct platform_driver davinci_driver = {
- .remove = __exit_p(davinci_remove),
+ .probe = davinci_probe,
+ .remove = __devexit_p(davinci_remove),
.driver = {
.name = "musb-davinci",
},
@@ -620,7 +621,7 @@ MODULE_LICENSE("GPL v2");
static int __init davinci_init(void)
{
- return platform_driver_probe(&davinci_driver, davinci_probe);
+ return platform_driver_register(&davinci_driver);
}
subsys_initcall(davinci_init);
let me know if that doesn't solve all your problems.
--
balbi
Attachment:
signature.asc
Description: Digital signature
