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;
Message-ID: <8d33abb0-51ab-67c4-50b3-ae95a5d9fb55@nvidia.com>
Date:   Tue, 6 Sep 2022 10:43:55 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH v2 1/2] phy: tegra: xusb: add utmi pad power on/down ops
Content-Language: en-US
To:     Vinod Koul <vkoul@kernel.org>, Jim Lin <jilin@nvidia.com>
Cc:     thierry.reding@gmail.com, jonathanh@nvidia.com, balbi@kernel.org,
        gregkh@linuxfoundation.org, bhsieh@nvidia.com,
        linux-phy@lists.infradead.org, linux-tegra@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org
References: <20220816082353.13390-1-jilin@nvidia.com>
 <20220816082353.13390-2-jilin@nvidia.com> <YxS6FBeyS1Cs7xhI@matsya>
From:   JC Kuo <jckuo@nvidia.com>
Organization: NVIDIA
In-Reply-To: <YxS6FBeyS1Cs7xhI@matsya>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VnNJY1JTVVpoL1dYeFhFajBQYmVJbHhXQXFrd0dNUDVUWW5tVktzaVNHRXZi?=
 =?utf-8?B?aXc4aTdqWXdlY1JnQ3ZUd3Y1eXNWRHRkTklvWkFBY0Z0S25yNnJHZVZheFQx?=
 =?utf-8?B?R0d6YzlDRjNlbUVEL2UxY0ppYW9ESmpsa3BjSENGMjJxMTJpVjNlbCtTckdE?=
 =?utf-8?B?emlvd29RQWRnc0FpRHYrSXQzeUEvUndRb2ZUcjVXck1teXpxdjdnclFWWUxn?=
 =?utf-8?B?ZmxWdWY2UUthc2ZQR2xyRG1RT3hmU25LUWVtVUdMaWk0S2VZM3RZQitoQnNa?=
 =?utf-8?B?U1lFdVRSV3JmUW12KzdwTnhWcHduRW15RHVFdkRJQ1hNc1pTTWt4ckhkckxy?=
 =?utf-8?B?aXB6bHBxZ0NhUzVlaXlUYVFEMXc0d0g3eURPOTFmOVFURyt4enFBc3FYMGJk?=
 =?utf-8?B?cUtZYUNNNU9LNEdMRnJqajBBUkVZTWF0MWlEL1AvL3huWldCaSszaWxxeHJq?=
 =?utf-8?B?NlVhbkdXNjlITHpYZE5tZk9mWEdTaGVPMUpxWkpmeDZhQ3JGNU5NeGorZE5l?=
 =?utf-8?B?T3VjOFpnb2FvT1BBMitFblBkamNYTEd0WDhSbkpmUzVQZmtIS3NXd000QWFF?=
 =?utf-8?B?amhjak15N0pNRk1VbXNZQXdsNzZ1d0Ivc292T1hMRUkzQjBsaStEdDdSb2VC?=
 =?utf-8?B?ZHg5cmQ0OXUwaWRFNGxXUThIRldMZWpvMFRtejBnTmdaUGg2ZjVBd1hveHQv?=
 =?utf-8?B?S1crclJDRHFuRUdVKzh3RDZwZ0grRWExRzVDUjdGVktoVVZyRERqQWU2VXBP?=
 =?utf-8?B?cXpqK0dyQzcwWnFyK3V6Sjlzcnc2ektiVnd6TmZKditsMERUblFrNDJEZVZn?=
 =?utf-8?B?VEd4MkI1bUZaemZWQVRyRjFEYU5CNlY1bWQ4cXMzRFJoUEtaaG5pbXh5eVZK?=
 =?utf-8?B?WGpTemdmMFVxOG5YOXJrK05OQ2VGOXg1M3VsNWRRdURmRUkrRDJJNTRsNFFO?=
 =?utf-8?B?bzVLZ2dYUlh5d01BUEovVm5ITTB3a1dCTm55Mm5qNitodmV3Wkc5dU9nSWsv?=
 =?utf-8?B?dDVqR2UyR3MzbVZNM3lSQklyUWtWT2hhT2NuSzZ4c2Z5Q3Z2aERDbTZFaHhL?=
 =?utf-8?B?Nm9ZeThsbDA5Q0hGczNZTUYxdkFKNTkwTkZKaFlBQmM3YU5kb0pDeVV4NGpE?=
 =?utf-8?B?QVNyK1FGZXZRVVIvVEpoUENtbHc5aDJUdDMzWEpjRmtRTStNQUdxSEZONWt2?=
 =?utf-8?B?SEZ2dXZGRncwbi9saFFkdzJRdTl0aWVhL3NaQ01qSUpoaTV0clFpNlV6R3pi?=
 =?utf-8?B?VDloRTFMamVtNUJXa0xmM2VXNUlDa1lDTTZzdklVbHA1YnVLemNrQm5DY1dn?=
 =?utf-8?B?SXluVmZNYWc4bWZlVmZGOUZmZlNYbjlaOW95ditOV2NQcklBYXIwNUQ5MDYv?=
 =?utf-8?B?QzRoNFYxN2Vhc3FETEE5Q0RlYWZpZVZVQTJzaHJqbyt2QmJaeTdGaG5TamNK?=
 =?utf-8?B?QnNFaUY2ODNsWUJYKzYwd0Y1K2JSSFJuaHczRzZUK2w1UVRaT2dKN0luR2Rx?=
 =?utf-8?B?NFJqK0o0MXVuZis1S0orQTVZc3FUY0RKbUlFd2Z3TWN6WElobVppMWFJdHRj?=
 =?utf-8?B?eTBhKzN0WWk3RFdxUmJ2UFNnTzZhSjVTY0FRa2RVWEJLc2ovcFNKNHNRYmFW?=
 =?utf-8?B?bk5maVpnV2w3Q0puUGNxZFNCTXk2cTRJQ3NVblVDTkFTaWZnYm9HOXN3QkRo?=
 =?utf-8?B?Wk9xUlYzSGdiaE9TTGN6VzRTZW05YlJwajd6YmFLVEIyek5HeFJtcnVxa1JY?=
 =?utf-8?B?bUJlajRUUmlIdTRDM3FvTWdSbGZOMzI4MWd3NzkyVUFjaG1MaUpPa3hGSWRX?=
 =?utf-8?B?bFYxMlc5T1lzeGJrNm5LWlJjelFrNGVjVzRjRS9YR2I4UEp5NGFBaHdDd0VT?=
 =?utf-8?B?K2ZGZDlhTFptdmRTdVM5M0grR0Nucm5yYnFDb1hwUXN0TlorTk9neUxleHJ6?=
 =?utf-8?B?ZFZ4KzNyN045NXFuZGVCVkROdTh0dmk4YXJXeHhuWlM5NmlGU0JiTVNmaXBO?=
 =?utf-8?B?RHhqOVYzK1RGalc0TjMyVTAvTlRHaVBPWTFpbXdyYUI4Q3RWYWtnRm4xSkVx?=
 =?utf-8?B?dGZkK2pEN3BJMVlnbkpMV2pMbXU0Zk9EeU93RXg2ZHk3NTI1SGU1NzgrYWxK?=
 =?utf-8?Q?xcN/hWNii+ukY+RnoC3062K7K?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: d9de1149-2287-472e-83bc-08da8fb1aaf7
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5195.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Sep 2022 02:44:07.0270
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: G0sZznjMa9Qt/BWcisj2r4jGXYHwQERkywE5f5bPsexsJdkbxD8ouUskFngLsnOLagkEktuYv24FLUtJETL9aw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB5643
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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

Hi Vinod,
Before the device or host is being attached, we can keep most of the
transceivers powered down (PD=1/PD_DR=1) to minimize power consumption. At this
stage, in .phy_power_on(), we enable only the single-ended receiver (PD_ZI=0)
for detecting connection. Upon detecting device's or host's connection, host or
controller driver will invoke tegra_phy_xusb_utmi_pad_power_on() to power on all
of the transceivers (PD=0/PD_DR=0) to equip full link functionality.

Thanks,
JC


On 9/4/22 22:45, Vinod Koul wrote:
> On 16-08-22, 16:23, Jim Lin wrote:
>> Add utmi_pad_power_on/down ops for each SOC instead of exporting
>> tegra_phy_xusb_utmi_pad_power_on/down directly for Tegra186 chip.
> 
> Can you please help me understand why do we need to utmi power_on/down
> exported and cant be handled thry phy-ops..
> 
>>
>> Signed-off-by: BH Hsieh <bhsieh@nvidia.com>
>> Signed-off-by: Jim Lin <jilin@nvidia.com>
>> ---
>> v2: update copyright year 
>>
>>  drivers/phy/tegra/xusb-tegra186.c | 19 ++++++++++++-------
>>  drivers/phy/tegra/xusb.c          | 22 +++++++++++++++++++++-
>>  drivers/phy/tegra/xusb.h          |  4 +++-
>>  include/linux/phy/tegra/xusb.h    |  4 +++-
>>  4 files changed, 39 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/phy/tegra/xusb-tegra186.c b/drivers/phy/tegra/xusb-tegra186.c
>> index ae3915ed9fef..5abdf81aa143 100644
>> --- a/drivers/phy/tegra/xusb-tegra186.c
>> +++ b/drivers/phy/tegra/xusb-tegra186.c
>> @@ -1,6 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  /*
>> - * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
>>   */
>>  
>>  #include <linux/delay.h>
>> @@ -638,7 +638,7 @@ static void tegra186_utmi_bias_pad_power_off(struct tegra_xusb_padctl *padctl)
>>  	mutex_unlock(&padctl->lock);
>>  }
>>  
>> -static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>> +static void tegra186_utmi_pad_power_on(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> @@ -656,6 +656,8 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>>  		return;
>>  	}
>>  
>> +	dev_dbg(dev, "power on UTMI pad %u\n", index);
>> +
>>  	tegra186_utmi_bias_pad_power_on(padctl);
>>  
>>  	udelay(2);
>> @@ -669,7 +671,7 @@ static void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>>  }
>>  
>> -static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>> +static void tegra186_utmi_pad_power_down(struct phy *phy)
>>  {
>>  	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>>  	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> @@ -679,6 +681,8 @@ static void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>>  	if (!phy)
>>  		return;
>>  
>> +	dev_dbg(padctl->dev, "power down UTMI pad %u\n", index);
>> +
>>  	value = padctl_readl(padctl, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>>  	value |= USB2_OTG_PD;
>>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL0(index));
>> @@ -849,15 +853,14 @@ static int tegra186_utmi_phy_power_on(struct phy *phy)
>>  	value |= RPD_CTRL(priv->calib.rpd_ctrl);
>>  	padctl_writel(padctl, value, XUSB_PADCTL_USB2_OTG_PADX_CTL1(index));
>>  
>> -	/* TODO: pad power saving */
>> -	tegra_phy_xusb_utmi_pad_power_on(phy);
>> +	tegra186_utmi_pad_power_on(phy);
>> +
>>  	return 0;
>>  }
>>  
>>  static int tegra186_utmi_phy_power_off(struct phy *phy)
>>  {
>> -	/* TODO: pad power saving */
>> -	tegra_phy_xusb_utmi_pad_power_down(phy);
>> +	tegra186_utmi_pad_power_down(phy);
>>  
>>  	return 0;
>>  }
>> @@ -1486,6 +1489,8 @@ static const struct tegra_xusb_padctl_ops tegra186_xusb_padctl_ops = {
>>  	.suspend_noirq = tegra186_xusb_padctl_suspend_noirq,
>>  	.resume_noirq = tegra186_xusb_padctl_resume_noirq,
>>  	.vbus_override = tegra186_xusb_padctl_vbus_override,
>> +	.utmi_pad_power_on = tegra186_utmi_pad_power_on,
>> +	.utmi_pad_power_down = tegra186_utmi_pad_power_down,
>>  };
>>  
>>  #if IS_ENABLED(CONFIG_ARCH_TEGRA_186_SOC)
>> diff --git a/drivers/phy/tegra/xusb.c b/drivers/phy/tegra/xusb.c
>> index 963de5913e50..49873718d54a 100644
>> --- a/drivers/phy/tegra/xusb.c
>> +++ b/drivers/phy/tegra/xusb.c
>> @@ -1,6 +1,6 @@
>>  // SPDX-License-Identifier: GPL-2.0-only
>>  /*
>> - * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
>>   */
>>  
>>  #include <linux/delay.h>
>> @@ -1458,6 +1458,26 @@ int tegra_phy_xusb_utmi_port_reset(struct phy *phy)
>>  }
>>  EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_port_reset);
>>  
>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	if (padctl->soc->ops->utmi_pad_power_on)
>> +		padctl->soc->ops->utmi_pad_power_on(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_on);
>> +
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy)
>> +{
>> +	struct tegra_xusb_lane *lane = phy_get_drvdata(phy);
>> +	struct tegra_xusb_padctl *padctl = lane->pad->padctl;
>> +
>> +	if (padctl->soc->ops->utmi_pad_power_down)
>> +		padctl->soc->ops->utmi_pad_power_down(phy);
>> +}
>> +EXPORT_SYMBOL_GPL(tegra_phy_xusb_utmi_pad_power_down);
>> +
>>  int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
>>  				    unsigned int port)
>>  {
>> diff --git a/drivers/phy/tegra/xusb.h b/drivers/phy/tegra/xusb.h
>> index 034f7a2c28d6..8cfbbdbd6e0c 100644
>> --- a/drivers/phy/tegra/xusb.h
>> +++ b/drivers/phy/tegra/xusb.h
>> @@ -1,6 +1,6 @@
>>  /* SPDX-License-Identifier: GPL-2.0-only */
>>  /*
>> - * Copyright (c) 2014-2020, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2014-2022, NVIDIA CORPORATION.  All rights reserved.
>>   * Copyright (c) 2015, Google Inc.
>>   */
>>  
>> @@ -412,6 +412,8 @@ struct tegra_xusb_padctl_ops {
>>  				    unsigned int index, bool enable);
>>  	int (*vbus_override)(struct tegra_xusb_padctl *padctl, bool set);
>>  	int (*utmi_port_reset)(struct phy *phy);
>> +	void (*utmi_pad_power_on)(struct phy *phy);
>> +	void (*utmi_pad_power_down)(struct phy *phy);
>>  };
>>  
>>  struct tegra_xusb_padctl_soc {
>> diff --git a/include/linux/phy/tegra/xusb.h b/include/linux/phy/tegra/xusb.h
>> index 3a35e74cdc61..70998e6dd6fd 100644
>> --- a/include/linux/phy/tegra/xusb.h
>> +++ b/include/linux/phy/tegra/xusb.h
>> @@ -1,6 +1,6 @@
>>  /* SPDX-License-Identifier: GPL-2.0-only */
>>  /*
>> - * Copyright (c) 2016-2020, NVIDIA CORPORATION.  All rights reserved.
>> + * Copyright (c) 2016-2022, NVIDIA CORPORATION.  All rights reserved.
>>   */
>>  
>>  #ifndef PHY_TEGRA_XUSB_H
>> @@ -21,6 +21,8 @@ int tegra_xusb_padctl_usb3_set_lfps_detect(struct tegra_xusb_padctl *padctl,
>>  					   unsigned int port, bool enable);
>>  int tegra_xusb_padctl_set_vbus_override(struct tegra_xusb_padctl *padctl,
>>  					bool val);
>> +void tegra_phy_xusb_utmi_pad_power_on(struct phy *phy);
>> +void tegra_phy_xusb_utmi_pad_power_down(struct phy *phy);
>>  int tegra_phy_xusb_utmi_port_reset(struct phy *phy);
>>  int tegra_xusb_padctl_get_usb3_companion(struct tegra_xusb_padctl *padctl,
>>  					 unsigned int port);
>> -- 
>> 2.17.1
>