Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932541AbcJNPAi (ORCPT ); Fri, 14 Oct 2016 11:00:38 -0400 Received: from mail.kernel.org ([198.145.29.136]:47194 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932460AbcJNPAh (ORCPT ); Fri, 14 Oct 2016 11:00:37 -0400 MIME-Version: 1.0 In-Reply-To: <1476401397-26497-3-git-send-email-john.stultz@linaro.org> References: <1476401397-26497-1-git-send-email-john.stultz@linaro.org> <1476401397-26497-3-git-send-email-john.stultz@linaro.org> From: Rob Herring Date: Fri, 14 Oct 2016 10:00:04 -0500 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [RFC][PATCH 2/2] usb: dwc2: Add a quirk to allow speed negotiation for Hisilicon Hi6220 To: John Stultz Cc: lkml , Chen Yu , Wei Xu , Guodong Xu , Amit Pundir , Mark Rutland , John Youn , Douglas Anderson , Greg Kroah-Hartman , Linux USB List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6690 Lines: 174 On Thu, Oct 13, 2016 at 6:29 PM, John Stultz wrote: > From: Chen Yu > > The Hi6220's usb controller is limited in that it does not > automatically autonegotiate the usb speed. Thus it requires a > quirk so that we can manually negotiate the best usb speed for > the attached device. > > Cc: Wei Xu > Cc: Guodong Xu > Cc: Amit Pundir > Cc: Rob Herring > Cc: Mark Rutland > Cc: John Youn > Cc: Douglas Anderson > Cc: Greg Kroah-Hartman > Cc: linux-usb@vger.kernel.org > Signed-off-by: Chen Yu > Signed-off-by: John Stultz > --- > arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 1 + > drivers/usb/dwc2/core.h | 7 +++ > drivers/usb/dwc2/hcd.c | 75 +++++++++++++++++++++++++++++++ > drivers/usb/dwc2/platform.c | 3 ++ > 4 files changed, 86 insertions(+) > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > index 17839db..2c8f435 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > @@ -752,6 +752,7 @@ > g-np-tx-fifo-size = <128>; > g-tx-fifo-size = <128 128 128 128 128 128>; > interrupts = <0 77 0x4>; > + hi6220,change_speed_quirk; > }; > > mailbox: mailbox@f7510000 { > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h > index 2a21a04..24fea73 100644 > --- a/drivers/usb/dwc2/core.h > +++ b/drivers/usb/dwc2/core.h > @@ -823,6 +823,10 @@ struct dwc2_hregs_backup { > * @frame_list_sz: Frame list size > * @desc_gen_cache: Kmem cache for generic descriptors > * @desc_hsisoc_cache: Kmem cache for hs isochronous descriptors > + * @change_speed_quirk: Change speed configuration to DWC2_SPEED_PARAM_FULL > + * while full&low speed device connect. And change speed > + * back to DWC2_SPEED_PARAM_HIGH while device is gone. > + * @device_count: Number of devices connect to root hub > * > * These are for peripheral mode: > * > @@ -951,6 +955,9 @@ struct dwc2_hsotg { > struct kmem_cache *desc_gen_cache; > struct kmem_cache *desc_hsisoc_cache; > > + int change_speed_quirk; > + unsigned int device_count; > + > #ifdef DEBUG > u32 frrem_samples; > u64 frrem_accum; > diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c > index b374e60..663033e 100644 > --- a/drivers/usb/dwc2/hcd.c > +++ b/drivers/usb/dwc2/hcd.c > @@ -4868,6 +4868,75 @@ static void _dwc2_hcd_clear_tt_buffer_complete(struct usb_hcd *hcd, > spin_unlock_irqrestore(&hsotg->lock, flags); > } > > +/* > + * HPRT0_SPD_HIGH_SPEED: high speed > + * HPRT0_SPD_FULL_SPEED: full speed > + */ > +static void dwc2_change_bus_speed(struct usb_hcd *hcd, int speed) > +{ > + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); > + > + if (hsotg->core_params->speed == speed) > + return; > + > + hsotg->core_params->speed = speed; > + queue_work(hsotg->wq_otg, &hsotg->wf_otg); > +} > + > +static int dwc2_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev) > +{ > + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); > + > + if (!hsotg->change_speed_quirk) > + return 1; > + > + hsotg->device_count++; > + dev_info(hsotg->dev, "Device count is %u after alloc dev\n", > + hsotg->device_count); > + > + return 1; > +} > + > +static void dwc2_free_dev(struct usb_hcd *hcd, struct usb_device *udev) > +{ > + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); > + > + if (!hsotg->change_speed_quirk) > + return; > + > + if (hsotg->device_count) > + hsotg->device_count--; > + > + dev_info(hsotg->dev, "Device count is %u after free dev\n", > + hsotg->device_count); > + > + if (hsotg->device_count == 1 && udev->parent && > + udev->parent->speed > USB_SPEED_UNKNOWN && > + udev->parent->speed < USB_SPEED_HIGH) { > + dev_info(hsotg->dev, "Set speed to default high-speed\n"); > + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED); > + } > +} > + > +static int dwc2_reset_device(struct usb_hcd *hcd, struct usb_device *udev) > +{ > + struct dwc2_hsotg *hsotg = dwc2_hcd_to_hsotg(hcd); > + > + if (!hsotg->change_speed_quirk) > + return 0; > + > + if (udev->speed == USB_SPEED_HIGH) { > + dev_info(hsotg->dev, "Set speed to high-speed\n"); > + dwc2_change_bus_speed(hcd, HPRT0_SPD_HIGH_SPEED); > + } else if (udev->speed == USB_SPEED_FULL > + || udev->speed == USB_SPEED_LOW) { > + dev_info(hsotg->dev, "Set speed to full-speed\n"); > + dwc2_change_bus_speed(hcd, HPRT0_SPD_FULL_SPEED); > + } > + > + return 0; > +} > + > static struct hc_driver dwc2_hc_driver = { > .description = "dwc2_hsotg", > .product_desc = "DWC OTG Controller", > @@ -5023,6 +5092,12 @@ int dwc2_hcd_init(struct dwc2_hsotg *hsotg, int irq) > dev_warn(hsotg->dev, "can't set coherent DMA mask\n"); > } > > + if (hsotg->change_speed_quirk) { > + dwc2_hc_driver.alloc_dev = dwc2_alloc_dev; > + dwc2_hc_driver.free_dev = dwc2_free_dev; > + dwc2_hc_driver.reset_device = dwc2_reset_device; > + } > + > hcd = usb_create_hcd(&dwc2_hc_driver, hsotg->dev, dev_name(hsotg->dev)); > if (!hcd) > goto error1; > diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c > index 8e1728b..21c328b 100644 > --- a/drivers/usb/dwc2/platform.c > +++ b/drivers/usb/dwc2/platform.c > @@ -585,6 +585,9 @@ static int dwc2_driver_probe(struct platform_device *dev) > dev_dbg(&dev->dev, "mapped PA %08lx to VA %p\n", > (unsigned long)res->start, hsotg->regs); > > + hsotg->change_speed_quirk = device_property_read_bool(&dev->dev, > + "hi6220,change_speed_quirk"); Can't this be determined from the hi6220's compatible string? In any case, "hi6220" is not a vendor. This should be hisilicon,change... and don't use '_'. And to bikeshed things, "no-speed-change-quirk". Rob