From: Li Yang <leoli@freescale.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
Arnd Bergmann <arnd@arndb.de>, Peter Chen <Peter.Chen@nxp.com>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
"linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
lkml <linux-kernel@vger.kernel.org>,
Rajesh Bhagat <rajesh.bhagat@nxp.com>
Subject: Re: [PATCH] usb: chipidea: Configure DMA properties and ops from DT
Date: Tue, 8 Mar 2016 13:52:29 -0600 [thread overview]
Message-ID: <CADRPPNS0q3Q9j4AYvqRfcSaQ6BzLiL6zM5tn7MwJ613iNA1iPQ@mail.gmail.com> (raw)
In-Reply-To: <CADRPPNRH28W6u8adSb9Ja0JSJvFgcKF+2jYa3GSKBcGVsZcdLQ@mail.gmail.com>
On Wed, Mar 2, 2016 at 4:59 PM, Li Yang <leoli@freescale.com> wrote:
> On Mon, Feb 22, 2016 at 4:07 PM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> On Mon 22 Feb 02:03 PST 2016, Srinivas Kandagatla wrote:
>>
>>>
>>>
>>> On 22/02/16 05:32, Bjorn Andersson wrote:
>>> >On certain platforms (e.g. ARM64) the dma_ops needs to be explicitly set
>>> >to be able to do DMA allocations, so use the of_dma_configure() helper
>>> >to populate the dma properties and assign an appropriate dma_ops.
>
> We also hit the same issue with the dwc3 driver.
>
>>> >
>>> >Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
>>> >---
>>> > drivers/usb/chipidea/core.c | 4 ++++
>>> > 1 file changed, 4 insertions(+)
>>> >
>>> >diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
>>> >index 7404064b9bbc..047b9d4e67aa 100644
>>> >--- a/drivers/usb/chipidea/core.c
>>> >+++ b/drivers/usb/chipidea/core.c
>>> >@@ -62,6 +62,7 @@
>>> > #include <linux/usb/chipidea.h>
>>> > #include <linux/usb/of.h>
>>> > #include <linux/of.h>
>>> >+#include <linux/of_device.h>
>>> > #include <linux/phy.h>
>>> > #include <linux/regulator/consumer.h>
>>> > #include <linux/usb/ehci_def.h>
>>> >@@ -834,6 +835,9 @@ struct platform_device *ci_hdrc_add_device(struct device *dev,
>>> > pdev->dev.dma_parms = dev->dma_parms;
>>> > dma_set_coherent_mask(&pdev->dev, dev->coherent_dma_mask);
>>> >
>>> >+ if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>>> >+ of_dma_configure(&pdev->dev, dev->of_node);
>>> >+
>>> Would we hit the same issue if we are on non Device tree platforms like ACPI
>>> or platform device style itself?
>>>
>>
>> As far as I can see, yes.
>>
>>>
>>> > ret = platform_device_add_resources(pdev, res, nres);
>>> > if (ret)
>>> > goto err;
>>> >
>>>
>>> I think this is the side effect of commit
>>> 1dccb598df549d892b6450c261da54cdd7af44b4(arm64: simplify dma_get_ops)
>>>
>>
>> I agree, before that we would have hit:
>>
>> __generic_dma_ops() {
>> ..
>> else if (acpi_disabled)
>> return dma_ops;
>> ...
>> }
>>
>> with dma_ops being swiotlb_dma_ops from arm64_dma_init().
>>
>>
>> But this would not have saved us in the ACPI case, i.e. the result would
>> have been as with my suggested patch. Poking Arnd here to see if he has
>> any input.
>>
>>> None of the drivers call of_dma_configure() explicitly, which makes me feel
>>> that we are doing something wrong. TBH, this should be handled in more
>>> generic way rather than driver like this having an explicit call to
>>> of_dma_configure().
>>>
>>
>> I agree, trying to figure out if it should be inherited or something.
>
> I also agree. We need address it in a more generic way. I did a
> search for platform_device_add()/platform_device_register() in the
> kernel source code. I found a lot of them and many could be also
> doing DMA. Looks like it is still too early to assume every device is
> already getting dma_ops set through bus probe. Otherwise, many
> drivers are potentially broken by this assumption.
Any further comment on this topic? I added the linux-arm mailing list
which was missing from previous discussion.
Regards,
Leo
next prev parent reply other threads:[~2016-03-08 19:53 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-22 5:32 [PATCH] usb: chipidea: Configure DMA properties and ops from DT Bjorn Andersson
2016-02-22 6:02 ` Peter Chen
2016-02-22 6:14 ` Bjorn Andersson
2016-02-22 10:03 ` Srinivas Kandagatla
2016-02-22 22:07 ` Bjorn Andersson
2016-02-23 1:31 ` Peter Chen
2016-03-02 22:59 ` Li Yang
2016-03-08 19:52 ` Li Yang [this message]
2016-03-09 3:40 ` Bjorn Andersson
2016-03-09 23:16 ` Li Yang
2016-03-14 10:51 ` Peter Chen
2016-03-17 15:52 ` Arnd Bergmann
2016-03-18 1:54 ` Peter Chen
2016-03-18 3:25 ` Rajesh Bhagat
2016-03-18 7:28 ` Arnd Bergmann
2016-03-18 10:56 ` Russell King - ARM Linux
2016-03-25 4:02 ` Peter Chen
2016-03-25 4:39 ` [PATCH 1/1] usb: chipidea: add DMA mask configuration API kbuild test robot
2016-10-21 16:59 ` [RESEND PATCH] usb: chipidea: Configure DMA properties and ops from DT Bjorn Andersson
2016-10-21 17:38 ` Stephen Boyd
2016-10-21 17:53 ` Bjorn Andersson
2016-10-22 6:22 ` Sriram Dash
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=CADRPPNS0q3Q9j4AYvqRfcSaQ6BzLiL6zM5tn7MwJ613iNA1iPQ@mail.gmail.com \
--to=leoli@freescale.com \
--cc=Peter.Chen@nxp.com \
--cc=arnd@arndb.de \
--cc=bjorn.andersson@linaro.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=rajesh.bhagat@nxp.com \
--cc=srinivas.kandagatla@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).