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: <20200324102030.31000-1-narmstrong@baylibre.com> <20200324102030.31000-6-narmstrong@baylibre.com>
In-Reply-To: <20200324102030.31000-6-narmstrong@baylibre.com>
From:   Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Date:   Tue, 24 Mar 2020 15:33:22 +0100
Message-ID: <CAFBinCDrycKy6sJfpUjoB3_jkHP-ssmwvr-Cab1-fOmz_i_bGQ@mail.gmail.com>
Subject: Re: [PATCH 05/13] usb: dwc3: meson-g12a: refactor usb2 phy init
To:     Neil Armstrong <narmstrong@baylibre.com>
Cc:     kishon@ti.com, balbi@kernel.org, khilman@baylibre.com,
        linux-amlogic@lists.infradead.org, linux-usb@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: multipart/mixed; boundary="000000000000dec06e05a19aa2dd"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

--000000000000dec06e05a19aa2dd
Content-Type: text/plain; charset="UTF-8"

Hi Neil,

On Tue, Mar 24, 2020 at 11:20 AM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> @@ -195,23 +239,9 @@ static int dwc3_meson_g12a_usb2_init(struct dwc3_meson_g12a *priv)
>                 if (!strstr(priv->drvdata->phy_names[i], "usb2"))
>                         continue;
>
> -               regmap_update_bits(priv->u2p_regmap[i], U2P_R0,
> -                                  U2P_R0_POWER_ON_RESET,
> -                                  U2P_R0_POWER_ON_RESET);
> -
> -               if (priv->drvdata->otg_switch_supported && i == USB2_OTG_PHY) {
> -                       regmap_update_bits(priv->u2p_regmap[i], U2P_R0,
> -                               U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS,
> -                               U2P_R0_ID_PULLUP | U2P_R0_DRV_VBUS);
> -
> -                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> -                                                     priv->otg_phy_mode);
> -               } else
> -                       dwc3_meson_g12a_usb2_set_mode(priv, i,
> -                                                     PHY_MODE_USB_HOST);
> -
> -               regmap_update_bits(priv->u2p_regmap[i], U2P_R0,
> -                                  U2P_R0_POWER_ON_RESET, 0);
> +               ret = priv->drvdata->usb2_init_phy(priv, i, mode);
> +               if (ret)
> +                       return ret;
>         }
this doesn't compile for me, it complains that mode is undefined
I believe we need something like the attached patch on top.

[...]
> @@ -580,7 +612,9 @@ static int dwc3_meson_g12a_probe(struct platform_device *pdev)
>         /* Get dr_mode */
>         priv->otg_mode = usb_get_dr_mode(dev);
>
> -       dwc3_meson_g12a_usb_init(priv);
> +       ret = dwc3_meson_g12a_usb_init(priv);
> +       if (ret)
> +               goto err_disable_clks;
this looks like an unrelated fix, dwc3_meson_g12a_usb_init has always
returned int (as potential error)
also the same check is missing in dwc3_meson_g12a_resume
can you please move this to a separate patch with the appropriate tag:
Fixes: c99993376f72ca ("usb: dwc3: Add Amlogic G12A DWC3 glue")


Martin

--000000000000dec06e05a19aa2dd
Content-Type: text/x-patch; charset="US-ASCII"; name="fixup.patch"
Content-Disposition: attachment; filename="fixup.patch"
Content-Transfer-Encoding: base64
Content-ID: <f_k85zzkgb0>
X-Attachment-Id: f_k85zzkgb0

ZGlmZiAtLWdpdCBhL2RyaXZlcnMvdXNiL2R3YzMvZHdjMy1tZXNvbi1nMTJhLmMgYi9kcml2ZXJz
L3VzYi9kd2MzL2R3YzMtbWVzb24tZzEyYS5jCmluZGV4IDc0ZDA3ZGVkOGE3ZS4uZDJhNjBlNDAw
NjFkIDEwMDY0NAotLS0gYS9kcml2ZXJzL3VzYi9kd2MzL2R3YzMtbWVzb24tZzEyYS5jCisrKyBi
L2RyaXZlcnMvdXNiL2R3YzMvZHdjMy1tZXNvbi1nMTJhLmMKQEAgLTIzMywxMiArMjMzLDE5IEBA
IHN0YXRpYyBpbnQgZHdjM19tZXNvbl9nMTJhX3VzYjJfaW5pdChzdHJ1Y3QgZHdjM19tZXNvbl9n
MTJhICpwcml2KQogCQlwcml2LT5vdGdfcGh5X21vZGUgPSBQSFlfTU9ERV9VU0JfSE9TVDsKIAog
CWZvciAoaSA9IDA7IGkgPCBwcml2LT5kcnZkYXRhLT5udW1fcGh5czsgKytpKSB7CisJCWVudW0g
cGh5X21vZGUgbW9kZTsKKwogCQlpZiAoIXByaXYtPnBoeXNbaV0pCiAJCQljb250aW51ZTsKIAog
CQlpZiAoIXN0cnN0cihwcml2LT5kcnZkYXRhLT5waHlfbmFtZXNbaV0sICJ1c2IyIikpCiAJCQlj
b250aW51ZTsKIAorCQlpZiAocHJpdi0+ZHJ2ZGF0YS0+b3RnX3N3aXRjaF9zdXBwb3J0ZWQgJiYg
aSA9PSBVU0IyX09UR19QSFkpCisJCQltb2RlID0gcHJpdi0+b3RnX3BoeV9tb2RlOworCQllbHNl
CisJCQltb2RlID0gUEhZX01PREVfVVNCX0hPU1Q7CisKIAkJcmV0ID0gcHJpdi0+ZHJ2ZGF0YS0+
dXNiMl9pbml0X3BoeShwcml2LCBpLCBtb2RlKTsKIAkJaWYgKHJldCkKIAkJCXJldHVybiByZXQ7
Cg==
--000000000000dec06e05a19aa2dd--