linux-kernel - Re: [PATCH v3 2/4] misc: hisi_hikey

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Sep 2021 11:45:28 -0700
From:   John Stultz <john.stultz@...aro.org>
To:     Mauro Carvalho Chehab <mchehab+huawei@...nel.org>
Cc:     Rob Herring <robh@...nel.org>, linuxarm@...wei.com,
        mauro.chehab@...wei.com, Arnd Bergmann <arnd@...db.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/4] misc: hisi_hikey_usb: change the DT schema

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