Re: [PATCH V3 10/16] scsi: ufs: add UFS power management support

["Dolev Raviv" <draviv@codeaurora.org>]

Hi Kiran,
Thanks for bringing it to my attention. Fortunately I already noticed it
and fixed it. It will be sent soon when all comments on this series are
addressed.

Thanks,
Dolev

> Hi Dolev,
>
> On Wednesday 10 September 2014 05:24 PM, Dolev Raviv wrote:
>> From: Subhash Jadavani <subhashj@codeaurora.org>
>>
>> This patch adds support for UFS device and UniPro link power management
>> during runtime/system PM.
>>
>
> <snip>
>
>> +vccq_lpm:
>> +	ufshcd_config_vreg_lpm(hba, hba->vreg_info.vccq);
>> +vcc_disable:
>> +	ufshcd_toggle_vreg(hba->dev, hba->vreg_info.vcc, false);
>> +out:
>> +	return ret;
>> +}
>> +
>> +static void ufshcd_hba_vreg_set_lpm(struct ufs_hba *hba)
>> +{
>> +	if (ufshcd_is_link_off(hba))
>> +		ufshcd_setup_hba_vreg(hba, false);
>
> I didn't find any declaration and definition of above API in your patch
> set.
> During compilation of this driver I got a below error, am I missing any
> thing?
>
> drivers/scsi/ufs/ufshcd.c: In function ‘ufshcd_hba_vreg_set_lpm’:
> drivers/scsi/ufs/ufshcd.c:4656:3: error: implicit declaration of
> function ‘ufshcd_setup_hba_vreg’ [-Werror=implicit-function-declaration]
>    ufshcd_setup_hba_vreg(hba, false);
>    ^
>
>> +}
>> +
>> +static void ufshcd_hba_vreg_set_hpm(struct ufs_hba *hba)
>> +{
>> +	if (ufshcd_is_link_off(hba))
>> +		ufshcd_setup_hba_vreg(hba, true);
>
> Ditto.
>
>>  }
>> -EXPORT_SYMBOL_GPL(ufshcd_suspend);
>>
>>  /**
>> - * ufshcd_resume - resume power management function
>> + * ufshcd_suspend - helper function for suspend operations
>>   * @hba: per adapter instance
>> + * @pm_op: desired low power operation type
>> + *
>> + * This function will try to put the UFS device and link into low power
>> + * mode based on the "rpm_lvl" (Runtime PM level) or "spm_lvl"
>> + * (System PM level).
>>   *
>
> <snip>
>
>> +int ufshcd_system_resume(struct ufs_hba *hba)
>> +{
>> +	if (!hba || !hba->is_powered || pm_runtime_suspended(hba->dev))
>> +		/*
>> +		 * Let the runtime resume take care of resuming
>> +		 * if runtime suspended.
>> +		 */
>> +		return 0;
>> +	else
>
> "else" is not generally useful after a break or return.
>
>> +		return ufshcd_resume(hba, UFS_SYSTEM_PM);
>> +}
>> +EXPORT_SYMBOL(ufshcd_system_resume);
>> +
>> +/**
>> + * ufshcd_runtime_suspend - runtime suspend routine
>> + * @hba: per adapter instance
>> + *
>> + * Check the description of ufshcd_suspend() function for more details.
>> + *
>> + * Returns 0 for success and non-zero for failure
>> + */
>> +int ufshcd_runtime_suspend(struct ufs_hba *hba)
>> +{
>> +	if (!hba || !hba->is_powered)
>> +		return 0;
>> +	else
>
> "else" is not generally useful after a break or return.
>
>> +		return ufshcd_suspend(hba, UFS_RUNTIME_PM);
>>  }
>>  EXPORT_SYMBOL(ufshcd_runtime_suspend);
>>
>> +/**
>> + * ufshcd_runtime_resume - runtime resume routine
>> + * @hba: per adapter instance
>> + *
>> + * This function basically brings the UFS device, UniPro link and
>> controller
>> + * to active state. Following operations are done in this function:
>> + *
>
> Thanks,
> --Kiran
>
>


-- 
QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation