Re: [PATCH V1 1/3] scsi: ufs: export api for use in vendor file

[nitirawa@codeaurora.org]

On 2021-01-31 19:29, Avri Altman wrote:
>> 
>> Exporting functions ufshcd_set_dev_pwr_mode, ufshcd_disable_vreg
>> and ufshcd_enable_vreg so that vendor drivers can make use of
>> them in setting vendor specific regulator setting
>> in vendor specific file.
> As for ufshcd_{enable,disable}_vreg - maybe inline ufshcd_toggle_vreg
> and use it instead?
> 
>> 
>> Signed-off-by: Nitin Rawat <nitirawa@codeaurora.org>
>> Signed-off-by: Bao D. Nguyen <nguyenb@codeaurora.org>
>> Signed-off-by: Veerabhadrarao Badiganti <vbadigan@codeaurora.org>
>> ---
>>  drivers/scsi/ufs/ufshcd.c | 9 ++++++---
>>  drivers/scsi/ufs/ufshcd.h | 4 ++++
>>  2 files changed, 10 insertions(+), 3 deletions(-)
>> 
>> diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
>> index 9c691e4..000a03a 100644
>> --- a/drivers/scsi/ufs/ufshcd.c
>> +++ b/drivers/scsi/ufs/ufshcd.c
>> @@ -8091,7 +8091,7 @@ static int ufshcd_config_vreg(struct device 
>> *dev,
>>         return ret;
>>  }
>> 
>> -static int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg 
>> *vreg)
>> +int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>  {
>>         int ret = 0;
>> 
>> @@ -8110,8 +8110,9 @@ static int ufshcd_enable_vreg(struct device 
>> *dev,
>> struct ufs_vreg *vreg)
>>  out:
>>         return ret;
>>  }
>> +EXPORT_SYMBOL(ufshcd_enable_vreg);
> Why do you need to export it across the kernel?
> Isn't making it non-static suffices?
> Do you need it for a loadable module?
> 
>> 
>> -static int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg 
>> *vreg)
>> +int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg *vreg)
>>  {
>>         int ret = 0;
>> 
>> @@ -8131,6 +8132,7 @@ static int ufshcd_disable_vreg(struct device 
>> *dev,
>> struct ufs_vreg *vreg)
>>  out:
>>         return ret;
>>  }
>> +EXPORT_SYMBOL(ufshcd_disable_vreg);
>> 
>>  static int ufshcd_setup_vreg(struct ufs_hba *hba, bool on)
>>  {
>> @@ -8455,7 +8457,7 @@ ufshcd_send_request_sense(struct ufs_hba *hba,
>> struct scsi_device *sdp)
>>   * Returns 0 if requested power mode is set successfully
>>   * Returns non-zero if failed to set the requested power mode
>>   */
>> -static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>> +int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>>                                      enum ufs_dev_pwr_mode pwr_mode)
>>  {
>>         unsigned char cmd[6] = { START_STOP };
>> @@ -8513,6 +8515,7 @@ static int ufshcd_set_dev_pwr_mode(struct 
>> ufs_hba
>> *hba,
>>         hba->host->eh_noresume = 0;
>>         return ret;
>>  }
>> +EXPORT_SYMBOL(ufshcd_set_dev_pwr_mode);
>> 
>>  static int ufshcd_link_state_transition(struct ufs_hba *hba,
>>                                         enum uic_link_state 
>> req_link_state,
>> diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
>> index ee61f82..1410c95 100644
>> --- a/drivers/scsi/ufs/ufshcd.h
>> +++ b/drivers/scsi/ufs/ufshcd.h
>> @@ -997,6 +997,10 @@ extern int ufshcd_dme_get_attr(struct ufs_hba 
>> *hba,
>> u32 attr_sel,
>>                                u32 *mib_val, u8 peer);
>>  extern int ufshcd_config_pwr_mode(struct ufs_hba *hba,
>>                         struct ufs_pa_layer_attr *desired_pwr_mode);
>> +extern int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba,
>> +                                               enum ufs_dev_pwr_mode 
>> pwr_mode);
>> +extern int ufshcd_enable_vreg(struct device *dev, struct ufs_vreg 
>> *vreg);
>> +extern int ufshcd_disable_vreg(struct device *dev, struct ufs_vreg 
>> *vreg);
>> 
>>  /* UIC command interfaces for DME primitives */
>>  #define DME_LOCAL      0
>> --
>> 2.7.4

Hi Avri,
Thanks for reviewing it.
ufs-qcom.c can be a loadable module, so just inlining won't suffice in 
that case.
Hence export is needed.

Thanks,
Nitin