From: bigeasy@linutronix•de (Sebastian Andrzej Siewior)
To: linux-arm-kernel@lists•infradead.org
Subject: [RESEND PATCH v3 2/4] usb: musb: Bugfix of_node assignment
Date: Mon, 25 Nov 2013 17:48:54 +0100 [thread overview]
Message-ID: <52937F76.107@linutronix.de> (raw)
In-Reply-To: <20131125161421.GH18046@saruman.home>
On 11/25/2013 05:14 PM, Felipe Balbi wrote:
> Hi,
>
> On Mon, Nov 18, 2013 at 04:54:36PM +0100, Markus Pargmann wrote:
>> It is not safe to assign the of_node to a device without driver.
>> The device is matched against a list of drivers and the of_node
>> could lead to a DT match with the parent driver.
>>
>> Signed-off-by: Markus Pargmann <mpa@pengutronix•de> ---
>> drivers/usb/musb/musb_core.c | 12 +++++++++++-
>> drivers/usb/musb/musb_dsps.c | 1 - 2 files changed, 11
>> insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/usb/musb/musb_core.c
>> b/drivers/usb/musb/musb_core.c index 8b7d903..d5ad9db 100644 ---
>> a/drivers/usb/musb/musb_core.c +++
>> b/drivers/usb/musb/musb_core.c @@ -1966,6 +1966,7 @@ static int
>> musb_probe(struct platform_device *pdev) int irq =
>> platform_get_irq_byname(pdev, "mc"); struct resource *iomem; void
>> __iomem *base; + int ret;
>>
>> iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0); if
>> (!iomem || irq <= 0) @@ -1975,7 +1976,16 @@ static int
>> musb_probe(struct platform_device *pdev) if (IS_ERR(base)) return
>> PTR_ERR(base);
>>
>> - return musb_init_controller(dev, irq, base); + if
>> (dev->parent)
don't we always have a parent?
>> + dev->of_node = dev->parent->of_node; + + ret =
>> musb_init_controller(dev, irq, base); + if (ret) { +
>> dev->of_node = NULL; + return ret; + } + + return 0; }
>>
>> static int musb_remove(struct platform_device *pdev) diff --git
>> a/drivers/usb/musb/musb_dsps.c b/drivers/usb/musb/musb_dsps.c
>> index 82e1da0..7b36d32 100644 --- a/drivers/usb/musb/musb_dsps.c
>> +++ b/drivers/usb/musb/musb_dsps.c @@ -485,7 +485,6 @@ static int
>> dsps_create_musb_pdev(struct dsps_glue *glue, musb->dev.parent =
>> dev; musb->dev.dma_mask = &musb_dmamask;
>> musb->dev.coherent_dma_mask = musb_dmamask; - musb->dev.of_node
>> = of_node_get(dn);
>
> Sebastian, does this look correct to you ?
Is this fixing a bug? Commit 4fc4b2 ("usb: musb: dsps: do not bind to
"musb-hdrc") [0] should fix the problem that is described here.
ux500 is affected by the same problem as dsps but it is only visible on
dsps because we DEFER probe for few reasons there test the fail path.
So by just looking at it should fix the same problem as the change I
cited _but_ it would also cover ux500. Currently I can't think of any
side effects by assigning of_node if the musb_init_*() did not do it.
If we take this in I would suggest to remove the check I added (because
it does no longer serve any purpose) and the description (why we do
this, what is the problem exactly) is could be taken from my patch
since I think it is better described (it safe to assign a node to a
driver, it becomes unsafe if after platform_probe _also_ the DT probe
is executed which was not to designed to work like this).
In the long term I would suggest to move the DT probe over to the musb
core code and we wouldn't need the node assignment anymore?
[0] http://www.spinics.net/lists/linux-usb/msg94701.html
Sebastian
next prev parent reply other threads:[~2013-11-25 16:48 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-11-18 15:54 [RESEND PATCH v3 0/4] usb: musb bugfixes Markus Pargmann
2013-11-18 15:54 ` [RESEND PATCH v3 1/4] usb: musb: gadget, stay IDLE without gadget driver Markus Pargmann
2013-11-25 15:56 ` Felipe Balbi
2013-11-18 15:54 ` [RESEND PATCH v3 2/4] usb: musb: Bugfix of_node assignment Markus Pargmann
2013-11-25 16:14 ` Felipe Balbi
2013-11-25 16:48 ` Sebastian Andrzej Siewior [this message]
2013-11-26 9:34 ` Markus Pargmann
2014-08-04 10:13 ` Markus Pargmann
2014-08-05 12:33 ` Sebastian Andrzej Siewior
2014-08-05 20:17 ` Felipe Balbi
2013-11-18 15:54 ` [RESEND PATCH v3 3/4] usb: musb: dsps, debugfs files Markus Pargmann
2013-11-25 15:57 ` Felipe Balbi
2013-11-26 9:25 ` Markus Pargmann
2013-11-26 16:40 ` Felipe Balbi
2013-11-18 15:54 ` [RESEND PATCH v3 4/4] usb: musb: dsps, use devm_kzalloc Markus Pargmann
2013-11-25 15:58 ` Felipe Balbi
2013-11-26 9:26 ` Markus Pargmann
2013-11-26 16:41 ` Felipe Balbi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=52937F76.107@linutronix.de \
--to=bigeasy@linutronix$(echo .)de \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox