On 01/10/2013 10:28 PM, Alan Stern wrote:
> On Thu, 10 Jan 2013, Felipe Balbi wrote:
>
>> -ENOROGER :-p
>>
>> On Thu, Jan 10, 2013 at 08:23:56PM +0200, Felipe Balbi wrote:
>>> Hi
>>>
>>> On Thu, Jan 10, 2013 at 12:01:09PM -0500, Alan Stern wrote:
>>>> On Thu, 10 Jan 2013, Felipe Balbi wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> On Thu, Jan 10, 2013 at 10:36:27AM -0500, Alan Stern wrote:
>>>>>> On Thu, 10 Jan 2013, Felipe Balbi wrote:
>>>>>>
>>>>>>> Hi Alan,
>>>>>>>
>>>>>>> with v3.8-rc3, ehci-hcd fails to compile for ARM if I use allmodconfig:
>>>>>>>
>>>>>>> drivers/usb/host/ehci-hcd.c:1285:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
>>>>>>> drivers/usb/host/ehci-hcd.c:1255:0: note: this is the location of the previous definition
>>>>>>> drivers/usb/host/ehci-mxc.c:280:31: warning: 'ehci_mxc_driver' defined but not used [-Wunused-variable]
>>>>>>> drivers/usb/host/ehci-hcd.c:1285:0: warning: "PLATFORM_DRIVER" redefined [enabled by default]
>>>>>>> drivers/usb/host/ehci-hcd.c:1255:0: note: this is the location of the previous definition
>>>>>>>
>>>>>>> Looks like the recent ARM multi-arch patches didn't take into account
>>>>>>> EHCI-hcd.
>>>>>>
>>>>>> Do you know why this problem didn't occur in 3.6-rc2? I don't see any
>>>>>> changes in -rc3 that would account for it (although I didn't look
>>>>>> terribly thoroughly).
>>>>>
>>>>> It's probably there, I just didn't look at it either :-)
>>>>
>>>> It seems clear that CONFIG_EHCI_HCD_PLATFORM needs to be mutually
>>>> exclusive with all the old-style drivers, in order to prevent this sort
>>>> of conflict. I just don't see why it never came up before...
>>>
>>> you can't do that as you will break ARM single zImage builds.
>
> As you point out below, they are already broken.
>
>>>>>>> Now it becomes easy to see why giving ehci its own
>>>>>>> platform_device/driver would've been a lot easier ;-)
>>>>>>
>>>>>> Not at all. We merely need to finish the conversion that I started.
>>>>>> It should be pretty easy to convert ehci-mxc over to the new scheme.
>>>>>> I don't have time to do it right now, but soon...
>>>>>
>>>>> ehci-mxc isn't the only one redefining PLATFORM_DRIVER. All of below are
>>>>> potential offenders:
>>>>>
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_fsl_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_mxc_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_sh_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_omap_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_orion_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_w90x900_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_atmel_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_octeon_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER vt8500_ehci_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER spear_ehci_hcd_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_msm_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_tilegx_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_msp_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER tegra_ehci_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER s5p_ehci_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_grlib_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_mv_driver
>>>>> drivers/usb/host/ehci-hcd.c:#define PLATFORM_DRIVER ehci_hcd_sead3_driver
>>>>
>>>> Yes; those all are old-style drivers. Most of them can be converted to
>>>> the new scheme with relatively little effort; a few will require more
>>>> work.
>>>>
>>>> In fact, the only drivers that _have_ been converted so far are
>>>> ehci-pci, chipidea, and ehci-platform.
>>>
>>> Fair enough... Roger Quadros (now in Cc) can probably help converting
>>> ehci-omap to the new scheme if you're willing to provide your review :-)
>
> Yes, I saw that and was going to recommend to him that the conversion
> should be done before his changes are applied. :-)
>
> Roger, I'll send you a patch for ehci-omap.c shortly. You should base
> your work on top of it (after testing to make sure that it works okay).
>
>>>> It wouldn't hurt to add some restrictions to the Kconfig entry for
>>>> CONFIG_USB_EHCI_HCD_PLATFORM too.
>>>
>>> Ok, maybe add a #warn "fix your freaking driver dude!!" to the
>>> non-converted ones would make people help ?
>
> Heh. :-)
>
> Here's my attempt to convert ehci-mxc. I don't have any simple way
> even to compile it. Please try it out to make sure that it fixes the
> build problem without introducing any other errors.
>
> Alan Stern
>
>
>
> Index: usb-3.7/drivers/usb/host/Kconfig
> ===================================================================
> --- usb-3.7.orig/drivers/usb/host/Kconfig
> +++ usb-3.7/drivers/usb/host/Kconfig
> @@ -148,7 +148,7 @@ config USB_EHCI_FSL
> Variation of ARC USB block used in some Freescale chips.
>
> config USB_EHCI_MXC
> - bool "Support for Freescale i.MX on-chip EHCI USB controller"
> + tristate "Support for Freescale i.MX on-chip EHCI USB controller"
> depends on USB_EHCI_HCD && ARCH_MXC
> select USB_EHCI_ROOT_HUB_TT
> ---help---
> Index: usb-3.7/drivers/usb/host/Makefile
> ===================================================================
> --- usb-3.7.orig/drivers/usb/host/Makefile
> +++ usb-3.7/drivers/usb/host/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_PCI) += pci-quirks.o
> obj-$(CONFIG_USB_EHCI_HCD) += ehci-hcd.o
> obj-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
> obj-$(CONFIG_USB_EHCI_HCD_PLATFORM) += ehci-platform.o
> +obj-$(CONFIG_USB_EHCI_MXC) += ehci-mxc.o
>
> obj-$(CONFIG_USB_OXU210HP_HCD) += oxu210hp-hcd.o
> obj-$(CONFIG_USB_ISP116X_HCD) += isp116x-hcd.o
> Index: usb-3.7/drivers/usb/host/ehci.h
> ===================================================================
> --- usb-3.7.orig/drivers/usb/host/ehci.h
> +++ usb-3.7/drivers/usb/host/ehci.h
> @@ -221,6 +221,8 @@ struct ehci_hcd { /* one per controlle
> #ifdef DEBUG
> struct dentry *debug_dir;
> #endif
> +
> + unsigned long priv[0]; /* platform-specific data */
Why can't it be void * ?
Also we need to put a comment there stating that priv must be the last
member in this structure, else it won't work.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
