From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 8F571C433F5 for ; Wed, 12 Jan 2022 08:17:39 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 1293882F91; Wed, 12 Jan 2022 09:17:37 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=walle.cc Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=walle.cc header.i=@walle.cc header.b="Vcz+lp+i"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id B4F2A81429; Wed, 12 Jan 2022 09:17:35 +0100 (CET) Received: from ssl.serverraum.org (ssl.serverraum.org [IPv6:2a01:4f8:151:8464::1:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 5AEA582F91 for ; Wed, 12 Jan 2022 09:17:31 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=walle.cc Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=michael@walle.cc Received: from ssl.serverraum.org (web.serverraum.org [172.16.0.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ssl.serverraum.org (Postfix) with ESMTPSA id A6CAE22247; Wed, 12 Jan 2022 09:17:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=walle.cc; s=mail2016061301; t=1641975449; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=1sQjgd+MAoA+HuGbmxociqPyqhN/6TyZNzpVvYjf4J8=; b=Vcz+lp+iQpsx5oqh1B/O/cNCSnKh94ikuYErZizoXZz0D0rVd/HTXF3buIaevNk0VVarXS 2haEssQFBcxRKZp/5IjTX9ULt1KmxAliMKdr3VdUXmFMy/3xjvq2gfFZX8GlooQjr5RPab uDXWxXEwD4iAEXsGpY19vhMw+95OpKo= MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 12 Jan 2022 09:17:28 +0100 From: Michael Walle To: Adam Ford Cc: Fabio Estevam , Marek Vasut , Simon Glass , U-Boot Mailing List , Tim Harvey Subject: Re: [PATCH V2] usb: ehci-mx6: Enable OTG detection on imx8mm and imx8mn In-Reply-To: References: <20211223140756.304527-1-aford173@gmail.com> <20220104083148.1328623-1-michael@walle.cc> User-Agent: Roundcube Webmail/1.4.12 Message-ID: <6f8b8bccf3eb19742d386acaa9bf59e3@walle.cc> X-Sender: michael@walle.cc X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Am 2022-01-11 17:52, schrieb Adam Ford: > On Tue, Jan 11, 2022 at 9:31 AM Michael Walle wrote: >> >> Hi, >> >> Am 2022-01-11 15:20, schrieb Adam Ford: >> > On Tue, Jan 4, 2022 at 2:32 AM Michael Walle wrote: >> >> > The imx8mm and imx8mn appear compatible with imx7d-usb >> >> > flags in the OTG driver. If the dr_mode is defined as >> >> > host or peripheral, the device appears to operate correctly, >> >> > however the auto host/peripheral detection results in an error. >> >> > >> >> > The solution isn't just adding checks for imx8mm and imx8mn to >> >> > the check for imx7, because the USB clock needs to be running >> >> > to read from the USBNC_PHY_STATUS_OFFSET register or it will hang. >> >> > >> >> > The init_type in both priv and plat data are the same, so it doesn't >> >> > make sense to configure the data in the plat data and copy the >> >> > data to priv when priv can be configured directly. Instead, rename >> >> > ehci_usb_of_to_plat to ehci_usb_dr_mode and call it from the >> >> > probe functions after the clocks are enabled, but before the data >> >> > is required. >> >> > >> >> > With that added, the additional checks for imx8mm and imx8mn will >> >> > allow reading the register to automatically determine the state >> >> > (host or device) of the OTG controller. >> >> > >> >> > Signed-off-by: Adam Ford >> >> > --- >> >> > V2: Rename ehci_usb_of_to_plat to ehci_usb_dr_mode and call it >> >> > from the probe after the clocks are enabled, but before >> >> > the data is needed. >> >> > >> >> > diff --git a/drivers/usb/host/ehci-mx6.c b/drivers/usb/host/ehci-mx6.c >> >> > index 1bd6147c76..f2a34b7f06 100644 >> >> > --- a/drivers/usb/host/ehci-mx6.c >> >> > +++ b/drivers/usb/host/ehci-mx6.c >> >> >> >> .. >> >> >> >> > @@ -639,10 +639,8 @@ static int mx6_parse_dt_addrs(struct udevice *dev) >> >> > >> >> > static int ehci_usb_probe(struct udevice *dev) >> >> > { >> >> > - struct usb_plat *plat = dev_get_plat(dev); >> >> > struct usb_ehci *ehci = dev_read_addr_ptr(dev); >> >> > struct ehci_mx6_priv_data *priv = dev_get_priv(dev); >> >> > - enum usb_init_type type = plat->init_type; >> >> > struct ehci_hccr *hccr; >> >> > struct ehci_hcor *hcor; >> >> > int ret; >> >> > @@ -660,8 +658,6 @@ static int ehci_usb_probe(struct udevice *dev) >> >> > return ret; >> >> > >> >> > priv->ehci = ehci; >> >> > - priv->init_type = type; >> >> >> > >> > Michael, >> > >> >> I'm not sure this is correct. There is also this: >> >> https://elixir.bootlin.com/u-boot/v2022.01-rc4/source/drivers/usb/host/usb-uclass.c#L407 >> >> >> > >> > Further down in the code, you should see: >> > priv->phy_type = usb_get_phy_mode(dev_ofnode(dev)); >> >> But that is just fetching the mode from the device tree, the >> usb_setup_ehci_gadget() referenced above, change the mode during >> runtime by writing the plat->init_type and reprobing the device. >> >> >> Which won't work anymore. usb_setup_ehci_gadget() is called from >> >> usb_gadget_register_driver() in ci_udc.c. The latter is the one used >> >> on the imx, right? But I might be wrong. I'm still trying to figure >> >> out how this all works together, because I also try to get OTG >> >> running on the dwc3 driver. It looks like the ci_udc.c is special >> >> here, and I wonder how a transition to UCLASS_USB_GADGET_GENERIC >> >> might look like for this driver. >> > >> > This driver really isn't changing anything, it's just an order of >> > operations. >> >> It doesn't use the plat->init_type at all anymore, no? > > From what I could tell, the only thing that plat->init_type did was to > set priv->init_type. > The priv structure appears to do most of the work. but plat->init_type seems to change during runtime (by usb_setup_ehci_gadget()) and with this patch applied, it doesn't do that anymore. >> > Previously there was a separate that was being called to >> > determine the init_type as being either a peripheral or host. If it >> > wasn't explicitly set in the device tree, the helper function would >> > query a register, however, on the imx8mm and imx8mn platforms, the >> > clocks were not running which resulted in a hang. What I've done is >> > simply move the helper function into the probe and have it run after >> > the clocks start. In theory the register values will be the same as >> > they would be with the help function still in place. >> > >> > Both Tim Harvey and I have tested on an imx8mm and Fabio tested it on >> > an imx7. For me, the peripheral mode worked when the ID pin of the >> > USB-OTG was not grounded. When it was grounded, the system corrected >> > identified it as a host and I was able to mount a USB drive. >> >> Again, I'm missing something here, but the only user of >> usb_setup_ehci_gadget() is usb/gadget/ci_udc.c. > > I can't speak to what those functions do. What are you suggesting > that I do differently? I'd start by seeing if/when usb_setup_ehci_gadget() is called and see if plat->init_type changes. If its not called, is it dead code then? Sorry I just noticed this, while reviewing how this particular driver works, because I still like to find a working OTG driver in u-boot. I don't have any imx boards. -michael