Re: [PATCH v3 04/12] scsi: ufs: core: Add support for TX Equalization

[Can Guo <can.guo@oss.qualcomm.com>]

On 3/14/2026 4:19 PM, Can Guo wrote:
>
>
> On 3/14/2026 6:19 AM, Bart Van Assche wrote:
>> On 3/8/26 8:14 AM, Can Guo wrote:
>>> +static int txeq_gear_set(const char *val, const struct kernel_param 
>>> *kp)
>>> +{
>>> +    return param_set_uint_minmax(val, kp, UFS_HS_G1, UFS_HS_G6);
>>> +}
>>
>> Why UFS_HS_G6 instead of UFS_HS_GEAR_MAX?
> I will use 'UFS_HS_GEAR_MAX - 1' in next version.
On second thought, to make the code more readable and scalable, I will 
use UFS_HS_GEAR_MAX
here. To achieve so, I am going to tweak the code like below:

enum ufs_hs_gear_tag {
         UFS_HS_DONT_CHANGE,     /* Don't change Gear */
         UFS_HS_G1,              /* HS Gear 1 (default for reset) */
         UFS_HS_G2,              /* HS Gear 2 */
         UFS_HS_G3,              /* HS Gear 3 */
         UFS_HS_G4,              /* HS Gear 4 */
         UFS_HS_G5,              /* HS Gear 5 */
+      UFS_HS_G6,              /* HS Gear 6 */
+      UFS_HS_GEAR_MAX_INVALID,
};
+
+ #define UFS_HS_GEAR_MAX         UFS_HS_GEAR_MAX_INVALID - 1

Thanks,
Can Guo.
>>
>>> @@ -955,6 +1045,11 @@ enum ufshcd_mcq_opr {
>>> + * @host_preshoot_cap: host TX PreShoot capability
>>> + * @host_deemphasis_cap: host TX DeEmphasis capability
>>> + * @device_preshoot_cap: device TX PreShoot capability
>>> + * @device_deemphasis_cap: device TX DeEmphasis capability
>>
>> Please either explain the meaning of the bits in the above four new
>> member variables or add a reference to the standard that defines the
>> meaning of the bits in these member variables.
> OK.
>>
>>> +#define UFS_HS_RATE_STRING(rate) \
>>> +    ((rate) == PA_HS_MODE_A ? "A" : \
>>> +     (rate) == PA_HS_MODE_B ? "B" : \
>>> +     "Unknown")
>>
>> Why a #define instead of an inline function? Aren't inline functions
>> preferred over preprocessor macros?
> Let me change to inline function in next version.
>
> Thanks,
> Can Guo.
>>
>> Thanks,
>>
>> Bart.
>