Re: UFS API in the kernel
On 2016-09-28 02:19, Joao Pinto wrote: Hi again! Could you also send me an example of how you are using the IOCTL from your user app (send/receive data)? I already have my implemented but you use a different mechanism (I have checked your structures in uapi/scsi/ufs/) and I have to port it! Thanks! You may use the SG_IO ioctl with sg device node for any data read/write operations. http://www.tldp.org/HOWTO/SCSI-Generic-HOWTO/sg_io.html has some details. On 9/28/2016 10:06 AM, Joao Pinto wrote: Hi Subhash, On 9/28/2016 12:05 AM, subha...@codeaurora.org wrote: Hi Joao, On 2016-09-26 18:10, Kiwoong Kim wrote: Hi. If you want to declare some things for user interface, is it be better to put those thing include/uapi/linux/ than include/linux? Agreed with Mr. Pinto's opinion with respect to implementing additional ioctls. Yes, "scsi_host_template" allows the LLD's to export their ioctl callback and then you can use the sg interface to issue UFS specific ioctls. We had implemented similar ioctl for our use, here is the reference code: https://source.codeaurora.org/quic/la/kernel/msm-3.18/tree/drivers/scsi/ufs/ufshcd.c?h=LA.HB.1.1.1.c2#n6791. This was mainly done to export the UFS query request interface to user space. Declarations where exported under include/uapi/scsi/ufs/ . If you want to build upon this already existing functionality, i can post the formal patch on mailing list. Yes, of course I have interest on building on top of what you have done already. Did yu submit this patch to the kernel already? Could you please send me the patch and kernel version to apply? Thanks, Joao Regards, Subhash Regards. -Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Shaun Tancheff Sent: Tuesday, September 27, 2016 4:23 AM To: Joao Pinto Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: UFS API in the kernel On Thu, Sep 22, 2016 at 10:21 AM, Joao Pinto wrote: Hi! I am designing an application that has the goal to be an utility for Unipro and UFS testing purposes. This application is going to run on top of a recent Linux Kernel containing the new UFS stack (including the new DWC drivers). I am considering doing the following: a) Create a new config item called CONFIG_UFS_CHARDEV which is going to create a char device responsible to make some IOCTL available for user-space applications b) Create a linux/ufs.h header file that contains data structures declarations that will be needed in user-space applications I am not very familiar with UFS devices, that said you should have an sgX chardev being created already so you can handle SG_IO requests. There also appear to be some sysfs entries being created. So between sg and sysfs you should be able to handle any user-space out of band requests without resorting to making a new chardev. Adding more sysfs entries, if you need them, should be fine. You may find it easier to expand on the existing interfaces than to get consensus on a new driver and ioctls. Hope this helps, Shaun Could you please advise me about what the correct approach should be to make it as standard as possible and usable in the future? Thank you very much for your help! regards, Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_ma jordomo-2Dinfo.html&d=DQICaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ug l8V50qIHLe856QW0qfG3WVYGOrWzA&m=vJFB6pCywWtdvkgHz9Vc0jQz0xzeyZlr-7eCWY u88nM&s=yiQLPFpqmMrbqLZz1Jb3aNqOje2dRMLJHEzUDobwcXc&e= -- Shaun Tancheff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH -next] scsi: ufs: fix error return code in ufshcd_init()
Looks good to me. Reviewed-by: Subhash Jadavani On 2016-09-28 07:49, Wei Yongjun wrote: From: Wei Yongjun Fix to return a negative error code from the error handling case instead of 0, as done elsewhere in this function. Signed-off-by: Wei Yongjun --- 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 37f3c51..6aebb7e 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6500,6 +6500,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) if (IS_ERR(hba->devfreq)) { dev_err(hba->dev, "Unable to register with devfreq %ld\n", PTR_ERR(hba->devfreq)); + err = PTR_ERR(hba->devfreq); goto out_remove_scsi_host; } /* Suspend devfreq until the UFS device is detected */ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UFS API in the kernel
On 2016-09-28 02:57, Joao Pinto wrote: I was able to get the 7 patches to have the UFS IOCTL features from your repo. BTW, why weren't these features submitted to the kernel? I checked lots of tweaks that you have been making to the UFS... do you synchronize them periodically with the mainline? Thanks! We had mainlined quite a lot UFS patches in past and I am also planning to push out the remaining deltas to mainline over next couple of months. Thanks. On 9/28/2016 10:19 AM, Joao Pinto wrote: Hi again! Could you also send me an example of how you are using the IOCTL from your user app (send/receive data)? I already have my implemented but you use a different mechanism (I have checked your structures in uapi/scsi/ufs/) and I have to port it! Thanks! On 9/28/2016 10:06 AM, Joao Pinto wrote: Hi Subhash, On 9/28/2016 12:05 AM, subha...@codeaurora.org wrote: Hi Joao, On 2016-09-26 18:10, Kiwoong Kim wrote: Hi. If you want to declare some things for user interface, is it be better to put those thing include/uapi/linux/ than include/linux? Agreed with Mr. Pinto's opinion with respect to implementing additional ioctls. Yes, "scsi_host_template" allows the LLD's to export their ioctl callback and then you can use the sg interface to issue UFS specific ioctls. We had implemented similar ioctl for our use, here is the reference code: https://source.codeaurora.org/quic/la/kernel/msm-3.18/tree/drivers/scsi/ufs/ufshcd.c?h=LA.HB.1.1.1.c2#n6791. This was mainly done to export the UFS query request interface to user space. Declarations where exported under include/uapi/scsi/ufs/ . If you want to build upon this already existing functionality, i can post the formal patch on mailing list. Yes, of course I have interest on building on top of what you have done already. Did yu submit this patch to the kernel already? Could you please send me the patch and kernel version to apply? Thanks, Joao Regards, Subhash Regards. -Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Shaun Tancheff Sent: Tuesday, September 27, 2016 4:23 AM To: Joao Pinto Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: UFS API in the kernel On Thu, Sep 22, 2016 at 10:21 AM, Joao Pinto wrote: Hi! I am designing an application that has the goal to be an utility for Unipro and UFS testing purposes. This application is going to run on top of a recent Linux Kernel containing the new UFS stack (including the new DWC drivers). I am considering doing the following: a) Create a new config item called CONFIG_UFS_CHARDEV which is going to create a char device responsible to make some IOCTL available for user-space applications b) Create a linux/ufs.h header file that contains data structures declarations that will be needed in user-space applications I am not very familiar with UFS devices, that said you should have an sgX chardev being created already so you can handle SG_IO requests. There also appear to be some sysfs entries being created. So between sg and sysfs you should be able to handle any user-space out of band requests without resorting to making a new chardev. Adding more sysfs entries, if you need them, should be fine. You may find it easier to expand on the existing interfaces than to get consensus on a new driver and ioctls. Hope this helps, Shaun Could you please advise me about what the correct approach should be to make it as standard as possible and usable in the future? Thank you very much for your help! regards, Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_ma jordomo-2Dinfo.html&d=DQICaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ug l8V50qIHLe856QW0qfG3WVYGOrWzA&m=vJFB6pCywWtdvkgHz9Vc0jQz0xzeyZlr-7eCWY u88nM&s=yiQLPFpqmMrbqLZz1Jb3aNqOje2dRMLJHEzUDobwcXc&e= -- Shaun Tancheff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR
On 2016-09-27 22:14, Martin K. Petersen wrote: "Subhash" == subhashj writes: Subhash> Looks good to me. - /* Data segment length */ - ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD( - 0, 0, len >> 8, (u8)len); + /* Data segment length only need for WRITE_DESC */ + if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC) + ucd_req_ptr->header.dword_2 = + UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len); + else + ucd_req_ptr->header.dword_2 = 0; What about READ_DESC? This patch is changing the value written to "Data Segment Length" field in the Basic header section of Query Request UPIU. This is description of "Data Segment Length" from UFS device v2.1 spec (JESD220C, line#1429-1430): "The Data Segment Length field contains the number of valid bytes within the Data Segment of the UPIU". Because we will not be sending any data segment in the request UPIU itself for the read descriptor hence this field can be zeroed out for descriptor read. Read length for the "read descriptor" transaction needs to be specified (in ) in "LENGTH" of transaction specific fields of query request UPIU. Thanks, Subhash -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: UFS API in the kernel
Hi Joao, On 2016-09-26 18:10, Kiwoong Kim wrote: Hi. If you want to declare some things for user interface, is it be better to put those thing include/uapi/linux/ than include/linux? Agreed with Mr. Pinto's opinion with respect to implementing additional ioctls. Yes, "scsi_host_template" allows the LLD's to export their ioctl callback and then you can use the sg interface to issue UFS specific ioctls. We had implemented similar ioctl for our use, here is the reference code: https://source.codeaurora.org/quic/la/kernel/msm-3.18/tree/drivers/scsi/ufs/ufshcd.c?h=LA.HB.1.1.1.c2#n6791. This was mainly done to export the UFS query request interface to user space. Declarations where exported under include/uapi/scsi/ufs/ . If you want to build upon this already existing functionality, i can post the formal patch on mailing list. Regards, Subhash Regards. -Original Message- From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- ow...@vger.kernel.org] On Behalf Of Shaun Tancheff Sent: Tuesday, September 27, 2016 4:23 AM To: Joao Pinto Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org Subject: Re: UFS API in the kernel On Thu, Sep 22, 2016 at 10:21 AM, Joao Pinto wrote: > Hi! > > I am designing an application that has the goal to be an utility for > Unipro and UFS testing purposes. This application is going to run on > top of a recent Linux Kernel containing the new UFS stack (including the new DWC drivers). > > I am considering doing the following: > a) Create a new config item called CONFIG_UFS_CHARDEV which is going > to create a char device responsible to make some IOCTL available for > user-space applications > b) Create a linux/ufs.h header file that contains data structures > declarations that will be needed in user-space applications I am not very familiar with UFS devices, that said you should have an sgX chardev being created already so you can handle SG_IO requests. There also appear to be some sysfs entries being created. So between sg and sysfs you should be able to handle any user-space out of band requests without resorting to making a new chardev. Adding more sysfs entries, if you need them, should be fine. You may find it easier to expand on the existing interfaces than to get consensus on a new driver and ioctls. Hope this helps, Shaun > Could you please advise me about what the correct approach should be > to make it as standard as possible and usable in the future? > > Thank you very much for your help! > > regards, > Joao > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" > in the body of a message to majord...@vger.kernel.org More majordomo > info at > https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_ma > jordomo-2Dinfo.html&d=DQICaQ&c=IGDlg0lD0b-nebmJJ0Kp8A&r=Wg5NqlNlVTT7Ug > l8V50qIHLe856QW0qfG3WVYGOrWzA&m=vJFB6pCywWtdvkgHz9Vc0jQz0xzeyZlr-7eCWY > u88nM&s=yiQLPFpqmMrbqLZz1Jb3aNqOje2dRMLJHEzUDobwcXc&e= -- Shaun Tancheff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][SCSI] scsi: ufs: get a TM service response from the correct offset
Hi Kiwoong Kim, This is a good catch, Looks good to me. Reviewed-by: Subhash Jadavani Thanks, Subhash On 2016-09-08 16:22, Kiwoong Kim wrote: From: Kiwoong Kim When any UFS host controller receives a TM(Task Management) response from a UFS device, UFS driver has been recognize like receiving a message of "Task Management Function Complete"(00h) in all cases, so far. That means there is no pending task for a tag of the TM request sent before in the UFS device. That's because the byte offset 6 in TM response which has been used to get a TM service response so far represents just whether or not a TM transmission passes. Regarding UFS spec, the correct byte offset to get TM service response is 15, not 6. I tested that UFS driver responds properly for the TM response From a UFS device with an reference board with exynos8890, as follow: No pending task -> Task Management Function Complete (00h) Pending task -> Task Management Function Succeeded (08h) Signed-off-by: Kiwoong Kim Signed-off-by: HeonGwang Chu Tested-by: : Kiwoong Kim --- drivers/scsi/ufs/ufs.h|1 + drivers/scsi/ufs/ufshcd.c |4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index 42c459a..89c121e 100644 --- a/drivers/scsi/ufs/ufs.h +++ b/drivers/scsi/ufs/ufs.h @@ -295,6 +295,7 @@ enum { MASK_QUERY_DATA_SEG_LEN = 0x, MASK_RSP_UPIU_DATA_SEG_LEN = 0x, MASK_RSP_EXCEPTION_EVENT= 0x1, + MASK_TM_SERVICE_RESP= 0xFF, }; /* Task management service response */ diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e8a706b..c641cd3 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3013,8 +3013,8 @@ static int ufshcd_task_req_compl(struct ufs_hba *hba, u32 index, u8 *resp) if (ocs_value == OCS_SUCCESS) { task_rsp_upiup = (struct utp_upiu_task_rsp *) task_req_descp[index].task_rsp_upiu; - task_result = be32_to_cpu(task_rsp_upiup->header.dword_1); - task_result = ((task_result & MASK_TASK_RESPONSE) >> 8); + task_result = be32_to_cpu(task_rsp_upiup->output_param1); + task_result = task_result & MASK_TM_SERVICE_RESP; if (resp) *resp = (u8)task_result; } else { -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] UFS: Date Segment only need for WRITE DESCRIPTOR
Looks good to me. Reviewed-by: Subhash Jadavani On 2016-08-25 02:39, Zang Leigang wrote: Some device may cause a compatibility issue while receiving a Query UPIU with Data Segment which does not expected. Signed-off-by: Zang Leigang --- drivers/scsi/ufs/ufshcd.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index f08d41a..9b21d88 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1266,9 +1266,12 @@ static void ufshcd_prepare_utp_query_req_upiu(struct ufs_hba *hba, ucd_req_ptr->header.dword_1 = UPIU_HEADER_DWORD( 0, query->request.query_func, 0, 0); - /* Data segment length */ - ucd_req_ptr->header.dword_2 = UPIU_HEADER_DWORD( - 0, 0, len >> 8, (u8)len); + /* Data segment length only need for WRITE_DESC */ + if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC) + ucd_req_ptr->header.dword_2 = + UPIU_HEADER_DWORD(0, 0, (len >> 8), (u8)len); + else + ucd_req_ptr->header.dword_2 = 0; /* Copy the Query Request buffer as is */ memcpy(&ucd_req_ptr->qr, &query->request.upiu_req, -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ufs: add support for BLKSECDISCARD
Hi Pawel, Please find some comments inline. On 2016-07-26 04:56, Pawel Wodkowski wrote: Add BLKSECDISCAD feature support if LU is provisioned for TPRZ (bProvisioningType = 3). Did you mean sending purge when bProvisioningType is set to 02h (TPRZ = 0)? why do we want to send the purge if TPRZ is 1? To perform BLKSECDISCAD driver issue purge operation after each discard SCSI command with REQ_SECURE flag set, and delay calling scsi_done() till purge finish. This operation might long so block requests from SCSI layer in ufshcd_queueucommand() and then unblock it after purge finish. We had seen purge taking few mins to complete with some of the UFS device vendors. Did you run any experiments to major the time taken for purge to complete? Signed-off-by: Pawel Wodkowski --- drivers/scsi/ufs/ufs.h| 19 + drivers/scsi/ufs/ufshcd.c | 187 +- drivers/scsi/ufs/ufshcd.h | 6 ++ 3 files changed, 208 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h index b291fa6ed2ad..2f769974fda1 100644 --- a/drivers/scsi/ufs/ufs.h +++ b/drivers/scsi/ufs/ufs.h @@ -132,12 +132,14 @@ enum flag_idn { QUERY_FLAG_IDN_FDEVICEINIT = 0x01, QUERY_FLAG_IDN_PWR_ON_WPE = 0x03, QUERY_FLAG_IDN_BKOPS_EN = 0x04, + QUERY_FLAG_IDN_PURGE_EN = 0x06, }; /* Attribute idn for Query requests */ enum attr_idn { QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03, QUERY_ATTR_IDN_BKOPS_STATUS = 0x05, + QUERY_ATTR_IDN_PURGE_STATUS = 0x06, QUERY_ATTR_IDN_EE_CONTROL = 0x0D, QUERY_ATTR_IDN_EE_STATUS= 0x0E, }; @@ -247,6 +249,13 @@ enum { UFSHCD_AMP = 3, }; +/* Provisioning type */ +enum unit_desc_param_provisioning_type { + THIN_PROVISIONING_DISABLED = 0x00, + THIN_PROVISIONING_ENABLED_TPRZ_0= 0x02, + THIN_PROVISIONING_ENABLED_TPRZ_1= 0x03, +}; + #define POWER_DESC_MAX_SIZE0x62 #define POWER_DESC_MAX_ACTV_ICC_LVLS 16 @@ -279,6 +288,16 @@ enum bkops_status { BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL, }; +/* Purge operation status */ +enum purge_status { + PURGE_STATUS_IDLE = 0x0, + PURGE_STATUS_IN_PROGRESS = 0x1, + PURGE_STATUS_STOP_BY_HOST = 0x2, + PURGE_STATUS_SUCCESS = 0x3, + PURGE_STATUS_QUEUE_NOT_EMPTY = 0x4, + PURGE_STATUS_GENERAL_FAIL = 0x5 +}; + /* UTP QUERY Transaction Specific Fields OpCode */ enum query_opcode { UPIU_QUERY_OPCODE_NOP = 0x0, diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index f8fa72c31a9d..4ca15a6f294c 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -70,6 +70,9 @@ /* Task management command timeout */ #define TM_CMD_TIMEOUT 100 /* msecs */ +/* Purge operation timeout */ +#define PURGE_TIMEOUT 9000 /* msecs */ + /* maximum number of retries for a general UIC command */ #define UFS_UIC_COMMAND_RETRIES 3 @@ -1382,11 +1385,13 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) struct ufshcd_lrb *lrbp; struct ufs_hba *hba; unsigned long flags; + bool secure; int tag; int err = 0; hba = shost_priv(host); + secure = !!(cmd->request->cmd_flags & REQ_SECURE); tag = cmd->request->tag; if (!ufshcd_valid_tag(hba, tag)) { dev_err(hba->dev, @@ -1420,6 +1425,17 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) cmd->scsi_done(cmd); goto out_unlock; } + + if (secure) { + if (hba->is_purge_in_progress) { + secure = false; + err = SCSI_MLQUEUE_HOST_BUSY; + goto out_unlock; + } + + hba->is_purge_in_progress = true; + } + spin_unlock_irqrestore(hba->host->host_lock, flags); /* acquire the tag to make sure device cmds don't use it */ @@ -1465,9 +1481,19 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) /* issue command to the controller */ spin_lock_irqsave(hba->host->host_lock, flags); ufshcd_send_command(hba, tag); + + if (secure) { + hba->purge_timeout = jiffies + msecs_to_jiffies(PURGE_TIMEOUT); + + scsi_block_requests(hba->host); + } + out_unlock: spin_unlock_irqrestore(hba->host->host_lock, flags); out: + if (err && secure && hba->is_purge_in_progress) + hba->is_purge_in_progress = false; + return err; } @@ -1641,7 +1667,7 @@ static inline void ufshcd_put_dev_cmd_tag(struct ufs_hba *hba, int tag) * ufshcd_exec_dev_cmd - API for sending device management requests * @hba - UFS hba * @cmd_type - specifi
Re: [PATCH] scsi: ufs: enable no vccq quirk for skhynix device
Looks good to me. Reviewed-by: Subhash Jadavani On 2016-09-26 07:58, Kyuho Choi wrote: This patch enable no vccq quirk for SKHynix devices. SKHynix ufs device don't need vccq vrail for device operation. Signed-off-by: Kyuho Choi --- drivers/scsi/ufs/ufs_quirks.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/ufs/ufs_quirks.h b/drivers/scsi/ufs/ufs_quirks.h index ee4ab85..22f881e 100644 --- a/drivers/scsi/ufs/ufs_quirks.h +++ b/drivers/scsi/ufs/ufs_quirks.h @@ -25,6 +25,7 @@ #define UFS_VENDOR_TOSHIBA 0x198 #define UFS_VENDOR_SAMSUNG 0x1CE +#define UFS_VENDOR_SKHYNIX 0x1AD /** * ufs_device_info - ufs device details @@ -145,6 +146,7 @@ static struct ufs_dev_fix ufs_fixups[] = { UFS_DEVICE_QUIRK_PA_TACTIVATE), UFS_FIX(UFS_VENDOR_TOSHIBA, "THGLF2G9D8KBADG", UFS_DEVICE_QUIRK_PA_TACTIVATE), + UFS_FIX(UFS_VENDOR_SKHYNIX, UFS_ANY_MODEL, UFS_DEVICE_NO_VCCQ), END_FIX }; -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 04/15] scsi: ufs: clear outstanding_request bit in case query timeout
Looks good to me. Reviewed-by: Subhash Jadavani > When sending a query to the device returns with a timeout error, > we clear the corresponding bit in the DOORBELL register but > we don't clear the outstanding_request field as we should. > This patch fixes this bug. > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufshcd.c | 22 -- > 1 file changed, 20 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 8860a57..e0b8755 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -364,6 +364,16 @@ static inline void ufshcd_utrl_clear(struct ufs_hba > *hba, u32 pos) > } > > /** > + * ufshcd_outstanding_req_clear - Clear a bit in outstanding request > field > + * @hba: per adapter instance > + * @tag: position of the bit to be cleared > + */ > +static inline void ufshcd_outstanding_req_clear(struct ufs_hba *hba, int > tag) > +{ > + __clear_bit(tag, &hba->outstanding_reqs); > +} > + > +/** > * ufshcd_get_lists_status - Check UCRDY, UTRLRDY and UTMRLRDY > * @reg: Register value of host controller status > * > @@ -1502,9 +1512,17 @@ static int ufshcd_wait_for_dev_cmd(struct ufs_hba > *hba, > > if (!time_left) { > err = -ETIMEDOUT; > + dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n", > + __func__, lrbp->task_tag); > if (!ufshcd_clear_cmd(hba, lrbp->task_tag)) > - /* sucessfully cleared the command, retry if needed */ > + /* successfully cleared the command, retry if needed */ > err = -EAGAIN; > + /* > + * in case of an error, after clearing the doorbell, > + * we also need to clear the outstanding_request > + * field in hba > + */ > + ufshcd_outstanding_req_clear(hba, lrbp->task_tag); > } > > return err; > @@ -3942,7 +3960,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > scsi_dma_unmap(cmd); > > spin_lock_irqsave(host->host_lock, flags); > - __clear_bit(tag, &hba->outstanding_reqs); > + ufshcd_outstanding_req_clear(hba, tag); > hba->lrb[tag].cmd = NULL; > spin_unlock_irqrestore(host->host_lock, flags); > > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 03/15] scsi: ufs: verify command tag validity
Looks good to me. Reviewed-by: Subhash Jadavani > A race condition appear to exist between request completion when > scsi_done() is called to end the request and set the tag back to > -1 (at blk_queue_end_tag() scsi_end_request), and scsi layer error > handling which aborts the command and reuses it to request sense > data. Sending the request sense is done with tag which was set to -1 > and so it is invalid. > Assert command tag passed from scsi layer is valid. > > Signed-off-by: Gilad Broner > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufshcd.c | 24 ++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 2d3ebca..8860a57 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -190,6 +190,10 @@ static int ufshcd_config_pwr_mode(struct ufs_hba > *hba, > struct ufs_pa_layer_attr *desired_pwr_mode); > static int ufshcd_change_power_mode(struct ufs_hba *hba, >struct ufs_pa_layer_attr *pwr_mode); > +static inline bool ufshcd_valid_tag(struct ufs_hba *hba, int tag) > +{ > + return tag >= 0 && tag < hba->nutrs; > +} > > static inline int ufshcd_enable_irq(struct ufs_hba *hba) > { > @@ -1310,6 +1314,12 @@ static int ufshcd_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *cmd) > hba = shost_priv(host); > > tag = cmd->request->tag; > + if (!ufshcd_valid_tag(hba, tag)) { > + dev_err(hba->dev, > + "%s: invalid command tag %d: cmd=0x%p, > cmd->request=0x%p", > + __func__, tag, cmd, cmd->request); > + BUG(); > + } > > spin_lock_irqsave(hba->host->host_lock, flags); > switch (hba->ufshcd_state) { > @@ -3862,13 +3872,23 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) > host = cmd->device->host; > hba = shost_priv(host); > tag = cmd->request->tag; > + if (!ufshcd_valid_tag(hba, tag)) { > + dev_err(hba->dev, > + "%s: invalid command tag %d: cmd=0x%p, > cmd->request=0x%p", > + __func__, tag, cmd, cmd->request); > + BUG(); > + } > > ufshcd_hold(hba, false); > + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); > /* If command is already aborted/completed, return SUCCESS */ > - if (!(test_bit(tag, &hba->outstanding_reqs))) > + if (!(test_bit(tag, &hba->outstanding_reqs))) { > + dev_err(hba->dev, > + "%s: cmd at tag %d already completed, outstanding=0x%lx, > doorbell=0x%x\n", > + __func__, tag, hba->outstanding_reqs, reg); > goto out; > + } > > - reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); > if (!(reg & (1 << tag))) { > dev_err(hba->dev, > "%s: cmd was completed, but without a notifying intr, tag = %d", > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/15] scsi: ufs: clear fields UTRD, UPIU req and rsp before new transfers
Looks good to me. Reviewed-by: Subhash Jadavani > Some of the data structures (like response UPIU) and/or its elements > (unused fields) should be cleared before sending out the respective > command to UFS device. > > This change clears the UPIU response data structure for query commands > and NOP command before sending out the command. We also initialize the > PRDT table length to zero which should take care of commands which doesn't > have any data associated with it. We are also clearing the unused fields > in > request UPIU for NOP command. > > Signed-off-by: Subhash Jadavani > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufshcd.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 3428f72..2d3ebca 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -1129,6 +1129,8 @@ static void ufshcd_prepare_req_desc_hdr(struct > ufshcd_lrb *lrbp, > cpu_to_le32(OCS_INVALID_COMMAND_STATUS); > /* dword_3 is reserved, hence it is set to 0 */ > req_desc->header.dword_3 = 0; > + > + req_desc->prd_table_length = 0; > } > > /** > @@ -1198,6 +1200,7 @@ static void ufshcd_prepare_utp_query_req_upiu(struct > ufs_hba *hba, > if (query->request.upiu_req.opcode == UPIU_QUERY_OPCODE_WRITE_DESC) > memcpy(descp, query->descriptor, len); > > + memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp)); > } > > static inline void ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp) > @@ -1210,6 +1213,11 @@ static inline void > ufshcd_prepare_utp_nop_upiu(struct ufshcd_lrb *lrbp) > ucd_req_ptr->header.dword_0 = > UPIU_HEADER_DWORD( > UPIU_TRANSACTION_NOP_OUT, 0, 0, lrbp->task_tag); > + /* clear rest of the fields of basic header */ > + ucd_req_ptr->header.dword_1 = 0; > + ucd_req_ptr->header.dword_2 = 0; > + > + memset(lrbp->ucd_rsp_ptr, 0, sizeof(struct utp_upiu_rsp)); > } > > /** > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 8/8] scsi: ufs-qcom: add QUniPro hardware support and power optimizations
Looks good to me. Reviewed-by: Subhash Jadavani > New revisions of UFS host controller supports the new UniPro > hardware controller (referred as QUniPro). This patch adds > the support to enable this new UniPro controller hardware. > > This change also adds power optimization for bus scaling feature, > as well as support for HS-G3 power mode. > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 640 > > drivers/scsi/ufs/ufs-qcom.h | 31 ++- > drivers/scsi/ufs/ufshcd.c | 8 +- > drivers/scsi/ufs/ufshcd.h | 27 +- > 4 files changed, 525 insertions(+), 181 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 1633808..4f38d00 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -44,11 +44,11 @@ enum { > > static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS]; > > -static void ufs_qcom_get_speed_mode(struct ufs_pa_layer_attr *p, char > *result); > -static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host, > - const char *speed_mode); > static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote); > static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); > +static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba > *hba, > +u32 clk_cycles); > + > static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len, > char *prefix) > { > @@ -177,6 +177,7 @@ static int ufs_qcom_init_lane_clks(struct > ufs_qcom_host *host) > > err = ufs_qcom_host_clk_get(dev, "tx_lane1_sync_clk", > &host->tx_l1_sync_clk); > + > out: > return err; > } > @@ -209,7 +210,9 @@ static int ufs_qcom_check_hibern8(struct ufs_hba *hba) > > do { > err = ufshcd_dme_get(hba, > - UIC_ARG_MIB(MPHY_TX_FSM_STATE), &tx_fsm_val); > + UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, > + UIC_ARG_MPHY_TX_GEN_SEL_INDEX(0)), > + &tx_fsm_val); > if (err || tx_fsm_val == TX_FSM_HIBERN8) > break; > > @@ -223,7 +226,9 @@ static int ufs_qcom_check_hibern8(struct ufs_hba *hba) >*/ > if (time_after(jiffies, timeout)) > err = ufshcd_dme_get(hba, > - UIC_ARG_MIB(MPHY_TX_FSM_STATE), &tx_fsm_val); > + UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE, > + UIC_ARG_MPHY_TX_GEN_SEL_INDEX(0)), > + &tx_fsm_val); > > if (err) { > dev_err(hba->dev, "%s: unable to get TX_FSM_STATE, err %d\n", > @@ -237,6 +242,15 @@ static int ufs_qcom_check_hibern8(struct ufs_hba > *hba) > return err; > } > > +static void ufs_qcom_select_unipro_mode(struct ufs_qcom_host *host) > +{ > + ufshcd_rmwl(host->hba, QUNIPRO_SEL, > +ufs_qcom_cap_qunipro(host) ? QUNIPRO_SEL : 0, > +REG_UFS_CFG1); > + /* make sure above configuration is applied before we return */ > + mb(); > +} > + > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > @@ -251,9 +265,11 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba > *hba) > usleep_range(1000, 1100); > > ret = ufs_qcom_phy_calibrate_phy(phy, is_rate_B); > + > if (ret) { > - dev_err(hba->dev, "%s: ufs_qcom_phy_calibrate_phy() failed, ret > = > %d\n", > - __func__, ret); > + dev_err(hba->dev, > + "%s: ufs_qcom_phy_calibrate_phy()failed, ret = %d\n", > + __func__, ret); > goto out; > } > > @@ -274,9 +290,12 @@ static int ufs_qcom_power_up_sequence(struct ufs_hba > *hba) > > ret = ufs_qcom_phy_is_pcs_ready(phy); > if (ret) > - dev_err(hba->dev, "%s: is_physical_coding_sublayer_ready() > failed, ret > = %d\n", > + dev_err(hba->dev, > + "%s: is_physical_coding_sublayer_ready() failed, ret = > %d\n", > __func__, ret); > > + ufs_qcom_select_unipro_mode(host); > + > out: > return ret; > } > @@ -299,7 +318,8 @@ static void ufs_qcom_enable_hw_clk_gating(struct > ufs_hba *hba) > mb(); > } > > -static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, bool status) > +static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, > + enum ufs_notify_change_status status) > { > struct ufs_qcom_host *host = ufshcd_get_variant(hba); > int err = 0; > @@ -329,12 +349,12 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba > *hba, bool status) > } > > /** > - * Returns non-zero for success (which rate of core_clk) and 0 > - * in case of a failure > + * Returns zero for success and non-zero
Re: [PATCH v7 7/8] scsi: ufs-qcom: add debug prints for test bus
Looks good to me. Reviewed-by: Subhash Jadavani > Adds support for configuring and reading the test bus and debug > registers. This change also adds another vops in order to print the > debug registers. > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 165 > +++- > drivers/scsi/ufs/ufs-qcom.h | 37 +- > drivers/scsi/ufs/ufshcd.c | 2 + > drivers/scsi/ufs/ufshcd.h | 8 +++ > 4 files changed, 208 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index b275a9a..1633808 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -23,6 +23,24 @@ > #include "unipro.h" > #include "ufs-qcom.h" > #include "ufshci.h" > +#define UFS_QCOM_DEFAULT_DBG_PRINT_EN\ > + (UFS_QCOM_DBG_PRINT_REGS_EN | UFS_QCOM_DBG_PRINT_TEST_BUS_EN) > + > +enum { > + TSTBUS_UAWM, > + TSTBUS_UARM, > + TSTBUS_TXUC, > + TSTBUS_RXUC, > + TSTBUS_DFC, > + TSTBUS_TRLUT, > + TSTBUS_TMRLUT, > + TSTBUS_OCSC, > + TSTBUS_UTP_HCI, > + TSTBUS_COMBINED, > + TSTBUS_WRAPPER, > + TSTBUS_UNIPRO, > + TSTBUS_MAX, > +}; > > static struct ufs_qcom_host *ufs_qcom_hosts[MAX_UFS_QCOM_HOSTS]; > > @@ -30,6 +48,15 @@ static void ufs_qcom_get_speed_mode(struct > ufs_pa_layer_attr *p, char *result); > static int ufs_qcom_get_bus_vote(struct ufs_qcom_host *host, > const char *speed_mode); > static int ufs_qcom_set_bus_vote(struct ufs_qcom_host *host, int vote); > +static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host); > +static void ufs_qcom_dump_regs(struct ufs_hba *hba, int offset, int len, > + char *prefix) > +{ > + print_hex_dump(KERN_ERR, prefix, > + len > 4 ? DUMP_PREFIX_OFFSET : DUMP_PREFIX_NONE, > + 16, 4, (void __force *)hba->mmio_base + offset, > + len * 4, false); > +} > > static int ufs_qcom_get_connected_tx_lanes(struct ufs_hba *hba, u32 > *tx_lanes) > { > @@ -996,6 +1023,15 @@ static int ufs_qcom_init(struct ufs_hba *hba) > if (hba->dev->id < MAX_UFS_QCOM_HOSTS) > ufs_qcom_hosts[hba->dev->id] = host; > > + host->dbg_print_en |= UFS_QCOM_DEFAULT_DBG_PRINT_EN; > + ufs_qcom_get_default_testbus_cfg(host); > + err = ufs_qcom_testbus_config(host); > + if (err) { > + dev_warn(dev, "%s: failed to configure the testbus %d\n", > + __func__, err); > + err = 0; > + } > + > goto out; > > out_disable_phy: > @@ -1025,12 +1061,134 @@ void ufs_qcom_clk_scale_notify(struct ufs_hba > *hba) > > if (!dev_req_params) > return; > +} > + > +static void ufs_qcom_get_default_testbus_cfg(struct ufs_qcom_host *host) > +{ > + /* provide a legal default configuration */ > + host->testbus.select_major = TSTBUS_UAWM; > + host->testbus.select_minor = 1; > +} > + > +static bool ufs_qcom_testbus_cfg_is_ok(struct ufs_qcom_host *host) > +{ > + if (host->testbus.select_major >= TSTBUS_MAX) { > + dev_err(host->hba->dev, > + "%s: UFS_CFG1[TEST_BUS_SEL} may not equal 0x%05X\n", > + __func__, host->testbus.select_major); > + return false; > + } > + > + /* > + * Not performing check for each individual select_major > + * mappings of select_minor, since there is no harm in > + * configuring a non-existent select_minor > + */ > + if (host->testbus.select_minor > 0x1F) { > + dev_err(host->hba->dev, > + "%s: 0x%05X is not a legal testbus option\n", > + __func__, host->testbus.select_minor); > + return false; > + } > + > + return true; > +} > + > +int ufs_qcom_testbus_config(struct ufs_qcom_host *host) > +{ > + int reg; > + int offset; > + u32 mask = TEST_BUS_SUB_SEL_MASK; > + > + if (!host) > + return -EINVAL; > > - ufs_qcom_cfg_timers(hba, dev_req_params->gear_rx, > - dev_req_params->pwr_rx, > - dev_req_params->hs_rate); > + if (!ufs_qcom_testbus_cfg_is_ok(host)) > + return -EPERM; > + > + switch (host->testbus.select_major) { > + case TSTBUS_UAWM: > + reg = UFS_TEST_BUS_CTRL_0; > + offset = 24; > + break; > + case TSTBUS_UARM: > + reg = UFS_TEST_BUS_CTRL_0; > + offset = 16; > + break; > + case TSTBUS_TXUC: > + reg = UFS_TEST_BUS_CTRL_0; > + offset = 8; > + break; > + case TSTBUS_RXUC: > + reg = UFS_TEST_BUS_CTRL_0; > + offset = 0; > + break; > + case TSTBUS_DFC: > + reg = UFS_TEST_BUS_CTRL_1; > + offset = 24; > + break; > + case TSTBUS_TRLUT: > + reg
Re: [PATCH v7 6/8] scsi: ufs: make the UFS variant a platform device
Comments inline below: > This change turns the UFS variant (SCSI_UFS_QCOM) into a UFS > a platform device. > In order to do so a few additional changes are required: > 1. The ufshcd-pltfrm is no longer serves as a platform device. >Now it only serves as a group of platform APIs such as PM APIs >(runtime suspend/resume, system suspend/resume etc), parsers of >clocks, regulators and pm_levels from DT. > 2. What used to be the old platform "probe" is now "only" >a pltfrm_init() routine, that does exactly the same, but only >being called by the new probe function of the UFS variant. > > Signed-off-by: Yaniv Gardi > > --- > Documentation/devicetree/bindings/ufs/ufs-qcom.txt | 57 + > .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 4 +- > drivers/scsi/ufs/ufs-qcom.c| 62 +- > drivers/scsi/ufs/ufshcd-pltfrm.c | 98 > ++ > drivers/scsi/ufs/ufshcd-pltfrm.h | 41 + > drivers/scsi/ufs/ufshcd.c | 10 +++ > drivers/scsi/ufs/ufshcd.h | 1 + > 7 files changed, 199 insertions(+), 74 deletions(-) > create mode 100644 Documentation/devicetree/bindings/ufs/ufs-qcom.txt > create mode 100644 drivers/scsi/ufs/ufshcd-pltfrm.h > > diff --git a/Documentation/devicetree/bindings/ufs/ufs-qcom.txt > b/Documentation/devicetree/bindings/ufs/ufs-qcom.txt > new file mode 100644 > index 000..452e4ef > --- /dev/null > +++ b/Documentation/devicetree/bindings/ufs/ufs-qcom.txt > @@ -0,0 +1,57 @@ > +* Qualcomm Technologies Inc Universal Flash Storage (UFS) PHY > + > +UFSPHY nodes are defined to describe on-chip UFS PHY hardware macro. > +Each UFS PHY node should have its own node. > + > +To bind UFS PHY with UFS host controller, the controller node should > +contain a phandle reference to UFS PHY node. > + > +Required properties: > +- compatible: compatible list, contains "qcom,ufs-phy-qmp-20nm" > + or "qcom,ufs-phy-qmp-14nm" according to the relevant phy > in use. > +- reg : should contain PHY register address space > (mandatory), > +- reg-names : indicates various resources passed to driver (via > reg proptery) by name. > + Required "reg-names" is "phy_mem". > +- #phy-cells: This property shall be set to 0 > +- vdda-phy-supply : phandle to main PHY supply for analog domain > +- vdda-pll-supply : phandle to PHY PLL and Power-Gen block power supply > +- clocks : List of phandle and clock specifier pairs > +- clock-names : List of clock input name strings sorted in the same > + order as the clocks property. "ref_clk_src", "ref_clk", > + "tx_iface_clk" & "rx_iface_clk" are mandatory but > + "ref_clk_parent" is optional > + > +Optional properties: > +- vdda-phy-max-microamp : specifies max. load that can be drawn from phy > supply > +- vdda-pll-max-microamp : specifies max. load that can be drawn from pll > supply > +- vddp-ref-clk-supply : phandle to UFS device ref_clk pad power supply > +- vddp-ref-clk-max-microamp : specifies max. load that can be drawn from > this supply > +- vddp-ref-clk-always-on : specifies if this supply needs to be kept > always on > + > +Example: > + > + ufsphy1: ufsphy@0xfc597000 { > + compatible = "qcom,ufs-phy-qmp-20nm"; > + reg = <0xfc597000 0x800>; > + reg-names = "phy_mem"; > + #phy-cells = <0>; > + vdda-phy-supply = <&pma8084_l4>; > + vdda-pll-supply = <&pma8084_l12>; > + vdda-phy-max-microamp = <5>; > + vdda-pll-max-microamp = <1000>; > + clock-names = "ref_clk_src", > + "ref_clk_parent", > + "ref_clk", > + "tx_iface_clk", > + "rx_iface_clk"; > + clocks = <&clock_rpm clk_ln_bb_clk>, > + <&clock_gcc clk_pcie_1_phy_ldo >, > + <&clock_gcc clk_ufs_phy_ldo>, > + <&clock_gcc clk_gcc_ufs_tx_cfg_clk>, > + <&clock_gcc clk_gcc_ufs_rx_cfg_clk>; > + }; > + > + ufshc@0xfc598000 { > + ... > + phys = <&ufsphy1>; > + }; > diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > index 5357919..c0f10d3 100644 > --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt > @@ -4,7 +4,9 @@ UFSHC nodes are defined to describe on-chip UFS host > controllers. > Each UFS controller instance should have its own node. > > Required properties: > -- compatible: compatible list, contains "jedec,ufs-1.1" > +- compatible: compatible list, contains "jedec,ufs-1.1" or > + "qcom,msm8994-ufshc" or "qcom,msm8996-
Re: [PATCH v7 4/8] add ufshcd_get_variant ufshcd_set_variant
Looks good to me. Reviewed-by: Subhash Jadavani > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 34 +- > drivers/scsi/ufs/ufshcd.h | 21 + > 2 files changed, 38 insertions(+), 17 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 6c23bbf..64c54b7 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -155,7 +155,7 @@ out: > > static int ufs_qcom_link_startup_post_change(struct ufs_hba *hba) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct phy *phy = host->generic_phy; > u32 tx_lanes; > int err = 0; > @@ -211,7 +211,7 @@ static int ufs_qcom_check_hibern8(struct ufs_hba *hba) > > static int ufs_qcom_power_up_sequence(struct ufs_hba *hba) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct phy *phy = host->generic_phy; > int ret = 0; > bool is_rate_B = (UFS_QCOM_LIMIT_HS_RATE == PA_HS_MODE_B) > @@ -273,7 +273,7 @@ static void ufs_qcom_enable_hw_clk_gating(struct > ufs_hba *hba) > > static int ufs_qcom_hce_enable_notify(struct ufs_hba *hba, bool status) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > int err = 0; > > switch (status) { > @@ -307,7 +307,7 @@ static int ufs_qcom_hce_enable_notify(struct ufs_hba > *hba, bool status) > static unsigned long > ufs_qcom_cfg_timers(struct ufs_hba *hba, u32 gear, u32 hs, u32 rate) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct ufs_clk_info *clki; > u32 core_clk_period_in_ns; > u32 tx_clk_cycles_per_us = 0; > @@ -448,7 +448,7 @@ static int ufs_qcom_link_startup_notify(struct ufs_hba > *hba, bool status) > > static int ufs_qcom_suspend(struct ufs_hba *hba, enum ufs_pm_op pm_op) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct phy *phy = host->generic_phy; > int ret = 0; > > @@ -479,7 +479,7 @@ out: > > static int ufs_qcom_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct phy *phy = host->generic_phy; > int err; > > @@ -621,7 +621,7 @@ static int ufs_qcom_pwr_change_notify(struct ufs_hba > *hba, > struct ufs_pa_layer_attr *dev_req_params) > { > u32 val; > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > struct phy *phy = host->generic_phy; > struct ufs_qcom_dev_params ufs_qcom_cap; > int ret = 0; > @@ -696,7 +696,7 @@ out: > > static u32 ufs_qcom_get_ufs_hci_version(struct ufs_hba *hba) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > if (host->hw_ver.major == 0x1) > return UFSHCI_VERSION_11; > @@ -715,7 +715,7 @@ static u32 ufs_qcom_get_ufs_hci_version(struct ufs_hba > *hba) > */ > static void ufs_qcom_advertise_quirks(struct ufs_hba *hba) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > if (host->hw_ver.major == 0x01) { > hba->quirks |= UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS > @@ -740,7 +740,7 @@ static void ufs_qcom_advertise_quirks(struct ufs_hba > *hba) > > static void ufs_qcom_set_caps(struct ufs_hba *hba) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > if (host->hw_ver.major >= 0x2) > host->caps = UFS_QCOM_CAP_QUNIPRO; > @@ -811,7 +811,7 @@ static void ufs_qcom_get_speed_mode(struct > ufs_pa_layer_attr *p, char *result) > > static int ufs_qcom_setup_clocks(struct ufs_hba *hba, bool on) > { > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > int err = 0; > int vote = 0; > > @@ -866,7 +866,7 @@ show_ufs_to_mem_max_bus_bw(struct device *dev, struct > device_attribute *attr, > char *buf) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba); > > return snprintf(buf, PAGE_SIZE, "%u\n", > host->bus_vote.is_max_bw_needed); > @@ -877,7 +877,7 @@ store_ufs_to_mem_max_bus_bw(struct device *dev, struct > device_attribute *attr, > const char *buf, size_t count) > { > struct ufs_hba *hba = dev_get_drvdata(dev); > - struct ufs_qcom_host *host = hba->priv; > + struct ufs_qcom_host *host = ufshcd_get_variant(hba)
Re: [PATCH v7 3/8] scsi: ufs-qcom: update configuration option of SCSI_UFS_QCOM component
Looks good to me. Reviewed-by: Subhash Jadavani > This change is required in order to be able to build the component > as a module. > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/Kconfig b/drivers/scsi/ufs/Kconfig > index e945383..5f45307 100644 > --- a/drivers/scsi/ufs/Kconfig > +++ b/drivers/scsi/ufs/Kconfig > @@ -72,7 +72,7 @@ config SCSI_UFSHCD_PLATFORM > If unsure, say N. > > config SCSI_UFS_QCOM > - bool "QCOM specific hooks to UFS controller platform driver" > + tristate "QCOM specific hooks to UFS controller platform driver" > depends on SCSI_UFSHCD_PLATFORM && ARCH_QCOM > select PHY_QCOM_UFS > help > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 5/8] scsi: ufs: creates wrapper functions for vops
Looks good to me. Reviewed-by: Subhash Jadavani > In order to simplify the code a set of wrapper functions is created > to test and call each of the variant operations. > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 1 - > drivers/scsi/ufs/ufshcd.c | 104 > +--- > drivers/scsi/ufs/ufshcd.h | 98 > + > 3 files changed, 137 insertions(+), 66 deletions(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 64c54b7..329ac84 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -1049,6 +1049,5 @@ static const struct ufs_hba_variant_ops > ufs_hba_qcom_vops = { > .suspend= ufs_qcom_suspend, > .resume = ufs_qcom_resume, > }; > -EXPORT_SYMBOL(ufs_hba_qcom_vops); > > MODULE_LICENSE("GPL v2"); > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index b0ade73..9e79c33 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -271,10 +271,8 @@ static inline u32 ufshcd_get_intr_mask(struct ufs_hba > *hba) > */ > static inline u32 ufshcd_get_ufs_version(struct ufs_hba *hba) > { > - if (hba->quirks & UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION) { > - if (hba->vops && hba->vops->get_ufs_hci_version) > - return hba->vops->get_ufs_hci_version(hba); > - } > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION) > + return ufshcd_vops_get_ufs_hci_version(hba); > > return ufshcd_readl(hba, REG_UFS_VERSION); > } > @@ -2473,9 +2471,8 @@ static int ufshcd_change_power_mode(struct ufs_hba > *hba, > dev_err(hba->dev, > "%s: power mode change failed %d\n", __func__, ret); > } else { > - if (hba->vops && hba->vops->pwr_change_notify) > - hba->vops->pwr_change_notify(hba, > - POST_CHANGE, NULL, pwr_mode); > + ufshcd_vops_pwr_change_notify(hba, POST_CHANGE, NULL, > + pwr_mode); > > memcpy(&hba->pwr_info, pwr_mode, > sizeof(struct ufs_pa_layer_attr)); > @@ -2495,10 +2492,10 @@ static int ufshcd_config_pwr_mode(struct ufs_hba > *hba, > struct ufs_pa_layer_attr final_params = { 0 }; > int ret; > > - if (hba->vops && hba->vops->pwr_change_notify) > - hba->vops->pwr_change_notify(hba, > - PRE_CHANGE, desired_pwr_mode, &final_params); > - else > + ret = ufshcd_vops_pwr_change_notify(hba, PRE_CHANGE, > + desired_pwr_mode, &final_params); > + > + if (ret) > memcpy(&final_params, desired_pwr_mode, sizeof(final_params)); > > ret = ufshcd_change_power_mode(hba, &final_params); > @@ -2647,8 +2644,7 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > /* UniPro link is disabled at this point */ > ufshcd_set_link_off(hba); > > - if (hba->vops && hba->vops->hce_enable_notify) > - hba->vops->hce_enable_notify(hba, PRE_CHANGE); > + ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); > > /* start controller initialization sequence */ > ufshcd_hba_start(hba); > @@ -2681,8 +2677,7 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) > /* enable UIC related interrupts */ > ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); > > - if (hba->vops && hba->vops->hce_enable_notify) > - hba->vops->hce_enable_notify(hba, POST_CHANGE); > + ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); > > return 0; > } > @@ -2735,8 +2730,7 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > int retries = DME_LINKSTARTUP_RETRIES; > > do { > - if (hba->vops && hba->vops->link_startup_notify) > - hba->vops->link_startup_notify(hba, PRE_CHANGE); > + ufshcd_vops_link_startup_notify(hba, PRE_CHANGE); > > ret = ufshcd_dme_link_startup(hba); > > @@ -2767,11 +2761,9 @@ static int ufshcd_link_startup(struct ufs_hba *hba) > } > > /* Include any host controller configuration via UIC commands */ > - if (hba->vops && hba->vops->link_startup_notify) { > - ret = hba->vops->link_startup_notify(hba, POST_CHANGE); > - if (ret) > - goto out; > - } > + ret = ufshcd_vops_link_startup_notify(hba, POST_CHANGE); > + if (ret) > + goto out; > > ret = ufshcd_make_hba_operational(hba); > out: > @@ -4578,8 +4570,7 @@ static int __ufshcd_setup_clocks(struct ufs_hba > *hba, bool on, > } > } > > - if (hba->vops && hba->vops->setup_clocks) > - ret = hba->vops->setup_clocks(hba, on); > + ret = ufshcd_vops_setup_clocks(hba, on); > out: > if (ret) { > list_for_each_entry(clki, head, list)
Re: [PATCH v7 2/8] scsi: ufs-qcom: fix compilation warning if compiled as a module
Looks good to me. Reviewed-by: Subhash Jadavani > This change fixes a compilation warning that happens if SCSI_UFS_QCOM > is compiled as a module. > Also this patch fixes an error happens when insmod the module: > "ufs_qcom: module license 'unspecified' taints kernel." > > Signed-off-by: Yaniv Gardi > > --- > drivers/scsi/ufs/ufs-qcom.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c > index 4cdffa4..6c23bbf 100644 > --- a/drivers/scsi/ufs/ufs-qcom.c > +++ b/drivers/scsi/ufs/ufs-qcom.c > @@ -917,12 +917,15 @@ out: > > #define ANDROID_BOOT_DEV_MAX30 > static char android_boot_dev[ANDROID_BOOT_DEV_MAX]; > -static int get_android_boot_dev(char *str) > + > +#ifndef MODULE > +static int __init get_android_boot_dev(char *str) > { > strlcpy(android_boot_dev, str, ANDROID_BOOT_DEV_MAX); > return 1; > } > __setup("androidboot.bootdevice=", get_android_boot_dev); > +#endif > > /** > * ufs_qcom_init - bind phy with controller > @@ -1047,3 +1050,5 @@ static const struct ufs_hba_variant_ops > ufs_hba_qcom_vops = { > .resume = ufs_qcom_resume, > }; > EXPORT_SYMBOL(ufs_hba_qcom_vops); > + > +MODULE_LICENSE("GPL v2"); > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 1/8] phy: qcom-ufs: fix build error when the component is built as a module
Looks good to me. Reviewed-by: Subhash Jadavani > Export the following functions in order to avoid build errors > when the component PHY_QCOM_UFS is compiled as a module: > > ERROR: "ufs_qcom_phy_disable_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_is_pcs_ready" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_disable_iface_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_start_serdes" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_calibrate_phy" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_dev_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_set_tx_lane_enable" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_disable_dev_ref_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_save_controller_version" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > ERROR: "ufs_qcom_phy_enable_iface_clk" > [drivers/scsi/ufs/ufs-qcom.ko] undefined! > make[1]: *** [__modpost] Error 1 > > Signed-off-by: Yaniv Gardi > > --- > drivers/phy/phy-qcom-ufs.c | 11 +++ > 1 file changed, 11 insertions(+) > > diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c > index f9c618f..6140a8b 100644 > --- a/drivers/phy/phy-qcom-ufs.c > +++ b/drivers/phy/phy-qcom-ufs.c > @@ -432,6 +432,7 @@ out_disable_src: > out: > return ret; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_ref_clk); > > static > int ufs_qcom_phy_disable_vreg(struct phy *phy, > @@ -474,6 +475,7 @@ void ufs_qcom_phy_disable_ref_clk(struct phy > *generic_phy) > phy->is_ref_clk_enabled = false; > } > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_ref_clk); > > #define UFS_REF_CLK_EN (1 << 5) > > @@ -517,11 +519,13 @@ void ufs_qcom_phy_enable_dev_ref_clk(struct phy > *generic_phy) > { > ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, true); > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_dev_ref_clk); > > void ufs_qcom_phy_disable_dev_ref_clk(struct phy *generic_phy) > { > ufs_qcom_phy_dev_ref_clk_ctrl(generic_phy, false); > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_dev_ref_clk); > > /* Turn ON M-PHY RMMI interface clocks */ > int ufs_qcom_phy_enable_iface_clk(struct phy *generic_phy) > @@ -550,6 +554,7 @@ int ufs_qcom_phy_enable_iface_clk(struct phy > *generic_phy) > out: > return ret; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_iface_clk); > > /* Turn OFF M-PHY RMMI interface clocks */ > void ufs_qcom_phy_disable_iface_clk(struct phy *generic_phy) > @@ -562,6 +567,7 @@ void ufs_qcom_phy_disable_iface_clk(struct phy > *generic_phy) > phy->is_iface_clk_enabled = false; > } > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_disable_iface_clk); > > int ufs_qcom_phy_start_serdes(struct phy *generic_phy) > { > @@ -578,6 +584,7 @@ int ufs_qcom_phy_start_serdes(struct phy *generic_phy) > > return ret; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_start_serdes); > > int ufs_qcom_phy_set_tx_lane_enable(struct phy *generic_phy, u32 > tx_lanes) > { > @@ -595,6 +602,7 @@ int ufs_qcom_phy_set_tx_lane_enable(struct phy > *generic_phy, u32 tx_lanes) > > return ret; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_set_tx_lane_enable); > > void ufs_qcom_phy_save_controller_version(struct phy *generic_phy, > u8 major, u16 minor, u16 step) > @@ -605,6 +613,7 @@ void ufs_qcom_phy_save_controller_version(struct phy > *generic_phy, > ufs_qcom_phy->host_ctrl_rev_minor = minor; > ufs_qcom_phy->host_ctrl_rev_step = step; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_save_controller_version); > > int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B) > { > @@ -625,6 +634,7 @@ int ufs_qcom_phy_calibrate_phy(struct phy > *generic_phy, bool is_rate_B) > > return ret; > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy); > > int ufs_qcom_phy_remove(struct phy *generic_phy, > struct ufs_qcom_phy *ufs_qcom_phy) > @@ -662,6 +672,7 @@ int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy) > return ufs_qcom_phy->phy_spec_ops-> > is_physical_coding_sublayer_ready(ufs_qcom_phy); > } > +EXPORT_SYMBOL_GPL(ufs_qcom_phy_is_pcs_ready); > > int ufs_qcom_phy_power_on(struct phy *generic_phy) > { > -- > 1.8.5.2 > > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] scsi: ufs: add ioctl interface for query request
Looks good to me. Reviewed-by: Subhash Jadavani > This patch exposes the ioctl interface for UFS driver via SCSI device > ioctl interface. As of now UFS driver would provide the ioctl for query > interface to connected UFS device. > > Signed-off-by: Dolev Raviv > Signed-off-by: Noa Rubens > Signed-off-by: Raviv Shvili > Signed-off-by: Gilad Broner > Signed-off-by: Yaniv Gardi > --- > drivers/scsi/ufs/ufs.h| 53 +++ > drivers/scsi/ufs/ufshcd.c | 208 > +- > include/uapi/scsi/Kbuild | 1 + > include/uapi/scsi/ufs/Kbuild | 3 + > include/uapi/scsi/ufs/ioctl.h | 58 > include/uapi/scsi/ufs/ufs.h | 67 ++ > 6 files changed, 347 insertions(+), 43 deletions(-) > create mode 100644 include/uapi/scsi/ufs/Kbuild > create mode 100644 include/uapi/scsi/ufs/ioctl.h > create mode 100644 include/uapi/scsi/ufs/ufs.h > > diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h > index 05e0f23..eab739d 100644 > --- a/drivers/scsi/ufs/ufs.h > +++ b/drivers/scsi/ufs/ufs.h > @@ -38,6 +38,7 @@ > > #include > #include > +#include > > #define MAX_CDB_SIZE 16 > #define GENERAL_UPIU_REQUEST_SIZE 32 > @@ -72,6 +73,16 @@ enum { > UFS_UPIU_RPMB_WLUN = 0xC4, > }; > > +/** > + * ufs_is_valid_unit_desc_lun - checks if the given LUN has a unit > descriptor > + * @lun: LU number to check > + * @return: true if the lun has a matching unit descriptor, false > otherwise > + */ > +static inline bool ufs_is_valid_unit_desc_lun(u8 lun) > +{ > + return lun == UFS_UPIU_RPMB_WLUN || (lun < UFS_UPIU_MAX_GENERAL_LUN); > +} > + > /* > * UFS Protocol Information Unit related definitions > */ > @@ -127,35 +138,6 @@ enum { > UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST = 0x81, > }; > > -/* Flag idn for Query Requests*/ > -enum flag_idn { > - QUERY_FLAG_IDN_FDEVICEINIT = 0x01, > - QUERY_FLAG_IDN_PWR_ON_WPE = 0x03, > - QUERY_FLAG_IDN_BKOPS_EN = 0x04, > -}; > - > -/* Attribute idn for Query requests */ > -enum attr_idn { > - QUERY_ATTR_IDN_ACTIVE_ICC_LVL = 0x03, > - QUERY_ATTR_IDN_BKOPS_STATUS = 0x05, > - QUERY_ATTR_IDN_EE_CONTROL = 0x0D, > - QUERY_ATTR_IDN_EE_STATUS= 0x0E, > -}; > - > -/* Descriptor idn for Query requests */ > -enum desc_idn { > - QUERY_DESC_IDN_DEVICE = 0x0, > - QUERY_DESC_IDN_CONFIGURAION = 0x1, > - QUERY_DESC_IDN_UNIT = 0x2, > - QUERY_DESC_IDN_RFU_0= 0x3, > - QUERY_DESC_IDN_INTERCONNECT = 0x4, > - QUERY_DESC_IDN_STRING = 0x5, > - QUERY_DESC_IDN_RFU_1= 0x6, > - QUERY_DESC_IDN_GEOMETRY = 0x7, > - QUERY_DESC_IDN_POWER= 0x8, > - QUERY_DESC_IDN_MAX, > -}; > - > enum desc_header_offset { > QUERY_DESC_LENGTH_OFFSET= 0x00, > QUERY_DESC_DESC_TYPE_OFFSET = 0x01, > @@ -278,19 +260,6 @@ enum bkops_status { > BKOPS_STATUS_MAX = BKOPS_STATUS_CRITICAL, > }; > > -/* UTP QUERY Transaction Specific Fields OpCode */ > -enum query_opcode { > - UPIU_QUERY_OPCODE_NOP = 0x0, > - UPIU_QUERY_OPCODE_READ_DESC = 0x1, > - UPIU_QUERY_OPCODE_WRITE_DESC= 0x2, > - UPIU_QUERY_OPCODE_READ_ATTR = 0x3, > - UPIU_QUERY_OPCODE_WRITE_ATTR= 0x4, > - UPIU_QUERY_OPCODE_READ_FLAG = 0x5, > - UPIU_QUERY_OPCODE_SET_FLAG = 0x6, > - UPIU_QUERY_OPCODE_CLEAR_FLAG= 0x7, > - UPIU_QUERY_OPCODE_TOGGLE_FLAG = 0x8, > -}; > - > /* Query response result code */ > enum { > QUERY_RESULT_SUCCESS= 0x00, > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 786df28..efeb252 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -38,6 +38,7 @@ > */ > > #include > +#include > #include > #include > #include > @@ -2208,7 +2209,7 @@ static inline int ufshcd_read_unit_desc_param(struct > ufs_hba *hba, >* Unit descriptors are only available for general purpose LUs (LUN id >* from 0 to 7) and RPMB Well known LU. >*/ > - if (lun != UFS_UPIU_RPMB_WLUN && (lun >= UFS_UPIU_MAX_GENERAL_LUN)) > + if (!ufs_is_valid_unit_desc_lun(lun)) > return -EOPNOTSUPP; > > return ufshcd_read_desc_param(hba, QUERY_DESC_IDN_UNIT, lun, > @@ -5047,6 +5048,207 @@ out: > } > > /** > + * ufshcd_query_ioctl - perform user read queries > + * @hba: per-adapter instance > + * @lun: used for lun specific queries > + * @buffer: user space buffer for reading and submitting query data and > params > + * @return: 0 for success negative error code otherwise > + * > + * Expected/Submitted buffer structure is struct ufs_ioctl_query_data. > + * It will read the opcode, idn and buf_length parameters, and, put the > + * response in the buffer field while updating the used size in > buf_length. > + */ > +static int ufshcd_query_ioctl(struct uf
Re: [PATCH/RFC V2 07/16] scsi: support well known logical units
> On Fri, Aug 22, 2014 at 12:02:30AM -, subha...@codeaurora.org wrote: >> >> + /* >> >> + * put runtime pm reference for well-known logical units, >> >> + * drivers are expected to _get_* again during probe. >> >> + */ >> >> + if (scsi_is_wlun(sdev->lun)) >> >> + scsi_autopm_put_device(sdev); >> > Special casing the well known LUNs here seems wrong. Shouldn't we do >> this >> > for any devices that don't get a driver attached to them? >> >> Agreed, i will replace "if (scsi_is_wlun(sdev->lun))" check with "if >> (sdev->sdev_gendev.driver)". > > I would really prefer the callers of autopm get/put to be symmetric to > issues with later than probe driver attach or similar. Think of the > case say the CDROM or tape driver only gets loaded after we already > did a bus scan. A quick check of the autopm code and it's underlying > implementation seems to suggest that nesting them is okay. If that's > indeed the case I would suggest to restructure it the following way: > > - make the scsi_autopm_put_device call you add at the end of >scsi_sysfs_add_sdev unconditional to make it balance out >the get earlier in the function (and delete the now incorrect comment >above the scsi_autopm_get_device call). > - let drivers do paired get/put calls in their probe/remove methods. > > If you still need any wlun changes after that please send them as a > separate patch with a detailed description on why you need it. Thanks, yes I was also thinking on same lines but was not sure if it would work or not. But let me check this more and if it works, will make a different patch for it. > All in all the autopm code and it's underlying implementation is highly > confusing, so additional documentation on it would also be very welcome. I would agree but not sure if I would be able to add it now, hopefully some time in future. > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] scsi: ufs-msm: add UFS controller support for Qualcomm MSM chips
>> On Aug 14, 2014, at 9:22 AM, Yaniv Gardi wrote: >>> The files in this change implement the UFS HW (controller & PHY) specific >>> behavior in Qualcomm MSM chips. >>> Signed-off-by: Yaniv Gardi >>> --- >>> Documentation/devicetree/bindings/ufs/ufs-msm.txt | 37 + >>> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt |4 + >>> drivers/scsi/ufs/Kconfig | 12 + >>> drivers/scsi/ufs/Makefile |4 + >>> drivers/scsi/ufs/ufs-msm-phy-qmp-20nm.c| 254 + drivers/scsi/ufs/ufs-msm-phy-qmp-20nm.h| 216 drivers/scsi/ufs/ufs-msm-phy-qmp-28nm.c| 368 +++ drivers/scsi/ufs/ufs-msm-phy-qmp-28nm.h| 735 + >>> drivers/scsi/ufs/ufs-msm-phy.c | 646 drivers/scsi/ufs/ufs-msm-phy.h | 193 >> Any reason not to put the phy driver in drivers/phy ? > Yes. Phy driver introduces a generic phy framework. > And as a framework it provides with API's, callbacks, > And data structures. > I think the right place to have the >implementation< of the ufs-msm-phy code > Is under drivers/scsi/ufs as it's more related to ufs than it's related to > the framework itself. I would agree with Kumar that PHY specific platform driver (which uses the generic PHY framwork) can actually go under drivers/phy/*, if i look at the 3.17-rc1, there are many platform specific phy drivers under drivers/phy/. >>> drivers/scsi/ufs/ufs-msm.c | 1105 >>> >>> drivers/scsi/ufs/ufs-msm.h | 158 +++ >>> 12 files changed, 3732 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/ufs/ufs-msm.txt create mode 100644 drivers/scsi/ufs/ufs-msm-phy-qmp-20nm.c >>> create mode 100644 drivers/scsi/ufs/ufs-msm-phy-qmp-20nm.h >>> create mode 100644 drivers/scsi/ufs/ufs-msm-phy-qmp-28nm.c >>> create mode 100644 drivers/scsi/ufs/ufs-msm-phy-qmp-28nm.h >>> create mode 100644 drivers/scsi/ufs/ufs-msm-phy.c >>> create mode 100644 drivers/scsi/ufs/ufs-msm-phy.h >>> create mode 100644 drivers/scsi/ufs/ufs-msm.c >>> create mode 100644 drivers/scsi/ufs/ufs-msm.h >> Seems like we should spit this into two patches, one for the phy and one >> for the UFS driver itself. Maybe even three, one for the 20nm phy, one for the 28nm phy, and one for ufs-msm.c,h. > we could try to split it, but since we didn't split this change into functional sub-changes, we decided to upload this change as a whole, as one change without the other wouldn't work anyhow, and they are both needed for proper functionality. >>> diff --git a/Documentation/devicetree/bindings/ufs/ufs-msm.txt b/Documentation/devicetree/bindings/ufs/ufs-msm.txt >>> new file mode 100644 >>> index 000..b5caace >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/ufs/ufs-msm.txt >> This should probably be bindings/phy/qcom-ufs-phy.txt If Yaniv also agrees that Qualcomm UFS MSM PHY specific driver should move under drivers/phy then yes, "ufs-msm.txt" should also move under bindings/phy/. Btw, regarding using "qcom-ufs-phy.txt" instead "ufs-msm.txt", i would agree that we need to add "phy" in file name but not sure we would like to replace "msm" with "qcom" because we used "msm" prefix/postfix almost everywhere. >>> @@ -0,0 +1,37 @@ >>> +* MSM Universal Flash Storage (UFS) PHY >>> + >>> +UFSPHY nodes are defined to describe on-chip UFS PHY hardware macro. +Each UFS PHY node should have its own node. >>> + >>> +To bind UFS PHY with UFS host controller, the controller node should +contain a phandle reference to UFS PHY node. >>> + >>> +Required properties: >>> +- compatible: compatible list, contains >>> "qcom,ufs-msm-phy-qmp-28nm" >>> + or "qcom,ufs-msm-phy-qmp-20nm" according to the relevant >>> + phy in use >> Do we really need -msm in the compat name? That's the convention we followed for all file names and hence carry forwarded to compatible name as well. Any specific issues with it? >>> +- reg : >>> +- #phy-cells : This property shall be set to 0 >>> +- vdda-phy-supply : phandle to main PHY supply for analog domain +- vdda-pll-supply : phandle to PHY PLL and Power-Gen block power supply >>> + >>> +Optional properties: >>> +- vdda-phy-max-microamp : specifies max. load that can be drawn from phy supply >>> +- vdda-pll-max-microamp : specifies max. load that can be drawn from pll supply >>> + >>> +Example: >>> + >>> + ufsphy1: ufsphy@0xfc597000 { >>> + compatible = "qcom,ufs-msm-phy-qmp-28nm"; >>> + reg = <0xfc597000 0x800>; >>> + #phy-cells = <0>; >>> + vdda-phy-supply = <&pma8084_l4>; >>> + vdda-pll-supply = <&pma8084_l12>; >>> + vdda-phy-max-microamp = <5>; >>> + vdda-pll-max-microamp = <1000>; >>> + }; >>> + >>> + ufshc@0xfc598000 { >>> + ... >>> + phys = <&ufsphy1>; >>> +
Re: [PATCH/RFC V2 07/16] scsi: support well known logical units
> On Thu, Aug 14, 2014 at 04:30:58PM +0300, Dolev Raviv wrote: >> From: Subhash Jadavani >> REPORT LUNS command has "SELECT REPORT" field which controls what type of >> logical units to be reported by device server. According to UFS device standard, if this field is set to 0, REPORT LUNS would report only report >> standard logical units. If it's set to 1 then it would report only well known logical unit and if it's set to 2 then device would report both standard and well known logical units. >> Some well-known logical units might not have scsi upper-layer drivers. In >> such cases, the runtime PM reference count increased during device enumeration will not be decreased, causing the parent device (host) to always be on even during idle. >> This change allows the SCSI LLD (Low Level Driver) to choose which type of logical units should be detected. >> Signed-off-by: Subhash Jadavani >> Signed-off-by: Sujit Reddy Thumma >> Signed-off-by: Dolev Raviv >> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 4a6e4ba..5a0e164 100644 >> --- a/drivers/scsi/scsi_scan.c >> +++ b/drivers/scsi/scsi_scan.c >> @@ -802,6 +802,14 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result, >> } else { >> sdev->type = (inq_result[0] & 0x1f); >> sdev->removable = (inq_result[1] & 0x80) >> 7; >> + >> +/* >> + * some devices may respond with wrong type for >> + * well-known logical units. Force well-known type >> + * to enumerate them correctly. >> + */ >> +if (scsi_is_wlun(sdev->lun) && (sdev->type != TYPE_WLUN)) >> +sdev->type = TYPE_WLUN; > no need to test for TYPE_WLUN here.] Agreed, will fix it. >> switch (sdev->type) { >> @@ -817,6 +825,7 @@ static int scsi_add_lun(struct scsi_device *sdev, unsigned char *inq_result, >> case TYPE_COMM: >> case TYPE_RAID: >> case TYPE_OSD: >> +case TYPE_WLUN: >> sdev->writeable = 1; >> break; >> case TYPE_ROM: > This switch statements has been removed in 3.17-rc1, please rebase your series on top of it. Sure. > While you're at it please make this patch, which > introduceÑ core scsi changes first in the series. Ok. Will move these core changes first in the current patch series. >> @@ -1412,6 +1421,13 @@ static int scsi_report_lun_scan(struct >> scsi_target *starget, int bflags, >> */ >> memset(&scsi_cmd[1], 0, 5); >> +if (shost->report_wlus) >> +/* >> + * Set "SELECT REPORT" field to 0x2 which will make device to + >> * report well known logical units along with standard LUs. >> + */ >> +scsi_cmd[2] = 0x2; > So we're finally getting well known LUN support. I think we should key this > off the SBC compliance level and not have the drivers opt in. If i understood this correctly, we want to make all devices report well known LUs irrespective of device driver (LLD)'s preference? If yes then i will knock off "if (shost->report_wlus)" check. >> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 5f36788..ba9d0f0 100644 >> --- a/drivers/scsi/scsi_sysfs.c >> +++ b/drivers/scsi/scsi_sysfs.c >> @@ -1060,6 +1060,13 @@ int scsi_sysfs_add_sdev(struct scsi_device *sdev) >> } >> } >> +/* >> + * put runtime pm reference for well-known logical units, >> + * drivers are expected to _get_* again during probe. >> + */ >> +if (scsi_is_wlun(sdev->lun)) >> +scsi_autopm_put_device(sdev); > Special casing the well known LUNs here seems wrong. Shouldn't we do this > for any devices that don't get a driver attached to them? Agreed, i will replace "if (scsi_is_wlun(sdev->lun))" check with "if (sdev->sdev_gendev.driver)". Regards, Subhash -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html