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]
Signed-off-by: Mark Langsdorf <[email protected]>
Tested-by: Mark Salter <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ 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));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_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
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]
Signed-off-by: Mark Langsdorf <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 | 11 +++++++++++
2 files changed, 17 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 5d03f8b..14b40d2 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"
@@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
#endif
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id usb_xhci_acpi_match[] = {
+ /* APM X-Gene USB Controller */
+ { "PNP0D10", },
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, usb_xhci_acpi_match);
+#endif
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
@@ -269,6 +279,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
On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
> 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]
> Signed-off-by: Mark Langsdorf <[email protected]>
> Signed-off-by: Duc Dang <[email protected]>
>
> ---
> 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 | 11 +++++++++++
> 2 files changed, 17 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 5d03f8b..14b40d2 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"
> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> #endif
>
> +#ifdef CONFIG_ACPI
You shoudn't need this #ifdef, right?
thanks,
greg k-h
On Fri, Aug 7, 2015 at 6:29 PM, Greg KH <[email protected]> wrote:
> On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> 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]
>> Signed-off-by: Mark Langsdorf <[email protected]>
>> Signed-off-by: Duc Dang <[email protected]>
>>
>> ---
>> 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 | 11 +++++++++++
>> 2 files changed, 17 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 5d03f8b..14b40d2 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"
>> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> #endif
>>
>> +#ifdef CONFIG_ACPI
>
> You shoudn't need this #ifdef, right?
You are correct, Greg.
I will post a new version that remove this #ifdef CONFIG_ACPI shortly
>
> thanks,
>
> greg k-h
--
Regards,
Duc Dang.
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]
Signed-off-by: Mark Langsdorf <[email protected]>
Tested-by: Mark Salter <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d03f8b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,14 @@ 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));
+ /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_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
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.
Signed-off-by: Mark Langsdorf <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 5d03f8b..bd282cd 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"
@@ -262,6 +263,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[] = {
+ /* APM X-Gene 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,
@@ -269,6 +277,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
Hello Greg,
On Sat, Aug 8, 2015 at 3:29 AM, Greg KH <[email protected]> wrote:
> On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> 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]
>> Signed-off-by: Mark Langsdorf <[email protected]>
>> Signed-off-by: Duc Dang <[email protected]>
>>
>> ---
>> 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 | 11 +++++++++++
>> 2 files changed, 17 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 5d03f8b..14b40d2 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"
>> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> #endif
>>
>> +#ifdef CONFIG_ACPI
>
> You shoudn't need this #ifdef, right?
>
Why it is not needed?
The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
AFAIU since the driver also supports OF. So without the #ifdef guards,
.acpi_match_table = NULL and the struct acpi_device_id
usb_xhci_acpi_match[] will be built but not used.
Or am I missing something?
> thanks,
>
> greg k-h
>
Best regards,
Javier
On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index 890ad9d..5d03f8b 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -93,14 +93,14 @@ 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));
> + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> + if (ret) {
> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
> + if (ret)
> + return ret;
> + }
Note that dma_set_mask_and_coherent() and the original code are not
equivalent because of this:
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
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.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
> Hello Greg,
>
> On Sat, Aug 8, 2015 at 3:29 AM, Greg KH <[email protected]> wrote:
> > On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
> >> 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]
> >> Signed-off-by: Mark Langsdorf <[email protected]>
> >> Signed-off-by: Duc Dang <[email protected]>
> >>
> >> ---
> >> 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 | 11 +++++++++++
> >> 2 files changed, 17 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 5d03f8b..14b40d2 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"
> >> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
> >> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
> >> #endif
> >>
> >> +#ifdef CONFIG_ACPI
> >
> > You shoudn't need this #ifdef, right?
> >
>
> Why it is not needed?
Why is it needed?
> The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
> ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
> AFAIU since the driver also supports OF. So without the #ifdef guards,
> .acpi_match_table = NULL and the struct acpi_device_id
> usb_xhci_acpi_match[] will be built but not used.
Which is just fine, right?
> Or am I missing something?
Don't put #ifdef in .c files if at all possible is the kernel style
rules.
thanks,
greg k-h
On Sat, Aug 8, 2015 at 8:37 AM, Greg KH <[email protected]> wrote:
> On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Greg,
>>
>> On Sat, Aug 8, 2015 at 3:29 AM, Greg KH <[email protected]> wrote:
>> > On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> >> 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]
>> >> Signed-off-by: Mark Langsdorf <[email protected]>
>> >> Signed-off-by: Duc Dang <[email protected]>
>> >>
>> >> ---
>> >> 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 | 11 +++++++++++
>> >> 2 files changed, 17 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 5d03f8b..14b40d2 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"
>> >> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> >> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> >> #endif
>> >>
>> >> +#ifdef CONFIG_ACPI
>> >
>> > You shoudn't need this #ifdef, right?
>> >
>>
>> Why it is not needed?
>
> Why is it needed?
>
>> The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
>> ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
>> AFAIU since the driver also supports OF. So without the #ifdef guards,
>> .acpi_match_table = NULL and the struct acpi_device_id
>> usb_xhci_acpi_match[] will be built but not used.
>
> Which is just fine, right?
>
>> Or am I missing something?
>
> Don't put #ifdef in .c files if at all possible is the kernel style
> rules.
I tested booting with both device tree and ACPI with the new code that
has #ifdef CONFIG_ACPI removed and USB works fine with my X-Gene Arm64
platform.
>
> thanks,
>
> greg k-h
--
Regards,
Duc Dang.
On Sat, Aug 8, 2015 at 2:22 AM, Russell King - ARM Linux
<[email protected]> wrote:
> On Fri, Aug 07, 2015 at 08:18:48PM -0700, Duc Dang wrote:
>> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
>> index 890ad9d..5d03f8b 100644
>> --- a/drivers/usb/host/xhci-plat.c
>> +++ b/drivers/usb/host/xhci-plat.c
>> @@ -93,14 +93,14 @@ 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));
>> + /* Try setting the coherent_dma_mask to 64 bits, then try 32 bits */
>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
>> + if (ret) {
>> + ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
>> + if (ret)
>> + return ret;
>> + }
>
> Note that dma_set_mask_and_coherent() and the original code are not
> equivalent because of this:
>
> if (!pdev->dev.dma_mask)
> pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
>
> 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.
> --
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
Regards,
Duc Dang.
Hello Greg,
On Sat, Aug 8, 2015 at 5:37 PM, Greg KH <[email protected]> wrote:
> On Sat, Aug 08, 2015 at 07:43:40AM +0200, Javier Martinez Canillas wrote:
>> Hello Greg,
>>
>> On Sat, Aug 8, 2015 at 3:29 AM, Greg KH <[email protected]> wrote:
>> > On Fri, Aug 07, 2015 at 06:03:36PM -0700, Duc Dang wrote:
>> >> 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]
>> >> Signed-off-by: Mark Langsdorf <[email protected]>
>> >> Signed-off-by: Duc Dang <[email protected]>
>> >>
>> >> ---
>> >> 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 | 11 +++++++++++
>> >> 2 files changed, 17 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 5d03f8b..14b40d2 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"
>> >> @@ -262,6 +263,15 @@ static const struct of_device_id usb_xhci_of_match[] = {
>> >> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
>> >> #endif
>> >>
>> >> +#ifdef CONFIG_ACPI
>> >
>> > You shoudn't need this #ifdef, right?
>> >
>>
>> Why it is not needed?
>
> Why is it needed?
>
As explained, to have avoid having an unused variable.
>> The driver does .acpi_match_table = ACPI_PTR(usb_xhci_acpi_match) and
>> ACPI_PTR() is NULL if CONFIG_ACPI is not enabled. Which can happen
>> AFAIU since the driver also supports OF. So without the #ifdef guards,
>> .acpi_match_table = NULL and the struct acpi_device_id
>> usb_xhci_acpi_match[] will be built but not used.
>
> Which is just fine, right?
>
I've seen people having different opinions about this specific case
(using #ifdef guards for ACPI, OF, etc match tables definition),
that's why I asked.
>> Or am I missing something?
>
> Don't put #ifdef in .c files if at all possible is the kernel style
> rules.
>
I know but as you said the rule is to not have #ifdef if possible. But
I understand now that for you this case doesn't justify the #ifdefery.
> thanks,
>
> greg k-h
Best regards,
Javier
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 <[email protected]>
Tested-by: Mark Salter <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 890ad9d..5d1b84b 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -93,14 +93,18 @@ 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;
+ /*
+ * Try setting dma_mask and coherent_dma_mask to 64 bits,
+ * then try 32 bits
+ */
if (!pdev->dev.dma_mask)
pdev->dev.dma_mask = &pdev->dev.coherent_dma_mask;
- else
- dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+ ret = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
+ if (ret) {
+ ret = dma_set_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
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 <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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 5d1b84b..7ec2d31 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"
@@ -266,6 +267,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,
@@ -273,6 +281,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
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.
Arnd
On Sat, Aug 15, 2015 at 1:05 PM, Arnd Bergmann <[email protected]> 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.
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
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 <[email protected]>
Tested-by: Mark Salter <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Duc Dang <[email protected]>
---
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
On Thu, Aug 20, 2015 at 12:38 PM, Duc Dang <[email protected]> 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 <[email protected]>
> Tested-by: Mark Salter <[email protected]>
> Signed-off-by: Duc Dang <[email protected]>
>
> ---
> 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.