* [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode [not found] <CGME20220117103107epcas2p3d7223ff63f6cb575316695cc8fb155a4@epcas2p3.samsung.com> @ 2022-01-17 10:29 ` Kiwoong Kim 2022-01-19 0:45 ` Bart Van Assche 0 siblings, 1 reply; 5+ messages in thread From: Kiwoong Kim @ 2022-01-17 10:29 UTC (permalink / raw) To: linux-scsi, linux-kernel, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, cang, adrian.hunter, sc.suh, hy50.seo, sh425.lee, bhoon95.kim, vkumar.1997 Cc: Kiwoong Kim The return value of ufshcd_set_dev_pwr_mode is given to device pm core. However, the function currently returns a result in scsi command and the device pm core doesn't understand it. It might lead to unexpected behaviors of user land. I found the return value led to platform reset in Android. This patch is to use an generic code for SSU failures. Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 1049e41..a60816c 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -8669,6 +8669,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, pwr_mode, ret); if (ret > 0 && scsi_sense_valid(&sshdr)) scsi_print_sense_hdr(sdp, NULL, &sshdr); + ret = -EIO; } if (!ret) -- 2.7.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode 2022-01-17 10:29 ` [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode Kiwoong Kim @ 2022-01-19 0:45 ` Bart Van Assche 2022-01-20 2:51 ` Kiwoong Kim 0 siblings, 1 reply; 5+ messages in thread From: Bart Van Assche @ 2022-01-19 0:45 UTC (permalink / raw) To: Kiwoong Kim, linux-scsi, linux-kernel, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, cang, adrian.hunter, sc.suh, hy50.seo, sh425.lee, bhoon95.kim, vkumar.1997 On 1/17/22 02:29, Kiwoong Kim wrote: > The return value of ufshcd_set_dev_pwr_mode is given to > device pm core. However, the function currently returns a result > in scsi command and the device pm core doesn't understand it. > It might lead to unexpected behaviors of user land. I found > the return value led to platform reset in Android. > > This patch is to use an generic code for SSU failures. > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 1049e41..a60816c 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -8669,6 +8669,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba *hba, > pwr_mode, ret); > if (ret > 0 && scsi_sense_valid(&sshdr)) > scsi_print_sense_hdr(sdp, NULL, &sshdr); > + ret = -EIO; > } > > if (!ret) Shouldn't "ret = -EIO" only be executed if ret > 0? Additionally, please update the documentation of ufshcd_set_dev_pwr_mode(). I'm referring to the following comment: "Returns non-zero if failed to set the requested power mode". Thanks, Bart. ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode 2022-01-19 0:45 ` Bart Van Assche @ 2022-01-20 2:51 ` Kiwoong Kim 2022-01-20 17:57 ` Bart Van Assche 0 siblings, 1 reply; 5+ messages in thread From: Kiwoong Kim @ 2022-01-20 2:51 UTC (permalink / raw) To: 'Bart Van Assche', linux-scsi, sc.suh, hy50.seo, sh425.lee, bhoon95.kim, vkumar.1997 > > @@ -8669,6 +8669,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba > *hba, > > pwr_mode, ret); > > if (ret > 0 && scsi_sense_valid(&sshdr)) > > scsi_print_sense_hdr(sdp, NULL, &sshdr); > > + ret = -EIO; > > } > > > > if (!ret) > > Shouldn't "ret = -EIO" only be executed if ret > 0? Additionally, please > update the documentation of ufshcd_set_dev_pwr_mode(). I'm referring to > the following comment: "Returns non-zero if failed to set the requested > power mode". > > Thanks, > > Bart. scsi_execute returns cmd->result(int type) but I think there is no case that the valaue is negative because all values defined for its most significant byte, i.e. driver byte, are smaller than 0x80. And I understand the 'ret > 0' presents that something wrong happens during the process. So I'm not sure if 'ret = -EIO;' should be executed under 'ret > 0'. -- #define DRIVER_BUSY 0x01 #define DRIVER_SOFT 0x02 And for the comment, I got it. Thanks Kiwoong ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode 2022-01-20 2:51 ` Kiwoong Kim @ 2022-01-20 17:57 ` Bart Van Assche 2022-01-21 5:34 ` Kiwoong Kim 0 siblings, 1 reply; 5+ messages in thread From: Bart Van Assche @ 2022-01-20 17:57 UTC (permalink / raw) To: Kiwoong Kim, linux-scsi, sc.suh, hy50.seo, sh425.lee, bhoon95.kim, vkumar.1997 On 1/19/22 18:51, Kiwoong Kim wrote: >>> @@ -8669,6 +8669,7 @@ static int ufshcd_set_dev_pwr_mode(struct ufs_hba >> *hba, >>> pwr_mode, ret); >>> if (ret > 0 && scsi_sense_valid(&sshdr)) >>> scsi_print_sense_hdr(sdp, NULL, &sshdr); >>> + ret = -EIO; >>> } >>> >>> if (!ret) >> >> Shouldn't "ret = -EIO" only be executed if ret > 0? Additionally, please >> update the documentation of ufshcd_set_dev_pwr_mode(). I'm referring to >> the following comment: "Returns non-zero if failed to set the requested >> power mode". >> >> Thanks, >> >> Bart. > > scsi_execute returns cmd->result(int type) but I think there is no case that the valaue is negative > because all values defined for its most significant byte, i.e. driver byte, are smaller than 0x80. > And I understand the 'ret > 0' presents that something wrong happens during the process. > > So I'm not sure if 'ret = -EIO;' should be executed under 'ret > 0'. I think that scsi_execute() can return a negative value. From __scsi_execute(): req = scsi_alloc_request(sdev->request_queue, data_direction == DMA_TO_DEVICE ? REQ_OP_DRV_OUT : REQ_OP_DRV_IN, rq_flags & RQF_PM ? BLK_MQ_REQ_PM : 0); if (IS_ERR(req)) return PTR_ERR(req); As you probably know PTR_ERR() returns a negative error code if IS_ERR() returns true. Thanks, Bart. ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode 2022-01-20 17:57 ` Bart Van Assche @ 2022-01-21 5:34 ` Kiwoong Kim 0 siblings, 0 replies; 5+ messages in thread From: Kiwoong Kim @ 2022-01-21 5:34 UTC (permalink / raw) To: 'Bart Van Assche', linux-scsi, sc.suh, hy50.seo, sh425.lee, bhoon95.kim, vkumar.1997 > I think that scsi_execute() can return a negative value. From > __scsi_execute(): > > req = scsi_alloc_request(sdev->request_queue, > data_direction == DMA_TO_DEVICE ? > REQ_OP_DRV_OUT : REQ_OP_DRV_IN, > rq_flags & RQF_PM ? BLK_MQ_REQ_PM : 0); > if (IS_ERR(req)) > return PTR_ERR(req); > > As you probably know PTR_ERR() returns a negative error code if IS_ERR() > returns true. Right, Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-21 5:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20220117103107epcas2p3d7223ff63f6cb575316695cc8fb155a4@epcas2p3.samsung.com>
2022-01-17 10:29 ` [PATCH v1] scsi: ufs: use an generic error code in ufshcd_set_dev_pwr_mode Kiwoong Kim
2022-01-19 0:45 ` Bart Van Assche
2022-01-20 2:51 ` Kiwoong Kim
2022-01-20 17:57 ` Bart Van Assche
2022-01-21 5:34 ` Kiwoong Kim
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox