From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7FBE035839C; Mon, 6 Jul 2026 03:32:02 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783308723; cv=none; b=TTsSpMezr6njG94PjwdaMVHmwYpRoO9fVNLeAA/iz8/LS1LNnZmHe79q7rP8QDssWLkmsQEXhr9gpcLwUts16Cq4w9N33QH2WB+BlH/Tlu6gSxdFDp0JfIxraj5/bTSxk6ht6qT+VmLZCQwNi4yABkL0vvJ77piEQJ5r+KHTEOc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783308723; c=relaxed/simple; bh=GBaSpkX/ZWZ8FKBzWzRYdMKpcyVa+GFYiM6rKv2byGY=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=maeKFvRBhg1Vyee+9JaB6WNW9L6vKPGCXfKG0tPeRyZD7ygQXgKMeogCyrBDEhiWCi9dzC2GmdNoz6RLX1UoSq3gxgQddzCf/L9sGnVTrnGaZy/K5kzbS95msoVLalqZPI6JnRrWxJB3AqpSrEMcrg5q6ru5MkwhTjIT4UA0xhc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=S4LLoTMg; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="S4LLoTMg" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8E2551F000E9; Mon, 6 Jul 2026 03:32:01 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783308722; bh=pgK1AhmGSy4aon6P61BNRyRaQlwaG8JpQ/TTXfTW+so=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=S4LLoTMgX5Frd4+I3ALkC1K+q2wrqr+SFmn19E3s7YcZK7/SkSaBaJFuezpe2CKwn 68jnToZBcuz1mFPuMLF7YiVruGQdKS4bnpjVA/l9ufvCrUJcN44+7ca3oENHeEIY2W EXcRf54OV3/K/ClX8U2MT74Sle858zHULjxXt4PbTNKeG3BvY2E3O6ooRTuZjJfqKw wdudwo2M27+Yb134Ps/A/QvwuZTWDIyN5sXERbi+jrchqo9p/V2hwZMzejIktBo9SM i2nzN2W54jSfwuasY/DoU6fuWELExMREWxnUr4bF5Mpikf93WNAwHRGS/UUr7/ctGf jjU7GfEaiCVHw== Date: Sun, 5 Jul 2026 20:32:00 -0700 From: Peter Chen To: Elson Serrao Cc: Bjorn Andersson , Dmitry Baryshkov , Greg Kroah-Hartman , Konrad Dybcio , Krzysztof Kozlowski , Rob Herring , Conor Dooley , Souradeep Chowdhury , linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v4 4/9] usb: misc: qcom_eud: add per-path role switch support Message-ID: References: <20260501170635.2641748-1-elson.serrao@oss.qualcomm.com> <20260501170635.2641748-5-elson.serrao@oss.qualcomm.com> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260501170635.2641748-5-elson.serrao@oss.qualcomm.com> On 26-05-01 10:06:30, Elson Serrao wrote: > The EUD hardware can support multiple High-Speed USB paths, each connected > to different USB controllers. The current implementation uses a single > chip-level role switch, which cannot properly handle multi-path > configurations where each path needs independent role management. Since > EUD is physically present between the USB connector and the controller, > it should also relay the role change requests from the connector. > > Restructure the driver to support per-path role switches and remove the > chip-level role switch. Additionally, as EUD need not modify the USB > role upon enabling, remove the unnecessary role switch call from > enable_eud(). > > Signed-off-by: Elson Serrao > --- > drivers/usb/misc/qcom_eud.c | 89 +++++++++++++++++++++++++++++-------- > 1 file changed, 70 insertions(+), 19 deletions(-) > > diff --git a/drivers/usb/misc/qcom_eud.c b/drivers/usb/misc/qcom_eud.c > index a624c44d5d59..7fbb74bf1599 100644 > --- a/drivers/usb/misc/qcom_eud.c > +++ b/drivers/usb/misc/qcom_eud.c > @@ -44,12 +44,15 @@ static const char * const eud_port_names[] = { > struct eud_path { > struct eud_chip *chip; > struct phy *phy; > + struct usb_role_switch *controller_sw; > + struct usb_role_switch *eud_sw; > + enum usb_role curr_role; > + char name[16]; > u8 num; > }; > > struct eud_chip { > struct device *dev; > - struct usb_role_switch *role_sw; > void __iomem *base; > struct eud_path *paths[EUD_MAX_PORTS]; > phys_addr_t mode_mgr; > @@ -123,7 +126,7 @@ static int enable_eud(struct eud_chip *priv) > writel(EUD_INT_VBUS | EUD_INT_SAFE_MODE, > priv->base + EUD_REG_INT1_EN_MASK); > > - return usb_role_switch_set_role(priv->role_sw, USB_ROLE_DEVICE); > + return 0; > } > > static int disable_eud(struct eud_chip *priv) > @@ -274,12 +277,15 @@ static irqreturn_t handle_eud_irq(int irq, void *data) > static irqreturn_t handle_eud_irq_thread(int irq, void *data) > { > struct eud_chip *chip = data; > + struct eud_path *path; > int ret; > > + path = chip->paths[chip->port_idx]; > + > if (chip->usb_attached) > - ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_DEVICE); > + ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_DEVICE); > else > - ret = usb_role_switch_set_role(chip->role_sw, USB_ROLE_HOST); > + ret = usb_role_switch_set_role(path->controller_sw, USB_ROLE_HOST); > if (ret) > dev_err(chip->dev, "failed to set role switch\n"); > > @@ -290,15 +296,36 @@ static irqreturn_t handle_eud_irq_thread(int irq, void *data) > return IRQ_HANDLED; > } > > -static void eud_role_switch_release(void *data) > +static int eud_role_switch_set(struct usb_role_switch *sw, enum usb_role role) > { > - struct eud_chip *chip = data; > + struct eud_path *path = usb_role_switch_get_drvdata(sw); > + int ret; > + > + /* Forward the role request to the USB controller */ > + ret = usb_role_switch_set_role(path->controller_sw, role); > + if (ret) { > + dev_err(path->chip->dev, "failed to set role %s for port %u: %d\n", > + usb_role_string(role), path->num, ret); > + return ret; > + } > > - usb_role_switch_put(chip->role_sw); > + path->curr_role = role; > + > + return 0; > +} > + > +static void eud_path_role_switch_release(void *data) > +{ > + struct eud_path *path = data; > + > + usb_role_switch_unregister(path->eud_sw); > + usb_role_switch_put(path->controller_sw); > } > > static int eud_init_path(struct eud_chip *chip, struct device_node *np) > { > + struct usb_role_switch_desc role_sw_desc; Should role_sw_desc be zero-initialized here? Otherwise, some entries of path->eud_sw are uncertain after calling usb_role_switch_register? Peter > + struct usb_role_switch *sw; > struct device_node *controller_node; > struct eud_path *path; > u32 path_num; > @@ -331,11 +358,44 @@ static int eud_init_path(struct eud_chip *chip, struct device_node *np) > "failed to get controller node for path %u\n", path_num); > > path->phy = devm_of_phy_get_by_index(chip->dev, controller_node, 0); > - of_node_put(controller_node); > - > - if (IS_ERR(path->phy)) > + if (IS_ERR(path->phy)) { > + of_node_put(controller_node); > return dev_err_probe(chip->dev, PTR_ERR(path->phy), > "failed to get PHY for path %d\n", path_num); > + } > + > + path->curr_role = USB_ROLE_NONE; > + > + /* Fetch controller role switch if it is role switch capable */ > + if (of_property_read_bool(controller_node, "usb-role-switch")) { > + sw = usb_role_switch_find_by_fwnode(of_fwnode_handle(controller_node)); > + if (!sw) { > + of_node_put(controller_node); > + return dev_err_probe(chip->dev, -EPROBE_DEFER, > + "Failed to get controller role switch for path %d\n", > + path_num); > + } > + path->controller_sw = sw; > + } > + > + of_node_put(controller_node); > + > + role_sw_desc.fwnode = of_fwnode_handle(np); > + role_sw_desc.set = eud_role_switch_set; > + role_sw_desc.driver_data = path; > + snprintf(path->name, sizeof(path->name), "eud-path%u", path_num); > + role_sw_desc.name = path->name; > + > + path->eud_sw = usb_role_switch_register(chip->dev, &role_sw_desc); > + if (IS_ERR(path->eud_sw)) { > + usb_role_switch_put(path->controller_sw); > + return dev_err_probe(chip->dev, PTR_ERR(path->eud_sw), > + "Failed to register EUD role switch for path %d\n", path_num); > + } > + > + ret = devm_add_action_or_reset(chip->dev, eud_path_role_switch_release, path); > + if (ret) > + return ret; > > chip->paths[path_num] = path; > > @@ -365,15 +425,6 @@ static int eud_probe(struct platform_device *pdev) > if (!chip->paths[0]) > return dev_err_probe(chip->dev, -ENODEV, "primary path not found\n"); > > - chip->role_sw = usb_role_switch_get(&pdev->dev); > - if (IS_ERR(chip->role_sw)) > - return dev_err_probe(chip->dev, PTR_ERR(chip->role_sw), > - "failed to get role switch\n"); > - > - ret = devm_add_action_or_reset(chip->dev, eud_role_switch_release, chip); > - if (ret) > - return ret; > - > chip->base = devm_platform_ioremap_resource(pdev, 0); > if (IS_ERR(chip->base)) > return PTR_ERR(chip->base); > -- > 2.34.1 > > -- Thanks, Peter Chen