Re: [PATCH 3/3] usb host/MIPS: Remove hard-coded OCTEON platform information.

[David Daney <ddaney@caviumnetworks.com>]

On 05/29/2014 01:55 PM, Alan Stern wrote:
> On Thu, 29 May 2014, David Daney wrote:
>
>> Several points of clarification:
>>
>> 1) I wrote the patch in question, not Florian.
>>
>> 2) I agree that OCTEON ehci/ohci support could probably be refactored
>> along the lines of Alan's suggestion.
>>
>> 3) This patch is a relatively minor change to an *existing* driver
>> rather than a completely new thing that hasn't yet been merged.
>>
>> 4) There is a lot of precedent for merging minor enhancements and bug
>> fixes instead of requiring a complete refactoring of *existing* code.
>>
>> All that said, I haven't dug into the ehci-platform and ohci-platform
>> enough to be able to opine on the best course of action in this
>> particular case.  I hope to be able to make a more educated follow-up
>> next week.
>
> Moving these into the respective platform drivers shouldn't be a big
> deal.  Below is a totally untested preliminary pass -- I'm pleased that
> it removes a lot more lines than it adds.
>
> This first attempt leaves a few matters unresolved:
>
> 	The EHCI DMA mask is coerced to 32 bits by ehci-platform.c.
> 	Maybe this should be controllable by a flag in the
> 	usb_ehci_pdata structure.
>
> 	The timing of the calls to octeon_[eo]hci_hw_start() might
> 	be wrong.  The patch does it before the memory resources
> 	are mapped, rather than afterward as it is done now.  I don't
> 	know if this will matter.
>
> 	The clock management is awkward at best.  But it's about the
> 	same as what we do now.
>
> Anyway, it shouldn't be hard to fix this up and get it working, and
> then rebase your patch on top of it.
>
> Alan Stern
>
>
>
>   arch/mips/cavium-octeon/octeon-platform.c |  274 +++++++++++++++++++++++++++++-
>   arch/mips/configs/cavium_octeon_defconfig |    3
>   drivers/usb/host/Kconfig                  |   18 +
>   drivers/usb/host/Makefile                 |    1
>   drivers/usb/host/ehci-hcd.c               |    5
>   drivers/usb/host/ehci-octeon.c            |  188 --------------------
>   drivers/usb/host/octeon2-common.c         |  200 ---------------------
>   drivers/usb/host/ohci-hcd.c               |    5
>   drivers/usb/host/ohci-octeon.c            |  202 ----------------------
>   9 files changed, 285 insertions(+), 611 deletions(-)
>

Thanks Alan,

That goes beyond the call of duty!


I will try to test (and improve if necessary) this patch.

David Daney