Re: [PATCH v2] scsi: ufs: core: Add ufshcd_send_bsg_uic_cmd() for UFS BSG

[Ziqi Chen <quic_ziqichen@quicinc.com>]

Hi Peter,

Thank you for your comment. We also recognize that the H8 Enter/Exit has 
the same issue. In fact, we have tried to add the H8 command before. 
Unfortunately, the situation with Hibern8 is complex, there are a lot of 
status checks, and some vendors have their own sequence that is 
implemented in their vendor driver by Vops. Simply including the H8 
command here would cause other issues since the vendor sequence for H8 
enter/exit is required. If we add the full sequence here, the code will 
be bloated. We haven’t come up with a good solution for the Hibern8 
situation yet. However, whether we include the H8 command or not, the 
above issue is present as well (directly call ufshcd_send_uic_cmd() via 
BSG framework).
Right now, we have tested the PA_PWRMODE case and confirmed that it 
works, so we want to quickly resolve the PA_PWRMODE issue to unblock our 
customers’ urgent cases first. As for the Hibern8 situation, we have 
plan to fix it but we want to fix it afterward, in a separated change. 
If you have any good suggestions, we can discuss separately.

BRs,
Ziqi

On 11/19/2024 2:40 PM, Peter Wang (王信友) wrote:
> On Wed, 2024-11-13 at 19:14 +0800, Ziqi Chen wrote:
> 
>> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
>> index e338867bc96c..c01f4b0c1b4f 100644
>> --- a/drivers/ufs/core/ufshcd.c
>> +++ b/drivers/ufs/core/ufshcd.c
>> @@ -4319,6 +4319,42 @@ static int ufshcd_uic_pwr_ctrl(struct ufs_hba
>> *hba, struct uic_command *cmd)
>>          return ret;
>>   }
>>
>> +/**
>> + * ufshcd_send_bsg_uic_cmd - Send UIC commands requested via BSG
>> layer and retrieve the result
>> + * @hba: per adapter instance
>> + * @uic_cmd: UIC command
>> + *
>> + * Return: 0 only if success.
>> + */
>> +int ufshcd_send_bsg_uic_cmd(struct ufs_hba *hba, struct uic_command
>> *uic_cmd)
>> +{
>> +       int ret;
>> +
>> +       if (hba->quirks & UFSHCD_QUIRK_BROKEN_UIC_CMD)
>> +               return 0;
>> +
>> +       ufshcd_hold(hba);
>> +
>> +       if (uic_cmd->argument1 == UIC_ARG_MIB(PA_PWRMODE) &&
>> +           uic_cmd->command == UIC_CMD_DME_SET) {
>>
> 
> Hi Ziqi,
> 
> Should we also check if uic_cmd->command == UIC_CMD_DME_HIBER_ENTER
> or UIC_CMD_DME_HIBER_EXIT?
> 
> Thanks
> Peter
> 
> 
> 
>> +               ret = ufshcd_uic_pwr_ctrl(hba, uic_cmd);
>> +               goto out;
>> +       }
>> +
>> +       mutex_lock(&hba->uic_cmd_mutex);
>> +       ufshcd_add_delay_before_dme_cmd(hba);
>> +
>> +       ret = __ufshcd_send_uic_cmd(hba, uic_cmd);
>> +       if (!ret)
>> +               ret = ufshcd_wait_for_uic_cmd(hba, uic_cmd);
>> +
>> +       mutex_unlock(&hba->uic_cmd_mutex);
>> +
>> +out:
>> +       ufshcd_release(hba);
>> +       return ret;
>> +}
>> +
>>   /**
>>    * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
>>    *                             using DME_SET primitives.
>> --
>> 2.34.1
>>