Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Minas Harutyunyan <Minas.Harutyunyan@synopsys.com>
To:     Fabrice Gasnier <fabrice.gasnier@foss.st.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "quentin.schulz@theobroma-systems.com" 
        <quentin.schulz@theobroma-systems.com>
CC:     "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "amelie.delaunay@foss.st.com" <amelie.delaunay@foss.st.com>,
        "alexandre.torgue@foss.st.com" <alexandre.torgue@foss.st.com>
Subject: RE: [PATCH] usb: dwc2: fix a race, don't power off/on phy for
 dual-role mode
Thread-Topic: [PATCH] usb: dwc2: fix a race, don't power off/on phy for
 dual-role mode
Thread-Index: AQHZV0y86Vo9MmsmT0qa2vDc2LUTSq8Jbprg
Date:   Fri, 24 Mar 2023 05:09:10 +0000
Message-ID: <BYAPR12MB3399A285F4DAF09F2AB18DACA7849@BYAPR12MB3399.namprd12.prod.outlook.com>
References: <20230315144433.3095859-1-fabrice.gasnier@foss.st.com>
In-Reply-To: <20230315144433.3095859-1-fabrice.gasnier@foss.st.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?H4J2RVDrTBEFK4TAjARuVlAIt84hzVByapC/Yn8rG0BR7v32+iikz3z5aV6p?=
 =?us-ascii?Q?zRsrock7AYUMbC6v08+pn+OwcRRceEsUdum2JpRONt6zrqwfXwruu7kcWcy4?=
 =?us-ascii?Q?t8BBEfrBCXSaMGYCNJahechreoiCp6JUDeTAs/BF3H57ArZ1ZPvZfdDV9VG6?=
 =?us-ascii?Q?wgwq8NwqCYuS1XeRT10L3j2ysx0F/KLZLWS6NPqgq7U+KJVaQm1dI3N+AG8X?=
 =?us-ascii?Q?K+Ghvr+0GTGJIVmaLymuq5w2P5vlsPXWaLbZ6kEHfsFY6+ls4TI9KDUzgpaN?=
 =?us-ascii?Q?gIkex+KlD3Ow9rJXb7SB7xtvp6NsCS3zk9b8b15OfjQHgWrh40MHlY4Alcc3?=
 =?us-ascii?Q?v8Gui4v3P0qlCRC9GStQceXLej/KSu7y45TVTsx5l6/xmz4VkpeLksLEK+VK?=
 =?us-ascii?Q?xXh51gbxvxLx909zeAEiQrnj+6nQ0ehmAebbq+hn/fJbYcGwc9QVwTQhWE6e?=
 =?us-ascii?Q?VK8hwzzNrB5ks3I18LBRgXp7SYNwpRSzgMklFF4jWvDAM5JN9vqvSe4nI2dF?=
 =?us-ascii?Q?CEzG47Os6lctFeTIlXaSl8zGB+pZewBh1yGkWKKlBhaDjqYoNStoLT0IkV4y?=
 =?us-ascii?Q?OuHhR+zdltJPnNwZiZMAZyZhD1rY4JmCRczxBDUvvuQQXo5KcCHIL5QUUcCx?=
 =?us-ascii?Q?rmJIhWb5yHlRLSvWYB9r+nFJ1y7RgE4Ofb++jXIhoK+EQ6qUnvFnLBjziOle?=
 =?us-ascii?Q?FYv3AFOe4hAKcf61JjoZxsFlescA0dvOi070Sc2bQ7Zskx90GwtEsPVZ4DEc?=
 =?us-ascii?Q?jLGB4cYLQtAg1/+Pvh5OUhtpJduTThK7yeOCWH21wKznT6d30wzvDnB1ylPz?=
 =?us-ascii?Q?taSQts+/yPptzsqxzMB9jnR4OkypN5M83P9kFA+DsyLpGnUpqadKISQfQ/3x?=
 =?us-ascii?Q?CGUwdpW9rEIcELaHDGrzhAtojgi2yCzSdWl57tacZYEAgxV5bPnSMWSwomIn?=
 =?us-ascii?Q?CjcxVJkcL3BbwYrgdri1PuRku1gMCFy1xYqe+AJpN8hzD6ILES1Se09xqPhJ?=
 =?us-ascii?Q?vxCdqzBFb5yjnViPdNJ258hUo8qXF/JIwlBO4lhodYTjxdf+AxwujJlccvhQ?=
 =?us-ascii?Q?MKPHp3RLnH2G5O55MfayXl/5JGBbG5IZ1hMW9tb8ijN5zxehJIMp7KgQwA2d?=
 =?us-ascii?Q?j4vd8B5O5WFuF6tc1EWHO2GuFB4HoNL8PWtxmyZ1ss3JJBdO1n0P5O5FT8E8?=
 =?us-ascii?Q?uJ5/78TMR2FkuixCoGmwOlSxSr8j3AoBdxiJkufZqO8/vOl2RhxdItKO1RsU?=
 =?us-ascii?Q?SCTyIEZXG9voe5GG2DbAWDzIzNpKoRrE/V4HObPszJ31/Rrdwrvm508iag3I?=
 =?us-ascii?Q?7pJe4Bg8i5Hyk2usy30Tib/Qn0o5m3bfsgTNB2K7igL6A7meIKNkI344oeMF?=
 =?us-ascii?Q?bIvqDBZNMSQAiEo72h1LjF3BKjFTg+PHAl0wZHD1lFkE6qRpWW4Pc6UrGq0R?=
 =?us-ascii?Q?UCNsNduI8+OpMTj3Y5wo+nG2B9nxqLFXM4eqdLFBM/NRBOFc83htWA2kQOfC?=
 =?us-ascii?Q?XuDVdZLetVHj7iGLeD9Gcswu/yO0VBvW8ZIOCcO+KwuFihywKMyNnFOkVmMr?=
 =?us-ascii?Q?hAj72sjhZ1D/NBoLXNRs7kPN46Y/gmd97xDkwRb+?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?E79K2NXS9uQ0/WvAwUBNz3t4qbvorOQYpfJNIlqDl7fZ1+OJAmx5G8OsJkV9?=
 =?us-ascii?Q?0RbwQkdbOzHQTKH04Jlb2BFd1W9+LrkoI+g4u8GO/mDrhXeeTeG2vJsoc8Xi?=
 =?us-ascii?Q?Tar2DxBTKDL/8mCegToDHIt7hROqR2V04+vv9pCrU6Ish4GL+t1a2CfbFSHX?=
 =?us-ascii?Q?oMA1bkocB94PiOozhWtMW7xXKuY7ZBzhW1E+FrUxNBqXno2h2SEulTNdTSJo?=
 =?us-ascii?Q?Lpx0RSvTpmvRl1/wjjGITO/BiQxH/feJhhQAR8WGGNfjjIji4XWydz7XifJZ?=
 =?us-ascii?Q?9QWX1kSfCocgAR2RDdAKHSr0YNOtXojVybx+yobOc9pHi/BleOGZ8MDlMXKF?=
 =?us-ascii?Q?rjrZkU4fbEoxNfs1ooTrCRCoYH/xS52mMHw0h2u/XS9phkpARN/b68yUvoq1?=
 =?us-ascii?Q?8J3bOHzVcxmU4kvGQ3SamXLsTWOw0+ykd1/hEOVRid6TvZdWEjvW3TRaWuCc?=
 =?us-ascii?Q?/XMcp63kQJAk4eRFn2sWtZ4x7HHywJEdVA5Lb1nvHigL7IUxdylA/NGye/Yr?=
 =?us-ascii?Q?OV0mogNSx1o1PZ6k/IycoX7t34nvtSsQOHxUuvEEYPC2hpctqkcXNMNyyhL1?=
 =?us-ascii?Q?OSkN2mUHwowdLWMMmR2c1Dmg4gwoRaMG67fY4tBQU2r57SXyZvh/1WPfGuBu?=
 =?us-ascii?Q?moxgQRY0SeCRpMhuuz7EqpgSl+fwdfxuE6+eMp5ebD0d1WFlZtg7Sdh+bcmB?=
 =?us-ascii?Q?zNiLeXA30rCwNFCsWvK+o0+Qnk2mk5zyzpWXZ30inK7URYppcEdXs5jaV6fr?=
 =?us-ascii?Q?bMQEqGNFF/Eytza4lUEmkdbBvJIokR+f+1tVfgeTaw3auFJtJl2kva34x/m9?=
 =?us-ascii?Q?LHnWiYm78vnBGm/7jn7O4zwGjkJ8V3eC5wEEdLgKFGj1JBfi+xWVtqbU71Uv?=
 =?us-ascii?Q?9B+adq25R9HiJzskhkg0M9uyayo2yfZtFBMfB2kCKa5L8fMrZlxiKcNAYpqM?=
 =?us-ascii?Q?3JuYto3VnucPARsrJ8b2Zss+17J4wy9kTOYAiPLzzXh3W05pYbDp3utEW6mU?=
 =?us-ascii?Q?86UO+TZO3SngmhfFQNSTX2woxg=3D=3D?=
