Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190521151952.2779-1-narmstrong@baylibre.com> <20190521151952.2779-4-narmstrong@baylibre.com>
In-Reply-To: <20190521151952.2779-4-narmstrong@baylibre.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Tue, 21 May 2019 20:01:51 +0200
Message-ID: <CAFBinCDzvroNfzhZHhDdvc+VR1eQNg5JMJ7F9=++hdWKcEXSOw@mail.gmail.com>
Subject: Re: [PATCH v3 3/3] arm64: dts: meson: Add minimal support for Odroid-N2
To:     Neil Armstrong <narmstrong@baylibre.com>
Cc:     khilman@baylibre.com, linux-amlogic@lists.infradead.org,
        linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Neil,

On Tue, May 21, 2019 at 5:20 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> +       hub_5v: regulator-hub_5v {
> +               compatible = "regulator-fixed";
> +               regulator-name = "HUB_5V";
> +               regulator-min-microvolt = <5000000>;
> +               regulator-max-microvolt = <5000000>;
> +               vin-supply = <&vcc_5v>;
> +
> +               gpio = <&gpio GPIOH_5 GPIO_ACTIVE_HIGH>;
I missed this in the review of v1:
according to the schematics GPIOH_5 is routed to GL3523 (soldered down
USB hub) CHIP_EN signal.
The datasheet [0] mentions that this will "Disable whole chip and keep
hub in lowest power state (standby mode)"

do you know if this is a similar case as GPIOH_4 (USB hub reset line,
we configure this using a gpio-hog)?

[...]
> +&ext_mdio {
> +       external_phy: ethernet-phy@0 {
> +               /* Realtek RTL8211F (0x001cc916) */
> +               reg = <0>;
> +               max-speed = <1000>;
> +               eee-broken-1000t;
are we in the same situation that we have on the X96 Max where network
dies without eee-broken-1000t?

[...]
> +&usb2_phy0 {
> +       phy-supply = <&usb_pwr_en>;
is usb_pwr_en really the phy-supply or is it the vbus-supply of the
USB top control block (&usb node)?

if these three questions are answered then you can add my:
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>


Regards
Martin


[0] https://datasheet.lcsc.com/szlcsc/GL3523-OV3S1_C157363.pdf