mbox series

[v2,0/6] rockchip: rk3328: sync dts and add ROC-RK3328-CC board

Message ID 20200405022544.31856-1-wens@kernel.org
Headers show
Series rockchip: rk3328: sync dts and add ROC-RK3328-CC board | expand

Message

Chen-Yu Tsai April 5, 2020, 2:25 a.m. UTC
From: Chen-Yu Tsai <wens at csie.org>

Hi everyone,

This is v2 of my ROC-RK3328-CC series. Changes from v1 are mainly
dropping the custom board target, and dealing with the pinmuxing
through proper use of DM regulators / GPIO / pinctrl in SPL.

This series adds proper support for Firefly / Libre Computer ROC-RK3328-CC
single board computer.

The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
card size development board based on the Rockchip RK3328 SoC, with:

  - 1/2/4 GB DDR4 DRAM
  - eMMC connector for optional module
  - micro SD card slot
  - 1 x USB 3.0 host port
  - 2 x USB 2.0 host port
  - 1 x USB 2.0 OTG port
  - HDMI video output
  - TRRS connector with audio and composite video output
  - gigabit Ethernet
  - consumer IR receiver
  - debug UART pins

Originally I started with Loic's patches, and syncing the device tree
files from Linux. That didn't get very far, with SPL failing to detect
the SD card. Examining the schematics and internal state of GRF and
GPIOs, I realized that the logic for the SD card power enable switch
is opposite that of what the SD card controller's SDMMC0_PWREN pin
would use. Instead, directly using the GPIO is required.

To deal with this, DM regulator and GPIO are enabled in SPL, and
various device nodes are marked with u-boot,dm-spl to have them work.
pinctrl properties are not stripped, so as to have the SDMMC0_PWREN
pin muxed over to GPIO.

Along the way, there are some clean-ups of existing dts files, moving
U-boot only features to -u-boot.dtsi files, and then a wholesale sync
from Linux. Only boards already existing in U-boot are synced. DT
binding header files are synced separately as there is already one
patch floating around. The DT sync also includes clean-up changes only
recently posted, and likely won't make it in for at least a few weeks.

Please have a look, and test if possible. I cc-ed a couple people that
showed interest in this board on mailing lists recently.

Regards
ChenYu


Chen-Yu Tsai (6):
  rockchip: dts: rk3328-evb: Move vcc5v0-host-xhci-drv to -u-boot.dtsi
  rockchip: dts: rk3328-evb: Move gmac2io related nodes to -u-boot.dtsi
  dt-bindings: clock: rk3328: sync from upstream Linux kernel
  dt-bindings: power: rk3328-power: sync from upstream Linux kernel
  rockchip: dts: rk3328: Sync device tree files from Linux
  rockchip: rk3328: Add support for ROC-RK3328-CC board

 arch/arm/dts/Makefile                         |    1 +
 arch/arm/dts/rk3328-evb-u-boot.dtsi           |   39 +
 arch/arm/dts/rk3328-evb.dts                   |  220 +--
 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi        |   38 +
 .../{rk3328-rock64.dts => rk3328-roc-cc.dts}  |  135 +-
 arch/arm/dts/rk3328-rock64.dts                |  132 +-
 arch/arm/dts/rk3328.dtsi                      | 1420 +++++++++++------
 board/rockchip/evb_rk3328/MAINTAINERS         |    7 +
 configs/roc-cc-rk3328_defconfig               |  103 ++
 doc/README.rockchip                           |    4 +-
 include/dt-bindings/clock/rk3328-cru.h        |  212 +--
 include/dt-bindings/power/rk3328-power.h      |   19 +
 12 files changed, 1578 insertions(+), 752 deletions(-)
 create mode 100644 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
 copy arch/arm/dts/{rk3328-rock64.dts => rk3328-roc-cc.dts} (68%)
 create mode 100644 configs/roc-cc-rk3328_defconfig
 create mode 100644 include/dt-bindings/power/rk3328-power.h

Comments

