Message ID | 20221213124854.3779-2-sinthu.raja@ti.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp100752wrn; Tue, 13 Dec 2022 04:58:14 -0800 (PST) X-Google-Smtp-Source: AA0mqf7CZG++zxVitAAeEJWlr7+bM57Vwx+7oWobMISZe6yaDvGXHYMloLhBKr14iKpgqurT6xFH X-Received: by 2002:a05:6402:294f:b0:46c:e226:7717 with SMTP id ed15-20020a056402294f00b0046ce2267717mr18711024edb.31.1670936294611; Tue, 13 Dec 2022 04:58:14 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1670936294; cv=none; d=google.com; s=arc-20160816; b=PtrQnYZT7BrZugz3hpL2b3ojfhSCK2ErTqUVqXJ0vKsMlNfyHu5vewk0WWB27cgim7 JwRR7bNGjlZT2hPLE+3Fbf5I9ZkHhteZSMvzhm+JwGSlHLS1N3VG+n0+bglWI3bT3Szi /ziGNMNG6P5mtrRkBpC3bEnVZUL/DX9Sm3bwfwalDDCGcnwz11K+Mo2QdNRIyKOBPFYy HPxDBHIjOEuOW+ClweC7J+E7L6ejBvaIscPluFo+Q1I4asqLKAdkcyJoQhw1NghWV8l+ Tzrnb66IZVhEyV8rnh5P21HX7hWCKNihceD5BUAytXRizLsq00VK3uKmSCz17XLi0rK2 WutA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=mJx0rqXAD4/GS75NvFhhlmtznHYCg5Ca/9JyH7cg9Uo=; b=KjDo56pFbPi4NGvKvvNX/3nMnuFkXacN7UTlFYK0A7Xnzh5CvuKoRaL/r+VDJAVp6/ bX6T+HR+3xoZxsf7/u0vkUhwjBgXag3+cpX1OKqWuPqhrrwVqAYKstR2cHQa9E4i37hQ GZAsxdo55GRtRJjBXNjpQ9yHmQRobtHiBhxqT/QEh9GwB3OJ13RetjP+6dKhGtJu8B+s m4io/w/pr1qe8NNL8O16SlxqQXKz5rBZ55bKFDquMSMU87WMKHIseQta55G4MRJQbJ1t QMAgffAQq6RPRuROXj6BfgJUEjCznTgIGu1wigZdzMXejZ/S0JySdYrYV+9+Q07Z0xfI D75A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@mistralsolutions.com header.s=google header.b=kJcvRSdI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=mistralsolutions.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id b4-20020a0564021f0400b0046ae1883662si9031671edb.597.2022.12.13.04.57.50; Tue, 13 Dec 2022 04:58:14 -0800 (PST) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@mistralsolutions.com header.s=google header.b=kJcvRSdI; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=QUARANTINE sp=QUARANTINE dis=NONE) header.from=mistralsolutions.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234607AbiLMMtV (ORCPT <rfc822;jeantsuru.cumc.mandola@gmail.com> + 99 others); Tue, 13 Dec 2022 07:49:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:51456 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235436AbiLMMtQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 13 Dec 2022 07:49:16 -0500 Received: from egress-ip4b.ess.de.barracuda.com (egress-ip4b.ess.de.barracuda.com [18.185.115.208]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A6ABC1B9F1 for <linux-kernel@vger.kernel.org>; Tue, 13 Dec 2022 04:49:14 -0800 (PST) Received: from mail-pf1-f197.google.com (mail-pf1-f197.google.com [209.85.210.197]) by mx-outbound20-247.eu-central-1b.ess.aws.cudaops.com (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 13 Dec 2022 12:49:12 +0000 Received: by mail-pf1-f197.google.com with SMTP id z13-20020aa79f8d000000b00576b614b7d2so2004027pfr.14 for <linux-kernel@vger.kernel.org>; Tue, 13 Dec 2022 04:49:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mistralsolutions.com; s=google; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=mJx0rqXAD4/GS75NvFhhlmtznHYCg5Ca/9JyH7cg9Uo=; b=kJcvRSdIp5NMz5v1W2KrvA5RQ6yRnVaMEmRm9XeDvp9L9VdtmbkgqW/ajupMeLa5C8 LYH1eSD2TDLLz7yzclYpZzqrSSJIDUMS6YJ1c9JX2pO9hXEawe8hnibZOn4njwmTYQs0 PsxVRQq0//XNw635iCjmc9cBeLcyKAdYiG5ao= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mJx0rqXAD4/GS75NvFhhlmtznHYCg5Ca/9JyH7cg9Uo=; b=It/lv7oEG2pgssI4bSFZmNQVPjGbt0OPG7fBZsHfv3ZOI5VQjXHzJw36EbUCxLciZb mAx7Ss50pup39O8+ivThHqdE1W1grNohI+UOgFKP8Gg/9qsZo55iaDtULXOUylLmq4lV QCfh4ntPWHvsjmQIK3mycJvk/oW/A80tmb+GJ5YEVPhj9UJniCTcTu0k6M/ro+e2Jhfu 5+JzlSG7vvIZGInVuPaW+TrsGcgmOj2GvEibmvH/hu4rGM86uXlg03CJ+3p6aOH3lrq1 hYgc0dbrwWl3p4mTqOysu3lGyPMafnjhp54y9ktITof85pQQY+pK7Fl2oipPrkNPrZq3 oVbQ== X-Gm-Message-State: ANoB5pmcbdrAjeG46TM7bGwFNojCusORPY4WKQ2f3DmgkjXh5ZL1xErP PgsQw0GKNvY1Z1vWTEY74VyNiirsVa2c+jAOapXBVWuzkgrOkNasdWOvJYzgGq/Dlfov/Jamu2L 9+A3zba5urUxo4oUMyFiw2oCZZtEQgb8mvJd+MhSTKk2+fmcZ+bUzkIjIrEV0 X-Received: by 2002:a62:6303:0:b0:578:83f5:554d with SMTP id x3-20020a626303000000b0057883f5554dmr7396315pfb.23.1670935750568; Tue, 13 Dec 2022 04:49:10 -0800 (PST) X-Received: by 2002:a62:6303:0:b0:578:83f5:554d with SMTP id x3-20020a626303000000b0057883f5554dmr7396294pfb.23.1670935750251; Tue, 13 Dec 2022 04:49:10 -0800 (PST) Received: from LAP568U.mistral.in ([106.51.227.150]) by smtp.gmail.com with ESMTPSA id y15-20020aa7942f000000b0057622e8e82csm7605485pfo.191.2022.12.13.04.49.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 13 Dec 2022 04:49:09 -0800 (PST) From: Sinthu Raja <sinthu.raja@mistralsolutions.com> X-Google-Original-From: Sinthu Raja <sinthu.raja@ti.com> To: Vinod Koul <vkoul@kernel.org>, Ravi Gunasekaran <r-gunasekaran@ti.com>, Siddharth Vadapalli <s-vadapalli@ti.com> Cc: Vignesh Raghavendra <vigneshr@ti.com>, Roger Quadros <rogerq@kernel.org>, linux-phy@lists.infradead.org, linux-kernel@vger.kernel.org, Sinthu Raja <sinthu.raja@ti.com> Subject: [PATCH 1/2] phy: ti: j721e-wiz: Manage TypeC lane swap if typec-gpio-dir not specified Date: Tue, 13 Dec 2022 18:18:53 +0530 Message-Id: <20221213124854.3779-2-sinthu.raja@ti.com> X-Mailer: git-send-email 2.36.1 In-Reply-To: <20221213124854.3779-1-sinthu.raja@ti.com> References: <20221213124854.3779-1-sinthu.raja@ti.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-BESS-ID: 1670935751-305367-5380-5821-1 X-BESS-VER: 2019.1_20221212.2317 X-BESS-Apparent-Source-IP: 209.85.210.197 X-BESS-Outbound-Spam-Score: 0.00 X-BESS-Outbound-Spam-Report: Code version 3.2, rules version 3.2.2.244774 [from cloudscan15-235.eu-central-1a.ess.aws.cudaops.com] Rule breakdown below pts rule name description ---- ---------------------- -------------------------------- 0.00 BSF_BESS_OUTBOUND META: BESS Outbound 0.00 BSF_SC0_MISMATCH_TO META: Envelope rcpt doesn't match header X-BESS-Outbound-Spam-Status: SCORE=0.00 using account:ESS91090 scores of KILL_LEVEL=7.0 tests=BSF_BESS_OUTBOUND, BSF_SC0_MISMATCH_TO X-BESS-BRTS-Status: 1 X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW, RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS autolearn=ham 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 X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1752103696059685813?= X-GMAIL-MSGID: =?utf-8?q?1752103696059685813?= |
Series |
phy: ti: j721e-wiz: Add support to manage type-C swap on Lane2 and lane3
|
|
Commit Message
Sinthu Raja
Dec. 13, 2022, 12:48 p.m. UTC
It's possible that the Type-C plug orientation on the DIR line will be
implemented through hardware design. In that situation, there won't be
an external GPIO line available, but the driver still needs to address
this since the DT won't use the typec-gpio-dir property.
Add code to handle LN10 Type-C swap if typec-gpio-dir property is not
specified in DT.
Remove typec-gpio-dir check to use minimum debounce from Type-C spec if
it is not provided in DT
Signed-off-by: Sinthu Raja <sinthu.raja@ti.com>
---
drivers/phy/ti/phy-j721e-wiz.c | 65 +++++++++++++++++++++-------------
1 file changed, 40 insertions(+), 25 deletions(-)
Comments
Hi Sinthu, On 13/12/2022 14:48, Sinthu Raja wrote: > It's possible that the Type-C plug orientation on the DIR line will be > implemented through hardware design. In that situation, there won't be > an external GPIO line available, but the driver still needs to address > this since the DT won't use the typec-gpio-dir property. The property is actually "typec-dir-gpios" > > Add code to handle LN10 Type-C swap if typec-gpio-dir property is not > specified in DT. > > Remove typec-gpio-dir check to use minimum debounce from Type-C spec if > it is not provided in DT Why? > > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com> > --- > drivers/phy/ti/phy-j721e-wiz.c | 65 +++++++++++++++++++++------------- > 1 file changed, 40 insertions(+), 25 deletions(-) > > diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c > index 141b51af4427..b17eec632d49 100644 > --- a/drivers/phy/ti/phy-j721e-wiz.c > +++ b/drivers/phy/ti/phy-j721e-wiz.c > @@ -375,6 +375,7 @@ struct wiz { > struct gpio_desc *gpio_typec_dir; > int typec_dir_delay; > u32 lane_phy_type[WIZ_MAX_LANES]; > + u32 lane_phy_reg[WIZ_MAX_LANES]; > struct clk *input_clks[WIZ_MAX_INPUT_CLOCKS]; > struct clk *output_clks[WIZ_MAX_OUTPUT_CLOCKS]; > struct clk_onecell_data clk_data; > @@ -1231,14 +1232,28 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev, > int ret; > > /* if typec-dir gpio was specified, set LN10 SWAP bit based on that */ > - if (id == 0 && wiz->gpio_typec_dir) { > - if (wiz->typec_dir_delay) > - msleep_interruptible(wiz->typec_dir_delay); > - > - if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > - regmap_field_write(wiz->typec_ln10_swap, 1); > - else > - regmap_field_write(wiz->typec_ln10_swap, 0); > + if (id == 0 && wiz->typec_dir_delay) { > + msleep_interruptible(wiz->typec_dir_delay); Why do you need to have this debounce delay if there was no GPIO to begin with. You need to move the msleep call within the next if {} block. > + > + if (wiz->gpio_typec_dir) { > + if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > + regmap_field_write(wiz->typec_ln10_swap, 1); > + else > + regmap_field_write(wiz->typec_ln10_swap, 0); > + } else { > + /* if no typec-dir gpio was specified, and USB lines > + * are connected to Lane 0 then set LN10 SWAP bit to 1. > + */ > + u32 num_lanes = wiz->num_lanes; > + int i; > + > + for (i = 0; i < num_lanes; i++) {typec-dir-gpios: > + if ((wiz->lane_phy_type[i] == PHY_TYPE_USB3) \ > + && wiz->lane_phy_reg[i] == 0) { > + regmap_field_write(wiz->typec_ln10_swap, 1); > + } > + } > + } > } > > if (id == 0) { > @@ -1370,8 +1385,10 @@ static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz) > dev_dbg(dev, "%s: Lanes %u-%u have phy-type %u\n", __func__, > reg, reg + num_lanes - 1, phy_type); > > - for (i = reg; i < reg + num_lanes; i++) > + for (i = reg; i < reg + num_lanes; i++) { > + wiz->lane_phy_reg[i] = reg; > wiz->lane_phy_type[i] = phy_type; > + } > } > > return 0; > @@ -1464,24 +1481,22 @@ static int wiz_probe(struct platform_device *pdev) > goto err_addr_to_resource; > } > > - if (wiz->gpio_typec_dir) { > - ret = of_property_read_u32(node, "typec-dir-debounce-ms", > - &wiz->typec_dir_delay); > - if (ret && ret != -EINVAL) { > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > - goto err_addr_to_resource; > - } > + ret = of_property_read_u32(node, "typec-dir-debounce-ms", > + &wiz->typec_dir_delay); > + if (ret && ret != -EINVAL) { > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > + goto err_addr_to_resource; > + } Why do you need to know this debounce value if you don't have a valid GPIO line? > > - /* use min. debounce from Type-C spec if not provided in DT */ > - if (ret == -EINVAL) > - wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > + /* use min. debounce from Type-C spec if not provided in DT */ > + if (ret == -EINVAL) > + wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > > - if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > - wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > - ret = -EINVAL; > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > - goto err_addr_to_resource; > - } > + if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > + wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > + ret = -EINVAL; > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > + goto err_addr_to_resource; > } All these changes are unnecessary. > > ret = wiz_get_lane_phy_types(dev, wiz); cheers, -roger
Hi, Some more comments below. On 13/12/2022 14:48, Sinthu Raja wrote: > It's possible that the Type-C plug orientation on the DIR line will be > implemented through hardware design. In that situation, there won't be > an external GPIO line available, but the driver still needs to address > this since the DT won't use the typec-gpio-dir property. > > Add code to handle LN10 Type-C swap if typec-gpio-dir property is not > specified in DT. > > Remove typec-gpio-dir check to use minimum debounce from Type-C spec if > it is not provided in DT > > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com> > --- > drivers/phy/ti/phy-j721e-wiz.c | 65 +++++++++++++++++++++------------- > 1 file changed, 40 insertions(+), 25 deletions(-) > > diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c > index 141b51af4427..b17eec632d49 100644 > --- a/drivers/phy/ti/phy-j721e-wiz.c > +++ b/drivers/phy/ti/phy-j721e-wiz.c > @@ -375,6 +375,7 @@ struct wiz { > struct gpio_desc *gpio_typec_dir; > int typec_dir_delay; > u32 lane_phy_type[WIZ_MAX_LANES]; > + u32 lane_phy_reg[WIZ_MAX_LANES]; This name looks misleading. I'll discuss about it where you are setting it. > struct clk *input_clks[WIZ_MAX_INPUT_CLOCKS]; > struct clk *output_clks[WIZ_MAX_OUTPUT_CLOCKS]; > struct clk_onecell_data clk_data; > @@ -1231,14 +1232,28 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev, > int ret; > > /* if typec-dir gpio was specified, set LN10 SWAP bit based on that */ > - if (id == 0 && wiz->gpio_typec_dir) { > - if (wiz->typec_dir_delay) > - msleep_interruptible(wiz->typec_dir_delay); > - > - if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > - regmap_field_write(wiz->typec_ln10_swap, 1); > - else > - regmap_field_write(wiz->typec_ln10_swap, 0); > + if (id == 0 && wiz->typec_dir_delay) { > + msleep_interruptible(wiz->typec_dir_delay); > + > + if (wiz->gpio_typec_dir) { > + if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > + regmap_field_write(wiz->typec_ln10_swap, 1); > + else > + regmap_field_write(wiz->typec_ln10_swap, 0); > + } else { > + /* if no typec-dir gpio was specified, and USB lines > + * are connected to Lane 0 then set LN10 SWAP bit to 1. > + */ Why should lanes 1 and 0 be swapped if USB is connected to lane 0? > + u32 num_lanes = wiz->num_lanes; > + int i; > + > + for (i = 0; i < num_lanes; i++) { > + if ((wiz->lane_phy_type[i] == PHY_TYPE_USB3) \ > + && wiz->lane_phy_reg[i] == 0) { > + regmap_field_write(wiz->typec_ln10_swap, 1); > + } > + } I really don't understand what you are doing here. It definitely doesn't match your comment. As an example. If num_lanes = 2 then wiz->lane_phy_reg[1] is being used without being really initialized (see later). Just because of kzalloc, it would of course be 0. > + } > } > > if (id == 0) { > @@ -1370,8 +1385,10 @@ static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz) > dev_dbg(dev, "%s: Lanes %u-%u have phy-type %u\n", __func__, > reg, reg + num_lanes - 1, phy_type); > > - for (i = reg; i < reg + num_lanes; i++) > + for (i = reg; i < reg + num_lanes; i++) { > + wiz->lane_phy_reg[i] = reg; As per DT binding reg: description: The master lane number. This is the lowest numbered lane in the lane group. So you are in fact storing the Master lane number of every Link (or lane group). A link may have 1 or more lanes in it. Also notice that if num_lanes has been 2 then wiz->lane_phy_reg[1] is not initialized. > wiz->lane_phy_type[i] = phy_type; > + } > } > > return 0; > @@ -1464,24 +1481,22 @@ static int wiz_probe(struct platform_device *pdev) > goto err_addr_to_resource; > } > > - if (wiz->gpio_typec_dir) { > - ret = of_property_read_u32(node, "typec-dir-debounce-ms", > - &wiz->typec_dir_delay); > - if (ret && ret != -EINVAL) { > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > - goto err_addr_to_resource; > - } > + ret = of_property_read_u32(node, "typec-dir-debounce-ms", > + &wiz->typec_dir_delay); > + if (ret && ret != -EINVAL) { > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > + goto err_addr_to_resource; > + } > > - /* use min. debounce from Type-C spec if not provided in DT */ > - if (ret == -EINVAL) > - wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > + /* use min. debounce from Type-C spec if not provided in DT */ > + if (ret == -EINVAL) > + wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > > - if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > - wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > - ret = -EINVAL; > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > - goto err_addr_to_resource; > - } > + if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > + wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > + ret = -EINVAL; > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > + goto err_addr_to_resource; > } > > ret = wiz_get_lane_phy_types(dev, wiz); cheers, -roger
Hi Roger, On Wed, Dec 14, 2022 at 3:12 PM Roger Quadros <rogerq@kernel.org> wrote: > > Hi, > > Some more comments below. > > On 13/12/2022 14:48, Sinthu Raja wrote: > > It's possible that the Type-C plug orientation on the DIR line will be > > implemented through hardware design. In that situation, there won't be > > an external GPIO line available, but the driver still needs to address > > this since the DT won't use the typec-gpio-dir property. > > > > Add code to handle LN10 Type-C swap if typec-gpio-dir property is not > > specified in DT. > > > > Remove typec-gpio-dir check to use minimum debounce from Type-C spec if > > it is not provided in DT > > > > Signed-off-by: Sinthu Raja <sinthu.raja@ti.com> > > --- > > drivers/phy/ti/phy-j721e-wiz.c | 65 +++++++++++++++++++++------------- > > 1 file changed, 40 insertions(+), 25 deletions(-) > > > > diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c > > index 141b51af4427..b17eec632d49 100644 > > --- a/drivers/phy/ti/phy-j721e-wiz.c > > +++ b/drivers/phy/ti/phy-j721e-wiz.c > > @@ -375,6 +375,7 @@ struct wiz { > > struct gpio_desc *gpio_typec_dir; > > int typec_dir_delay; > > u32 lane_phy_type[WIZ_MAX_LANES]; > > + u32 lane_phy_reg[WIZ_MAX_LANES]; > > This name looks misleading. I'll discuss about it where you are setting it. Will change the name to master_lane_num[]; > > > struct clk *input_clks[WIZ_MAX_INPUT_CLOCKS]; > > struct clk *output_clks[WIZ_MAX_OUTPUT_CLOCKS]; > > struct clk_onecell_data clk_data; > > @@ -1231,14 +1232,28 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev, > > int ret; > > > > /* if typec-dir gpio was specified, set LN10 SWAP bit based on that */ > > - if (id == 0 && wiz->gpio_typec_dir) { > > - if (wiz->typec_dir_delay) > > - msleep_interruptible(wiz->typec_dir_delay); > > - > > - if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > > - regmap_field_write(wiz->typec_ln10_swap, 1); > > - else > > - regmap_field_write(wiz->typec_ln10_swap, 0); > > + if (id == 0 && wiz->typec_dir_delay) { > > + msleep_interruptible(wiz->typec_dir_delay); > > + > > + if (wiz->gpio_typec_dir) { > > + if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) > > + regmap_field_write(wiz->typec_ln10_swap, 1); > > + else > > + regmap_field_write(wiz->typec_ln10_swap, 0); > > + } else { > > + /* if no typec-dir gpio was specified, and USB lines > > + * are connected to Lane 0 then set LN10 SWAP bit to 1. > > + */ > > Why should lanes 1 and 0 be swapped if USB is connected to lane 0? My Bad! I should have been more precise in mentioning the USB3 Type C. Lanes 0 and 2 are reserved for USB3 for type-C connector lane swap. > > > + u32 num_lanes = wiz->num_lanes; > > + int i; > > + > > + for (i = 0; i < num_lanes; i++) { > > + if ((wiz->lane_phy_type[i] == PHY_TYPE_USB3) \ > > + && wiz->lane_phy_reg[i] == 0) { > > + regmap_field_write(wiz->typec_ln10_swap, 1); > > + } > > + } > > I really don't understand what you are doing here. > It definitely doesn't match your comment. > As an example. If num_lanes = 2 then wiz->lane_phy_reg[1] is being used without being > really initialized (see later). Just because of kzalloc, it would of course be 0. This register is used to configure the external lanes selections that need to be swapped for SerDes type C. The initialization of the lanes is done separately. This bit is set to configure that all control for lane 0 will apply to lane 1 and vice versa. Will update the commit description. > > > + } > > } > > > > if (id == 0) { > > @@ -1370,8 +1385,10 @@ static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz) > > dev_dbg(dev, "%s: Lanes %u-%u have phy-type %u\n", __func__, > > reg, reg + num_lanes - 1, phy_type); > > > > - for (i = reg; i < reg + num_lanes; i++) > > + for (i = reg; i < reg + num_lanes; i++) { > > + wiz->lane_phy_reg[i] = reg; > > As per DT binding > reg: > description: > The master lane number. This is the lowest numbered lane in the lane group. > > So you are in fact storing the Master lane number of every Link (or lane group). > A link may have 1 or more lanes in it. > > Also notice that if num_lanes has been 2 then wiz->lane_phy_reg[1] is not initialized. Irrespective of the number of lanes that are connected to the link, if the master lane is '0' or '2' and the PHY type is USB3, then this bit needs to be set in the SerDes WIZ control register (according to the design). > > > wiz->lane_phy_type[i] = phy_type; > > + } > > } > > > > return 0; > > @@ -1464,24 +1481,22 @@ static int wiz_probe(struct platform_device *pdev) > > goto err_addr_to_resource; > > } > > > > - if (wiz->gpio_typec_dir) { > > - ret = of_property_read_u32(node, "typec-dir-debounce-ms", > > - &wiz->typec_dir_delay); > > - if (ret && ret != -EINVAL) { > > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > > - goto err_addr_to_resource; > > - } > > + ret = of_property_read_u32(node, "typec-dir-debounce-ms", > > + &wiz->typec_dir_delay); > > + if (ret && ret != -EINVAL) { > > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > > + goto err_addr_to_resource; > > + } > > > > - /* use min. debounce from Type-C spec if not provided in DT */ > > - if (ret == -EINVAL) > > - wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > > + /* use min. debounce from Type-C spec if not provided in DT */ > > + if (ret == -EINVAL) > > + wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; > > > > - if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > > - wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > > - ret = -EINVAL; > > - dev_err(dev, "Invalid typec-dir-debounce property\n"); > > - goto err_addr_to_resource; > > - } > > + if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || > > + wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { > > + ret = -EINVAL; > > + dev_err(dev, "Invalid typec-dir-debounce property\n"); > > + goto err_addr_to_resource; > > } > > > > ret = wiz_get_lane_phy_types(dev, wiz); > > cheers, > -roger
diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c index 141b51af4427..b17eec632d49 100644 --- a/drivers/phy/ti/phy-j721e-wiz.c +++ b/drivers/phy/ti/phy-j721e-wiz.c @@ -375,6 +375,7 @@ struct wiz { struct gpio_desc *gpio_typec_dir; int typec_dir_delay; u32 lane_phy_type[WIZ_MAX_LANES]; + u32 lane_phy_reg[WIZ_MAX_LANES]; struct clk *input_clks[WIZ_MAX_INPUT_CLOCKS]; struct clk *output_clks[WIZ_MAX_OUTPUT_CLOCKS]; struct clk_onecell_data clk_data; @@ -1231,14 +1232,28 @@ static int wiz_phy_reset_deassert(struct reset_controller_dev *rcdev, int ret; /* if typec-dir gpio was specified, set LN10 SWAP bit based on that */ - if (id == 0 && wiz->gpio_typec_dir) { - if (wiz->typec_dir_delay) - msleep_interruptible(wiz->typec_dir_delay); - - if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) - regmap_field_write(wiz->typec_ln10_swap, 1); - else - regmap_field_write(wiz->typec_ln10_swap, 0); + if (id == 0 && wiz->typec_dir_delay) { + msleep_interruptible(wiz->typec_dir_delay); + + if (wiz->gpio_typec_dir) { + if (gpiod_get_value_cansleep(wiz->gpio_typec_dir)) + regmap_field_write(wiz->typec_ln10_swap, 1); + else + regmap_field_write(wiz->typec_ln10_swap, 0); + } else { + /* if no typec-dir gpio was specified, and USB lines + * are connected to Lane 0 then set LN10 SWAP bit to 1. + */ + u32 num_lanes = wiz->num_lanes; + int i; + + for (i = 0; i < num_lanes; i++) { + if ((wiz->lane_phy_type[i] == PHY_TYPE_USB3) \ + && wiz->lane_phy_reg[i] == 0) { + regmap_field_write(wiz->typec_ln10_swap, 1); + } + } + } } if (id == 0) { @@ -1370,8 +1385,10 @@ static int wiz_get_lane_phy_types(struct device *dev, struct wiz *wiz) dev_dbg(dev, "%s: Lanes %u-%u have phy-type %u\n", __func__, reg, reg + num_lanes - 1, phy_type); - for (i = reg; i < reg + num_lanes; i++) + for (i = reg; i < reg + num_lanes; i++) { + wiz->lane_phy_reg[i] = reg; wiz->lane_phy_type[i] = phy_type; + } } return 0; @@ -1464,24 +1481,22 @@ static int wiz_probe(struct platform_device *pdev) goto err_addr_to_resource; } - if (wiz->gpio_typec_dir) { - ret = of_property_read_u32(node, "typec-dir-debounce-ms", - &wiz->typec_dir_delay); - if (ret && ret != -EINVAL) { - dev_err(dev, "Invalid typec-dir-debounce property\n"); - goto err_addr_to_resource; - } + ret = of_property_read_u32(node, "typec-dir-debounce-ms", + &wiz->typec_dir_delay); + if (ret && ret != -EINVAL) { + dev_err(dev, "Invalid typec-dir-debounce property\n"); + goto err_addr_to_resource; + } - /* use min. debounce from Type-C spec if not provided in DT */ - if (ret == -EINVAL) - wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; + /* use min. debounce from Type-C spec if not provided in DT */ + if (ret == -EINVAL) + wiz->typec_dir_delay = WIZ_TYPEC_DIR_DEBOUNCE_MIN; - if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || - wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { - ret = -EINVAL; - dev_err(dev, "Invalid typec-dir-debounce property\n"); - goto err_addr_to_resource; - } + if (wiz->typec_dir_delay < WIZ_TYPEC_DIR_DEBOUNCE_MIN || + wiz->typec_dir_delay > WIZ_TYPEC_DIR_DEBOUNCE_MAX) { + ret = -EINVAL; + dev_err(dev, "Invalid typec-dir-debounce property\n"); + goto err_addr_to_resource; } ret = wiz_get_lane_phy_types(dev, wiz);