Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <cover.1630581434.git.mchehab+huawei@kernel.org> <d990e75f28c443c0c5a5fc857b87acc4be3f9464.1630581434.git.mchehab+huawei@kernel.org>
In-Reply-To: <d990e75f28c443c0c5a5fc857b87acc4be3f9464.1630581434.git.mchehab+huawei@kernel.org>
From:   John Stultz <john.stultz@linaro.org>
Date:   Thu, 2 Sep 2021 11:45:28 -0700
Message-ID: <CALAqxLXM=qOk3cGru+x0pfddWViRPdoF67w1xNO8gZdWd7G2uw@mail.gmail.com>
Subject: Re: [PATCH v3 2/4] misc: hisi_hikey_usb: change the DT schema
To:     Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Cc:     Rob Herring <robh@kernel.org>, linuxarm@huawei.com,
        mauro.chehab@huawei.com, Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        lkml <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 2, 2021 at 4:28 AM Mauro Carvalho Chehab
<mchehab+huawei@kernel.org> wrote:
>
> As there's no upstream DT bindings for this driver, let's
> update its DT schema, while it is not too late.
>
> While here, add error messages, in order to help discovering
> problems during probing time.
>
> Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>

I gave this series a spin on the hikey960 booting in a number of
different usb configurations (usb-c gadget, usb-c host, usb-a host) as
well as switching between them and didn't see any issue or new
regressions.  Admittedly not super rigorous but the usb issues we have
seen in the past usually took a number of days to trip over, so it's
probably as good as I can say now.

Tested-by: John Stultz <john.stultz@linaro.org>

I've also added it to my mainline tracking tree and will let you know
if I run into anything over the next few days.

But one nit on the patch below:

> ---
>  drivers/misc/hisi_hikey_usb.c | 81 +++++++++++++++--------------------
>  1 file changed, 35 insertions(+), 46 deletions(-)
>
> diff --git a/drivers/misc/hisi_hikey_usb.c b/drivers/misc/hisi_hikey_usb.c
> index 989d7d129469..8be7d28cdd71 100644
> --- a/drivers/misc/hisi_hikey_usb.c
> +++ b/drivers/misc/hisi_hikey_usb.c
> @@ -248,8 +237,8 @@ static int  hisi_hikey_usb_remove(struct platform_device *pdev)
>  }
>
>  static const struct of_device_id id_table_hisi_hikey_usb[] = {
> -       { .compatible = "hisilicon,gpio_hubv1" },
> -       { .compatible = "hisilicon,kirin970_hikey_usbhub" },
> +       { .compatible = "hisilicon,hikey960-usbhub" },
> +       { .compatible = "hisilicon,hikey970-usbhub" },
>         {}

So with the gpio/regulator change on the hikey960 side, there isn't
any more 970 specific logic, so should we unify the compat string?
Also, I personally would rather use something that wasn't so "branded"
to a specific board should a new device use the same approach (which
is why we switched to the gpio_hubv1 naming instead of "hikey960_usb"
that we had earlier).  This is definitely bikeshed territory, and I'm
not picky, but "hisilicon,usbhub_notifierv1" maybe?

thanks
-john