On Mon, Dec 26, 2022 at 03:17:12AM +0000, Jun Li wrote:
> > From: Jun Li
> > Sent: Thursday, November 3, 2022 7:53 PM
> > > From: Francesco Dolcini <francesco@xxxxxxxxxx>
> > > Sent: Thursday, November 3, 2022 1:59 AM
> > > On Wed, Nov 02, 2022 at 10:12:42AM +0000, Jun Li wrote:
> > > > > > On Tue, Nov 01, 2022 at 03:10:46AM +0000, Jun Li wrote:
> > > > > > > > On Mon, Oct 31, 2022 at 01:40:39PM +0000, Jun Li (OSS) wrote:
> > > > > > > > > > I am debugging some unexpected USB behavior on a
> > > > > > > > > > i.MX6ULL SOC, chipidea controller ("fsl,imx6ul-usb") and
> > > > > > > > > > a fsl mxs usbphy ("fsl,imx6ul-usbphy").
> > > > > > > > > >
> > > > > > > > > > The HW design has 2 USB interface, the first one is
> > > > > > > > > > dual-role, while the second one is a host port with NO
> > > > > > > > > > way to re-read
> > > the
> > > > > > > > > > VBUS (USB_OTG2_VBUS is not really connected, there is
> > > > > > > > > > just
> > > a
> > > > > > > > > > capacitor to GND).
> > > > > > > > >
> > > > > > > > > How is your USB_OTG1_VBUS status? Can you try to make your
> > > > > > > > > USB_OTG1_VBUS pad has a valid VBUS voltage, then run your
> > > > > > > > > Host only port test with runtime PM enabled?
> > > > > > > >
> > > > > > > > USB_OTG1_VBUS is tied to GND the same way as USB_OTG2_VBUS,
> > > > > > > > not really straightforward to do such a test.
> > > > > > >
> > > > > > > iMX6ULL need at least one valid VBUS(either from OTG1 or OTG2)
> > > > > > > as input to power the internal USB LDO if I understand correctly.
> > > > > > This surprise me a little bit, since
> > > > > > - the i.MX6ULL datasheet prescribe to keep the VBUS disconnected
> > if
> > > > > > unused
> > > > >
> > > > > I think "unused" here means you do not need/enable the port at all.
> > > > >
> > > > > > - downstream NXP kernel seems to work fine ("seems" since we do
> > have
> > > > > > some patches there, so I could be wrong)
> > > > >
> > > > > What do you mean by " downstream NXP kernel seems to work fine"?
> > > > > The downstream kernel can work on your HW? But upstream kernel
> > > > > driver does not?
> > >
> > > Correct, NXP downstream kernel is working fine, upstream kernel
> > > requires runtime PM disabled to work correctly.
> > >
> > > > > > - disabling runtime pm on upstream Linux kernel make it works
> > > > > > perfectly, so there is a way in SW to have this HW configuration
> > > > > > working.
> > > > >
> > > > > Again I want to make sure the both VBUS pads(OTG1 and OTG2)
> > > > > voltage are always at 0v on your HW, can you double check and confirm?
> > > > > I ask this again because such situation should cause the USB port
> > > > > Cannot work at any cases, but your current status is: only low
> > > > > power wakeup cannot work.
> > > >
> > > > Could you please check the voltage of pad of VDD_USB_CAP on your HW?
> > >
> > > I was about to clarify you this point, it's important and I forgot
> > > about it, sorry about that!
> > >
> > > VDD_USB_CAP in our design is connected to a 3.0V external LDO, voltage
> > > on both VBUS pads is 0V (FYI: this specific hardware design was
> > > validated by NXP hardware engineers).
> >
> > Then the HW design should be fine.
> > I need find time to try the upstream kernel on my iMX6ULL board to check
> > this.
>
> My iMX6ULL EVK board cannot reproduce this issue with upstream kernel.
>
> Could you try to set the bits [7,3] of 0x020c8200(for 2nd USB controller OTG2)
> to be value like 0x1000FC? This may be done at your bootloader(uboot), so
> make sure those targets bits are set before doing your test, or doing this
> with below change(not compiled or tested):
>
> diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
> index d2836ef5d15c..e390ef534a7c 100644
> --- a/drivers/usb/phy/phy-mxs-usb.c
> +++ b/drivers/usb/phy/phy-mxs-usb.c
> @@ -89,6 +89,9 @@
> #define ANADIG_USB1_CHRG_DET_STAT_CHRG_DETECTED BIT(1)
> #define ANADIG_USB1_CHRG_DET_STAT_PLUG_CONTACT BIT(0)
>
> +#define ANADIG_USB2_VBUS_DET_SET 0x204
> +#define ANADIG_USB2_VBUS_DET_VBUS_OVERRIDE GENMASK(7, 3)
> +
> #define ANADIG_USB2_VBUS_DET_STAT 0x220
>
> #define ANADIG_USB1_LOOPBACK_SET 0x1e4
> @@ -288,6 +291,11 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
> if (ret)
> goto disable_pll;
>
> + if (mxs_phy->regmap_anatop && (mxs_phy->port_id == 1))
> + regmap_write(mxs_phy->regmap_anatop,
> + ANADIG_USB2_VBUS_DET_SET,
> + ANADIG_USB2_VBUS_DET_VBUS_OVERRIDE);
> +
> /* Power up the PHY */
> writel(0, base + HW_USBPHY_PWD);
Hello,
I tested your patch and it does not work. I therefore tested a slightly
improved version that really ensure the right register value is written.
[ 8.408564] port=0 reg=0x200 val=0x1000fc
[ 8.440235] port=1 reg=0x204 val=0x1000fc
but it does not work never the less. Unfortunately bits 7-3 are not
documented, so I was not able to do much more.
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index d2836ef5d15c..3ff5489d679e 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -89,6 +89,10 @@
#define ANADIG_USB1_CHRG_DET_STAT_CHRG_DETECTED BIT(1)
#define ANADIG_USB1_CHRG_DET_STAT_PLUG_CONTACT BIT(0)
+#define ANADIG_USB1_VBUS_DET_SET 0x200
+#define ANADIG_USB2_VBUS_DET_SET 0x204
+#define ANADIG_USB1_VBUS_DET_VBUS_OVERRIDE GENMASK(7, 3)
+
#define ANADIG_USB2_VBUS_DET_STAT 0x220
#define ANADIG_USB1_LOOPBACK_SET 0x1e4
@@ -309,6 +313,7 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
writel(BM_USBPHY_IP_FIX, base + HW_USBPHY_IP_SET);
if (mxs_phy->regmap_anatop) {
+ unsigned int val;
unsigned int reg = mxs_phy->port_id ?
ANADIG_USB1_CHRG_DETECT_SET :
ANADIG_USB2_CHRG_DETECT_SET;
@@ -319,6 +324,15 @@ static int mxs_phy_hw_init(struct mxs_phy *mxs_phy)
regmap_write(mxs_phy->regmap_anatop, reg,
ANADIG_USB1_CHRG_DETECT_EN_B |
ANADIG_USB1_CHRG_DETECT_CHK_CHRG_B);
+
+ reg = mxs_phy->port_id ?
+ ANADIG_USB2_VBUS_DET_SET :
+ ANADIG_USB1_VBUS_DET_SET;
+
+ regmap_set_bits(mxs_phy->regmap_anatop, reg,
+ ANADIG_USB1_VBUS_DET_VBUS_OVERRIDE);
+ regmap_read(mxs_phy->regmap_anatop, reg, &val);
+ printk("port=%d reg=0x%x val=0x%x\n", mxs_phy->port_id, reg, val);
}
mxs_phy_tx_init(mxs_phy);
