| Message ID | 20191106083843.1718437-1-vkoul@kernel.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | usb: xhci: Add support for Renesas USB controllers | expand |
On 06-11-19, 14:08, Vinod Koul wrote: > This series add support for Renesas USB controllers uPD720201 and uPD720202. > These require firmware to be loaded and in case devices have ROM those can > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > This includes two patches from Christian which supported these controllers > w/o ROM and later my patches for ROM support and multiple firmware versions. Greg, Mathias Any feedback on this? > > Changes in v5: > Added a debugfs rom erase patch, helps in debugging > Squashed patch 1 & 2 as requested by Mathias > > Changes in v4: > Rollback the delay values as we got device failures > > Changes in v3: > Dropped patch 2 as discussed with Christian > Removed aligned 8 bytes check > Change order for firware search from highest version to lowest > Added entry for new firmware for device 0x14 as well > Add tested by Christian > > Changes in v2: > used macros for timeout count and delay > removed renesas_fw_alive_check > cleaned renesas_fw_callback > removed recurion for renesas_fw_download > added MODULE_FIRMWARE > added comment for multiple fw order > > Christian Lamparter (1): > usb: xhci: add firmware loader for uPD720201 and uPD720202 w/o ROM > > Vinod Koul (3): > usb: xhci: Add ROM loader for uPD720201 > usb: xhci: allow multiple firmware versions > usb: xhci: provide a debugfs hook for erasing rom > > drivers/usb/host/xhci-pci.c | 911 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 911 insertions(+) > > -- > 2.23.0
On 21.11.2019 6.54, Vinod Koul wrote: > On 06-11-19, 14:08, Vinod Koul wrote: >> This series add support for Renesas USB controllers uPD720201 and uPD720202. >> These require firmware to be loaded and in case devices have ROM those can >> also be programmed if empty. If ROM is programmed, it runs from ROM as well. >> >> This includes two patches from Christian which supported these controllers >> w/o ROM and later my patches for ROM support and multiple firmware versions. > > Greg, Mathias > > Any feedback on this? > I need to take a fresh look at this series, there is a lot of code. -Mathias
Hi Mathias, On 26-11-19, 17:59, Mathias Nyman wrote: > On 21.11.2019 6.54, Vinod Koul wrote: > > On 06-11-19, 14:08, Vinod Koul wrote: > > > This series add support for Renesas USB controllers uPD720201 and uPD720202. > > > These require firmware to be loaded and in case devices have ROM those can > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > > > > > This includes two patches from Christian which supported these controllers > > > w/o ROM and later my patches for ROM support and multiple firmware versions. > > > > Greg, Mathias > > > > Any feedback on this? > > > > I need to take a fresh look at this series, there is a lot of code. Let me know if you if you have any suggestions for review. I did try to split it up logically so we have chunks to review independently. Thanks
On Wed, Nov 6, 2019 at 12:40 AM Vinod Koul <vkoul@kernel.org> wrote: > > This series add support for Renesas USB controllers uPD720201 and uPD720202. > These require firmware to be loaded and in case devices have ROM those can > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > This includes two patches from Christian which supported these controllers > w/o ROM and later my patches for ROM support and multiple firmware versions. > Hey Vinod! In pushing this series to one of the Android trees for the db845c, there was some concern raised that this series is adding a lot of renesas specific logic to the more generic xhci-pci driver. There was some question if instead that logic should be added to its own file/module? Do you have any thoughts on this? Also, It seems there hasn't been much feedback on this for a few months now. Is there a newer version of the patchset I should sync with? Do you have plans to resubmit soon? thanks -john
Hi John, On 07-01-20, 11:51, John Stultz wrote: > On Wed, Nov 6, 2019 at 12:40 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202. > > These require firmware to be loaded and in case devices have ROM those can > > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > > > This includes two patches from Christian which supported these controllers > > w/o ROM and later my patches for ROM support and multiple firmware versions. > > > > Hey Vinod! > In pushing this series to one of the Android trees for the db845c, > there was some concern raised that this series is adding a lot of > renesas specific logic to the more generic xhci-pci driver. There was > some question if instead that logic should be added to its own > file/module? Do you have any thoughts on this? TBH I have not thought about that and in previous post neither Greg or Mathias gave a feedback that this was not acceptable... We can think about splitting but apart from firmware load there is not much extra functionality that we need to add, the controller behaviour as a standard xhci-pci. So i am not sure if we gain much by splitting. > Also, It seems there hasn't been much feedback on this for a few > months now. Is there a newer version of the patchset I should sync > with? Do you have plans to resubmit soon? Well am still waiting for feedback :( I dont have any update on top of this, I can repost but I dont think that really serves a purpose. I would really like to hear from Greg if this series is acceptable and if not what would he like to see changed. Thanks
On Wed, Jan 08, 2020 at 09:37:07AM +0530, Vinod Koul wrote: > Hi John, > > On 07-01-20, 11:51, John Stultz wrote: > > On Wed, Nov 6, 2019 at 12:40 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202. > > > These require firmware to be loaded and in case devices have ROM those can > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > > > > > This includes two patches from Christian which supported these controllers > > > w/o ROM and later my patches for ROM support and multiple firmware versions. > > > > > > > Hey Vinod! > > In pushing this series to one of the Android trees for the db845c, > > there was some concern raised that this series is adding a lot of > > renesas specific logic to the more generic xhci-pci driver. There was > > some question if instead that logic should be added to its own > > file/module? Do you have any thoughts on this? > > TBH I have not thought about that and in previous post neither Greg or > Mathias gave a feedback that this was not acceptable... > > We can think about splitting but apart from firmware load there is not > much extra functionality that we need to add, the controller behaviour > as a standard xhci-pci. So i am not sure if we gain much by splitting. > > > Also, It seems there hasn't been much feedback on this for a few > > months now. Is there a newer version of the patchset I should sync > > with? Do you have plans to resubmit soon? > > Well am still waiting for feedback :( I dont have any update on top of > this, I can repost but I dont think that really serves a purpose. > > I would really like to hear from Greg if this series is acceptable and > if not what would he like to see changed. Greg is not the xhci maintainer :)
On 8.1.2020 8.24, Greg Kroah-Hartman wrote: > On Wed, Jan 08, 2020 at 09:37:07AM +0530, Vinod Koul wrote: >> Hi John, >> >> On 07-01-20, 11:51, John Stultz wrote: >>> On Wed, Nov 6, 2019 at 12:40 AM Vinod Koul <vkoul@kernel.org> wrote: >>>> >>>> This series add support for Renesas USB controllers uPD720201 and uPD720202. >>>> These require firmware to be loaded and in case devices have ROM those can >>>> also be programmed if empty. If ROM is programmed, it runs from ROM as well. >>>> >>>> This includes two patches from Christian which supported these controllers >>>> w/o ROM and later my patches for ROM support and multiple firmware versions. >>>> >>> >>> Hey Vinod! >>> In pushing this series to one of the Android trees for the db845c, >>> there was some concern raised that this series is adding a lot of >>> renesas specific logic to the more generic xhci-pci driver. There was >>> some question if instead that logic should be added to its own >>> file/module? Do you have any thoughts on this? >> >> TBH I have not thought about that and in previous post neither Greg or >> Mathias gave a feedback that this was not acceptable... >> >> We can think about splitting but apart from firmware load there is not >> much extra functionality that we need to add, the controller behaviour >> as a standard xhci-pci. So i am not sure if we gain much by splitting. >> >>> Also, It seems there hasn't been much feedback on this for a few >>> months now. Is there a newer version of the patchset I should sync >>> with? Do you have plans to resubmit soon? >> >> Well am still waiting for feedback :( I dont have any update on top of >> this, I can repost but I dont think that really serves a purpose. >> >> I would really like to hear from Greg if this series is acceptable and >> if not what would he like to see changed. > > Greg is not the xhci maintainer :) > Reviewing this always got bumped down on my todo list as other urgent issues came up. I think the concern about adding this amount of renesas specific code to xhci-pci.c is valid. This series adds over 900 lines of Renesas FW loading code to a 600 line xhci-pci.c -Mathias
On Wed, Jan 08, 2020 at 06:00:48PM +0200, Mathias Nyman wrote: > On 8.1.2020 8.24, Greg Kroah-Hartman wrote: > > On Wed, Jan 08, 2020 at 09:37:07AM +0530, Vinod Koul wrote: > > > Hi John, > > > > > > On 07-01-20, 11:51, John Stultz wrote: > > > > On Wed, Nov 6, 2019 at 12:40 AM Vinod Koul <vkoul@kernel.org> wrote: > > > > > > > > > > This series add support for Renesas USB controllers uPD720201 and uPD720202. > > > > > These require firmware to be loaded and in case devices have ROM those can > > > > > also be programmed if empty. If ROM is programmed, it runs from ROM as well. > > > > > > > > > > This includes two patches from Christian which supported these controllers > > > > > w/o ROM and later my patches for ROM support and multiple firmware versions. > > > > > > > > > > > > > Hey Vinod! > > > > In pushing this series to one of the Android trees for the db845c, > > > > there was some concern raised that this series is adding a lot of > > > > renesas specific logic to the more generic xhci-pci driver. There was > > > > some question if instead that logic should be added to its own > > > > file/module? Do you have any thoughts on this? > > > > > > TBH I have not thought about that and in previous post neither Greg or > > > Mathias gave a feedback that this was not acceptable... > > > > > > We can think about splitting but apart from firmware load there is not > > > much extra functionality that we need to add, the controller behaviour > > > as a standard xhci-pci. So i am not sure if we gain much by splitting. > > > > > > > Also, It seems there hasn't been much feedback on this for a few > > > > months now. Is there a newer version of the patchset I should sync > > > > with? Do you have plans to resubmit soon? > > > > > > Well am still waiting for feedback :( I dont have any update on top of > > > this, I can repost but I dont think that really serves a purpose. > > > > > > I would really like to hear from Greg if this series is acceptable and > > > if not what would he like to see changed. > > > > Greg is not the xhci maintainer :) > > > > Reviewing this always got bumped down on my todo list as other urgent issues > came up. > > I think the concern about adding this amount of renesas specific code to > xhci-pci.c is valid. This series adds over 900 lines of Renesas FW loading > code to a 600 line xhci-pci.c Yeah, that's not good, should be simple to split it into a separate file that's only build if that hardware is selected. thanks, greg k-h
On 08-01-20, 19:29, Greg Kroah-Hartman wrote: > On Wed, Jan 08, 2020 at 06:00:48PM +0200, Mathias Nyman wrote: > > > > Reviewing this always got bumped down on my todo list as other urgent issues > > came up. > > > > I think the concern about adding this amount of renesas specific code to > > xhci-pci.c is valid. This series adds over 900 lines of Renesas FW loading > > code to a 600 line xhci-pci.c > > Yeah, that's not good, should be simple to split it into a separate file > that's only build if that hardware is selected. Okay let me redo the patches splitting it up. If you have any thoughts about how that should be done, do let me know. Thanks