From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB767C433DB for ; Tue, 5 Jan 2021 07:34:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id B0FEF21973 for ; Tue, 5 Jan 2021 07:34:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725894AbhAEHeY (ORCPT ); Tue, 5 Jan 2021 02:34:24 -0500 Received: from mga11.intel.com ([192.55.52.93]:55843 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725822AbhAEHeY (ORCPT ); Tue, 5 Jan 2021 02:34:24 -0500 IronPort-SDR: SInopDdxeDtLzkVQCTkZos2buFLX17jMlP0bA4fiLaAeifpgSh+8w6ePBsUfWAGyubeVFHCqKp FmeIulLuglGg== X-IronPort-AV: E=McAfee;i="6000,8403,9854"; a="173555946" X-IronPort-AV: E=Sophos;i="5.78,476,1599548400"; d="scan'208";a="173555946" Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga102.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jan 2021 23:33:44 -0800 IronPort-SDR: UxgStSIfqJrtaRejgRdQZElqt6gDSXZmA90ekirnF/Kh0JhEYyRaLS6yVcGB0Ww4TSJVzczTC9 LvxGLwNG0JhA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.78,476,1599548400"; d="scan'208";a="462213936" Received: from ahunter-desktop.fi.intel.com (HELO [10.237.72.94]) ([10.237.72.94]) by fmsmga001.fm.intel.com with ESMTP; 04 Jan 2021 23:33:36 -0800 Subject: Re: [PATCH RFC v4 1/1] scsi: ufs: Fix ufs power down/on specs violation To: Can Guo Cc: Bjorn Andersson , Ziqi Chen , asutoshd@codeaurora.org, nguyenb@codeaurora.org, hongwus@codeaurora.org, rnayak@codeaurora.org, vinholikatti@gmail.com, jejb@linux.vnet.ibm.com, martin.petersen@oracle.com, linux-scsi@vger.kernel.org, kernel-team@android.com, saravanak@google.com, salyzyn@google.com, kwmad.kim@samsung.com, stanley.chu@mediatek.com, Alim Akhtar , Avri Altman , "James E.J. Bottomley" , Andy Gross , Matthias Brugger , Bean Huo , Bart Van Assche , Satya Tangirala , "moderated list:UNIVERSAL FLASH STORAGE HOST CONTROLLER DRIVER..." , open list , "open list:ARM/QUALCOMM SUPPORT" , "moderated list:ARM/Mediatek SoC support" References: <1608644981-46267-1-git-send-email-ziqichen@codeaurora.org> From: Adrian Hunter Organization: Intel Finland Oy, Registered Address: PL 281, 00181 Helsinki, Business Identity Code: 0357606 - 4, Domiciled in Helsinki Message-ID: Date: Tue, 5 Jan 2021 09:33:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 5/01/21 9:28 am, Can Guo wrote: > On 2021-01-05 15:16, Adrian Hunter wrote: >> On 4/01/21 8:55 pm, Bjorn Andersson wrote: >>> On Mon 04 Jan 03:15 CST 2021, Adrian Hunter wrote: >>> >>>> On 22/12/20 3:49 pm, Ziqi Chen wrote: >>>>> As per specs, e.g, JESD220E chapter 7.2, while powering >>>>> off/on the ufs device, RST_N signal and REF_CLK signal >>>>> should be between VSS(Ground) and VCCQ/VCCQ2. >>>>> >>>>> To flexibly control device reset line, refactor the function >>>>> ufschd_vops_device_reset(sturct ufs_hba *hba) to ufshcd_ >>>>> vops_device_reset(sturct ufs_hba *hba, bool asserted). The >>>>> new parameter "bool asserted" is used to separate device reset >>>>> line pulling down from pulling up. >>>> >>>> This patch assumes the power is controlled by voltage regulators, but >>>> for us >>>> it is controlled by firmware (ACPI), so it is not correct to change RST_n >>>> for all host controllers as you are doing. >>>> >>>> Also we might need to use a firmware interface for device reset, in which >>>> case the 'asserted' value doe not make sense. >>>> >>> >>> Are you saying that the entire flip-flop-the-reset is a single firmware >>> operation in your case? >> >> Yes >> >>>                         If you look at the Mediatek driver, the >>> implementation of ufs_mtk_device_reset_ctrl() is a jump to firmware. >>> >>> >>> But perhaps "asserted" isn't the appropriate English word for saying >>> "the reset is in the resetting state"? >>> >>> I just wanted to avoid the use of "high"/"lo" as if you look at the >>> Mediatek code they pass the expected line-level to the firmware, while >>> in the Qualcomm code we pass the logical state to the GPIO code which is >>> setup up as "active low" and thereby flip the meaning before hitting the >>> pad. >>> >>>> Can we leave the device reset callback alone, and instead introduce a new >>>> variant operation for setting RST_n to match voltage regulator power >>>> changes? >>> >>> Wouldn't this new function just have to look like the proposed patches? >>> In which case for existing platforms we'd have both? >>> >>> How would you implement this, or would you simply skip implementing >>> this? >> >> Functionally, doing a device reset is not the same as adjusting signal >> levels to meet power up/off ramp requirements.  However, the issue is that >> we do not use regulators, so the power is not necessarily being changed at >> those points, and we definitely do not want to reset instead of entering >> DeepSleep for example. >> >> Off the top of my head, I imagine something like a callback called >> ufshcd_vops_prepare_power_ramp(hba, bool on) which is called only if >> hba->vreg_info->vcc is not NULL. > > Hi Adrian, > > I don't see you have the vops device_reset() implemented anywhere in > current code base, how is this change impacting you? Do I miss anything > or are you planning to push a change which implements device_reset() soon? At some point, yes.