Re: [PATCH v2 5/5] arm64: dts: meson: a1: support USB controller in OTG mode

[Dmitry Rokosov]

On Sun, Apr 23, 2023 at 07:51:31PM +0200, Martin Blumenstingl wrote:
> On Tue, Apr 18, 2023 at 1:16 PM Dmitry Rokosov <ddrokosov@xxxxxxxxxxxxxx> wrote:
> [...]
> > +                       usb2_phy1: phy@4000 {
> > +                               compatible = "amlogic,a1-usb2-phy";
> > +                               clocks = <&clkc CLKID_USB_PHY_IN>;
> > +                               clock-names = "xtal";
> Out of curiosity since there's also a CLKID_USB_PHY clock (which is
> used for the dwc3 controller below):
> Do we know that this part of the clock hierarchy is correct? I have no
> way to check this myself, so I'm curious if you could verify this
> somehow.
> 
> [...]

I've developed a clock driver for A1 and verified it against the Amlogic
custom driver and datasheet. As you pointed out, there are indeed two
USB phy clocks.
They are labeled as follows in my clock driver:
    * CLKID_USB_PHY_IN (xtal -> usb_phy gated clock) - the phy input clock
    * CLKID_USB_PHY (SYS_CLK_EN based gate) - the synopsys IP gated clock

The current representation of the USB phy clocks is solely based on
my technical opinion, as the datasheet does not provide any detailed
information about them.

Clock driver:
https://lore.kernel.org/all/20230405195927.13487-1-ddrokosov@xxxxxxxxxxxxxx/

> > +                       dwc2: usb@ff500000 {
> > +                               compatible = "amlogic,meson-a1-usb", "snps,dwc2";
> > +                               reg = <0x0 0xff500000 0x0 0x40000>;
> > +                               interrupts = <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
> > +                               phys = <&usb2_phy1>;
> > +                               phy-names = "usb2_phy";
> Documentation/devicetree/bindings/usb/dwc2.yaml only allows a
> "usb2-phy" (dash instead of underscore).
> 
> [...]

Ah, my fault..

> > +                       dwc3: usb@ff400000 {
> > +                               compatible = "snps,dwc3";
> > +                               reg = <0x0 0xff400000 0x0 0x100000>;
> Note to self: interesting that Amlogic swapped the register location
> of the dwc2 and dwc3 controllers since the G12 generation.

Indeed, during the bringup process, I was surprised to discover that
the dwc2 engine wasn't starting properly. It was quite unexpected, but
also admittedly intriguing as I delved into the issue and tried to
understand the root cause.

-- 
Thank you,
Dmitry