X-OriginatorOrg: synopsys.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BYAPR12MB3399.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c2025573-3d61-4813-3756-08db2c25e6e8
X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Mar 2023 05:09:10.2134
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: c33c9f88-1eb7-4099-9700-16013fd9e8aa
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: SYiyzAlmRG04HHDB216U4fLvPkXOILq/qI2tMwzM5BLRn0/UPNKXq5EX5LjrrWFNwdFK0EEkcon2UR6oWjvI+w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR12MB6946
X-Proofpoint-ORIG-GUID: vMdVOJuvu2jT1hkEmTZZxzeENAB8WcVb
X-Proofpoint-GUID: vMdVOJuvu2jT1hkEmTZZxzeENAB8WcVb
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.942,Hydra:6.0.573,FMLib:17.11.170.22
 definitions=2023-03-24_02,2023-03-23_02,2023-02-09_01
X-Proofpoint-Spam-Details: rule=outbound_active_cloned_notspam policy=outbound_active_cloned score=0
 spamscore=0 clxscore=1011 mlxscore=0 phishscore=0 impostorscore=0
 malwarescore=0 mlxlogscore=999 bulkscore=0 adultscore=0 lowpriorityscore=0
 priorityscore=1501 suspectscore=0 classifier=spam adjust=0 reason=mlx
 scancount=1 engine=8.12.0-2303200000 definitions=main-2303240040
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_NONE,SPF_NONE
        autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Fabrice,

