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 X-Spam-Level: X-Spam-Status: No, score=-2.2 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 20D61ECE58E for ; Thu, 17 Oct 2019 16:34:59 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id E9A0821835 for ; Thu, 17 Oct 2019 16:34:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="TD9Wgvrs" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org E9A0821835 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=atomide.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=GIScA41N00UDoopgrZ2Vs8hoYWzRgfzZMbMEnkHHdiI=; b=TD9WgvrsDn4VRt pxdM0MSpJSEh92P8/RIz32tbxGuAF29b1x3lq22BRxL1e7M3aVnL2fIvHwC3NYg1f7ddFlbU3NoXN Q9rwWs+uZmfc67UWSE45VuswsKkov4BLF68CV1nuF8Bdc39ifVmY7N94G3oE0QlJmM59tRuFAxE/V fSnpgFrkaPK+tojkBjmsI1gSKr/qCSrRUsg7CsU0y7RndAh40t+zLm2QrHDbbbkh1KJnQwTfCaKNk QzD011w+Sn+PWVssiw3SpDscj3yh35lRIG/OVo/et1uXD3PQR3dphlyCnW08PA5HxlDaD1N9gt9sv gVz4k47NsyjC8aZbUzaA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iL8jq-0000uA-5O; Thu, 17 Oct 2019 16:34:58 +0000 Received: from muru.com ([72.249.23.125]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1iL8jZ-0000XB-RK; Thu, 17 Oct 2019 16:34:44 +0000 Received: from atomide.com (localhost [127.0.0.1]) by muru.com (Postfix) with ESMTPS id C458C804F; Thu, 17 Oct 2019 16:35:10 +0000 (UTC) Date: Thu, 17 Oct 2019 09:34:33 -0700 From: Tony Lindgren To: min.guo@mediatek.com Subject: Re: [PATCH v8 6/6] usb: musb: Add support for MediaTek musb controller Message-ID: <20191017163433.GN5610@atomide.com> References: <20191017094126.29045-1-min.guo@mediatek.com> <20191017094126.29045-7-min.guo@mediatek.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20191017094126.29045-7-min.guo@mediatek.com> User-Agent: Mutt/1.12.1 (2019-06-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20191017_093441_976095_846456D3 X-CRM114-Status: GOOD ( 12.30 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , devicetree@vger.kernel.org, Yonglong Wu , hdegoede@redhat.com, Greg Kroah-Hartman , linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, chunfeng.yun@mediatek.com, Rob Herring , Alan Stern , Matthias Brugger , linux-mediatek@lists.infradead.org, Bin Liu , linux-arm-kernel@lists.infradead.org Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Hi, Just few comments for future changes that might help below. * min.guo@mediatek.com [191017 09:42]: > --- /dev/null > +++ b/drivers/usb/musb/mediatek.c > +static int musb_usb_role_sx_set(struct device *dev, enum usb_role role) > +{ > + struct mtk_glue *glue = dev_get_drvdata(dev); > + struct musb *musb = glue->musb; > + u8 devctl = readb(musb->mregs + MUSB_DEVCTL); > + enum usb_role new_role; > + > + if (role == glue->role) > + return 0; > + > + switch (role) { > + case USB_ROLE_HOST: > + musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE; > + glue->phy_mode = PHY_MODE_USB_HOST; > + new_role = USB_ROLE_HOST; > + if (glue->role == USB_ROLE_NONE) > + phy_power_on(glue->phy); > + > + devctl |= MUSB_DEVCTL_SESSION; > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); > + MUSB_HST_MODE(musb); > + break; > + case USB_ROLE_DEVICE: > + musb->xceiv->otg->state = OTG_STATE_B_IDLE; > + glue->phy_mode = PHY_MODE_USB_DEVICE; > + new_role = USB_ROLE_DEVICE; > + devctl &= ~MUSB_DEVCTL_SESSION; > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); > + if (glue->role == USB_ROLE_NONE) > + phy_power_on(glue->phy); > + > + MUSB_DEV_MODE(musb); > + break; > + case USB_ROLE_NONE: > + glue->phy_mode = PHY_MODE_USB_OTG; > + new_role = USB_ROLE_NONE; > + devctl &= ~MUSB_DEVCTL_SESSION; > + musb_writeb(musb->mregs, MUSB_DEVCTL, devctl); > + if (glue->role != USB_ROLE_NONE) > + phy_power_off(glue->phy); > + > + break; > + default: > + dev_err(glue->dev, "Invalid State\n"); > + return -EINVAL; > + } > + > + glue->role = new_role; > + phy_set_mode(glue->phy, glue->phy_mode); > + > + return 0; > +} For the role change, I recently posted a patch "[PATCH 4/7] usb: musb: Add musb_set_host and peripheral and use them for omap2430". That should work for you looking at the code above, so later on you might want to change to use that. Probably best done as a follow-up patch to avoid adding extra dependencies to your series. Please also note that musb core attempts to do things automagically on it's own. So trying to force mode in general does not work reliably. This is because VBUS may not yet have risen for example. The role change is best done based on the USB PHY as then usually musb has already switched to the right mode automatically :) > +static const struct musb_platform_ops mtk_musb_ops = { > + .quirks = MUSB_DMA_INVENTRA, > + .init = mtk_musb_init, > + .get_toggle = mtk_musb_get_toggle, > + .set_toggle = mtk_musb_set_toggle, > + .exit = mtk_musb_exit, > +#ifdef CONFIG_USB_INVENTRA_DMA > + .dma_init = musbhs_dma_controller_create_noirq, > + .dma_exit = musbhs_dma_controller_destroy, > +#endif > + .clearb = mtk_musb_clearb, > + .clearw = mtk_musb_clearw, > + .busctl_offset = mtk_musb_busctl_offset, > + .set_mode = mtk_musb_set_mode, > +}; So you may want to consider getting rid of .set_mode completely and rely on USB PHY calls instead. In some cases you need to use struct phy_companion for set_vbus depending how things are wired. Regards, Tony _______________________________________________ Linux-mediatek mailing list Linux-mediatek@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-mediatek