Peter Geis April 20, 2020, 5:35 p.m. UTC | #1
On Thu, Apr 16, 2020 at 5:53 AM Loic Devulder <LDevulder at suse.com> wrote:
>
> Hi Chen,
>
> I tested your patches and all work pretty well. I just had issues with
> USB2 that doesn't recognize any of my USB keys (it's OK on USB3).
>
> I also had these issues with XHCI driver:
> => usb reset
> resetting USB...
> BUG at drivers/usb/host/xhci-mem.c:84/xhci_ring_free()!
> BUG!
> ?esetting ...
>
> => usb stop
> stopping USB..
> Host not halted after 16000 microseconds.
> XHCI: failed to set VBus supply
> device_remove: Device 'usb at ff600000' failed to remove, but children are
> gone
>
> But for the whole series: Tested-by: Loic Devulder <ldevulder at suse.com>
>
> On Sun, 2020-04-05 at 10:25 +0800, Chen-Yu Tsai wrote:
> > From: Chen-Yu Tsai <wens at csie.org>
> >
> > Hi everyone,
> >
> > This is v2 of my ROC-RK3328-CC series. Changes from v1 are mainly
> > dropping the custom board target, and dealing with the pinmuxing
> > through proper use of DM regulators / GPIO / pinctrl in SPL.
> >
> > This series adds proper support for Firefly / Libre Computer ROC-
> > RK3328-CC
> > single board computer.
> >
> > The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
> > card size development board based on the Rockchip RK3328 SoC, with:
> >
> >   - 1/2/4 GB DDR4 DRAM
> >   - eMMC connector for optional module
> >   - micro SD card slot
> >   - 1 x USB 3.0 host port
> >   - 2 x USB 2.0 host port
> >   - 1 x USB 2.0 OTG port
> >   - HDMI video output
> >   - TRRS connector with audio and composite video output
> >   - gigabit Ethernet
> >   - consumer IR receiver
> >   - debug UART pins
> >
> > Originally I started with Loic's patches, and syncing the device tree
> > files from Linux. That didn't get very far, with SPL failing to
> > detect
> > the SD card. Examining the schematics and internal state of GRF and
> > GPIOs, I realized that the logic for the SD card power enable switch
> > is opposite that of what the SD card controller's SDMMC0_PWREN pin
> > would use. Instead, directly using the GPIO is required.
> >
> > To deal with this, DM regulator and GPIO are enabled in SPL, and
> > various device nodes are marked with u-boot,dm-spl to have them work.
> > pinctrl properties are not stripped, so as to have the SDMMC0_PWREN
> > pin muxed over to GPIO.
> >
> > Along the way, there are some clean-ups of existing dts files, moving
> > U-boot only features to -u-boot.dtsi files, and then a wholesale sync
> > from Linux. Only boards already existing in U-boot are synced. DT
> > binding header files are synced separately as there is already one
> > patch floating around. The DT sync also includes clean-up changes
> > only
> > recently posted, and likely won't make it in for at least a few
> > weeks.
> >
> > Please have a look, and test if possible. I cc-ed a couple people
> > that
> > showed interest in this board on mailing lists recently.
> >
> > Regards
> > ChenYu
> >
> >
> > Chen-Yu Tsai (6):
> >   rockchip: dts: rk3328-evb: Move vcc5v0-host-xhci-drv to -u-
> > boot.dtsi
> >   rockchip: dts: rk3328-evb: Move gmac2io related nodes to -u-
> > boot.dtsi
> >   dt-bindings: clock: rk3328: sync from upstream Linux kernel
> >   dt-bindings: power: rk3328-power: sync from upstream Linux kernel
> >   rockchip: dts: rk3328: Sync device tree files from Linux
> >   rockchip: rk3328: Add support for ROC-RK3328-CC board
> >
> >  arch/arm/dts/Makefile                         |    1 +
> >  arch/arm/dts/rk3328-evb-u-boot.dtsi           |   39 +
> >  arch/arm/dts/rk3328-evb.dts                   |  220 +--
> >  arch/arm/dts/rk3328-roc-cc-u-boot.dtsi        |   38 +
> >  .../{rk3328-rock64.dts => rk3328-roc-cc.dts}  |  135 +-
> >  arch/arm/dts/rk3328-rock64.dts                |  132 +-
> >  arch/arm/dts/rk3328.dtsi                      | 1420 +++++++++++--
> > ----
> >  board/rockchip/evb_rk3328/MAINTAINERS         |    7 +
> >  configs/roc-cc-rk3328_defconfig               |  103 ++
> >  doc/README.rockchip                           |    4 +-
> >  include/dt-bindings/clock/rk3328-cru.h        |  212 +--
> >  include/dt-bindings/power/rk3328-power.h      |   19 +
> >  12 files changed, 1578 insertions(+), 752 deletions(-)
> >  create mode 100644 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
> >  copy arch/arm/dts/{rk3328-rock64.dts => rk3328-roc-cc.dts} (68%)
> >  create mode 100644 configs/roc-cc-rk3328_defconfig
> >  create mode 100644 include/dt-bindings/power/rk3328-power.h
> >
> --
> Loic Devulder <ldevulder at suse.com> | ldevulder at irc
> 0x175A963893C85F55 | D220 DEF5 56A3 DE00 9DAA 78BA 175A 9638 93C8 5F55
> Senior QA Engineer | Container & Storage Solutions Quality Assurance
> team (qa-css)
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
> GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah HRB, 21284 (AG
> Nuernberg)

Good Afternoon,

I have tested this patch set against u-boot git as of 20 April 2020
and have some feedback.
The issue of booting off the sdmmc is fixed, thanks!

The USB issues above come from a few issues:
vcc_host1_5v is set to regulator-always-on, which prevents USB from
resetting properly, remove this option allows xhci to clean up.
USB 2.0 Host does not work because there is no rockchip,rk3328-usb2phy yet.
This causes the generic ohci and ehci drivers to fail, removing
CONFIG_PHY from your defconfig resolves this.
The USB-OTG port appears to not work because it's stuck in peripheral mode.
The vbus-supply = <&vcc_host1_5v> should be on all three USB
controllers, as it powers all three ports.

Overall, excellent work!
For the whole series: Tested-by: Peter Geis <pgwipeout at gmail.com>
Chen-Yu Tsai April 23, 2020, 9:24 a.m. UTC | #2
Hi,

On Tue, Apr 21, 2020 at 1:35 AM Peter Geis <pgwipeout at gmail.com> wrote:
>
> On Thu, Apr 16, 2020 at 5:53 AM Loic Devulder <LDevulder at suse.com> wrote:
> >
> > Hi Chen,
> >
> > I tested your patches and all work pretty well. I just had issues with
> > USB2 that doesn't recognize any of my USB keys (it's OK on USB3).
> >
> > I also had these issues with XHCI driver:
> > => usb reset
> > resetting USB...
> > BUG at drivers/usb/host/xhci-mem.c:84/xhci_ring_free()!
> > BUG!
> > ?esetting ...
> >
> > => usb stop
> > stopping USB..
> > Host not halted after 16000 microseconds.
> > XHCI: failed to set VBus supply
> > device_remove: Device 'usb at ff600000' failed to remove, but children are
> > gone
> >
> > But for the whole series: Tested-by: Loic Devulder <ldevulder at suse.com>
> >
> > On Sun, 2020-04-05 at 10:25 +0800, Chen-Yu Tsai wrote:
> > > From: Chen-Yu Tsai <wens at csie.org>
> > >
> > > Hi everyone,
> > >
> > > This is v2 of my ROC-RK3328-CC series. Changes from v1 are mainly
> > > dropping the custom board target, and dealing with the pinmuxing
> > > through proper use of DM regulators / GPIO / pinctrl in SPL.
> > >
> > > This series adds proper support for Firefly / Libre Computer ROC-
> > > RK3328-CC
> > > single board computer.
> > >
> > > The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
> > > card size development board based on the Rockchip RK3328 SoC, with:
> > >
> > >   - 1/2/4 GB DDR4 DRAM
> > >   - eMMC connector for optional module
> > >   - micro SD card slot
> > >   - 1 x USB 3.0 host port
> > >   - 2 x USB 2.0 host port
> > >   - 1 x USB 2.0 OTG port
> > >   - HDMI video output
> > >   - TRRS connector with audio and composite video output
> > >   - gigabit Ethernet
> > >   - consumer IR receiver
> > >   - debug UART pins
> > >
> > > Originally I started with Loic's patches, and syncing the device tree
> > > files from Linux. That didn't get very far, with SPL failing to
> > > detect
> > > the SD card. Examining the schematics and internal state of GRF and
> > > GPIOs, I realized that the logic for the SD card power enable switch
> > > is opposite that of what the SD card controller's SDMMC0_PWREN pin
> > > would use. Instead, directly using the GPIO is required.
> > >
> > > To deal with this, DM regulator and GPIO are enabled in SPL, and
> > > various device nodes are marked with u-boot,dm-spl to have them work.
> > > pinctrl properties are not stripped, so as to have the SDMMC0_PWREN
> > > pin muxed over to GPIO.
> > >
> > > Along the way, there are some clean-ups of existing dts files, moving
> > > U-boot only features to -u-boot.dtsi files, and then a wholesale sync
> > > from Linux. Only boards already existing in U-boot are synced. DT
> > > binding header files are synced separately as there is already one
> > > patch floating around. The DT sync also includes clean-up changes
> > > only
> > > recently posted, and likely won't make it in for at least a few
> > > weeks.
> > >
> > > Please have a look, and test if possible. I cc-ed a couple people
> > > that
> > > showed interest in this board on mailing lists recently.
> > >
> > > Regards
> > > ChenYu
> > >
> > >
> > > Chen-Yu Tsai (6):
> > >   rockchip: dts: rk3328-evb: Move vcc5v0-host-xhci-drv to -u-
> > > boot.dtsi
> > >   rockchip: dts: rk3328-evb: Move gmac2io related nodes to -u-
> > > boot.dtsi
> > >   dt-bindings: clock: rk3328: sync from upstream Linux kernel
> > >   dt-bindings: power: rk3328-power: sync from upstream Linux kernel
> > >   rockchip: dts: rk3328: Sync device tree files from Linux
> > >   rockchip: rk3328: Add support for ROC-RK3328-CC board
> > >
> > >  arch/arm/dts/Makefile                         |    1 +
> > >  arch/arm/dts/rk3328-evb-u-boot.dtsi           |   39 +
> > >  arch/arm/dts/rk3328-evb.dts                   |  220 +--
> > >  arch/arm/dts/rk3328-roc-cc-u-boot.dtsi        |   38 +
> > >  .../{rk3328-rock64.dts => rk3328-roc-cc.dts}  |  135 +-
> > >  arch/arm/dts/rk3328-rock64.dts                |  132 +-
> > >  arch/arm/dts/rk3328.dtsi                      | 1420 +++++++++++--
> > > ----
> > >  board/rockchip/evb_rk3328/MAINTAINERS         |    7 +
> > >  configs/roc-cc-rk3328_defconfig               |  103 ++
> > >  doc/README.rockchip                           |    4 +-
> > >  include/dt-bindings/clock/rk3328-cru.h        |  212 +--
> > >  include/dt-bindings/power/rk3328-power.h      |   19 +
> > >  12 files changed, 1578 insertions(+), 752 deletions(-)
> > >  create mode 100644 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
> > >  copy arch/arm/dts/{rk3328-rock64.dts => rk3328-roc-cc.dts} (68%)
> > >  create mode 100644 configs/roc-cc-rk3328_defconfig
> > >  create mode 100644 include/dt-bindings/power/rk3328-power.h
> > >
> > --
> > Loic Devulder <ldevulder at suse.com> | ldevulder at irc
> > 0x175A963893C85F55 | D220 DEF5 56A3 DE00 9DAA 78BA 175A 9638 93C8 5F55
> > Senior QA Engineer | Container & Storage Solutions Quality Assurance
> > team (qa-css)
> > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
> > GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah HRB, 21284 (AG
> > Nuernberg)
>
> Good Afternoon,
>
> I have tested this patch set against u-boot git as of 20 April 2020
> and have some feedback.
> The issue of booting off the sdmmc is fixed, thanks!
>
> The USB issues above come from a few issues:
> vcc_host1_5v is set to regulator-always-on, which prevents USB from
> resetting properly, remove this option allows xhci to clean up.

