Hi Bart,

On 2/28/2026 3:31 AM, Bart Van Assche wrote:

On 2/27/26 8:07 AM, Can Guo wrote:

quoted

Before power mode change to a target power mode, TX Equalzation Training

"Equalzation" -> "Equalization"

Done.

quoted

(EQTR) needs be done for that target power mode. In addition, before TX
EQTR we need to change the power mode to HS-G1. These cannot happen
before the vops pwr_change_notify(PRE_CHANGE) because we don't know the
negotiated target power mode yet. It is neither approprite if all these
happen post the vops pwr_change_notify(PRE_CHANGE) as we are going to
change the power mode to HS-G1 for TX EQTR.

approprite -> appropriate

Done.

Additionally, if "neither" occurs in a sentence, "nor" should occur in
the same sentence. I don't see "nor" in the above sentence?

Will improve the commit message in next version.

quoted

Introduce a new ufshcd vops negotiate_pwr_mode(), so that TX EQTR can be
done after vops negotiate_pwr_mode() and before vops 
pwr_change_notify().

This patch does much more than only introducing a new vendor operation.
Please make sure the patch description is complete.

Done.

quoted

-    return -ENOTSUPP;
+    return -EOPNOTSUPP;

Why has ENOTSUPP been changed into EOPNOTSUPP?

I got a warning from checkpatch.pl when I add the new vops, so I changed 
the same for
ufshcd_vops_pwr_change_notify() too.

WARNING: ENOTSUPP is not a SUSV4 error code, prefer EOPNOTSUPP
#59: FILE: drivers/ufs/core/ufshcd-priv.h:178:
+       return -ENOTSUPP;

quoted

-static int ufshcd_change_power_mode(struct ufs_hba *hba,
-                 struct ufs_pa_layer_attr *pwr_mode)
+static int __ufshcd_change_power_mode(struct ufs_hba *hba,
+                      struct ufs_pa_layer_attr *pwr_mode)
  {
      int ret;

The double underscore prefix is typically used in the Linux kernel to
indicate that the caller holds a lock. That is not the case here. Please
choose another name for this function, e.g.
ufshcd_dme_change_power_mode().

Done.

Thanks,

Bart.