Re: [PATCH v3 2/4] misc: hisi_hikey_usb: change the DT schema

[John Stultz]

On Thu, Sep 2, 2021 at 4:28 AM Mauro Carvalho Chehab
<mchehab+huawei@xxxxxxxxxx> 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@xxxxxxxxxx>

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@xxxxxxxxxx>

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