Re: [PATCH v4 2/2] usb: Add support for ACPI identification to xhci-platform

[Javier Martinez Canillas]

Hello Greg,

On Sat, Aug 8, 2015 at 3:29 AM, Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 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 <mlangsdo@xxxxxxxxxx>
>> Signed-off-by: Duc Dang <dhdang@xxxxxxx>
>>
>> ---
>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/