Hi Deepak,
The 0day bot reported an issue with this patch with Clang, would you
mind taking a look at it? The full report is attached at the bottom,
I've added a comment inline before it though.
On Tue, Dec 24, 2019 at 10:11:40AM +0530, Deepak Ukey wrote:
> From: Deepak Ukey <Deepak.Ukey@xxxxxxxxxxxxx>
>
> Added the IOCTL functionality for SGPIO.
>
> Signed-off-by: Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx>
> Signed-off-by: Viswas G <Viswas.G@xxxxxxxxxxxxx>
> Signed-off-by: Vishakha Channapattan <vishakhavc@xxxxxxxxxx>
> Signed-off-by: Bhavesh Jashnani <bjashnani@xxxxxxxxxx>
> Signed-off-by: Radha Ramachandran <radha@xxxxxxxxxx>
> Signed-off-by: Akshat Jain <akshatzen@xxxxxxxxxx>
> Signed-off-by: Yu Zheng <yuuzheng@xxxxxxxxxx>
> ---
> drivers/scsi/pm8001/pm8001_ctl.c | 73 ++++++++++++++++
> drivers/scsi/pm8001/pm8001_ctl.h | 72 ++++++++++++++++
> drivers/scsi/pm8001/pm8001_hwi.c | 172 +++++++++++++++++++++++++++++++++++++-
> drivers/scsi/pm8001/pm8001_hwi.h | 17 ++++
> drivers/scsi/pm8001/pm8001_init.c | 3 +
> drivers/scsi/pm8001/pm8001_sas.c | 37 ++++++++
> drivers/scsi/pm8001/pm8001_sas.h | 20 +++++
> drivers/scsi/pm8001/pm80xx_hwi.c | 6 ++
> drivers/scsi/pm8001/pm80xx_hwi.h | 3 +
> 9 files changed, 402 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/pm8001/pm8001_ctl.c b/drivers/scsi/pm8001/pm8001_ctl.c
> index 8292074c1e6f..3e59b2a7185a 100644
> --- a/drivers/scsi/pm8001/pm8001_ctl.c
> +++ b/drivers/scsi/pm8001/pm8001_ctl.c
> @@ -1009,6 +1009,76 @@ static long pm8001_gpio_ioctl(struct pm8001_hba_info *pm8001_ha,
> return ret;
> }
>
> +static long pm8001_sgpio_ioctl(struct pm8001_hba_info *pm8001_ha,
> + unsigned long arg)
> +{
> + struct sgpio_buffer buffer;
> + struct read_write_req_resp *req = &buffer.sgpio_req;
> + struct sgpio_req payload;
> + struct sgpio_ioctl_resp *sgpio_resp;
> + DECLARE_COMPLETION_ONSTACK(completion);
> + unsigned long timeout;
> + u32 ret = 0, i;
> +
> + if (copy_from_user(&buffer, (struct sgpio_buffer *)arg,
> + sizeof(struct sgpio_buffer))) {
> + return ADPT_IOCTL_CALL_FAILED;
> + }
> + mutex_lock(&pm8001_ha->ioctl_mutex);
> + pm8001_ha->ioctl_completion = &completion;
> +
> + payload.func_reg_index = cpu_to_le32((req->register_index << 24) |
> + (req->register_type << 16) | (req->function << 8) |
> + SMP_FRAME_REQ);
> + payload.count = req->register_count;
> +
> + if (req->function == WRITE_SGPIO_REGISTER) {
> + if (req->register_count > MAX_SGPIO_REQ_PAYLOAD) {
> + ret = ADPT_IOCTL_CALL_FAILED;
> + goto exit;
> + }
> + for (i = 0; i < req->register_count; i++)
> + payload.value[i] = req->read_write_data[i];
> + }
> +
> + ret = PM8001_CHIP_DISP->sgpio_req(pm8001_ha, &payload);
> + if (ret != 0) {
> + ret = ADPT_IOCTL_CALL_FAILED;
> + goto exit;
> + }
timeout is uninitialized here; could this if statement be removed and
have the msecs_to_jiffies just hardcode the 2000?
> + if (timeout < 2000)
> + timeout = 2000;
> +
> + timeout = wait_for_completion_timeout(&completion,
> + msecs_to_jiffies(timeout));
> + if (timeout == 0) {
> + ret = ADPT_IOCTL_CALL_TIMEOUT;
> + goto exit;
> + }
> +
> + sgpio_resp = &pm8001_ha->sgpio_resp;
> + req->frame_type = sgpio_resp->func_result & 0xff;
> + req->function = (sgpio_resp->func_result >> 8) & 0xff;
> + req->function_result = (sgpio_resp->func_result >> 16) & 0xff;
> + if (req->function == READ_SGPIO_REGISTER) {
> + for (i = 0; i < req->register_count; i++)
> + req->read_write_data[i] = sgpio_resp->value[i];
> + }
> + ret = ADPT_IOCTL_CALL_SUCCESS;
> +exit:
> + spin_lock_irq(&pm8001_ha->ioctl_lock);
> + pm8001_ha->ioctl_completion = NULL;
> + spin_unlock_irq(&pm8001_ha->ioctl_lock);
> + buffer.header.return_code = ret;
> + if (copy_to_user((void *)arg, (void *)&buffer,
> + sizeof(struct sgpio_buffer))) {
> + ret = ADPT_IOCTL_CALL_FAILED;
> + }
> + mutex_unlock(&pm8001_ha->ioctl_mutex);
> +
> + return ret;
> +}
> +
> static int pm8001_ioctl_get_phy_profile(struct pm8001_hba_info *pm8001_ha,
> unsigned long arg)
> {
> @@ -1172,6 +1242,9 @@ static long pm8001_ioctl(struct file *file,
> case ADPT_IOCTL_GPIO:
> ret = pm8001_gpio_ioctl(pm8001_ha, arg);
> break;
> + case ADPT_IOCTL_SGPIO:
> + ret = pm8001_sgpio_ioctl(pm8001_ha, arg);
> + break;
> case ADPT_IOCTL_GET_PHY_PROFILE:
> ret = pm8001_ioctl_get_phy_profile(pm8001_ha, arg);
> return ret;
On Thu, Dec 26, 2019 at 06:16:43AM +0800, kbuild test robot wrote:
> CC: kbuild-all@xxxxxxxxxxxx
> In-Reply-To: <20191224044143.8178-10-deepak.ukey@xxxxxxxxxxxxx>
> References: <20191224044143.8178-10-deepak.ukey@xxxxxxxxxxxxx>
> TO: Deepak Ukey <deepak.ukey@xxxxxxxxxxxxx>
> CC: linux-scsi@xxxxxxxxxxxxxxx
> CC: Vasanthalakshmi.Tharmarajan@xxxxxxxxxxxxx, Viswas.G@xxxxxxxxxxxxx, deepak.ukey@xxxxxxxxxxxxx, jinpu.wang@xxxxxxxxxxxxxxxx, martin.petersen@xxxxxxxxxx, dpf@xxxxxxxxxx, yuuzheng@xxxxxxxxxx, auradkar@xxxxxxxxxx, vishakhavc@xxxxxxxxxx, bjashnani@xxxxxxxxxx, radha@xxxxxxxxxx, akshatzen@xxxxxxxxxx
>
> Hi Deepak,
>
> Thank you for the patch! Perhaps something to improve:
>
> [auto build test WARNING on mkp-scsi/for-next]
> [cannot apply to scsi/for-next v5.5-rc3 next-20191220]
> [if your patch is applied to the wrong git tree, please drop us a note to help
> improve the system. BTW, we also suggest to use '--base' option to specify the
> base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
>
> url: https://github.com/0day-ci/linux/commits/Deepak-Ukey/pm80xx-Updates-for-the-driver-version-0-1-39/20191225-181036
> base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next
> config: x86_64-allyesconfig (attached as .config)
> compiler: clang version 10.0.0 (git://gitmirror/llvm_project 9a77c2095439ba41bd8f6f35931b94075b2fd45b)
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=x86_64
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@xxxxxxxxx>
>
> All warnings (new ones prefixed by >>):
>
> In file included from drivers/scsi/pm8001/pm8001_ctl.c:43:
> In file included from drivers/scsi/pm8001/pm8001_sas.h:58:
> drivers/scsi/pm8001/pm8001_defs.h:102:9: warning: 'CONFIG_SCSI_PM8001_MAX_DMA_SG' macro redefined [-Wmacro-redefined]
> #define CONFIG_SCSI_PM8001_MAX_DMA_SG 528
> ^
> ./include/generated/autoconf.h:10861:9: note: previous definition is here
> #define CONFIG_SCSI_PM8001_MAX_DMA_SG 128
> ^
> >> drivers/scsi/pm8001/pm8001_ctl.c:1049:6: warning: variable 'timeout' is uninitialized when used here [-Wuninitialized]
> if (timeout < 2000)
> ^~~~~~~
> drivers/scsi/pm8001/pm8001_ctl.c:1020:23: note: initialize the variable 'timeout' to silence this warning
> unsigned long timeout;
> ^
> = 0
> 2 warnings generated.
>
> vim +/timeout +1049 drivers/scsi/pm8001/pm8001_ctl.c
>
> 1011
> 1012 static long pm8001_sgpio_ioctl(struct pm8001_hba_info *pm8001_ha,
> 1013 unsigned long arg)
> 1014 {
> 1015 struct sgpio_buffer buffer;
> 1016 struct read_write_req_resp *req = &buffer.sgpio_req;
> 1017 struct sgpio_req payload;
> 1018 struct sgpio_ioctl_resp *sgpio_resp;
> 1019 DECLARE_COMPLETION_ONSTACK(completion);
> 1020 unsigned long timeout;
> 1021 u32 ret = 0, i;
> 1022
> 1023 if (copy_from_user(&buffer, (struct sgpio_buffer *)arg,
> 1024 sizeof(struct sgpio_buffer))) {
> 1025 return ADPT_IOCTL_CALL_FAILED;
> 1026 }
> 1027 mutex_lock(&pm8001_ha->ioctl_mutex);
> 1028 pm8001_ha->ioctl_completion = &completion;
> 1029
> 1030 payload.func_reg_index = cpu_to_le32((req->register_index << 24) |
> 1031 (req->register_type << 16) | (req->function << 8) |
> 1032 SMP_FRAME_REQ);
> 1033 payload.count = req->register_count;
> 1034
> 1035 if (req->function == WRITE_SGPIO_REGISTER) {
> 1036 if (req->register_count > MAX_SGPIO_REQ_PAYLOAD) {
> 1037 ret = ADPT_IOCTL_CALL_FAILED;
> 1038 goto exit;
> 1039 }
> 1040 for (i = 0; i < req->register_count; i++)
> 1041 payload.value[i] = req->read_write_data[i];
> 1042 }
> 1043
> 1044 ret = PM8001_CHIP_DISP->sgpio_req(pm8001_ha, &payload);
> 1045 if (ret != 0) {
> 1046 ret = ADPT_IOCTL_CALL_FAILED;
> 1047 goto exit;
> 1048 }
> > 1049 if (timeout < 2000)
> 1050 timeout = 2000;
> 1051
> 1052 timeout = wait_for_completion_timeout(&completion,
> 1053 msecs_to_jiffies(timeout));
> 1054 if (timeout == 0) {
> 1055 ret = ADPT_IOCTL_CALL_TIMEOUT;
> 1056 goto exit;
> 1057 }
> 1058
> 1059 sgpio_resp = &pm8001_ha->sgpio_resp;
> 1060 req->frame_type = sgpio_resp->func_result & 0xff;
> 1061 req->function = (sgpio_resp->func_result >> 8) & 0xff;
> 1062 req->function_result = (sgpio_resp->func_result >> 16) & 0xff;
> 1063 if (req->function == READ_SGPIO_REGISTER) {
> 1064 for (i = 0; i < req->register_count; i++)
> 1065 req->read_write_data[i] = sgpio_resp->value[i];
> 1066 }
> 1067 ret = ADPT_IOCTL_CALL_SUCCESS;
> 1068 exit:
> 1069 spin_lock_irq(&pm8001_ha->ioctl_lock);
> 1070 pm8001_ha->ioctl_completion = NULL;
> 1071 spin_unlock_irq(&pm8001_ha->ioctl_lock);
> 1072 buffer.header.return_code = ret;
> 1073 if (copy_to_user((void *)arg, (void *)&buffer,
> 1074 sizeof(struct sgpio_buffer))) {
> 1075 ret = ADPT_IOCTL_CALL_FAILED;
> 1076 }
> 1077 mutex_unlock(&pm8001_ha->ioctl_mutex);
> 1078
> 1079 return ret;
> 1080 }
> 1081
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/hyperkitty/list/kbuild-all@xxxxxxxxxxxx Intel Corporation
Cheers,
Nathan
