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

From: Anand Moon <linux.amoon@gmail.com>
To: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
	Vinod Koul <vkoul@kernel.org>,
	 Neil Armstrong <narmstrong@baylibre.com>,
	Kevin Hilman <khilman@baylibre.com>,
	 Jerome Brunet <jbrunet@baylibre.com>,
	Philipp Zabel <p.zabel@pengutronix.de>,
	 linux-phy@lists.infradead.org,
	 linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
	linux-amlogic@lists.infradead.org,
	 Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [RFCv1 6/8] phy: amlogic: meson8b-usb2: Use phy reset callback function
Date: Sat, 3 Jul 2021 00:43:19 +0530	[thread overview]
Message-ID: <CANAwSgSfzwYTtAFeKSxGUW=rtON42AObrXGsRTb31WevFKJQLA@mail.gmail.com> (raw)
In-Reply-To: <CAFBinCD+seUuYUrObiqJNgEi_DaULB3nU+QQFHz9d85dAU06ZA@mail.gmail.com>

Hi Martin,

On Mon, 28 Jun 2021 at 01:55, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
>
> Hi Anand,
>
> On Sun, Jun 27, 2021 at 10:07 PM Anand Moon <linux.amoon@gmail.com> wrote:
> [...]
> > Sorry for the delay.
> > We could switch the reset logic to
> > *devm_reset_control_get_optional_exclusive* as below
> > to fix the reset line, since both the dwc2 c90c0000.usb and c9040000.usb
> > will have their own context to reset control register, it means the
> > reset line is not share
> > between two USB PHY nodes.
> This is something I don't understand.
> As discussed in our previous mails reset_control_reset in case of the
> USB PHY driver (which uses the RESET_USB_OTG reset line for *both*
> PHYs) is equivalent to the following code in the vendor kernel:
>   aml_cbus_update_bits(0x1102, 0x1<<2, 0x1<<2)
>
> We have two PHYs but only one reset line. So in my own words I
> describe the reset line as being shared.
>
> >
> > -       priv->reset = devm_reset_control_get_optional_shared(&pdev->dev, NULL);
> > +       priv->reset = devm_reset_control_get_optional_exclusive(&pdev->dev,
> > +                                                               "reset");
> Have you boot-tested this?
> Without any .dts changes this will return NULL because there's no
> reset-names = "reset"; in the .dts(i).
> If you replace "reset" with NULL then I assume that the second PHY
> will fail to obtain the reset line because it's shared between two
> devices but we're trying to obtain it exclusively for both (PHYs).
>
Thanks for your review comments.

I have always tested with both the phy enable and with proper DTS changes.
Yes, it gives false-positive results while initialization of the USB PHY.
Odroid C2 it will pass but on Odroid C1 it will fail kid off.

But it seems to me that the order of the PHY reset is kind of a problem.

Thanks for looking into my changes.
>
> Best regards,
> Martin

Thanks
-Anand

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy