On Thu, 2024-11-21 at 10:24 -0800, Bart Van Assche wrote: > External email : Please do not click links or open attachments until > you have verified the sender or the content. > > > On 11/20/24 1:55 AM, peter.wang@xxxxxxxxxxxx wrote: > > @@ -4682,6 +4679,10 @@ int ufshcd_config_pwr_mode(struct ufs_hba > > *hba, > > > > ret = ufshcd_change_power_mode(hba, &final_params); > > > > + if (!ret) > > + ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, NULL, > > + &final_params); > > + > > return ret; > > } > > EXPORT_SYMBOL_GPL(ufshcd_config_pwr_mode); > > This patch causes the wrong power parameters to be passed to the > POST_CHANGE callback. I think that &final_params should be changed > into > &hba->pwr_info above. > Hi Bart, &final_params represents the final power mode to be switched to. If the power mode change is successful, it will be copied to &hba->pwr_info. So, &final_params should always be equal to &hba->pwr_info if successful, right?" > Additionally, this patch includes a subtle but important behavior > change. Without this patch, POST_CHANGE callback invocations can > influence the power settings that are copied into hba->pwr_info. With > this patch applied that is no longer possible. This behavior should > not > break any UFS host driver as far as I can tell. > > Please explain in the comment block above struct ufs_hba_variant_ops > that PRE_CHANGE invocations of the pwr_change_notify callback are > allowed to modify the power attribute arguments while POST_CHANGE > invocations must not modify the power attribute arguments. > > Thanks, > > Bart. Yes, currently, POST_CHANGE does not and should not have the host driver modifying final_params. I will add comments to clarify this. Thanks Peter
