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

[Greg KH]

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 <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?

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
--
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/