This seems a little odd. I suppose it's because of how U-boot's USB
stack works. On Linux, the XHCI is not supported. There seems to be
an issue in that the controller never detects disconnects.

> USB 2.0 Host does not work because there is no rockchip,rk3328-usb2phy yet.
> This causes the generic ohci and ehci drivers to fail, removing
> CONFIG_PHY from your defconfig resolves this.

Removing CONFIG_PHY stops XHCI from working.

It seems easier to remove the phy properties using the -u-boot.dtsi file.

> The USB-OTG port appears to not work because it's stuck in peripheral mode.
> The vbus-supply = <&vcc_host1_5v> should be on all three USB
> controllers, as it powers all three ports.

I tried setting dr_mode = "host" in the device tree but it still timeouts.
Setting vbus-supply doesn't help either as it is timing out during the
initializing phase in the driver, before vbus is enabled. Since it worked
before my patchset, I'm guessing it might be related to the assigned-clocks
changes. I have to do some more tests to narrow it down.


Some thoughts carried over from Linux regarding the always-on setting:

For this particular board, since all ports share the same GPIO to control
USB VCC, having it not always-on might result in other devices getting
power cycled, since U-boot doesn't do reference counting for regulators.

Another thing is that the bindings only allow the VBUS regulator to be
tied to the USB PHY, which is not supported in U-boot, and not the host.

To me it seems to make more sense to make enable/disable no-ops for
always-on regulators, instead of returning errors.


Regards
ChenYu



> Overall, excellent work!
> For the whole series: Tested-by: Peter Geis <pgwipeout at gmail.com>
Peter Geis April 23, 2020, 9:20 p.m. UTC | #3
On Thu, Apr 23, 2020 at 5:24 AM Chen-Yu Tsai <wens at kernel.org> wrote:
>
> Hi,
>
> On Tue, Apr 21, 2020 at 1:35 AM Peter Geis <pgwipeout at gmail.com> wrote:
> >
> > On Thu, Apr 16, 2020 at 5:53 AM Loic Devulder <LDevulder at suse.com> wrote:
> > >
> > > Hi Chen,
> > >
> > > I tested your patches and all work pretty well. I just had issues with
> > > USB2 that doesn't recognize any of my USB keys (it's OK on USB3).
> > >
> > > I also had these issues with XHCI driver:
> > > => usb reset
> > > resetting USB...
> > > BUG at drivers/usb/host/xhci-mem.c:84/xhci_ring_free()!
> > > BUG!
> > > ?esetting ...
> > >
> > > => usb stop
> > > stopping USB..
> > > Host not halted after 16000 microseconds.
> > > XHCI: failed to set VBus supply
> > > device_remove: Device 'usb at ff600000' failed to remove, but children are
> > > gone
> > >
> > > But for the whole series: Tested-by: Loic Devulder <ldevulder at suse.com>
> > >
> > > On Sun, 2020-04-05 at 10:25 +0800, Chen-Yu Tsai wrote:
> > > > From: Chen-Yu Tsai <wens at csie.org>
> > > >
> > > > Hi everyone,
> > > >
> > > > This is v2 of my ROC-RK3328-CC series. Changes from v1 are mainly
> > > > dropping the custom board target, and dealing with the pinmuxing
> > > > through proper use of DM regulators / GPIO / pinctrl in SPL.
> > > >
> > > > This series adds proper support for Firefly / Libre Computer ROC-
> > > > RK3328-CC
> > > > single board computer.
> > > >
> > > > The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
> > > > card size development board based on the Rockchip RK3328 SoC, with:
> > > >
> > > >   - 1/2/4 GB DDR4 DRAM
> > > >   - eMMC connector for optional module
> > > >   - micro SD card slot
> > > >   - 1 x USB 3.0 host port
> > > >   - 2 x USB 2.0 host port
> > > >   - 1 x USB 2.0 OTG port
> > > >   - HDMI video output
> > > >   - TRRS connector with audio and composite video output
> > > >   - gigabit Ethernet
> > > >   - consumer IR receiver
> > > >   - debug UART pins
> > > >
> > > > Originally I started with Loic's patches, and syncing the device tree
> > > > files from Linux. That didn't get very far, with SPL failing to
> > > > detect
> > > > the SD card. Examining the schematics and internal state of GRF and
> > > > GPIOs, I realized that the logic for the SD card power enable switch
> > > > is opposite that of what the SD card controller's SDMMC0_PWREN pin
> > > > would use. Instead, directly using the GPIO is required.
> > > >
> > > > To deal with this, DM regulator and GPIO are enabled in SPL, and
> > > > various device nodes are marked with u-boot,dm-spl to have them work.
> > > > pinctrl properties are not stripped, so as to have the SDMMC0_PWREN
> > > > pin muxed over to GPIO.
> > > >
> > > > Along the way, there are some clean-ups of existing dts files, moving
> > > > U-boot only features to -u-boot.dtsi files, and then a wholesale sync
> > > > from Linux. Only boards already existing in U-boot are synced. DT
> > > > binding header files are synced separately as there is already one
> > > > patch floating around. The DT sync also includes clean-up changes
> > > > only
> > > > recently posted, and likely won't make it in for at least a few
> > > > weeks.
> > > >
> > > > Please have a look, and test if possible. I cc-ed a couple people
> > > > that
> > > > showed interest in this board on mailing lists recently.
> > > >
> > > > Regards
> > > > ChenYu
> > > >
> > > >
> > > > Chen-Yu Tsai (6):
> > > >   rockchip: dts: rk3328-evb: Move vcc5v0-host-xhci-drv to -u-
> > > > boot.dtsi
> > > >   rockchip: dts: rk3328-evb: Move gmac2io related nodes to -u-
> > > > boot.dtsi
> > > >   dt-bindings: clock: rk3328: sync from upstream Linux kernel
> > > >   dt-bindings: power: rk3328-power: sync from upstream Linux kernel
> > > >   rockchip: dts: rk3328: Sync device tree files from Linux
> > > >   rockchip: rk3328: Add support for ROC-RK3328-CC board
> > > >
> > > >  arch/arm/dts/Makefile                         |    1 +
> > > >  arch/arm/dts/rk3328-evb-u-boot.dtsi           |   39 +
> > > >  arch/arm/dts/rk3328-evb.dts                   |  220 +--
> > > >  arch/arm/dts/rk3328-roc-cc-u-boot.dtsi        |   38 +
> > > >  .../{rk3328-rock64.dts => rk3328-roc-cc.dts}  |  135 +-
> > > >  arch/arm/dts/rk3328-rock64.dts                |  132 +-
> > > >  arch/arm/dts/rk3328.dtsi                      | 1420 +++++++++++--
> > > > ----
> > > >  board/rockchip/evb_rk3328/MAINTAINERS         |    7 +
> > > >  configs/roc-cc-rk3328_defconfig               |  103 ++
> > > >  doc/README.rockchip                           |    4 +-
> > > >  include/dt-bindings/clock/rk3328-cru.h        |  212 +--
> > > >  include/dt-bindings/power/rk3328-power.h      |   19 +
> > > >  12 files changed, 1578 insertions(+), 752 deletions(-)
> > > >  create mode 100644 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
> > > >  copy arch/arm/dts/{rk3328-rock64.dts => rk3328-roc-cc.dts} (68%)
> > > >  create mode 100644 configs/roc-cc-rk3328_defconfig
> > > >  create mode 100644 include/dt-bindings/power/rk3328-power.h
> > > >
> > > --
> > > Loic Devulder <ldevulder at suse.com> | ldevulder at irc
> > > 0x175A963893C85F55 | D220 DEF5 56A3 DE00 9DAA 78BA 175A 9638 93C8 5F55
> > > Senior QA Engineer | Container & Storage Solutions Quality Assurance
> > > team (qa-css)
> > > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
> > > GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah HRB, 21284 (AG
> > > Nuernberg)
> >
> > Good Afternoon,
> >
> > I have tested this patch set against u-boot git as of 20 April 2020
> > and have some feedback.
> > The issue of booting off the sdmmc is fixed, thanks!
> >
> > The USB issues above come from a few issues:
> > vcc_host1_5v is set to regulator-always-on, which prevents USB from
> > resetting properly, remove this option allows xhci to clean up.
>
> This seems a little odd. I suppose it's because of how U-boot's USB
> stack works. On Linux, the XHCI is not supported. There seems to be
> an issue in that the controller never detects disconnects.

Mainline Linux is missing the PHY driver, so you can enable the USB3
controller, but yes, connects/disconnects are not detected.
I've tried to port the downstream driver, but it is a mess and doesn't
trigger interrupts for 3.0 devices.

>
> > USB 2.0 Host does not work because there is no rockchip,rk3328-usb2phy yet.
> > This causes the generic ohci and ehci drivers to fail, removing
> > CONFIG_PHY from your defconfig resolves this.
>
> Removing CONFIG_PHY stops XHCI from working.

Odd, I never encountered that issue, maybe because I tagged all of the
phys to all of their devices.

>
> It seems easier to remove the phy properties using the -u-boot.dtsi file.
>
> > The USB-OTG port appears to not work because it's stuck in peripheral mode.
> > The vbus-supply = <&vcc_host1_5v> should be on all three USB
> > controllers, as it powers all three ports.
>
> I tried setting dr_mode = "host" in the device tree but it still timeouts.
> Setting vbus-supply doesn't help either as it is timing out during the
> initializing phase in the driver, before vbus is enabled. Since it worked
> before my patchset, I'm guessing it might be related to the assigned-clocks
> changes. I have to do some more tests to narrow it down.

I didn't test it, but it seemed to be the logical place to check.

>
>
> Some thoughts carried over from Linux regarding the always-on setting:
>
> For this particular board, since all ports share the same GPIO to control
> USB VCC, having it not always-on might result in other devices getting
> power cycled, since U-boot doesn't do reference counting for regulators.
>
> Another thing is that the bindings only allow the VBUS regulator to be
> tied to the USB PHY, which is not supported in U-boot, and not the host.
>
> To me it seems to make more sense to make enable/disable no-ops for
> always-on regulators, instead of returning errors.

Yes, but at least from my limited testing it seems U-Boot enables them
all en-mass during usb start.
It then shuts them all down during usb reset or usb stop.
It doesn't appear to implement runtime power management.

If you implement no-ops does that solve the issue of it not cleaning
up xhci on shutdown?

