linux-kernel - Re: [RFCv1 6/8] phy: amlogic: meson8b-usb2: Use phy reset callback function

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]

Message-ID: <CAFBinCD91eYNXSqnmDKoAvJHWqqRbXVjnaq9RuRNCnip9kKqkw@mail.gmail.com>
Date:   Tue, 22 Jun 2021 22:11:55 +0200
From:   Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To:     Anand Moon <linux.amoon@...il.com>
Cc:     Kishon Vijay Abraham I <kishon@...com>,
        Vinod Koul <vkoul@...nel.org>,
        Neil Armstrong <narmstrong@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        linux-phy@...ts.infradead.org,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
        linux-amlogic@...ts.infradead.org,
        Linux Kernel <linux-kernel@...r.kernel.org>
Subject: Re: [RFCv1 6/8] phy: amlogic: meson8b-usb2: Use phy reset callback function

Hi Anand,

On Mon, Jun 21, 2021 at 9:16 AM Anand Moon <linux.amoon@...il.com> wrote:
[...]
> Ok Thanks for the inputs. got your point.
>
> I was also looking into Amlogic source code for reset. (aml_cbus_update_bits)
> [0] https://github.com/khadas/linux/blob/khadas-vims-4.9.y/drivers/amlogic/usb/phy/phy-aml-new-usb.c
> is there some feature to iomap the USB with cbus?
for that specific code: that's what we do inside drivers/reset/reset-meson.c
Amlogic's vendor kernel uses an increment of 4 bytes per value, so
0x1102 translates to 0x4408

then in mainline's meson8b.dtsi we have:
    compatible = "amlogic,meson8b-reset";
    reg = <0x4404 0x9c>;
as you can see 0x4408 is part of the reset controller node.

next in include/dt-bindings/reset/amlogic,meson8b-reset.h we have:
    #define RESET_USB_OTG                 34

the register used for reset line 34 is translated using:
    0x4404 (first register) + 4 (4 * reset line / 32 = 1) = 0x4408
then the bit inside this register is translated using:
    reset line % 32 = 2

that's how we express aml_cbus_update_bits(0x1102, 0x1<<2, 0x1<<2); in
the mainline kernel (by going through the reset subsystem)

[...]
> > > > > -       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
> > > > > +       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, "phy");
> > > > I think this breaks compatibility with existing .dtbs and our
> > > > dt-bindings (as we're not documenting a "reset-names" property).
> > > > What is the goal of this one?
> > > >
> > >
> > > OK, If we pass NULL over here there is the possibility
> > > USB phy will not get registered.
> > I don't understand why - with NULL everything is working fine for me.
> > Also no matter which name you give to the reset line (in reset-names),
> > it will be the same reset line in all cases. If it's the same reset
> > line before and after: why is this needed?
> >
> I need to investigate this reset feature. With my setup with current changes
> after I update the below.
> -       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, "phy");
> +       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
>         if (PTR_ERR(priv->reset) == -EPROBE_DEFER)
>                 return PTR_ERR(priv->reset);
>
> Reset will break the USB initialization, see below output.
interesting, I have not seen that USB problem before and neither is
Kernel CI seeing it: [0]
Is it only happening with this patch or did you also see it before?


Best regards,
Martin


[0] https://storage.staging.kernelci.org/next/master/next-20210617/arm/multi_v7_defconfig+ltp-ima/gcc-8/lab-baylibre/baseline-meson8b-odroidc1.html