From: Sylvain Munaut <tnt@246tNt•com>
To: Nicolas DET <nd@bplan-gmbh•de>
Cc: linuxppc-embedded@ozlabs•org, sl@bplan-gmbh•de, linuxppc-dev@ozlabs•org
Subject: Re: [PATCH/RFC] powerpc: Add of_platform support for OHCI/Bigendian HC
Date: Sat, 04 Nov 2006 23:04:08 +0100 [thread overview]
Message-ID: <454D0E58.4010100@246tNt.com> (raw)
In-Reply-To: <454A59CC.6070902@bplan-gmbh.de>
>
> ------------------------------------------------------------------------
> +
> +static struct of_device_id ohci_hcd_ppc_of_match_be[] = {
> + {
> + .name = "usb",
> + .compatible = "ohci-bigendian",
> + },
> + {
> + .name = "usb",
> + .compatible = "ohci-be",
> + },
> + {},
> +};
>
> +
> +static struct of_device_id ohci_hcd_ppc_of_match_le[] = {
> + {
> + .name = "usb",
> + .compatible = "ohci-littledian",
> + },
> + {
> + .name = "usb",
> + .compatible = "ohci-le",
> + },
> + {},
> +};
>
Isn't it possible to specify "options" in the device tree rather than
different names ?
(just a though ...)
It's just duplicating all that code doesn't look nice.
> +config USB_OHCI_HCD_PPC_OF
> + bool "OHCI support for PPC USB controller for OpenFirmware platform"
> + depends on USB_OHCI_HCD && PPC_OF
> + default y
> + select USB_OHCI_BIG_ENDIAN
> + select USB_OHCI_LITTLE_ENDIAN
> + ---help---
> + Enables support for the USB controller PowerPC OpenFirmware platform
> +
>
I know what benh said but do we really want to select both all the
times. That adds
quite an overhead to the accesses ...
> --- a/drivers/usb/host/ohci-hcd.c 2006-11-01 09:18:56.000000000 +0100
> +++ b/drivers/usb/host/ohci-hcd.c 2006-11-02 18:06:02.000000000 +0100
> @@ -930,6 +930,12 @@ MODULE_LICENSE ("GPL");
> #include "ohci-ppc-soc.c"
> #endif
>
> +#ifdef CONFIG_USB_OHCI_HCD_PPC_OF
> +#include "ohci-ppc-of.c"
> +#endif
> +
> +
> +
> #if defined(CONFIG_ARCH_AT91RM9200) || defined(CONFIG_ARCH_AT91SAM9261)
> #include "ohci-at91.c"
> #endif
>
Don't add space for nothing. All the #if #endif are space by one empty
line, keep it that way.
You also missed the last #if #end
#if !(defined(CONFIG_PCI) \
|| defined(CONFIG_SA1111) \
|| defined(CONFIG_ARCH_S3C2410) \
|| defined(CONFIG_ARCH_OMAP) \
|| defined (CONFIG_ARCH_LH7A404) \
|| defined (CONFIG_PXA27x) \
|| defined (CONFIG_ARCH_EP93XX) \
|| defined (CONFIG_SOC_AU1X00) \
|| defined (CONFIG_USB_OHCI_HCD_PPC_SOC) \
|| defined (CONFIG_ARCH_AT91RM9200) \
|| defined (CONFIG_ARCH_AT91SAM9261) \
|| defined (CONFIG_ARCH_PNX4008) \
)
#error "missing bus glue for ohci-hcd"
#endif
You need to add a || defined(...) clause there.
Sylvain
next prev parent reply other threads:[~2006-11-04 22:04 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-01 20:31 [PATCH/RFC] powerpc: Add of_platform support for OHCI/Bigendian HC Nicolas DET
2006-11-01 22:23 ` Benjamin Herrenschmidt
2006-11-01 23:27 ` Benjamin Herrenschmidt
2006-11-02 20:49 ` Nicolas DET
2006-11-04 22:04 ` Sylvain Munaut [this message]
2006-11-04 22:15 ` Benjamin Herrenschmidt
2006-11-04 23:37 ` Benjamin Herrenschmidt
2006-11-01 23:49 ` Linas Vepstas
2006-11-02 0:07 ` Benjamin Herrenschmidt
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=454D0E58.4010100@246tNt.com \
--to=tnt@246tnt$(echo .)com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=linuxppc-embedded@ozlabs$(echo .)org \
--cc=nd@bplan-gmbh$(echo .)de \
--cc=sl@bplan-gmbh$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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