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=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 70CD6C433E1 for ; Tue, 7 Jul 2020 18:55:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4F291206CD for ; Tue, 7 Jul 2020 18:55:19 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=googlemail.com header.i=@googlemail.com header.b="MSqOBX8i" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728692AbgGGSzS (ORCPT ); Tue, 7 Jul 2020 14:55:18 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:37592 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728029AbgGGSzR (ORCPT ); Tue, 7 Jul 2020 14:55:17 -0400 Received: from mail-ej1-x642.google.com (mail-ej1-x642.google.com [IPv6:2a00:1450:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4CD8BC061755; Tue, 7 Jul 2020 11:55:17 -0700 (PDT) Received: by mail-ej1-x642.google.com with SMTP id y10so47719109eje.1; Tue, 07 Jul 2020 11:55:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RMd8tTCfjNB1oolYgjCGnbvueA8plaMd/4ysVp4hew8=; b=MSqOBX8iZf3N6uIy8Wnm5L4m29/XEr8WCDGWKf500fbPsnl0xzYC6LVlONOf42h0CR I4Rd/idVHgRuoGFtY+arKp+gSn5l1NRpVZPWVHS7fUt71j1qB3v2jBGggVQKiUaXawDQ qNM+I9ABuRJwyg5qaqOAP2v7E/mgtgDyAf07ig2saLp6YrXGZCFFphghvpbv74afVMS8 hgukBbVyPl5VtFibxfDALfdkB8feg3dsrXD3cpQ8FTKR3WiUU7+WwLET7Fw8fB0Y9Ntx fJHk1hgmFv1SXz42n9wzYn1nyBLlwu2BiOkVLXgOydApO9SPolevYiuOnuXVadp33hpE sucw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RMd8tTCfjNB1oolYgjCGnbvueA8plaMd/4ysVp4hew8=; b=EI62LqfxVmuWGRR0riabauCOYQMb4pWuSeGO6Vwy3Sm/tF38c1j1InDDIDN2+uYtRb Bw2u14sPc85z2UnwPVqXcMP762pEF7XFlFRlDKRVmr43PcD5PHgtlas6Y9NY279IoKW8 7CLVIkWIC+D0EOnUXhmYpR/yYk8oL68LecFc7BVlIelTbzuY0S0IulqE6+ow+7FhzXRz QnUD6FaaesDykqb84NymO7yoJDnvREnXIzp2GWoMtrfA986EyXqRZ8k4o/NcaC46Dmux haieheaV1l4GWN0m2TKarMJfHzEyKg5R7rhJw6CGsWSo3SWFNyM4SVdM+OEkZbyLGEJa 083A== X-Gm-Message-State: AOAM532e/2irmO3SuowHNQiiybJuDkAjP0mmHGxNzzZue2q4SzFcudtR rXr38tnsOduxVbKxFnVW+siAJuTZtjNgg4jdgtE= X-Google-Smtp-Source: ABdhPJyGaJyOnxq0gk8ybfmAthcx/e6Mw+KIY+a9E8de0FSUviN2dLI02eWmz8T4W3gH7eX3EhiPg+3nhWg5mmVWaSQ= X-Received: by 2002:a17:906:2287:: with SMTP id p7mr49705831eja.537.1594148115920; Tue, 07 Jul 2020 11:55:15 -0700 (PDT) MIME-Version: 1.0 References: <20200616140717.28465-2-amelie.delaunay@st.com> <20200704174219.612060-1-martin.blumenstingl@googlemail.com> <05a81997-5ddb-ea81-7a89-8078b8a2b610@st.com> In-Reply-To: <05a81997-5ddb-ea81-7a89-8078b8a2b610@st.com> From: Martin Blumenstingl Date: Tue, 7 Jul 2020 20:55:05 +0200 Message-ID: Subject: Re: [PATCH 1/3] usb: dwc2: override PHY input signals with usb role switch support To: Amelie DELAUNAY Cc: Alexandre TORGUE , "balbi@kernel.org" , "devicetree@vger.kernel.org" , Fabrice GASNIER , "gregkh@linuxfoundation.org" , "hminas@synopsys.com" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-stm32@st-md-mailman.stormreply.com" , "linux-usb@vger.kernel.org" , "mcoquelin.stm32@gmail.com" , "robh+dt@kernel.org" Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Amelie, On Tue, Jul 7, 2020 at 6:13 PM Amelie DELAUNAY wrote: > > Hi Martin, > > On 7/4/20 7:42 PM, Martin Blumenstingl wrote: > > Hello Amelie, > > > > thank you for this patch - I am hoping that it will help us on Amlogic > > Meson8, Meson8b, Meson8m2 and GXBB SoCs as well. > > On these SoCs the ID detection is performed by the PHY IP and needs to > > be polled. > > I think usb_role_switch is the perfect framework for this on dwc2 side. > > For the PHY driver I'm going to implement the cable state using the > > extcon framework and then having a new usb-conn-extcon driver. This is > > just to give you an overview why I'm interested in this. > > > > I'm wondering, why use extcon framework and not the usb role switch API > ? This patch on dwc2 is tested on STM32MP157C-DK2 board with STUSB160x > Type-C controller driver recently pushed with usb role switch. You can > have a look here https://lore.kernel.org/patchwork/patch/1256238/. one of the boards that I'm working on is for example the Odroid-C1. It has a Micro-USB port and there's no Type-C controller present. in the next few days I'll try to send my idea as RFC, but this is the .dts I've come up with so far: &usb0 { dr_mode = "otg"; usb-role-switch; connector { compatible = "extcon-usb-b-connector", "usb-b-connector"; type = "micro"; extcon = <&usb0_phy>; vbus-supply = <&usb_vbus>; }; }; I did this for two reasons: 1. I think the PHY is not a connector and thus it's driver shouldn't implement any connector specific logic (managing VBUS) 2. without the connector there would be a circular dependency: the USB controller needs the PHY to initialize but the PHY would need the USB controller so it can manage the role switch (or in other words: the connector replaces the Type-C controller in this case) > > [...] > >> +static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role) > >> +{ > >> + struct dwc2_hsotg *hsotg = usb_role_switch_get_drvdata(sw); > >> + unsigned long flags; > >> + > >> + /* Skip session not in line with dr_mode */ > >> + if ((role == USB_ROLE_DEVICE && hsotg->dr_mode == USB_DR_MODE_HOST) || > >> + (role == USB_ROLE_HOST && hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)) > >> + return -EINVAL; > >> + > >> + /* Skip session if core is in test mode */ > >> + if (role == USB_ROLE_NONE && hsotg->test_mode) { > >> + dev_dbg(hsotg->dev, "Core is in test mode\n"); > >> + return -EBUSY; > >> + } > >> + > >> + spin_lock_irqsave(&hsotg->lock, flags); > > due to this spin_lock_irqsave() ... > > > >> + if (role == USB_ROLE_HOST) { > >> + if (dwc2_ovr_avalid(hsotg, true)) > >> + goto unlock; > >> + > >> + if (hsotg->dr_mode == USB_DR_MODE_OTG) > >> + /* > >> + * This will raise a Connector ID Status Change > >> + * Interrupt - connID A > >> + */ > >> + dwc2_force_mode(hsotg, true); > > ... we cannot sleep in here. the call flow is: > > dwc2_drd_role_sw_set > > spin_lock_irqsave > > dwc2_force_mode > > dwc2_wait_for_mode > > usleep_range > > > > In fact, with the avalid or bvalid overriding + the debounce filter > bypass, GINTSTS_CURMOD is already in the expected mode, so that we exit > the loop directly, without running into usleep_range. on my Amlogic SoC this is not the case: The kernel complains because of that usleep_range from within the spinlock context Please let me know if/how I can help debug this. [...] > > +int dwc2_drd_init(struct dwc2_hsotg *hsotg) > > +{ > > + struct usb_role_switch_desc role_sw_desc = {0}; > > + struct usb_role_switch *role_sw; > > + int ret; > > + > > + if (!device_property_read_bool(hsotg->dev, "usb-role-switch")) > > + return 0; > > should we also return early here if dr_mode != "otg"? > > > > No, because when VBUS is not connected to the controller, you also need > to get the usb_role_none from the usb-role-switch to catch the > unattached state (also in Peripheral or Host only mode). I see - thank you for the explanation! Best regards, Martin 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=-0.7 required=3.0 tests=DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED,DKIM_VALID,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS 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 55225C433DF for ; Tue, 7 Jul 2020 18:56:42 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 21845206CD for ; Tue, 7 Jul 2020 18:56:42 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="h0etgVgh"; dkim=fail reason="signature verification failed" (2048-bit key) header.d=googlemail.com header.i=@googlemail.com header.b="MSqOBX8i" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 21845206CD Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=googlemail.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=Pst+0bF5uzNoNqiGcSKwB0n6hhFYvKiJuEmqSBVpbek=; b=h0etgVghziPDZXDj1iqac8IWT gWKJLm8RFYLJkz+16Fzy76PaFnN6u45dxCGIc3jp4JuSzCI8PZiFXpZNx3IQeQ02KGbN2jQyg97WG HjBXCF2o7Bb0cssWxUb0rsolYiDurDVa3f5xkM8vUMDkpMf3m1g4/4hbv7j6nOjcHAAW2nxk0UYsc d0pyC487N3S83bS9EztIb7V7cBctdsImJGYMF6Ca0348tPlenOajO16X6EshexDnZkuqFIpv+kcrX tiY6+GRr22OXOyZ+I3IhpLv8I2qwetTFyfQDKqj8lGNwOkj3Z85BpVknLKR35uVhKf5gwkhY8afi5 iibQagepA==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsska-0004gi-Be; Tue, 07 Jul 2020 18:55:28 +0000 Received: from mail-ej1-x642.google.com ([2a00:1450:4864:20::642]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jsskQ-0004cB-Oo for linux-arm-kernel@lists.infradead.org; Tue, 07 Jul 2020 18:55:19 +0000 Received: by mail-ej1-x642.google.com with SMTP id ga4so47691858ejb.11 for ; Tue, 07 Jul 2020 11:55:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RMd8tTCfjNB1oolYgjCGnbvueA8plaMd/4ysVp4hew8=; b=MSqOBX8iZf3N6uIy8Wnm5L4m29/XEr8WCDGWKf500fbPsnl0xzYC6LVlONOf42h0CR I4Rd/idVHgRuoGFtY+arKp+gSn5l1NRpVZPWVHS7fUt71j1qB3v2jBGggVQKiUaXawDQ qNM+I9ABuRJwyg5qaqOAP2v7E/mgtgDyAf07ig2saLp6YrXGZCFFphghvpbv74afVMS8 hgukBbVyPl5VtFibxfDALfdkB8feg3dsrXD3cpQ8FTKR3WiUU7+WwLET7Fw8fB0Y9Ntx fJHk1hgmFv1SXz42n9wzYn1nyBLlwu2BiOkVLXgOydApO9SPolevYiuOnuXVadp33hpE sucw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RMd8tTCfjNB1oolYgjCGnbvueA8plaMd/4ysVp4hew8=; b=gUVcYooXwaq2OjRjOWIhsIpOiq8xYSDQ75G+RqM6XvnY09vYOd3IF/5w1hkuGi+pqS lctpy21SkxUfktPFP/l6mpgcH/RCjKIUtAbIfyMwPNIYx4OHw7VkUjGn6h3Zn9N8SBCd rPoicOm/S4Rc+/IVRF+Q3x1SjyaAofRQ4rHCjFUVYgenJfKVKY1qlMFmvrxHDZUhZSsm /H3vln3Vh9CBZa4cYmn8kCkAsQvBkEhhEyJG7+N1D0utxV1z+j4Sl/CP6TZojSrlxVhq fZ9IrwBZOx4Zve6+Wf1KlAJbt2WNFcbJkQHuDyVevfW+X+WbOvibpv+ZdXwKXc9D/dCK i+Sw== X-Gm-Message-State: AOAM530yXNoAWh5F4yd5DQPVBah7BLdBlw5IeqyWsSSJYcOzVHkEvkJ3 +VISOTkkj8vD02V/ipmoyp638wpt2AgHIoCQs9Q= X-Google-Smtp-Source: ABdhPJyGaJyOnxq0gk8ybfmAthcx/e6Mw+KIY+a9E8de0FSUviN2dLI02eWmz8T4W3gH7eX3EhiPg+3nhWg5mmVWaSQ= X-Received: by 2002:a17:906:2287:: with SMTP id p7mr49705831eja.537.1594148115920; Tue, 07 Jul 2020 11:55:15 -0700 (PDT) MIME-Version: 1.0 References: <20200616140717.28465-2-amelie.delaunay@st.com> <20200704174219.612060-1-martin.blumenstingl@googlemail.com> <05a81997-5ddb-ea81-7a89-8078b8a2b610@st.com> In-Reply-To: <05a81997-5ddb-ea81-7a89-8078b8a2b610@st.com> From: Martin Blumenstingl Date: Tue, 7 Jul 2020 20:55:05 +0200 Message-ID: Subject: Re: [PATCH 1/3] usb: dwc2: override PHY input signals with usb role switch support To: Amelie DELAUNAY X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200707_145518_865562_8E2A89B0 X-CRM114-Status: GOOD ( 32.29 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "balbi@kernel.org" , Alexandre TORGUE , "devicetree@vger.kernel.org" , "gregkh@linuxfoundation.org" , "linux-usb@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "robh+dt@kernel.org" , "mcoquelin.stm32@gmail.com" , "hminas@synopsys.com" , Fabrice GASNIER , "linux-stm32@st-md-mailman.stormreply.com" , "linux-arm-kernel@lists.infradead.org" Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Amelie, On Tue, Jul 7, 2020 at 6:13 PM Amelie DELAUNAY wrote: > > Hi Martin, > > On 7/4/20 7:42 PM, Martin Blumenstingl wrote: > > Hello Amelie, > > > > thank you for this patch - I am hoping that it will help us on Amlogic > > Meson8, Meson8b, Meson8m2 and GXBB SoCs as well. > > On these SoCs the ID detection is performed by the PHY IP and needs to > > be polled. > > I think usb_role_switch is the perfect framework for this on dwc2 side. > > For the PHY driver I'm going to implement the cable state using the > > extcon framework and then having a new usb-conn-extcon driver. This is > > just to give you an overview why I'm interested in this. > > > > I'm wondering, why use extcon framework and not the usb role switch API > ? This patch on dwc2 is tested on STM32MP157C-DK2 board with STUSB160x > Type-C controller driver recently pushed with usb role switch. You can > have a look here https://lore.kernel.org/patchwork/patch/1256238/. one of the boards that I'm working on is for example the Odroid-C1. It has a Micro-USB port and there's no Type-C controller present. in the next few days I'll try to send my idea as RFC, but this is the .dts I've come up with so far: &usb0 { dr_mode = "otg"; usb-role-switch; connector { compatible = "extcon-usb-b-connector", "usb-b-connector"; type = "micro"; extcon = <&usb0_phy>; vbus-supply = <&usb_vbus>; }; }; I did this for two reasons: 1. I think the PHY is not a connector and thus it's driver shouldn't implement any connector specific logic (managing VBUS) 2. without the connector there would be a circular dependency: the USB controller needs the PHY to initialize but the PHY would need the USB controller so it can manage the role switch (or in other words: the connector replaces the Type-C controller in this case) > > [...] > >> +static int dwc2_drd_role_sw_set(struct usb_role_switch *sw, enum usb_role role) > >> +{ > >> + struct dwc2_hsotg *hsotg = usb_role_switch_get_drvdata(sw); > >> + unsigned long flags; > >> + > >> + /* Skip session not in line with dr_mode */ > >> + if ((role == USB_ROLE_DEVICE && hsotg->dr_mode == USB_DR_MODE_HOST) || > >> + (role == USB_ROLE_HOST && hsotg->dr_mode == USB_DR_MODE_PERIPHERAL)) > >> + return -EINVAL; > >> + > >> + /* Skip session if core is in test mode */ > >> + if (role == USB_ROLE_NONE && hsotg->test_mode) { > >> + dev_dbg(hsotg->dev, "Core is in test mode\n"); > >> + return -EBUSY; > >> + } > >> + > >> + spin_lock_irqsave(&hsotg->lock, flags); > > due to this spin_lock_irqsave() ... > > > >> + if (role == USB_ROLE_HOST) { > >> + if (dwc2_ovr_avalid(hsotg, true)) > >> + goto unlock; > >> + > >> + if (hsotg->dr_mode == USB_DR_MODE_OTG) > >> + /* > >> + * This will raise a Connector ID Status Change > >> + * Interrupt - connID A > >> + */ > >> + dwc2_force_mode(hsotg, true); > > ... we cannot sleep in here. the call flow is: > > dwc2_drd_role_sw_set > > spin_lock_irqsave > > dwc2_force_mode > > dwc2_wait_for_mode > > usleep_range > > > > In fact, with the avalid or bvalid overriding + the debounce filter > bypass, GINTSTS_CURMOD is already in the expected mode, so that we exit > the loop directly, without running into usleep_range. on my Amlogic SoC this is not the case: The kernel complains because of that usleep_range from within the spinlock context Please let me know if/how I can help debug this. [...] > > +int dwc2_drd_init(struct dwc2_hsotg *hsotg) > > +{ > > + struct usb_role_switch_desc role_sw_desc = {0}; > > + struct usb_role_switch *role_sw; > > + int ret; > > + > > + if (!device_property_read_bool(hsotg->dev, "usb-role-switch")) > > + return 0; > > should we also return early here if dr_mode != "otg"? > > > > No, because when VBUS is not connected to the controller, you also need > to get the usb_role_none from the usb-role-switch to catch the > unattached state (also in Peripheral or Host only mode). I see - thank you for the explanation! Best regards, Martin _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel