From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id AED6633A9CB for ; Thu, 21 May 2026 11:58:16 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779364698; cv=none; b=F0YNHrJ8cx/GmVcvdZwEga3VvRAU9N/34Jli2kLw9Uf0qZOwTYx+QGsPSNpvqkta0RS15/ll0jQFi4y/PxMEnyAenm1MZ/OatfeIuvS/sfIU2/wpfukhC9mWVtJpPnX0RMnKuUzbl1XJzZJwCHLZPw5pb/1VrRkFQO7//fHJEHA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779364698; c=relaxed/simple; bh=6ZVc+tCRCCkcEiJ6SjAllBlClzLxvxfyjkV50j9aGNA=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=uTcDkhrAyQXjwtwlOOTalFSYPSQ1TVRqW8/0s906hVITjqvEBWn6ZTA+aotwv8d9dpeXSvM0Bm5KHDazPgonk97vFJNU5lj62UrFQiZe/Y0wpDN5Z7VCDclNX7nMrwdVGRkAKO5/ErdwEeBsTYqIlp4z6O/+wOTXw6RvWh6RY9M= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=LqSOwycc; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="LqSOwycc" Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-449de065cb3so5642808f8f.2 for ; Thu, 21 May 2026 04:58:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1779364695; x=1779969495; darn=vger.kernel.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=4stJ0wGm3jktbxjyMiS2oEBiggoOgvX2rImz04PGH7E=; b=LqSOwycc1pKd8MEuRb4f4/wdTESwGmlobnw1cgU0S//O7yMYEzcL9+jhi7XVYDbW6j bpRg+FFJeT16gVqNP5HzofdOFKQCzYpN5zt1ILZJHWYABbUxGHewWvRDkt9a8aX+TjML rIEic5usCaj9x4bvUi85sf6if8EPaU51KI5+bkqcB2Ng6nttEfqU9vdJOx3iSXpljmQn kB56hTeSJPKaX6a/WzPszcOjioV8EBlTRga5a/AmbiciWXKPw6RdypGtLJxq6RY8o6kr J0XNvAcLlQ2iPgvCohV/5Tzy27R0QAFDr00RGMw5VbS72QAQVdcGtGCCparrTdvos6vU kmWg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779364695; x=1779969495; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=4stJ0wGm3jktbxjyMiS2oEBiggoOgvX2rImz04PGH7E=; b=aFVJUTaRgotK1zK+t6/2WA1JO9K0/3CbOABTlwbin3c+hnvlzSi0THyj3P3xrwOoIh +YxMMoGeoFeZRYkfo1lhrEXMOfCinKQeZJYOIQw32jytQ/bMaogk0DXpVdOKVvBUoxCM bCiKbtSeyLk7eu6YY/hKickmUUb1BujBQNvT8jJCskGLfACqj9/D+8YJkJ/KjNjFyWjq GrpeTNaEGDWCZlzl/ZyTBWzP4alqMUj6wA504CR5o53+7mGRxF/iWOzkGO8NZ1M7alKN QAURr7ZV1xYd/3/ZzqJ6RXIWd0S3YVUlh8eZCv+RxluFkfrH4cEot0k3tVRISmweSh3P KRBQ== X-Forwarded-Encrypted: i=1; AFNElJ+kHNJSnaaGQ9qwDR/C+TrIcWokKmqU22NYUGbpUCCW0XaTTw/3sjq0H5XmBqAzT1UPxveMRVX0YGm+2PY=@vger.kernel.org X-Gm-Message-State: AOJu0YzCGlgbpH7EltsNyreOU8vzuKvgu5G8ie9G0GYRly7r2nakECTP VolbUM51Faf2tpCHVro+vK6f1P3uN2OQt5WczlUSOYBfNZWYl6EppK4lCej1iUdP3Ns= X-Gm-Gg: Acq92OHhS+EWjK4s01zdhnVFhNGb4SdBxhvNlCSPy0cz8BhYlNNTTUFfrqJAfp1k5YQ ilSkWaaARYuMTm2nxVyZw5YGa0ohlBy3Hz2lLwopNNOQoQ2GAKdRH+I46+dSwciHepf6NGFfHF6 Lc4ah0na5HqcUtz1MKhQB4wnCay5Ght9LuFuKChegIYLsEDYOpWp8zBYl35QZw9m4pqeYcpk5uk a2pw+MUA3eVwbJIz07Sjzh885TW37cHBpHa4MJj8jKsZnnGe19Ztrpq3NAxkiCz1b5p97oyMKa7 M/oVxtdYZzpxROFZzIs8V29RM5K1dcMqKFdRPka/F3DpqP96DaqkL+2VlGzpSA5h9gBafbDjdP+ /oqZWKGupKsDYgxFVFyaSsYOF5N/6oNchZfkVqae/YH0K9JvYJUuDEdwpdhGYhfPHXKQ8i6KWzj giRhG9nQCrth+2UNV5kXYJ7N9DLn+KqoCdTQ== X-Received: by 2002:a05:6000:4615:b0:45b:d891:56bb with SMTP id ffacd0b85a97d-45ea4109a8dmr3655913f8f.38.1779364694940; Thu, 21 May 2026 04:58:14 -0700 (PDT) Received: from [192.168.0.35] ([109.76.55.220]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-45eaa93d291sm2227253f8f.36.2026.05.21.04.58.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 21 May 2026 04:58:14 -0700 (PDT) Message-ID: Date: Thu, 21 May 2026 12:58:12 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 1/4] phy: qcom: qmp-combo: skip USB power_off/exit after device teardown To: Michael Scott , linux-arm-msm@vger.kernel.org Cc: vkoul@kernel.org, neil.armstrong@linaro.org, dmitry.baryshkov@oss.qualcomm.com, wesley.cheng@oss.qualcomm.com, abelvesa@kernel.org, faisal.hassan@oss.qualcomm.com, linux-phy@lists.infradead.org, andersson@kernel.org, konradybcio@kernel.org, robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org, val@packett.cool, laurentiu.tudor1@dell.com, alex.vinarskis@gmail.com, linux-kernel@vger.kernel.org References: <20260521010935.1333494-1-mike.scott@oss.qualcomm.com> <20260521010935.1333494-2-mike.scott@oss.qualcomm.com> Content-Language: en-US From: Bryan O'Donoghue In-Reply-To: <20260521010935.1333494-2-mike.scott@oss.qualcomm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 21/05/2026 02:09, Michael Scott wrote: > qmp_combo_usb_power_off() is reachable from an external consumer > (notably dwc3 via phy_exit() during driver unbind) after this device's > backing resources have already been released along a separate teardown > chain. The dereference of qmp->pcs (whose ioremap mapping has been > freed by devm cleanup) then takes a level-3 translation fault and > oopses. > > Easily reproducible during testing of USB-C role-switch enablement on > Dell Latitude 7455 (X1E80100), by writing "none" to a USB-C DWC3's > usb_role_switch role attribute, e.g. > > echo none > /sys/class/usb_role/a800000.usb-role-switch/role > > which triggers the chain: > > Unable to handle kernel paging request at virtual address ffff8000876c5400 > pc : qmp_combo_usb_power_off.isra.0+0x58/0x470 [phy_qcom_qmp_combo] > Call trace: > qmp_combo_usb_power_off+0x58/0x470 [phy_qcom_qmp_combo] > qmp_combo_usb_exit+0x38/0x90 [phy_qcom_qmp_combo] > phy_exit > dwc3_phy_exit [dwc3] > dwc3_core_remove [dwc3] > dwc3_remove [dwc3] > platform_remove > device_release_driver_internal > device_driver_detach > unbind_store > sysfs_kf_write > vfs_write > ksys_write > __arm64_sys_write > el0_svc > > Two WARNs precede the oops from the same teardown chain, confirming > the resource ordering: > > WARNING: drivers/clk/clk.c:4494 at clk_nodrv_disable_unprepare+0x8/0x18 > WARNING: drivers/regulator/core.c:2657 at _regulator_put+0x84/0x98 > > i.e. the pipe clock provider has been unregistered and the regulators > released before qmp_combo_usb_power_off() runs. > > The proper long-term fix is a teardown-ordering rework so the QMP > PHY's backing resources outlive any consumer that may still call its > phy_ops. Pending that, guard the power_off/exit paths with the > existing usb_init_count balance so re-entry after teardown does not > oops. usb_init_count tracks the balance of usb_power_on/off; if it > is zero we have either never powered on or have already powered off, > and there is nothing to do. > > The same guard is added to qmp_combo_usb_exit() since it is the entry > point used by external consumers via phy_exit(). > > Signed-off-by: Michael Scott Something like this requires a Fixes: tag > --- > drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > index cdcfad2e86b1..0db200292642 100644 > --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c > @@ -3926,6 +3926,17 @@ static int qmp_combo_usb_power_off(struct phy *phy) > struct qmp_combo *qmp = phy_get_drvdata(phy); > const struct qmp_phy_cfg *cfg = qmp->cfg; > > + /* > + * Reachable as ->exit from external consumers (notably dwc3) after > + * this device's backing resources have already been released along > + * a teardown chain. Refuse to touch registers in that case. > + */ > + if (!qmp->usb_init_count) { > + dev_dbg(qmp->dev, "%s: PHY not powered on, skipping\n", > + __func__); > + return 0; > + } > + > /* PHY reset */ > qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET); > > @@ -3968,6 +3979,17 @@ static int qmp_combo_usb_exit(struct phy *phy) > struct qmp_combo *qmp = phy_get_drvdata(phy); > int ret; > > + /* > + * See qmp_combo_usb_power_off(): an external consumer may call > + * phy_exit() after the QMP device's resources have been torn > + * down. usb_init_count tracks usb_init/usb_exit balance. > + */ > + if (!qmp->usb_init_count) { > + dev_dbg(qmp->dev, "%s: PHY not initialised, skipping\n", > + __func__); > + return 0; > + } > + > mutex_lock(&qmp->phy_mutex); > ret = qmp_combo_usb_power_off(phy); This can't be right - you check usb_init_count before the mutex and then again inside the mutex @ qmp_combo_usb_power_off(); It seems like an error to even get to this function with !usb_init_count also check if that is a signed or an unsigned value as usb_init_count = -1 will evaluate true. > if (ret) > -- > 2.53.0 >