Date:   Mon, 10 Aug 2020 21:36:58 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc:     lkml <linux-kernel@...r.kernel.org>, Yu Chen <chenyu56@...wei.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        ShuFan Lee <shufan_lee@...htek.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Chunfeng Yun <chunfeng.yun@...iatek.com>,
        Felipe Balbi <balbi@...nel.org>,
        Hans de Goede <hdegoede@...hat.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Jun Li <lijun.kernel@...il.com>,
        Valentin Schneider <valentin.schneider@....com>,
        Jack Pham <jackp@...eaurora.org>,
        Linux USB List <linux-usb@...r.kernel.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>
Subject: Re: [RFC][PATCH v3 11/11] misc: hisi_hikey_usb: Driver to support usb
 functionality of Hikey960

On Mon, Aug 10, 2020 at 9:35 AM Mauro Carvalho Chehab
<mchehab+huawei@...nel.org> wrote:
> Em Wed, 16 Oct 2019 03:33:40 +0000
> John Stultz <john.stultz@...aro.org> escreveu:
>
> > From: Yu Chen <chenyu56@...wei.com>
> >
> > The HiKey960 has a fairly complex USB configuration due to it
> > needing to support a USB-C port for host/device mode and multiple
> > USB-A ports in host mode using a single USB controller.
> >
> > See schematics here:
> >   https://github.com/96boards/documentation/raw/master/consumer/hikey/hikey960/hardware-docs/HiKey960_Schematics.pdf
> >
> > This driver acts as a usb-role-switch intermediary, intercepting
> > the role switch notifications from the tcpm code, and passing
> > them on to the dwc3 core.
> >
> > In doing so, it also controls the onboard hub and power gpios in
> > order to properly route the data lines between the USB-C port
> > and the onboard hub to the USB-A ports.
> >
> > NOTE: It was noted that controlling the TYPEC_VBUS_POWER_OFF and
> > TYPEC_VBUS_POWER_ON values here is not reccomended. I'm looking
> > for a way to remove that bit from the logic here, but wanted to
> > still get feedback on this approach.
>
> Let me somewhat hijack this thread. I'm trying to add support here
> for the Hikey 970 driver. Maybe you might help me finding the remaing
> issues over there ;-)

So.. just as a heads up, this is a fairly old version of this patch. I
have the current version here:
  https://git.linaro.org/people/john.stultz/android-dev.git/commit/?h=dev/hikey960-mainline-WIP&id=1155346a06472177b8a7e7918de052549916f06f

So you may want to rework ontop of that.

That said, the last time I submitted the hub/mux driver, Rob pushed
back suggesting that the vbus, switch and hub power should probably be
DT describable:
  https://lore.kernel.org/lkml/20191218163738.GA12358@bogus/

I'm at the point where I probably don't have additional cycles to
spend to rework all the supporting drivers to support such a DT
binding, so I'm not very optimistic this patch will go upstream (its
much easier to float the current hub/mux driver).  So you may want to
focus on Rob's feedback there rather than any of my feedback here. :)


> The Hikey 970 has lots of things in common with Hikey 960, but
> the USB hub uses a somewhat different approach (based on what I
> saw at the Linaro's 4.9 official Hikey kernel tree).
>
> Basically, with the enclosed patch applied, the USB hub needs these
> at the DT file:
>
>                 hikey_usbhub: hikey_usbhub {
>                         compatible = "hisilicon,kirin970_hikey_usbhub";
>
>                         typec-vbus-gpios = <&gpio26 1 0>;
>                         otg-switch-gpios = <&gpio4 2 0>;
>                         hub_reset_en_gpio = <&gpio0 3 0>;
>                         hub-vdd-supply = <&ldo17>;
>                         usb-role-switch;
> ...
>                 }
>
> E.g. when compared with Hikey 960, the USB hub:
>
> - Hikey 970 uses a regulator instead of GPIO for powering on;

So, it might not be too hard to rework the hikey960 hub power gpio to
a gpio-regulator binding, and then both platforms can use the same
code?

> - Hikey 970 has a reset pin controlled via GPIO.

You might be able to put this reset pin under the dwc3 resets?


> It should be simple to add support for it, as done by the
> enclosed patch. With this, the phy driver for Hikey 970 and a new
> small driver to properly set clocks and reset lines at dwg3[1],
> I can now see the hub on my Hikey970:
>
>         $ lsusb
>         Bus 002 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub
>         Bus 001 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub
>
> Still, I'm missing something to make it work, as, besides the hub,
> right now, it doesn't detect the keyboard/mouse, which are
> attached at the USB hub.
>
> Do you have any ideas?

Not sure about the hub keyboard mouse issue. I worry that may be an
issue with the hub power not being on?
Make sure the mux driver is in the expected state when you boot up and
switch modes.

> [1] Right now, this is needed:
>         https://github.com/96boards-hikey/linux/blob/hikey970-v4.9/drivers/usb/dwc3/dwc3-hisi.c
>
>     Placing dwc3 directly under soc at DT causes some weird NMI, with
>     either produce an OOPS or hangs the machine at boot time.

I suspect you can drop the dwc3-hisi glue code once you move the clks
and resets to the dwc3 node directly, as we did for hikey960.
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/boot/dts/hisilicon/hi3660.dtsi?id=4bcf69e57063c9b1b15df1a293c969e80a1c97e6#n1169

thanks
-john

Powered by blists - more mailing lists