Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1210030 > unrolled thread

Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

Started byDuc Dang <dhdang@apm.com>
First post2015-08-19 23:30 +0200
Last post2015-09-01 14:10 +0200
Articles 7 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32  bit DMA Duc Dang <dhdang@apm.com> - 2015-08-19 23:30 +0200
    Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA Arnd Bergmann <arnd@arndb.de> - 2015-08-20 15:20 +0200
      [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA Duc Dang <dhdang@apm.com> - 2015-08-20 21:40 +0200
        [PATCH v7 2/2] usb: Add support for ACPI identification to xhci-platform Duc Dang <dhdang@apm.com> - 2015-08-20 21:50 +0200
        Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32  bit DMA Duc Dang <dhdang@apm.com> - 2015-08-31 21:00 +0200
          Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32  bit DMA Mathias Nyman <mathias.nyman@linux.intel.com> - 2015-09-01 14:00 +0200
            Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32  bit DMA Russell King - ARM Linux <linux@arm.linux.org.uk> - 2015-09-01 14:10 +0200

#1210030 — Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromDuc Dang <dhdang@apm.com>
Date2015-08-19 23:30 +0200
SubjectRe: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<pZj5N-15x-9@gated-at.bofh.it>
On Sat, Aug 15, 2015 at 1:05 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Saturday 08 August 2015 13:31:02 Duc Dang wrote:
> > >
> > > If we know that pdev->dev.dma_mask will always be initialised at this
> > > point, then the above change is fine.  If not, it's introducing a
> > > regression - dma_set_mask_and_coherent() will fail if pdev->dev.dma_mask
> > > is NULL (depending on the architectures implementation of dma_set_mask()).
> > >
> > > Prefixing the above change with the two lines I mention above would
> > > ensure equivalent behaviour.  Even if we do want to get rid of this,
> > > I'd advise to do it as a separate patch after this change, which can
> > > be independently reverted if there's problems with its removal.
> > >
> > Hi Russell,
> >
> > I will add the 2 lines you mentioned back to next version of the
> > patch. It is safer to do it that way as I do not see
> > pdev->dev.dma_mask gets initialized before the call
> > dma_set_mask_and_coherent  inside this xhci_plat.c file.
>
> It would be good to add a WARN_ON() to the case where dma_mask
> is a NULL pointer at the least. That way, we will at least
> find out if there are some broken platforms that do not correctly
> initialize the mask pointer.

Hi Arnd,

So the check will look like this, please let me know what do you think:
        if (!pdev->dev.dma_mask) {
                WARN_ON(1);
                /* Initialize dma_mask if the broken platform code has
not done so */
                pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
        }

>
>         Arnd




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

[toc] | [next] | [standalone]

#1210491 — Re: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromArnd Bergmann <arnd@arndb.de>
Date2015-08-20 15:20 +0200
SubjectRe: [PATCH v5 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<pZxV8-5W7-11@gated-at.bofh.it>
In reply to#1210030
On Wednesday 19 August 2015 14:28:33 Duc Dang wrote:
> 
> Hi Arnd,
> 
> So the check will look like this, please let me know what do you think:
>         if (!pdev->dev.dma_mask) {
>                 WARN_ON(1);
>                 /* Initialize dma_mask if the broken platform code has
> not done so */
>                 pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>         }

The condition can be written as 

	if (WARN_ON(!pdev->dev.dma_mask))

and I'd use dma_coerce_mask_and_coherent() instead of manually setting the
pointer, as an annotation for the fact that we are knowingly violating the
API here.

Those two points are just cosmetic though, aside from them, your code
above is what I had in mind.

Thanks,

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

[toc] | [prev] | [next] | [standalone]

#1210693 — [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromDuc Dang <dhdang@apm.com>
Date2015-08-20 21:40 +0200
Subject[PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<pZDQR-5Z9-1@gated-at.bofh.it>
In reply to#1210491
The xhci platform driver needs to work on systems that
either only support 64-bit DMA or only support 32-bit DMA.
Attempt to set a coherent dma mask for 64-bit DMA, and
attempt again with 32-bit DMA if that fails.

[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
Tested-by: Mark Salter <msalter@redhat.com>
Signed-off-by: Duc Dang <dhdang@apm.com>

---
Changes from v6:
	-Add WARN_ON if dma_mask is NULL
	-Use dma_coerce_mask_and_coherent to assign
	dma_mask and coherent_dma_mask

Changes from v5:
        -Change comment
        -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
        to make sure dma_set_mask_and_coherent does not fail prematurely.

Changes from v4:
        -None

Changes from v3:
        -Re-generate the patch over 4.2-rc5
        -No code change.

Changes from v2:
        -None

Changes from v1:
        -Consolidated to use dma_set_mask_and_coherent
        -Got rid of the check against sizeof(dma_addr_t)

 drivers/usb/host/xhci-plat.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..e4c7f9d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return -ENODEV;
 
-	/* Initialize dma_mask and coherent_dma_mask to 32-bits */
-	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
-	if (ret)
-		return ret;
-	if (!pdev->dev.dma_mask)
-		pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
-	else
-		dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+	/* Throw a waring if broken platform code didn't initialize dma_mask */
+	WARN_ON(!pdev->dev.dma_mask);
+	/*
+	 * Try setting dma_mask and coherent_dma_mask to 64 bits,
+	 * then try 32 bits
+	 */
+	ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+	if (ret) {
+		ret = dma_coerce_mask_and_coherent(&pdev->dev,
+						   DMA_BIT_MASK(32));
+		if (ret)
+			return ret;
+	}
 
 	hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
 	if (!hcd)
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1210704 — [PATCH v7 2/2] usb: Add support for ACPI identification to xhci-platform

FromDuc Dang <dhdang@apm.com>
Date2015-08-20 21:50 +0200
Subject[PATCH v7 2/2] usb: Add support for ACPI identification to xhci-platform
Message-ID<pZE0y-6as-15@gated-at.bofh.it>
In reply to#1210693
Provide the methods to let ACPI identify the need to use
xhci-platform. Change the Kconfig files so the
xhci-plat.o file is selectable during kernel config.

This has been tested on an ARM64 machine with platform XHCI, an
x86_64 machine with XHCI, and an x86_64 machine without XHCI.
There were no regressions or error messages on the machines
without platform XHCI.

[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
Signed-off-by: Duc Dang <dhdang@apm.com>

---
Changes from v6:
	-None

Change from v5:
        -Change comment to "XHCI-compliant USB Controller" as
        "PNP0D10" ID is not X-Gene specific

Changes from v4:
        -Remove #ifdef CONFIG_ACPI

Changes from v3:
        -Regenerate the patch over 4.2-rc5
        -No code change

Changes from v2
        -Replaced tristate with a boolean as the driver doesn't
                compile as a module
        -Correct --help-- to ---help---

Changes from v1
        -Renamed from "add support for APM X-Gene to xhci-platform"
        -Removed changes to arm64/Kconfig
        -Made CONFIG_USB_XHCI_PLATFORM a user selectable config option

 drivers/usb/host/Kconfig     | 7 ++++++-
 drivers/usb/host/xhci-plat.c | 9 +++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index 8afc3c1..96231ee 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -32,7 +32,12 @@ config USB_XHCI_PCI
        default y
 
 config USB_XHCI_PLATFORM
-	tristate
+	tristate "xHCI platform driver support"
+	---help---
+	  Say 'Y' to enable the support for the xHCI host controller
+	  as a platform device. Many ARM SoCs provide USB this way.
+
+	  If unsure, say 'Y'.
 
 config USB_XHCI_MVEBU
 	tristate "xHCI support for Marvell Armada 375/38x"
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index e4c7f9d..6c03e1c 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -19,6 +19,7 @@
 #include <linux/usb/phy.h>
 #include <linux/slab.h>
 #include <linux/usb/xhci_pdriver.h>
+#include <linux/acpi.h>
 
 #include "xhci.h"
 #include "xhci-mvebu.h"
@@ -267,6 +268,13 @@ static const struct of_device_id usb_xhci_of_match[] = {
 MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
 #endif
 
+static const struct acpi_device_id usb_xhci_acpi_match[] = {
+	/* XHCI-compliant USB Controller */
+	{ "PNP0D10", },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
+
 static struct platform_driver usb_xhci_driver = {
 	.probe	= xhci_plat_probe,
 	.remove	= xhci_plat_remove,
@@ -274,6 +282,7 @@ static struct platform_driver usb_xhci_driver = {
 		.name = "xhci-hcd",
 		.pm = DEV_PM_OPS,
 		.of_match_table = of_match_ptr(usb_xhci_of_match),
+		.acpi_match_table = ACPI_PTR(usb_xhci_acpi_match),
 	},
 };
 MODULE_ALIAS("platform:xhci-hcd");
-- 
1.9.1

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

[toc] | [prev] | [next] | [standalone]

#1216378 — Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromDuc Dang <dhdang@apm.com>
Date2015-08-31 21:00 +0200
SubjectRe: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<q3Ctc-7T0-11@gated-at.bofh.it>
In reply to#1210693
On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang <dhdang@apm.com> wrote:
> The xhci platform driver needs to work on systems that
> either only support 64-bit DMA or only support 32-bit DMA.
> Attempt to set a coherent dma mask for 64-bit DMA, and
> attempt again with 32-bit DMA if that fails.
>
> [dhdang: regenerate the patch over 4.2-rc5 and address new comments]
> Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
> Tested-by: Mark Salter <msalter@redhat.com>
> Signed-off-by: Duc Dang <dhdang@apm.com>
>
> ---
> Changes from v6:
>         -Add WARN_ON if dma_mask is NULL
>         -Use dma_coerce_mask_and_coherent to assign
>         dma_mask and coherent_dma_mask
>
> Changes from v5:
>         -Change comment
>         -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
>         to make sure dma_set_mask_and_coherent does not fail prematurely.
>
> Changes from v4:
>         -None
>
> Changes from v3:
>         -Re-generate the patch over 4.2-rc5
>         -No code change.
>
> Changes from v2:
>         -None
>
> Changes from v1:
>         -Consolidated to use dma_set_mask_and_coherent
>         -Got rid of the check against sizeof(dma_addr_t)
>
>  drivers/usb/host/xhci-plat.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 890ad9d..e4c7f9d 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
>         if (irq < 0)
>                 return -ENODEV;
>
> -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> -       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> -       if (ret)
> -               return ret;
> -       if (!pdev->dev.dma_mask)
> -               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> -       else
> -               dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> +       /* Throw a waring if broken platform code didn't initialize dma_mask */
> +       WARN_ON(!pdev->dev.dma_mask);
> +       /*
> +        * Try setting dma_mask and coherent_dma_mask to 64 bits,
> +        * then try 32 bits
> +        */
> +       ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> +       if (ret) {
> +               ret = dma_coerce_mask_and_coherent(&pdev->dev,
> +                                                  DMA_BIT_MASK(32));
> +               if (ret)
> +                       return ret;
> +       }
>
>         hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
>         if (!hcd)
> --
> 1.9.1
>

Hi Greg, Arnd, Russell,

Do you have any more comment about this patch set?

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

[toc] | [prev] | [next] | [standalone]

#1216801 — Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromMathias Nyman <mathias.nyman@linux.intel.com>
Date2015-09-01 14:00 +0200
SubjectRe: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<q3Soj-5Fd-29@gated-at.bofh.it>
In reply to#1216378
On 31.08.2015 21:58, Duc Dang wrote:
> On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang <dhdang@apm.com> wrote:
>> The xhci platform driver needs to work on systems that
>> either only support 64-bit DMA or only support 32-bit DMA.
>> Attempt to set a coherent dma mask for 64-bit DMA, and
>> attempt again with 32-bit DMA if that fails.
>>
>> [dhdang: regenerate the patch over 4.2-rc5 and address new comments]
>> Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
>> Tested-by: Mark Salter <msalter@redhat.com>
>> Signed-off-by: Duc Dang <dhdang@apm.com>
>>
>> ---
>> Changes from v6:
>>          -Add WARN_ON if dma_mask is NULL
>>          -Use dma_coerce_mask_and_coherent to assign
>>          dma_mask and coherent_dma_mask
>>
>> Changes from v5:
>>          -Change comment
>>          -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
>>          to make sure dma_set_mask_and_coherent does not fail prematurely.
>>
>> Changes from v4:
>>          -None
>>
>> Changes from v3:
>>          -Re-generate the patch over 4.2-rc5
>>          -No code change.
>>
>> Changes from v2:
>>          -None
>>
>> Changes from v1:
>>          -Consolidated to use dma_set_mask_and_coherent
>>          -Got rid of the check against sizeof(dma_addr_t)
>>
>>   drivers/usb/host/xhci-plat.c | 21 +++++++++++++--------
>>   1 file changed, 13 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 890ad9d..e4c7f9d 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
>>          if (irq < 0)
>>                  return -ENODEV;
>>
>> -       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
>> -       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
>> -       if (ret)
>> -               return ret;
>> -       if (!pdev->dev.dma_mask)
>> -               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>> -       else
>> -               dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
>> +       /* Throw a waring if broken platform code didn't initialize dma_mask */
>> +       WARN_ON(!pdev->dev.dma_mask);
>> +       /*
>> +        * Try setting dma_mask and coherent_dma_mask to 64 bits,
>> +        * then try 32 bits
>> +        */
>> +       ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>> +       if (ret) {
>> +               ret = dma_coerce_mask_and_coherent(&pdev->dev,
>> +                                                  DMA_BIT_MASK(32));
>> +               if (ret)
>> +                       return ret;
>> +       }
>>
>>          hcd = usb_create_hcd(driver, &pdev->dev, dev_name(&pdev->dev));
>>          if (!hcd)
>> --
>> 1.9.1
>>
>
> Hi Greg, Arnd, Russell,
>
> Do you have any more comment about this patch set?
>

I'm not sure I fully understand why we need to try the 64 bit DMA mask in platform probe.

As I understood it we just want to have some DMA mask set before calling usb_create_hcd()
to make sure USB core gets the "uses_dma" flag set and dma_set_mask() won't fail because of
missing dev->dma_mask.

The correct DMA mask is set later in xhci_gen_setup()

We also need to make sure the controller supports 64bit addressing capability before setting a 64 bit DMA mask.
(bit 0 in HCCPARAMS)

So for platform devices it goes look go this:

xhci_plat_probe()
   usb_create_hcd()
     usb_create_shared_hcd()
       hcd->self.uses_dma = (dev->dma_mask != NULL);
   usb_add_hcd()
     hcd->driver->reset()  (.reset = xhci_plat_setup)
       xhci_plat_setup()
         xhci_gen_setup()
           if (HCC_64BIT_ADDR(xhci->hcc_params) && !dma_set_mask(dev, DMA_BIT_MASK(64))) {
             xhci_dbg(xhci, "Enabling 64-bit DMA addresses.\n");
             dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
           }

or do we end up with dev->dma_mask = NULL on 64-bit DMA only after trying to set it to 32 in xhci_plat_probe(),
and thus also failing the dma_set_mask() in xhci_gen_setup()?

-Mathias










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

[toc] | [prev] | [next] | [standalone]

#1216805 — Re: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA

FromRussell King - ARM Linux <linux@arm.linux.org.uk>
Date2015-09-01 14:10 +0200
SubjectRe: [PATCH v7 1/2] usb: make xhci platform driver use 64 bit or 32 bit DMA
Message-ID<q3SxZ-65L-21@gated-at.bofh.it>
In reply to#1216801
On Tue, Sep 01, 2015 at 02:54:17PM +0300, Mathias Nyman wrote:
> On 31.08.2015 21:58, Duc Dang wrote:
> >On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang <dhdang@apm.com> wrote:
> >>The xhci platform driver needs to work on systems that
> >>either only support 64-bit DMA or only support 32-bit DMA.
> >>Attempt to set a coherent dma mask for 64-bit DMA, and
> >>attempt again with 32-bit DMA if that fails.
> >>
> >>[dhdang: regenerate the patch over 4.2-rc5 and address new comments]
> >>Signed-off-by: Mark Langsdorf <mlangsdo@redhat.com>
> >>Tested-by: Mark Salter <msalter@redhat.com>
> >>Signed-off-by: Duc Dang <dhdang@apm.com>
> >>
> >>---
> >>Changes from v6:
> >>         -Add WARN_ON if dma_mask is NULL
> >>         -Use dma_coerce_mask_and_coherent to assign
> >>         dma_mask and coherent_dma_mask
> >>
> >>Changes from v5:
> >>         -Change comment
> >>         -Assign dma_mask to coherent_dma_mask if dma_mask is NULL
> >>         to make sure dma_set_mask_and_coherent does not fail prematurely.
> >>
> >>Changes from v4:
> >>         -None
> >>
> >>Changes from v3:
> >>         -Re-generate the patch over 4.2-rc5
> >>         -No code change.
> >>
> >>Changes from v2:
> >>         -None
> >>
> >>Changes from v1:
> >>         -Consolidated to use dma_set_mask_and_coherent
> >>         -Got rid of the check against sizeof(dma_addr_t)
> >>
> >>  drivers/usb/host/xhci-plat.c | 21 +++++++++++++--------
> >>  1 file changed, 13 insertions(+), 8 deletions(-)
> >>
> >>diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> >>index 890ad9d..e4c7f9d 100644
> >>--- a/drivers/usb/host/xhci-plat.c
> >>+++ b/drivers/usb/host/xhci-plat.c
> >>@@ -93,14 +93,19 @@ static int xhci_plat_probe(struct platform_device *pdev)
> >>         if (irq < 0)
> >>                 return -ENODEV;
> >>
> >>-       /* Initialize dma_mask and coherent_dma_mask to 32-bits */
> >>-       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> >>-       if (ret)
> >>-               return ret;
> >>-       if (!pdev->dev.dma_mask)
> >>-               pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
> >>-       else
> >>-               dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> >>+       /* Throw a waring if broken platform code didn't initialize dma_mask */
> >>+       WARN_ON(!pdev->dev.dma_mask);
> >>+       /*
> >>+        * Try setting dma_mask and coherent_dma_mask to 64 bits,
> >>+        * then try 32 bits
> >>+        */
> >>+       ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> >>+       if (ret) {
> >>+               ret = dma_coerce_mask_and_coherent(&pdev->dev,
> >>+                                                  DMA_BIT_MASK(32));
> >>+               if (ret)
> >>+                       return ret;
> >>+       }

This isn't very good.  If dev.dma_mask is already set,
dma_coerce_mask_and_coherent() will always overwrite it.  There's also
no need to call it twice.  This, imho, is much better:

	/* Try to set a 64-bit DMA mask first */
	if (WARN_ON(!pdev->dev.dma_mask)) {
		/* Eek, platform didn't initialise the streaming DMA mask */
		ret = dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
	} else {
		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
	}

	/* If that failed, fall back to a 32-bit DMA mask */
	if (ret) {
		ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
		if (ret)
			return ret;
	}

since it preserves the dev.dma_mask pointer if it was properly setup

Really, drivers shouldn't be messing around with that pointer - especially
if it's already been correctly setup.  A platform may require separate
streaming and coherent masks, and we should respect that.

(The whole dma_mask being a pointer thing is a left-over from the PCI
layer which has never been cleaned up through fear of breaking something.)

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web