>On 3/15/2023 6:45 PM, Fabrice Gasnier <fabrice.gasnier@foss.st.com> wrote:
>From: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>Sent: Wednesday, March 15, 2023 6:45 PM
>To: Minas Harutyunyan <hminas@synopsys.com>; gregkh@linuxfoundation.org;
>quentin.schulz@theobroma-systems.com
>Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; linux-stm32@s=
t-
>md-mailman.stormreply.com; amelie.delaunay@foss.st.com;
>alexandre.torgue@foss.st.com; fabrice.gasnier@foss.st.com
>Subject: [PATCH] usb: dwc2: fix a race, don't power off/on phy for dual-ro=
le
>mode
>
>When in dual role mode (dr_mode =3D=3D USB_DR_MODE_OTG), platform probe
>successively basically calls:
>- dwc2_gadget_init()
>- dwc2_hcd_init()
>- dwc2_lowlevel_hw_disable() since recent change [1]
>- usb_add_gadget_udc()
>
>The PHYs (and so the clocks it may provide) shouldn't be disabled for all
>SoCs, in OTG mode, as the HCD part has been initialized.
>
>On STM32 this creates some weird race condition upon boot, when:
>- initially attached as a device, to a HOST
>- and there is a gadget script invoked to setup the device part.
>Below issue becomes systematic, as long as the gadget script isn't started
>by userland: the hardware PHYs (and so the clocks provided by the
>PHYs) remains disabled.
>It ends up in having an endless interrupt storm, before the watchdog reset=
s
>the platform.
>
>[   16.924163] dwc2 49000000.usb-otg: EPs: 9, dedicated fifos, 952 entries
>in SPRAM
>[   16.962704] dwc2 49000000.usb-otg: DWC OTG Controller
>[   16.966488] dwc2 49000000.usb-otg: new USB bus registered, assigned bus
>number 2
>[   16.974051] dwc2 49000000.usb-otg: irq 77, io mem 0x49000000
>[   17.032170] hub 2-0:1.0: USB hub found
>[   17.042299] hub 2-0:1.0: 1 port detected
>[   17.175408] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently i=
n
>Host mode
>[   17.181741] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently i=
n
>Host mode
>[   17.189303] dwc2 49000000.usb-otg: Mode Mismatch Interrupt: currently i=
n
>Host mode
>...
>
>The host part is also not functional, until the gadget part is configured.
>
>The HW may only be disabled for peripheral mode (original init), e.g.
>dr_mode =3D=3D USB_DR_MODE_PERIPHERAL, until the gadget driver initializes=
.
>
>But when in USB_DR_MODE_OTG, the HW should remain enabled, as the HCD part
>is able to run, while the gadget part isn't necessarily configured.
>
>I don't fully get the of purpose the original change, that claims disablin=
g
>the hardware is missing. It creates conditions on SOCs using the PHY
>initialization to be completely non working in OTG mode. Original change [=
1]
>should be reworked to be platform specific.
>
>[1] https://urldefense.com/v3/__https://lore.kernel.org/r/20221206-dwc2-
>gadget-dual-role-v1-2-36515e1092cd@theobroma-
>systems.com__;!!A4F2R9G_pg!Y21e8pRbIOVyLQTRP5HjdeDUHpSjbtiRQVFGVCOBBDu9yH3=
2W
>tdppqmP-8TLyGrBjyOBG5iI4qw6XMFEdfRJDhZ8HVc$
>
>Fixes: ade23d7b7ec5 ("usb: dwc2: power on/off phy for peripheral mode in
>dual-role mode")
>Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Acked-by: Minas Harutyunyan <hminas@synopsys.com>

>---
> drivers/usb/dwc2/gadget.c   | 6 ++----
> drivers/usb/dwc2/platform.c | 3 +--
> 2 files changed, 3 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index
>62fa6378d2d7..8b15742d9e8a 100644
>--- a/drivers/usb/dwc2/gadget.c
>+++ b/drivers/usb/dwc2/gadget.c
>@@ -4549,8 +4549,7 @@ static int dwc2_hsotg_udc_start(struct usb_gadget
>*gadget,
> 	hsotg->gadget.dev.of_node =3D hsotg->dev->of_node;
> 	hsotg->gadget.speed =3D USB_SPEED_UNKNOWN;
>
>-	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL ||
>-	    (hsotg->dr_mode =3D=3D USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)=
))
>{
>+	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL) {
> 		ret =3D dwc2_lowlevel_hw_enable(hsotg);
> 		if (ret)
> 			goto err;
>@@ -4612,8 +4611,7 @@ static int dwc2_hsotg_udc_stop(struct usb_gadget
>*gadget)
> 	if (!IS_ERR_OR_NULL(hsotg->uphy))
> 		otg_set_peripheral(hsotg->uphy->otg, NULL);
>
>-	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL ||
>-	    (hsotg->dr_mode =3D=3D USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)=
))
>+	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL)
> 		dwc2_lowlevel_hw_disable(hsotg);
>
> 	return 0;
>diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c ind=
ex
>23ef75996823..262c13b6362a 100644
>--- a/drivers/usb/dwc2/platform.c
>+++ b/drivers/usb/dwc2/platform.c
>@@ -576,8 +576,7 @@ static int dwc2_driver_probe(struct platform_device
>*dev)
> 	dwc2_debugfs_init(hsotg);
>
> 	/* Gadget code manages lowlevel hw on its own */
>-	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL ||
>-	    (hsotg->dr_mode =3D=3D USB_DR_MODE_OTG && dwc2_is_device_mode(hsotg)=
))
>+	if (hsotg->dr_mode =3D=3D USB_DR_MODE_PERIPHERAL)
> 		dwc2_lowlevel_hw_disable(hsotg);
>
> #if IS_ENABLED(CONFIG_USB_DWC2_PERIPHERAL) || \
>--
>2.25.1