>
>
> Regards
> ChenYu
>
>
>
> > Overall, excellent work!
> > For the whole series: Tested-by: Peter Geis <pgwipeout at gmail.com>
Chen-Yu Tsai April 24, 2020, 9:40 a.m. UTC | #4
On Fri, Apr 24, 2020 at 5:20 AM Peter Geis <pgwipeout at gmail.com> wrote:
>
> On Thu, Apr 23, 2020 at 5:24 AM Chen-Yu Tsai <wens at kernel.org> wrote:
> >
> > Hi,
> >
> > On Tue, Apr 21, 2020 at 1:35 AM Peter Geis <pgwipeout at gmail.com> wrote:
> > >
> > > On Thu, Apr 16, 2020 at 5:53 AM Loic Devulder <LDevulder at suse.com> wrote:
> > > >
> > > > Hi Chen,
> > > >
> > > > I tested your patches and all work pretty well. I just had issues with
> > > > USB2 that doesn't recognize any of my USB keys (it's OK on USB3).
> > > >
> > > > I also had these issues with XHCI driver:
> > > > => usb reset
> > > > resetting USB...
> > > > BUG at drivers/usb/host/xhci-mem.c:84/xhci_ring_free()!
> > > > BUG!
> > > > ?esetting ...
> > > >
> > > > => usb stop
> > > > stopping USB..
> > > > Host not halted after 16000 microseconds.
> > > > XHCI: failed to set VBus supply
> > > > device_remove: Device 'usb at ff600000' failed to remove, but children are
> > > > gone
> > > >
> > > > But for the whole series: Tested-by: Loic Devulder <ldevulder at suse.com>
> > > >
> > > > On Sun, 2020-04-05 at 10:25 +0800, Chen-Yu Tsai wrote:
> > > > > From: Chen-Yu Tsai <wens at csie.org>
> > > > >
> > > > > Hi everyone,
> > > > >
> > > > > This is v2 of my ROC-RK3328-CC series. Changes from v1 are mainly
> > > > > dropping the custom board target, and dealing with the pinmuxing
> > > > > through proper use of DM regulators / GPIO / pinctrl in SPL.
> > > > >
> > > > > This series adds proper support for Firefly / Libre Computer ROC-
> > > > > RK3328-CC
> > > > > single board computer.
> > > > >
> > > > > The ROC-RK3328-CC from Firefly and Libre Computer Project is a credit
> > > > > card size development board based on the Rockchip RK3328 SoC, with:
> > > > >
> > > > >   - 1/2/4 GB DDR4 DRAM
> > > > >   - eMMC connector for optional module
> > > > >   - micro SD card slot
> > > > >   - 1 x USB 3.0 host port
> > > > >   - 2 x USB 2.0 host port
> > > > >   - 1 x USB 2.0 OTG port
> > > > >   - HDMI video output
> > > > >   - TRRS connector with audio and composite video output
> > > > >   - gigabit Ethernet
> > > > >   - consumer IR receiver
> > > > >   - debug UART pins
> > > > >
> > > > > Originally I started with Loic's patches, and syncing the device tree
> > > > > files from Linux. That didn't get very far, with SPL failing to
> > > > > detect
> > > > > the SD card. Examining the schematics and internal state of GRF and
> > > > > GPIOs, I realized that the logic for the SD card power enable switch
> > > > > is opposite that of what the SD card controller's SDMMC0_PWREN pin
> > > > > would use. Instead, directly using the GPIO is required.
> > > > >
> > > > > To deal with this, DM regulator and GPIO are enabled in SPL, and
> > > > > various device nodes are marked with u-boot,dm-spl to have them work.
> > > > > pinctrl properties are not stripped, so as to have the SDMMC0_PWREN
> > > > > pin muxed over to GPIO.
> > > > >
> > > > > Along the way, there are some clean-ups of existing dts files, moving
> > > > > U-boot only features to -u-boot.dtsi files, and then a wholesale sync
> > > > > from Linux. Only boards already existing in U-boot are synced. DT
> > > > > binding header files are synced separately as there is already one
> > > > > patch floating around. The DT sync also includes clean-up changes
> > > > > only
> > > > > recently posted, and likely won't make it in for at least a few
> > > > > weeks.
> > > > >
> > > > > Please have a look, and test if possible. I cc-ed a couple people
> > > > > that
> > > > > showed interest in this board on mailing lists recently.
> > > > >
> > > > > Regards
> > > > > ChenYu
> > > > >
> > > > >
> > > > > Chen-Yu Tsai (6):
> > > > >   rockchip: dts: rk3328-evb: Move vcc5v0-host-xhci-drv to -u-
> > > > > boot.dtsi
> > > > >   rockchip: dts: rk3328-evb: Move gmac2io related nodes to -u-
> > > > > boot.dtsi
> > > > >   dt-bindings: clock: rk3328: sync from upstream Linux kernel
> > > > >   dt-bindings: power: rk3328-power: sync from upstream Linux kernel
> > > > >   rockchip: dts: rk3328: Sync device tree files from Linux
> > > > >   rockchip: rk3328: Add support for ROC-RK3328-CC board
> > > > >
> > > > >  arch/arm/dts/Makefile                         |    1 +
> > > > >  arch/arm/dts/rk3328-evb-u-boot.dtsi           |   39 +
> > > > >  arch/arm/dts/rk3328-evb.dts                   |  220 +--
> > > > >  arch/arm/dts/rk3328-roc-cc-u-boot.dtsi        |   38 +
> > > > >  .../{rk3328-rock64.dts => rk3328-roc-cc.dts}  |  135 +-
> > > > >  arch/arm/dts/rk3328-rock64.dts                |  132 +-
> > > > >  arch/arm/dts/rk3328.dtsi                      | 1420 +++++++++++--
> > > > > ----
> > > > >  board/rockchip/evb_rk3328/MAINTAINERS         |    7 +
> > > > >  configs/roc-cc-rk3328_defconfig               |  103 ++
> > > > >  doc/README.rockchip                           |    4 +-
> > > > >  include/dt-bindings/clock/rk3328-cru.h        |  212 +--
> > > > >  include/dt-bindings/power/rk3328-power.h      |   19 +
> > > > >  12 files changed, 1578 insertions(+), 752 deletions(-)
> > > > >  create mode 100644 arch/arm/dts/rk3328-roc-cc-u-boot.dtsi
> > > > >  copy arch/arm/dts/{rk3328-rock64.dts => rk3328-roc-cc.dts} (68%)
> > > > >  create mode 100644 configs/roc-cc-rk3328_defconfig
> > > > >  create mode 100644 include/dt-bindings/power/rk3328-power.h
> > > > >
> > > > --
> > > > Loic Devulder <ldevulder at suse.com> | ldevulder at irc
> > > > 0x175A963893C85F55 | D220 DEF5 56A3 DE00 9DAA 78BA 175A 9638 93C8 5F55
> > > > Senior QA Engineer | Container & Storage Solutions Quality Assurance
> > > > team (qa-css)
> > > > SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nuernberg, Germany
> > > > GF: Felix Imend?rffer, Mary Higgins, Sri Rasiah HRB, 21284 (AG
> > > > Nuernberg)
> > >
> > > Good Afternoon,
> > >
> > > I have tested this patch set against u-boot git as of 20 April 2020
> > > and have some feedback.
> > > The issue of booting off the sdmmc is fixed, thanks!
> > >
> > > The USB issues above come from a few issues:
> > > vcc_host1_5v is set to regulator-always-on, which prevents USB from
> > > resetting properly, remove this option allows xhci to clean up.
> >
> > This seems a little odd. I suppose it's because of how U-boot's USB
> > stack works. On Linux, the XHCI is not supported. There seems to be
> > an issue in that the controller never detects disconnects.
>
> Mainline Linux is missing the PHY driver, so you can enable the USB3
> controller, but yes, connects/disconnects are not detected.
> I've tried to port the downstream driver, but it is a mess and doesn't
> trigger interrupts for 3.0 devices.

OK. I figured out the failure pattern was that the USB 3.0 port only
gets USB 2.0 working.

At this point it's starting to get weird. If I only tie the regulator
to XHCI, then the OTG 2.0 port doesn't detect anything. If I tie the
regulator to all three hosts (EHCI+OHCI counted as one), or only the
OTG device, then the USB 3.0 port only works with USB 2.0 devices.

This seems like a preexisting issue.

On the Rock64 the regulator is set to "always-on" and the same issue
with XHCI not working with USB 3.0 exists as well.

Given the trade off, I believe having USB 3.0 is more useful than USB
OTG in host mode.

> >
> > > USB 2.0 Host does not work because there is no rockchip,rk3328-usb2phy yet.
> > > This causes the generic ohci and ehci drivers to fail, removing
> > > CONFIG_PHY from your defconfig resolves this.
> >
> > Removing CONFIG_PHY stops XHCI from working.
>
> Odd, I never encountered that issue, maybe because I tagged all of the
> phys to all of their devices.

Ah, it looks like some other issue, unrelated to CONFIG_PHY.

> > It seems easier to remove the phy properties using the -u-boot.dtsi file.
> >
> > > The USB-OTG port appears to not work because it's stuck in peripheral mode.
> > > The vbus-supply = <&vcc_host1_5v> should be on all three USB
> > > controllers, as it powers all three ports.
> >
> > I tried setting dr_mode = "host" in the device tree but it still timeouts.
> > Setting vbus-supply doesn't help either as it is timing out during the
> > initializing phase in the driver, before vbus is enabled. Since it worked
> > before my patchset, I'm guessing it might be related to the assigned-clocks
> > changes. I have to do some more tests to narrow it down.
>
> I didn't test it, but it seemed to be the logical place to check.

Looks like it just needed "hnp-srp-disable" set in the OTG node.

> >
> >
> > Some thoughts carried over from Linux regarding the always-on setting:
> >
> > For this particular board, since all ports share the same GPIO to control
> > USB VCC, having it not always-on might result in other devices getting
> > power cycled, since U-boot doesn't do reference counting for regulators.
> >
> > Another thing is that the bindings only allow the VBUS regulator to be
> > tied to the USB PHY, which is not supported in U-boot, and not the host.
> >
> > To me it seems to make more sense to make enable/disable no-ops for
> > always-on regulators, instead of returning errors.
>
> Yes, but at least from my limited testing it seems U-Boot enables them
> all en-mass during usb start.
> It then shuts them all down during usb reset or usb stop.
> It doesn't appear to implement runtime power management.
>
> If you implement no-ops does that solve the issue of it not cleaning
> up xhci on shutdown?

I'd prefer not changing core behavior that requires a whole lot of
testing.

Regards
ChenYu

> >
> >
> > Regards
> > ChenYu
> >
> >
> >
> > > Overall, excellent work!
> > > For the whole series: Tested-by: Peter Geis <pgwipeout at gmail.com>