Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Sender: nitirawa=codeaurora.org@mg.codeaurora.org
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Date:   Mon, 01 Feb 2021 13:40:10 +0530
From:   nitirawa@codeaurora.org
To:     Avri Altman <Avri.Altman@wdc.com>
Cc:     asutoshd@codeaurora.org, cang@codeaurora.org,
        stummala@codeaurora.org, vbadigan@codeaurora.org,
        alim.akhtar@samsung.com, jejb@linux.ibm.com,
        martin.petersen@oracle.com, stanley.chu@mediatek.com,
        beanhuo@micron.com, bvanassche@acm.org, linux-scsi@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        "Bao D . Nguyen" <nguyenb@codeaurora.org>
Subject: Re: [PATCH V1 1/3] scsi: ufs: export api for use in vendor file
In-Reply-To: <DM6PR04MB657574D3D8B99F3A4997D810FCB79@DM6PR04MB6575.namprd04.prod.outlook.com>
References: <1611852899-2171-1-git-send-email-nitirawa@codeaurora.org>
 <1611852899-2171-2-git-send-email-nitirawa@codeaurora.org>
 <DM6PR04MB657574D3D8B99F3A4997D810FCB79@DM6PR04MB6575.namprd04.prod.outlook.com>
Message-ID: <505fe60c90b13953d0fd0192aa9e6166@codeaurora.org>
User-Agent: Roundcube Webmail/1.3.9
Precedence: bulk

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