Messages in this thread | | | Date | Tue, 05 Jan 2021 15:28:47 +0800 | From | Can Guo <> | Subject | Re: [PATCH RFC v4 1/1] scsi: ufs: Fix ufs power down/on specs violation |
| |
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?
Thanks, Can Guo.
| |