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.
