RE: [PATCH] scsi_devinfo: remove synchronous ALUA for NETAPP/RDAC devices
As discussed with Hannes, this is not required for ONTAP's LUN C-Mode as well. So the ALUA blacklisting may be removed here as well, and not just for NetApp RDAC. -Martin -Original Message- From: Hannes Reinecke [mailto:h...@suse.de] Sent: Wednesday, November 9, 2016 12:20 PM To: Xose Vazquez PerezCc: George, Martin ; Stankey, Robert ; Schremmer, Steven ; Stewart, Sean ; Christophe Varoqui ; James E . J . Bottomley ; Martin K . Petersen ; SCSI ML ; device-mapper development Subject: Re: [PATCH] scsi_devinfo: remove synchronous ALUA for NETAPP/RDAC devices On 11/08/2016 08:32 PM, Xose Vazquez Perez wrote: > NetApp did confirm this is only required for ONTAP(LUN C-Mode). > > Cc: Martin George > Cc: Robert Stankey > Cc: Steven Schremmer > Cc: Sean Stewart > Cc: Hannes Reinecke > Cc: Christophe Varoqui > Cc: James E.J. Bottomley > Cc: Martin K. Petersen > Cc: SCSI ML > Cc: device-mapper development > Signed-off-by: Xose Vazquez Perez > --- > drivers/scsi/scsi_devinfo.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c > index 2464569..1fb7964 100644 > --- a/drivers/scsi/scsi_devinfo.c > +++ b/drivers/scsi/scsi_devinfo.c > @@ -221,7 +221,6 @@ static struct { > {"NEC", "PD-1 ODX654P", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, > {"NEC", "iStorage", NULL, BLIST_REPORTLUN2}, > {"NETAPP", "LUN C-Mode", NULL, BLIST_SYNC_ALUA}, > - {"NETAPP", "INF-01-00", NULL, BLIST_SYNC_ALUA}, > {"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, > {"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, > {"PIONEER", "CD-ROM DRM-600", NULL, BLIST_FORCELUN | > BLIST_SINGLELUN}, > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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_devinfo: remove synchronous ALUA for NETAPP/RDAC devices
On 11/08/2016 08:32 PM, Xose Vazquez Perez wrote: NetApp did confirm this is only required for ONTAP(LUN C-Mode). Cc: Martin GeorgeCc: Robert Stankey Cc: Steven Schremmer Cc: Sean Stewart Cc: Hannes Reinecke Cc: Christophe Varoqui Cc: James E.J. Bottomley Cc: Martin K. Petersen Cc: SCSI ML Cc: device-mapper development Signed-off-by: Xose Vazquez Perez --- drivers/scsi/scsi_devinfo.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index 2464569..1fb7964 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -221,7 +221,6 @@ static struct { {"NEC", "PD-1 ODX654P", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"NEC", "iStorage", NULL, BLIST_REPORTLUN2}, {"NETAPP", "LUN C-Mode", NULL, BLIST_SYNC_ALUA}, - {"NETAPP", "INF-01-00", NULL, BLIST_SYNC_ALUA}, {"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"PIONEER", "CD-ROM DRM-600", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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 v1] ufs: introcude UFSHCD_QUIRK_GET_VS_RESULT quirk
> > Some UFS host controllers may not report a result of UIC command > > completion properly. > > In such cases, they should get the result from UIC directly if their > > architectures support that. > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 27 +++ > > drivers/scsi/ufs/ufshcd.h | 20 > > 2 files changed, 43 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index d4a5a9c..8e19631 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -1011,7 +1011,10 @@ static inline bool > > ufshcd_ready_for_uic_cmd(struct ufs_hba *hba) > > */ > > static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba) { > > - return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7; > > + if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT) > > + return (u8)ufshcd_vops_get_vs_info(hba, VS_OP_03); > > + else > > + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7; > > } > > > > /** > > @@ -1038,6 +1041,17 @@ ufshcd_dispatch_uic_cmd(struct ufs_hba *hba, > > struct uic_command *uic_cmd) > > REG_UIC_COMMAND); > > } > > > > +static inline enum vs_opcode ufshcd_get_vs_opcode(struct uic_command > > *uic_cmd) > > +{ > > + if (uic_cmd->command == UIC_CMD_DME_LINK_STARTUP) > > + return VS_OP_00; > > + else if ((uic_cmd->command == UIC_CMD_DME_HIBER_ENTER)) > > + return VS_OP_01; > > + else if ((uic_cmd->command == UIC_CMD_DME_HIBER_EXIT)) > > + return VS_OP_02; > > + else > > + return VS_OP_INVALID; > > +} > > /** > > * ufshcd_wait_for_uic_cmd - Wait complectioin of UIC command > > * @hba: per adapter instance > > @@ -1051,11 +1065,16 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, > > struct uic_command *uic_cmd) { > > int ret; > > unsigned long flags; > > + enum vs_opcode vs_op = ufshcd_get_vs_opcode(uic_cmd); > > Please move this after we check the quirk below. Okay, I'll apply what you said. > > > > > if (wait_for_completion_timeout(_cmd->done, > > - msecs_to_jiffies(UIC_CMD_TIMEOUT))) > > - ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT; > > - else > > + msecs_to_jiffies(UIC_CMD_TIMEOUT))) { > > + if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT && > > + vs_op != VS_OP_INVALID) > > + ret = ufshcd_vops_get_vs_info(hba, vs_op); > > + else > > + ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT; > > + } else > > ret = -ETIMEDOUT; > > > > spin_lock_irqsave(hba->host->host_lock, flags); > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index 13504b4..8cf3991 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -245,6 +245,14 @@ struct ufs_pwr_mode_info { > > struct ufs_pa_layer_attr info; > > }; > > > > +enum vs_opcode { > > + VS_OP_00 = 0x00, > > + VS_OP_01, > > + VS_OP_02, > > + VS_OP_03, > > + VS_OP_INVALID = 0xFF, > > +}; > > + > > How do we interpret these codes? and how is this useful for non-exynos > UFS host controller? Please make it generic which can be used by other > controllers in future (if required). As you guessed, this patch is to get some information by using vendor specific ways directly, which was got from standard UFSHCI SFRs for some hardware problems. If other controllers have similar problems, but the problems are acutally another cases, you should add the function call - ufshcd_vops_get_vs_info - wherever you want to use this interface. This patch is intended to cover all results of link startup, hibern8 and power mode change. Because I don't want to add an additional interface whenever a new controller has a new problem similar with what I mentioned above. Anyway, could you give me your opinion about it? Or, if you don't agree applying something just to fix some simple problems, such as reading SFRs, I won't update this patch any more. Thank you. > > > /** > > * struct ufs_hba_variant_ops - variant specific callbacks > > * @name: variant name > > @@ -297,6 +305,7 @@ struct ufs_hba_variant_ops { > > int (*resume)(struct ufs_hba *, enum ufs_pm_op); > > void(*dbg_register_dump)(struct ufs_hba *hba); > > int (*phy_initialization)(struct ufs_hba *); > > + int (*get_vs_info)(struct ufs_hba *hba, enum vs_opcode); > > }; > > > > /* clock gating state */ > > @@ -484,6 +493,8 @@ struct ufs_hba { > > */ > > #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) > > > > + #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) > > Name is weird, can we name it to reflect the actual purpose of this > quirk? something like UFSHCD_QUIRK_BROKEN_UIC_COMPL ? Okay, I'll change the name
RE: [PATCH v1] ufs: introduce setup_hibern8 callback
> > Some UFS host controller may need to configure some things around > > hibern8 enter/exit > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 10 -- drivers/scsi/ufs/ufshcd.h > > | 10 ++ > > 2 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index fdb0502..d4a5a9c 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct > > ufs_hba *hba) > > int ret; > > struct uic_command uic_cmd = {0}; > > > > + ufshcd_vops_setup_hibern8(hba, true, PRE_CHANGE); > > + > > uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; > > ret = ufshcd_uic_pwr_ctrl(hba, _cmd); > > > > @@ -2710,7 +2712,8 @@ static int __ufshcd_uic_hibern8_enter(struct > > ufs_hba *hba) > > */ > > if (ufshcd_link_recovery(hba)) > > ret = -ENOLINK; > > - } > > + } else > > + ufshcd_vops_setup_hibern8(hba, true, POST_CHANGE); > > > > return ret; > > } > > @@ -2733,13 +2736,16 @@ static int ufshcd_uic_hibern8_exit(struct > > ufs_hba *hba) > > struct uic_command uic_cmd = {0}; > > int ret; > > > > + ufshcd_vops_setup_hibern8(hba, false, PRE_CHANGE); > > + > > uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; > > ret = ufshcd_uic_pwr_ctrl(hba, _cmd); > > if (ret) { > > dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n", > > __func__, ret); > > ret = ufshcd_link_recovery(hba); > > - } > > + } else > > + ufshcd_vops_setup_hibern8(hba, false, POST_CHANGE); > > > > return ret; > > } > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index b084d89..13504b4 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -265,6 +265,8 @@ struct ufs_pwr_mode_info { > > * to set some things > > * @setup_task_mgmt: called before any task management request is > > issued > > * to set some things > > + * @setup_hibern8: called around hibern8 enter/exit > > + * to configure some things > > * @suspend: called during host controller PM callback > > * @resume: called during host controller PM callback > > * @dbg_register_dump: used to dump controller debug information @@ > > -290,6 +292,7 @@ struct ufs_hba_variant_ops { > > struct ufs_pa_layer_attr *); > > void(*setup_xfer_req)(struct ufs_hba *, int, bool); > > void(*setup_task_mgmt)(struct ufs_hba *, int, u8); > > + void(*setup_hibern8)(struct ufs_hba *, bool, bool); > > Can we change the name to "hibern8_notify" ? You may check other > ufs_hba_variant_ops for reference. Okay, I'll apply what you said. > > > int (*suspend)(struct ufs_hba *, enum ufs_pm_op); > > int (*resume)(struct ufs_hba *, enum ufs_pm_op); > > void(*dbg_register_dump)(struct ufs_hba *hba); > > @@ -821,6 +824,13 @@ static inline void > > ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba, > > return hba->vops->setup_task_mgmt(hba, tag, tm_function); } > > > > +static inline void ufshcd_vops_setup_hibern8(struct ufs_hba *hba, > > + bool enter, bool notify) > > Using bool to specify whether it is hibern8 enter or hibern8 exit doesn't > seem to be readable. May be you can pass the UIC_CMD_DME_HIBER_ENTER or > UIC_CMD_DME_HIBER_EXIT (in other words use "enum uic_cmd_dme" type). > > also "notify" type should be changed from "bool" to "enum > ufs_notify_change_status". Okay, I'll apply what you said and use "enum uic_cmd_dme" as 2nd argument date type. > > > +{ > > + if (hba->vops && hba->vops->setup_hibern8) > > + return hba->vops->setup_hibern8(hba, enter, notify); } > > + > > static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum > > ufs_pm_op op) { > > if (hba->vops && hba->vops->suspend) > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project > -- > 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 v1] ufs: introduce setup_xfer_req callback
> > Some UFS host controller may need to configure some things before any > > transfer request is issued. > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 2 ++ > > drivers/scsi/ufs/ufshcd.h | 10 ++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 8cf5d8f..bf78321 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -1516,6 +1516,7 @@ 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_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false)); > > ufshcd_send_command(hba, tag); > > out_unlock: > > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ -1727,6 > > +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, > > /* Make sure descriptors are ready before ringing the doorbell */ > > wmb(); > > spin_lock_irqsave(hba->host->host_lock, flags); > > + ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false)); > > ufshcd_send_command(hba, tag); > > spin_unlock_irqrestore(hba->host->host_lock, flags); > > > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index afff7f4..f2a69c0 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -261,6 +261,8 @@ struct ufs_pwr_mode_info { > > * @pwr_change_notify: called before and after a power mode change > > * is carried out to allow vendor spesific capabilities > > * to be set. > > + * @setup_xfer_req: called before any transfer request is issued > > + * to set some things > > * @suspend: called during host controller PM callback > > * @resume: called during host controller PM callback > > * @dbg_register_dump: used to dump controller debug information @@ > > -284,6 +286,7 @@ struct ufs_hba_variant_ops { > > enum ufs_notify_change_status status, > > struct ufs_pa_layer_attr *, > > struct ufs_pa_layer_attr *); > > + void(*setup_xfer_req)(struct ufs_hba *, int, bool); > > int (*suspend)(struct ufs_hba *, enum ufs_pm_op); > > int (*resume)(struct ufs_hba *, enum ufs_pm_op); > > void(*dbg_register_dump)(struct ufs_hba *hba); > > @@ -801,6 +804,13 @@ static inline int > > ufshcd_vops_pwr_change_notify(struct ufs_hba *hba, > > return -ENOTSUPP; > > } > > > > +static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, > > +int > > tag, > > + bool is_cmd_not_null) > > This might be more readable: s/is_cmd_not_null/is_scsi_cmd , rest looks > good in this patch. Okay. I'll apply what you said. > > > > +{ > > + if (hba->vops && hba->vops->setup_xfer_req) > > + return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null); > > +} > > + > > static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum > > ufs_pm_op op) > > { > > if (hba->vops && hba->vops->suspend) > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > -- > 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 v1] ufs: introduce setup_task_mgmt
> > Some UFS host controller may need to configure some things before any > > task management request is issued > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 1 + > > drivers/scsi/ufs/ufshcd.h | 10 ++ > > 2 files changed, 11 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index bf78321..fdb0502 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba > > *hba, int lun_id, int task_id, > > task_req_upiup->input_param2 = cpu_to_be32(task_id); > > > > /* send command to the controller */ > > May be you need to move this comment just above __set_bit() call. Rest all > looks good in this patch. Okay. I'll apply what you said on new version of this patch.. > > > + ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function); > > __set_bit(free_slot, >outstanding_tasks); > > > > /* Make sure descriptors are ready before ringing the task doorbell > > */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index f2a69c0..b084d89 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -263,6 +263,8 @@ struct ufs_pwr_mode_info { > > * to be set. > > * @setup_xfer_req: called before any transfer request is issued > > * to set some things > > + * @setup_task_mgmt: called before any task management request is > > issued > > + * to set some things > > * @suspend: called during host controller PM callback > > * @resume: called during host controller PM callback > > * @dbg_register_dump: used to dump controller debug information @@ > > -287,6 +289,7 @@ struct ufs_hba_variant_ops { > > struct ufs_pa_layer_attr *, > > struct ufs_pa_layer_attr *); > > void(*setup_xfer_req)(struct ufs_hba *, int, bool); > > + void(*setup_task_mgmt)(struct ufs_hba *, int, u8); > > int (*suspend)(struct ufs_hba *, enum ufs_pm_op); > > int (*resume)(struct ufs_hba *, enum ufs_pm_op); > > void(*dbg_register_dump)(struct ufs_hba *hba); > > @@ -811,6 +814,13 @@ static inline void > > ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag, > > return hba->vops->setup_xfer_req(hba, tag, > is_cmd_not_null); } > > > > +static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba, > > + int tag, u8 tm_function) > > +{ > > + if (hba->vops && hba->vops->setup_task_mgmt) > > + return hba->vops->setup_task_mgmt(hba, tag, tm_function); } > > + > > static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum > > ufs_pm_op op) { > > if (hba->vops && hba->vops->suspend) > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project > -- > 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 v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk
> > If UFS driver resets interrupt aggregation timer and counter > > when there is a pending doorbell, an interrupt of IO completion > > of a corresponding task may be missed. > > That would cause a command timeout. > > > > If UFS driver resets interrupt aggregation timer and counter > > when there is a pending doorbell, a competion interrupt > > of a corresponing task may be issued. > > That would casue a command timeout. > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 4 +++- > > drivers/scsi/ufs/ufshcd.h | 1 + > > 2 files changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index c904854..65bbf59 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -3730,7 +3730,9 @@ static void ufshcd_transfer_req_compl(struct > > ufs_hba *hba) > > * false interrupt if device completes another request after > > resetting > > * aggregation and before reading the DB. > > */ > > - if (ufshcd_is_intr_aggr_allowed(hba)) > > + if ((ufshcd_is_intr_aggr_allowed(hba)) > > + && !(hba->quirks & UFSHCI_QUIRK_SKIP_INTR_AGGR)) > > Why do we need this new quirk? If controller has the issue with > interrupt aggregation, you can remove this UFSHCD_CAP_INTR_AGGR to > disable the aggregation altogether. This fragment doesn't mean disabling interrupt aggregation. It just means that interrupt aggregation logic would not be reset in ISR. But I think that the name of the quirk doesn't represent the function of it expectedly. I'll change the name with another one, such as UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR. > > > + ufshcd_reset_intr_aggr(hba); > > ufshcd_reset_intr_aggr(hba); > > > > tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index 6a96f24..6a9c6e9 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -497,6 +497,7 @@ struct ufs_hba { > > #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) > > #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) > > #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9) > > + #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10) > > > > > > unsigned int quirks;/* Deviations from standard UFSHCI spec. > */ > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project > -- > 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 v1] ufs: add a variety of definitions decribed in UFS spec
> > These things are defined to be used by some UFS Host controllers. > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/mphy.h | 38 ++ > > drivers/scsi/ufs/ufshci.h | 14 +++--- > > drivers/scsi/ufs/unipro.h | 24 > > 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 > > drivers/scsi/ufs/mphy.h > > > > diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new > > file mode 100644 index 000..c431f49 > > --- /dev/null > > +++ b/drivers/scsi/ufs/mphy.h > > @@ -0,0 +1,38 @@ > > +/* > > + * drivers/scsi/ufs/mphy.h > > + * > > + * Copyright (C) 2014 Samsung Electronics Co., Ltd. > > + * > > + * This program is free software; you can redistribute it and/or > > modify > > + * it under the terms of the GNU General Public License as published > > by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#ifndef _MPHY_H_ > > +#define _MPHY_H_ > > Do we really need separate file for MPHY? May be we can have these > accomodated in unipro.h itself? I think it's needed. MPHY spec isn't related with Unipro spec directly. Is there any reason why we say that this file is deprecated? Please let me know if you have. > > > + > > +#define TX_HIBERN8TIME_CAP 0x0f > > +#define TX_MIN_ACTIVATE_TIME 0x33 > > + > > +#define RX_HS_G1_SYNC_LEN_CAP 0x8b > > +#define RX_HS_G1_PREP_LEN_CAP 0x8c > > +#define RX_HS_G2_SYNC_LEN_CAP 0x94 > > +#define RX_HS_G3_SYNC_LEN_CAP 0x95 > > +#define RX_HS_G2_PREP_LEN_CAP 0x96 > > +#define RX_HS_G3_PREP_LEN_CAP 0x97 > > + #define SYNC_RANGE_FINE (0 << 6) > > + #define SYNC_RANGE_COARSE (1 << 6) > > + #define SYNC_LEN(x) ((x) & 0x3f) > > + #define PREP_LEN(x) ((x) & 0xf) > > +#define RX_ADV_GRANULARITY_CAP 0x98 > > + #define RX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) > > +#define TX_ADV_GRANULARITY_CAP 0x10 > > + #define TX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) > > +#define RX_MIN_ACTIVATETIME_CAP0x8f > > +#define RX_HIBERN8TIME_CAP 0x92 > > +#define RX_ADV_HIBERN8TIME_CAP 0x99 > > +#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a > > + > > +#endif /* _MPHY_H_ */ > > + > > diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h > > index 9599741..26dc340 100644 > > --- a/drivers/scsi/ufs/ufshci.h > > +++ b/drivers/scsi/ufs/ufshci.h > > @@ -170,17 +170,25 @@ enum { > > /* UECDL - Host UIC Error Code Data Link Layer 3Ch */ > > #define UIC_DATA_LINK_LAYER_ERROR UFS_BIT(31) > > #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF > > -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT 0x2000 > > -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001 > > -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002 > > +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0) > > +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT UFS_BIT(1) > > +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5) > > why don't we just add macros for all the bits in UECDL ? This makes it > easy in future. Okay. I'll think about it and apply new macros on new version of this patch. > > > +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT UFS_BIT(13) > > > > /* UECN - Host UIC Error Code Network Layer 40h */ > > #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31) > > #define UIC_NETWORK_LAYER_ERROR_CODE_MASK 0x7 > > +#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0) > > +#define UIC_NETWORK_BAD_DEVICEID_ENC BIT(1) > > +#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING BIT(2) > > > > /* UECT - Host UIC Error Code Transport Layer 44h */ > > #define UIC_TRANSPORT_LAYER_ERROR UFS_BIT(31) > > #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F > > +#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE BIT(0) > > +#define UIC_TRANSPORT_UNKNOWN_CPORTID BIT(1) > > +#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2) > > +#define UIC_TRANSPORT_BAD_TC BIT(4) > > May be add definition for bit-5 and 6 as well. Okay. I'll add those things on new version of this patch. > > > > > /* UECDME - Host UIC Error Code DME 48h */ > > #define UIC_DME_ERROR UFS_BIT(31) > > diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h > > index eff8b56..e47e2c2 100644 > > --- a/drivers/scsi/ufs/unipro.h > > +++ b/drivers/scsi/ufs/unipro.h > > @@ -127,6 +127,7 @@ > > #define PA_PACPREQEOBTIMEOUT 0x1591 > > #define PA_HIBERN8TIME 0x15A7 > > #define PA_LOCALVERINFO0x15A9 > > +#define PA_GRANULARITY 0x15AA > > #define PA_TACTIVATE 0x15A8 > > #define PA_PACPFRAMECOUNT 0x15C0 > > #define PA_PACPERRORCOUNT 0x15C1 > > @@ -170,6
Re: [PATCH 1/1] iscsi: fix regression caused by session lock patch
On Mon, Nov 07, 2016 at 04:23:10PM -0200, Guilherme G. Piccoli wrote: > > Sure! Count on us to test any patches. I guess the first step is to > reproduce on upstream right? We haven't tested specifically this > scenario for long time. Will try to reproduce on 4.9-rc4 and update here. Great, I'm looking forward to hearing the result. Assuming it reproduces, I don't think this level of fine grained locking is necessarily the best solution, but it may help confirm the problem. Especially if the WARN_ONCE I slipped in here triggers. - Chris --- diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c index f9b6fba..fbd18ab 100644 --- a/drivers/scsi/libiscsi.c +++ b/drivers/scsi/libiscsi.c @@ -560,8 +560,12 @@ static void iscsi_complete_task(struct iscsi_task *task, int state) WARN_ON_ONCE(task->state == ISCSI_TASK_FREE); task->state = state; - if (!list_empty(>running)) + spin_lock_bh(>taskqueuelock); + if (!list_empty(>running)) { + WARN_ONCE(1, "iscsi_complete_task while task on list"); list_del_init(>running); + } + spin_unlock_bh(>taskqueuelock); if (conn->task == task) conn->task = NULL; @@ -783,7 +787,9 @@ __iscsi_conn_send_pdu(struct iscsi_conn *conn, struct iscsi_hdr *hdr, if (session->tt->xmit_task(task)) goto free_task; } else { + spin_lock_bh(>taskqueuelock); list_add_tail(>running, >mgmtqueue); + spin_unlock_bh(>taskqueuelock); iscsi_conn_queue_work(conn); } @@ -1474,8 +1480,10 @@ void iscsi_requeue_task(struct iscsi_task *task) * this may be on the requeue list already if the xmit_task callout * is handling the r2ts while we are adding new ones */ + spin_lock_bh(>taskqueuelock); if (list_empty(>running)) list_add_tail(>running, >requeue); + spin_unlock_bh(>taskqueuelock); iscsi_conn_queue_work(conn); } EXPORT_SYMBOL_GPL(iscsi_requeue_task); @@ -1512,22 +1520,26 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) * only have one nop-out as a ping from us and targets should not * overflow us with nop-ins */ + spin_lock_bh(>taskqueuelock); check_mgmt: while (!list_empty(>mgmtqueue)) { conn->task = list_entry(conn->mgmtqueue.next, struct iscsi_task, running); list_del_init(>task->running); + spin_unlock_bh(>taskqueuelock); if (iscsi_prep_mgmt_task(conn, conn->task)) { /* regular RX path uses back_lock */ spin_lock_bh(>session->back_lock); __iscsi_put_task(conn->task); spin_unlock_bh(>session->back_lock); conn->task = NULL; + spin_lock_bh(>taskqueuelock); continue; } rc = iscsi_xmit_task(conn); if (rc) goto done; + spin_lock_bh(>taskqueuelock); } /* process pending command queue */ @@ -1535,19 +1547,24 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) conn->task = list_entry(conn->cmdqueue.next, struct iscsi_task, running); list_del_init(>task->running); + spin_unlock_bh(>taskqueuelock); if (conn->session->state == ISCSI_STATE_LOGGING_OUT) { fail_scsi_task(conn->task, DID_IMM_RETRY); + spin_lock_bh(>taskqueuelock); continue; } rc = iscsi_prep_scsi_cmd_pdu(conn->task); if (rc) { if (rc == -ENOMEM || rc == -EACCES) { + spin_lock_bh(>taskqueuelock); list_add_tail(>task->running, >cmdqueue); conn->task = NULL; + spin_unlock_bh(>taskqueuelock); goto done; } else fail_scsi_task(conn->task, DID_ABORT); + spin_lock_bh(>taskqueuelock); continue; } rc = iscsi_xmit_task(conn); @@ -1558,6 +1575,7 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) * we need to check the mgmt queue for nops that need to * be sent to aviod starvation */ + spin_lock_bh(>taskqueuelock); if (!list_empty(>mgmtqueue)) goto check_mgmt; } @@ -1577,12 +1595,15 @@ static int iscsi_data_xmit(struct iscsi_conn *conn) conn->task
Re: [PATCH v5 00/12] ufs-qcom: phy/hcd: Clean up qcom-ufs phy and ufs-qcom hcd
On Wed, Nov 9, 2016 at 4:36 AM, Martin K. Petersenwrote: >> "Vivek" == Vivek Gautam writes: > > Vivek> Here's the rebased version of patches based on 4.10/scsi-queue > Vivek> branch as requested. The patches can now be applied and > Vivek> pulled-in. > > Thanks! Applied to 4.10/scsi-queue. Thanks Martin. Regards Vivek -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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: Need some pointers to debug a target hang
On 11/04/2016 02:52 PM, Nicholas A. Bellinger wrote: Hi Zhu & Co, Thanks for the detailed logs. Comments below. On Mon, 2016-10-31 at 16:51 +0800, Zhu Lingshan wrote: Hi Nicholas, (sorry it would be a long mail) Sorry for the delay, I spent some test and debug work. I find the patch http://www.spinics.net/lists/target-devel/msg13530.html can solve two issues: (a). iscsit_stop_session() on the top of iscsi_np stack. (b).iscsi_check_for_session_reinstatement() on the top of iscsi_np stack, it is a must to reboot to recover. Great, thanks for confirming the patch above. The key wrt this scenario, and other scenarios below is once target-core ABORT_TASK logic locates a se_cmd descriptor for IBLOCK backend I/O still outstanding, both iscsi session reinstatement in iscsi_np context and iscsi session shutdown in iscsi_t[t,r]x context are blocked until the specific outstanding se_cmd I/Os are completed, back to target-core backend driver. Note this is expected behavior during target-core ABORT_TASK and iscsi-target session reinstatement + session shutdown. But I also find two more issues. - Please let me explain my setup: I have a target server, kernel version 4.4.21, it has a SATA HDD as a LUN, only one LUN. I have two initiators, both login to the target. Create two partitions on the LUN, each initiator mount a certain partition. Like initiator1 mount /dev/sdc1, initiator2 mount /dev/sdc2. looks like this: lszhu_DEV:~ # lsscsi [0:0:0:0]cd/dvd HL-DT-ST DVD+-RW GHB0NA1C0 /dev/sr0 [4:0:0:0]diskATA TOSHIBA DT01ACA2 MX4O /dev/sda [9:0:0:0]diskLIO-ORG IBLOCK 4.0 /dev/sdc lszhu_DEV:~ # lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:00 1.8T 0 disk ├─sda1 8:1016G 0 part [SWAP] ├─sda2 8:20 200G 0 part / └─sda3 8:30 1.6T 0 part /home sdc 8:32 0 465.8G 0 disk ├─sdc1 8:33 0 200G 0 part └─sdc2 8:34 0 265.8G 0 part /mnt sr0 11:01 1024M 0 rom bj-ha-5:~ # lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:00 465.8G 0 disk ├─sda1 8:10 8G 0 part [SWAP] ├─sda2 8:20 100G 0 part / └─sda3 8:30 357.8G 0 part /home sdb 8:16 0 465.8G 0 disk sdc 8:32 0 465.8G 0 disk ├─sdc1 8:33 0 200G 0 part /mnt └─sdc2 8:34 0 265.8G 0 part sr0 11:01 1024M 0 rom so you can see each initiator will read / write on their own partition from the LUN. mount the partition like this: mount -o dioread_nolock /dev/sdc1 /mnt, option dioread_nolock can help we reproduce this bug a little quicker. Then run fio on both initiators like this: fio -filename=/mnt/file1 -direct=1 -rw=randread -iodepth=64 -ioengin=libaio -bs=64K -size=5G -numjobs=24 -runtime=6 -time_based -group_reporting -name=init1 - Here are the three more issues I found, kernel version 4.4.21 (1) The first one looks like this, it is quite rare, only once, I failed to get stack information for iscsi_trx and hard to reproduce, call stack of iscsi_np is here: bj-ha-3:~ # ps -aux | grep iscsi root 10501 0.0 0.0 0 0 ?D16:45 0:00 [iscsi_np] root 10519 0.4 0.0 0 0 ?S16:49 0:02 [iscsi_ttx] root 10520 2.2 0.0 0 0 ?S16:49 0:14 [iscsi_trx] root 10533 0.9 0.0 0 0 ?D16:54 0:03 [iscsi_trx] root 10547 0.0 0.0 10508 1552 pts/0S+ 16:59 0:00 grep --color=auto iscsi bj-ha-3:~ # cat /proc/10501/stack [] inet_csk_accept+0x269/0x2e0 [] inet_accept+0x2a/0x100 [] kernel_accept+0x48/0xa0 [] iscsit_accept_np+0x31/0x230 [iscsi_target_mod] [] iscsi_target_login_thread+0xeb/0xfd0 [iscsi_target_mod] [] kthread+0xbd/0xe0 [] ret_from_fork+0x3f/0x70 [] kthread+0x0/0xe0 [] 0x The iscsi_np here is looks like it's doing normal Linux/NET accept. Is the uninterruptible sleep for PID=10533 waiting for outstanding I/O to complete during iscsit_close_connection() shutdown after ABORT_TASK..? sorry I have been trying to reproduce this case for a week, but can not reproduce this case anymore, it is quite rare. (2) The second issue I found has iscsi_check_for_session_reinstatement() on the top of iscsi_np call stack, but different from case (b) we mentioned before. In case (b), we must reboot the target to recover, but in this case, it will auto recover, here is the details: dmesg: [ 100.487421]
Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
> "Kashyap" == Kashyap Desaiwrites: Kashyap, >> Send a fix on top of current -git asap. The current tree is >> completely broken for any megaraid user. -rc4 is no time to send in >> untested patches, especially not something that claims to fix a 9 >> year old bug and is marked for stable as well. *sigh* It's always the most innocuous patches that cause the worst regressions :( Kashyap> I will run some more test (using patch set only marked for Kashyap> stable from last series) and submit fix ASAP. I just tried a system with a megaraid card and things are broken in both 4.9/scsi-fixes and 4.10/scsi-queue unless the controller firmware has the new sync flag set. So, yes. Let's get a fix for this queued up ASAP. Thanks! -- Martin K. Petersen Oracle Linux Engineering -- 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 v1] ufs: introduce UFSHCD_QUIRK_BROKEN_DWORD_UTRD quirk
> > Some UFS host controllers may think > > granularitys of PRDT length and offset as bytes, not double words. > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 24 +++- > > drivers/scsi/ufs/ufshcd.h | 2 ++ > > 2 files changed, 21 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 8e19631..549e3e8 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -1161,8 +1161,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb > > *lrbp) > > return sg_segments; > > > > if (sg_segments) { > > - lrbp->utr_descriptor_ptr->prd_table_length = > > - cpu_to_le16((u16) (sg_segments)); > > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) > > This might sound more specific: > s/UFSHCD_QUIRK_BROKEN_DWORD_UTRD/UFSHCD_QUIRK_PRDT_BYTE_GRAN . That sounds good. I'll apply what you said. > > > + lrbp->utr_descriptor_ptr->prd_table_length = > > + cpu_to_le16((u16)(sg_segments * > > + sizeof(struct ufshcd_sg_entry))); > > + else > > + lrbp->utr_descriptor_ptr->prd_table_length = > > + cpu_to_le16((u16) (sg_segments)); > > > > prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr; > > > > @@ -2385,12 +2390,21 @@ static void > > ufshcd_host_memory_configure(struct ufs_hba *hba) > > > cpu_to_le32(upper_32_bits(cmd_desc_element_addr)); > > > > /* Response upiu and prdt offset should be in double words > */ > > - utrdlp[i].response_upiu_offset = > > + if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) { > > + utrdlp[i].response_upiu_offset = > > + cpu_to_le16(response_offset); > > + utrdlp[i].prd_table_offset = > > + cpu_to_le16(prdt_offset); > > + utrdlp[i].response_upiu_length = > > + cpu_to_le16(ALIGNED_UPIU_SIZE); > > + } else { > > + utrdlp[i].response_upiu_offset = > > cpu_to_le16((response_offset >> 2)); > > - utrdlp[i].prd_table_offset = > > + utrdlp[i].prd_table_offset = > > cpu_to_le16((prdt_offset >> 2)); > > - utrdlp[i].response_upiu_length = > > + utrdlp[i].response_upiu_length = > > cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); > > + } > > > > hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); > > hba->lrb[i].ucd_req_ptr = > > diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h > > index 8cf3991..565f005 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -494,6 +494,8 @@ struct ufs_hba { > > #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) > > > > #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) > > + #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) > > + > > This extra line space isn't needed. Okay. > > > > > unsigned int quirks;/* Deviations from standard UFSHCI spec. > */ > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project > -- > 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: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
> -Original Message- > From: Jens Axboe [mailto:ax...@kernel.dk] > Sent: Wednesday, November 09, 2016 6:50 AM > To: Kashyap Desai; Martin K. Petersen > Cc: linux-scsi > Subject: Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box > > On 11/08/2016 05:20 PM, Kashyap Desai wrote: > >> -Original Message- > >> From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > >> Sent: Wednesday, November 09, 2016 4:45 AM > >> To: Jens Axboe > >> Cc: linux-scsi; Kashyap Desai; Martin K. Petersen > >> Subject: Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box > >> > >>> "Jens" == Jens Axboewrites: > >> > >> Jens> I wasted half a day on this, thinking it was something in my > >> Jens> 4.10 branches. But it turns out it is not, the regression is in > >> Jens> mainline. > > > > Jens - > > > > Sorry for trouble. I did not validated this single patch. I validated > > complete patch set. > > Issue is improper MACRO usage MEGASAS_IS_LOGICAL, which gives > > incorrect check condition in qcmd Path. > > > > Below is proposed fix. > > > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas.h > > b/drivers/scsi/megaraid/megaraid_sas.h > > index 74c7b44..0d2625b 100644 > > --- a/drivers/scsi/megaraid/megaraid_sas.h > > +++ b/drivers/scsi/megaraid/megaraid_sas.h > > @@ -2236,7 +2236,7 @@ struct megasas_instance_template { }; > > > > #define MEGASAS_IS_LOGICAL(scp) > > \ > > - (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1 > > Ugh... So we're completing everything immediately. > > > Martin - I validated whole series. Apologies for this. > > > > Please help me to know how to fix this ? Do I need to send only fix on > > top of latest commit (as posted above - MACRO definition) for this issue > > ? > > Send a fix on top of current -git asap. The current tree is completely > broken for > any megaraid user. -rc4 is no time to send in untested patches, especially > not > something that claims to fix a 9 year old bug and is marked for stable as > well. I will run some more test (using patch set only marked for stable from last series) and submit fix ASAP. > > -- > Jens Axboe -- 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 v1] ufs: introduce UFSHCD_QUIRK_USE_OF_HCE quirk
> On 2016-11-07 23:57, Kiwoong Kim wrote: > > Some host controller might not initialize itself by setting "Host > > Controller Enable" to 1. > > > They should do this to reset UIC. > > I am not sure if i understood this statment. can you give more details? UFSHCI spec says as follows: - When HCE is ‘0’ and software writes ‘1’, the host controller hardware shall execute the step 2 described in section 7.1.1 of this standard, including reset of the host UTP and UIC layers. I found that a specific controller didn't execute reset of UIC layers Properly for an internal problem. At that time, DME reset and DME enable could make UIC be reset properly instead of using "Host Controller Enable". > > > In such cases, 'DME reset' and 'DME enable' are required for normal > > subsequent operations. > > This means HCE implementation is broken, you should name the quirk as > UFSHCD_QUIRK_BROKEN_HCE . Okay. > > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/ufshcd.c | 44 > > +++- > > drivers/scsi/ufs/ufshcd.h | 1 + > > 2 files changed, 44 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 24d6ea7..c904854 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -2496,6 +2496,37 @@ static inline void > > ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) > > usleep_range(min_sleep_time_us, min_sleep_time_us + 50); } > > > > +static int ufshcd_dme_reset(struct ufs_hba *hba) { > > + struct uic_command uic_cmd = {0}; > > + int ret; > > + > > + uic_cmd.command = UIC_CMD_DME_RESET; > > + uic_cmd.argument1 = 0x1; > > + > > + ret = ufshcd_send_uic_cmd(hba, _cmd); > > + if (ret) > > + dev_err(hba->dev, > > + "dme-reset: error code %d\n", ret); > > This error message doesn't say which DME command failed, do you want to > add that? I think that the string "dme-reset" is useful when clarifying some problems. But if you don't agree merging it to mainline, I'll remove it in next version. > > > + > > + return ret; > > +} > > + > > +static int ufshcd_dme_enable(struct ufs_hba *hba) { > > + struct uic_command uic_cmd = {0}; > > + int ret; > > + > > + uic_cmd.command = UIC_CMD_DME_ENABLE; > > + > > + ret = ufshcd_send_uic_cmd(hba, _cmd); > > + if (ret) > > + dev_err(hba->dev, > > + "dme-enable: error code %d\n", ret); > > This error message doesn't say which DME command failed, do you want to > add that? Same with above. > > > + > > + return ret; > > +} > > + > > /** > > * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET > > * @hba: per adapter instance > > @@ -3101,6 +3132,7 @@ static inline void ufshcd_hba_stop(struct > > ufs_hba *hba, bool can_sleep) static int ufshcd_hba_enable(struct > > ufs_hba *hba) { > > int retry; > > + int ret = 0; > > > > /* > > * msleep of 1 and 5 used in this function might result in > > msleep(20), @@ -3117,6 +3149,9 @@ static int ufshcd_hba_enable(struct > > ufs_hba *hba) > > > > ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); > > > > + if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) > > + goto use_dme; > > + > > /* start controller initialization sequence */ > > ufshcd_hba_start(hba); > > > > @@ -3145,12 +3180,19 @@ static int ufshcd_hba_enable(struct ufs_hba > > *hba) > > msleep(5); > > } > > > > +use_dme: > > /* enable UIC related interrupts */ > > ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); > > > > + if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) { > > + ret = ufshcd_dme_reset(hba); > > + if (!ret) > > + ret = ufshcd_dme_enable(hba); > > + } > > + > > ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); > > > > - return 0; > > + return ret; > > } > > > > static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer) diff > > --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index > > c4abd76..6a96f24 100644 > > --- a/drivers/scsi/ufs/ufshcd.h > > +++ b/drivers/scsi/ufs/ufshcd.h > > @@ -496,6 +496,7 @@ struct ufs_hba { > > #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) > > #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) > > #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) > > + #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9) > > > > > > unsigned int quirks;/* Deviations from standard UFSHCI spec. > */ > > -- > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a > Linux Foundation Collaborative Project -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
James Bottomleywrites: > On Tue, 2016-11-08 at 18:57 -0600, Eric W. Biederman wrote: >> James Bottomley writes: >> >> > On Tue, 2016-11-08 at 13:13 -0600, Eric W. Biederman wrote: > [...] >> > > Is it really the dropping of the lock that is causing this? >> > > I don't see that when I read those traces. >> > >> > No, it's an ABBA lock inversion that causes this. The traces are >> > somewhat dense, but they say it here: >> > >> > Possible unsafe locking scenario: >> >CPU0CPU1 >> > >> > lock(s_active#336); >> >lock(>scan_mutex); >> >lock(s_active#336); >> > lock(>scan_mutex); >> > >> > *** DEADLOCK *** >> > >> > The detailed explanation of this is here: >> > >> > http://marc.info/?l=linux-scsi=147855187425596 >> > >> > The fix is ensuring that the CPU1 thread doesn't get into taking >> > s_active if CPU0 already has it using the KERNFS_SUICIDED/AL flag >> > as an indicator. >> >> So. The kernfs code does not look safe to have kernfs_remove_self >> and kernfs_remove_by_name_ns racing against each other I agree. >> >> The kernfs_remove_self path turns KERNFS_SUICIDAL into another >> blocking lock by another name, and without lockdep annotations so I >> don't know that it is safe. > > Yes ... the number of hand rolled locks in that code make it super hard > to follow. > >> The relevant bit from kernfs_remove_self is: >> >> if (!(kn->flags & KERNFS_SUICIDAL)) { >> kn->flags |= KERNFS_SUICIDAL; >> __kernfs_remove(kn); >> kn->flags |= KERNFS_SUICIDED; >> ret = true; >> } else { >> wait_queue_head_t *waitq = _root(kn) >> ->deactivate_waitq; >> DEFINE_WAIT(wait); >> >> while (true) { >> prepare_to_wait(waitq, , >> TASK_UNINTERRUPTIBLE); >> >> if ((kn->flags & KERNFS_SUICIDED) && >> atomic_read(>active) == >> KN_DEACTIVATED_BIAS) >> break; >> >> mutex_unlock(_mutex); >> schedule(); >> mutex_lock(_mutex); >> } >> finish_wait(waitq, ); >> WARN_ON_ONCE(!RB_EMPTY_NODE(>rb)); >> ret = false; >> } >> >> I am pretty certain that if you are going to make kernfs_remove_self >> and kernfs_remove_by_name_ns to be safe to race against each other, >> not just the KERNFS_SUICIDAL check, but the wait when KERNFS_SUICIDAL >> is set needs to be added ot kernfs_remove_by_name_ns. > > I don't think you can do that: waiting for SUICIDED would introduce > another potential lock entanglement. I'm reasonably happy that the > deactivation offset coupled with kernfs_drain in the non self remove > path means that the necessary cleanup is done when the directory itself > is removed. That seems to be a common pattern in all non-self > removes. But if we don't I am pretty certain there will be asynchrounous behavior in some cases that could potentially confuse userspace. Which is partly why I would like to kill kernfs_remove_self. >> And I suspect if you add the appropriate lockdep annotations to that >> mess you will find yourself in a similar but related ABBA deadlock. > > I can't prove the negative, but as long as there's no waiting on the > SUICIDED/AL flags in the non-self remove path, I believe we're safe > with the current patch. > >> Which is why I would like a simpler easier to understand mechanism if >> we can. > > I don't disagree: If you want to clean out the Augean Stables, I can > lend you the thigh length rubber boots and the gas mask. However, I > think that what we're currently proposing: a simple patch to make > device_remove_file_self() actually work for everyone, along with > stringent testing is the better approach. > > After all, if you look at > > commit ac0ece9174aca9aa895ce0accc54f1f8ff12d117 > Author: Tejun Heo > Date: Mon Feb 3 14:03:03 2014 -0500 > > scsi: use device_remove_file_self() instead of device_schedule_callback() > > You'll see Tejun added all this stuff just to remove the async callback > we originally had. Simply restoring the async callback back makes us > quite considerably worse off because the device_remove_file_self() > mechanism is in use elsewhere. We need either to fix it and move on or > junk it and go back to the original. I vote we junk it and go back to something that resembles the original. there are only about 5 other callers so this isn't that bad to do. Tejun's work clearly added this deadlock 2 and a half years ago, and it was nasty enough no one noticed until recently. Using task_work_add(current, ...) as I posted earlier let's us retain the synchronous property of the current API. While we debate the details I am happy to look at
Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock
On Tue, 2016-11-08 at 18:57 -0600, Eric W. Biederman wrote: > James Bottomleywrites: > > > On Tue, 2016-11-08 at 13:13 -0600, Eric W. Biederman wrote: [...] > > > Is it really the dropping of the lock that is causing this? > > > I don't see that when I read those traces. > > > > No, it's an ABBA lock inversion that causes this. The traces are > > somewhat dense, but they say it here: > > > > Possible unsafe locking scenario: > >CPU0CPU1 > > > > lock(s_active#336); > >lock(>scan_mutex); > >lock(s_active#336); > > lock(>scan_mutex); > > > > *** DEADLOCK *** > > > > The detailed explanation of this is here: > > > > http://marc.info/?l=linux-scsi=147855187425596 > > > > The fix is ensuring that the CPU1 thread doesn't get into taking > > s_active if CPU0 already has it using the KERNFS_SUICIDED/AL flag > > as an indicator. > > So. The kernfs code does not look safe to have kernfs_remove_self > and kernfs_remove_by_name_ns racing against each other I agree. > > The kernfs_remove_self path turns KERNFS_SUICIDAL into another > blocking lock by another name, and without lockdep annotations so I > don't know that it is safe. Yes ... the number of hand rolled locks in that code make it super hard to follow. > The relevant bit from kernfs_remove_self is: > > if (!(kn->flags & KERNFS_SUICIDAL)) { > kn->flags |= KERNFS_SUICIDAL; > __kernfs_remove(kn); > kn->flags |= KERNFS_SUICIDED; > ret = true; > } else { > wait_queue_head_t *waitq = _root(kn) > ->deactivate_waitq; > DEFINE_WAIT(wait); > > while (true) { > prepare_to_wait(waitq, , > TASK_UNINTERRUPTIBLE); > > if ((kn->flags & KERNFS_SUICIDED) && > atomic_read(>active) == > KN_DEACTIVATED_BIAS) > break; > > mutex_unlock(_mutex); > schedule(); > mutex_lock(_mutex); > } > finish_wait(waitq, ); > WARN_ON_ONCE(!RB_EMPTY_NODE(>rb)); > ret = false; > } > > I am pretty certain that if you are going to make kernfs_remove_self > and kernfs_remove_by_name_ns to be safe to race against each other, > not just the KERNFS_SUICIDAL check, but the wait when KERNFS_SUICIDAL > is set needs to be added ot kernfs_remove_by_name_ns. I don't think you can do that: waiting for SUICIDED would introduce another potential lock entanglement. I'm reasonably happy that the deactivation offset coupled with kernfs_drain in the non self remove path means that the necessary cleanup is done when the directory itself is removed. That seems to be a common pattern in all non-self removes. > And I suspect if you add the appropriate lockdep annotations to that > mess you will find yourself in a similar but related ABBA deadlock. I can't prove the negative, but as long as there's no waiting on the SUICIDED/AL flags in the non-self remove path, I believe we're safe with the current patch. > Which is why I would like a simpler easier to understand mechanism if > we can. I don't disagree: If you want to clean out the Augean Stables, I can lend you the thigh length rubber boots and the gas mask. However, I think that what we're currently proposing: a simple patch to make device_remove_file_self() actually work for everyone, along with stringent testing is the better approach. After all, if you look at commit ac0ece9174aca9aa895ce0accc54f1f8ff12d117 Author: Tejun Heo Date: Mon Feb 3 14:03:03 2014 -0500 scsi: use device_remove_file_self() instead of device_schedule_callback() You'll see Tejun added all this stuff just to remove the async callback we originally had. Simply restoring the async callback back makes us quite considerably worse off because the device_remove_file_self() mechanism is in use elsewhere. We need either to fix it and move on or junk it and go back to the original. James -- 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 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
> This patch uses the resource-managed to add the devfreq device. > This function will make it easy to handle the devfreq device. > > - struct devfreq *devm_devfreq_add_device(struct device *dev, > struct devfreq_dev_profile *profile, > const char *governor_name, > void *data); > Cc: Vinayak Holikatti> Cc: James E.J. Bottomley > Cc: Martin K. Petersen > Cc: linux-scsi@vger.kernel.org > Signed-off-by: Chanwoo Choi Acked-by: MyungJoo Ham > --- > drivers/scsi/ufs/ufshcd.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) >
Re: [PATCH] Avoid that SCSI device removal through sysfs triggers a deadlock
Bart Van Asschewrites: > On 11/08/2016 11:15 AM, Eric W. Biederman wrote: >> James Bottomley writes: >> >>> On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: On 11/08/2016 07:28 AM, James Bottomley wrote: > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: >> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c >> index cf4c636..44ec536 100644 >> --- a/fs/kernfs/dir.c >> +++ b/fs/kernfs/dir.c >> @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct >> kernfs_node >> *parent, const char *name, >> mutex_lock(_mutex); >> >> kn = kernfs_find_ns(parent, name, ns); >> -if (kn) >> +if (kn && !(kn->flags & KERNFS_SUICIDED)) > > Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that > kernfs_mutex is actually dropped half way through __kernfs_remove, > so KERNFS_SUICIDED is not set atomically with this mutex. Hello James, Sorry but what you wrote is not correct. >>> >>> I think you agree it is dropped. I don't need to add the bit about the >>> reacquisition because the race is mediated by the first acquisition not >>> the second one, if you mediate on KERNFS_SUICIDAL, you only need to >>> worry about this because the mediation is in the first acquisition. If >>> you mediate on KERNFS_SUICIDED, you need to explain that the final >>> thing that means the race can't happen is the unbreak in the sysfs >>> delete path re-acquiring s_active ... the explanation of what's going >>> on and why gets about 2x more complex. >> >> Is it really the dropping of the lock that is causing this? >> I don't see that when I read those traces. >> >> I am going to put my vote in for doing all of the self removal >> sem-asynchronously by using task_work_add, and killing >> device_remove_self, sysfs_remove_self, kernfs_remove_self. Using >> task_work_add remains synchronous with userspace so userspace should not >> care, and we get the benefit of not having two different variants of the >> same code racing with each other. >> >> It might take a little more work but will leave code that is much more >> maintainable in the long run. > > Hello Eric, > > I'm completely in favor of keeping code maintainable. But what's not clear to > me > is whether asynchronous I/O can be submitted to a sysfs attribute? If so, on > what context will task work queued through task_work_add() be executed if an > aio > write is used to write into a sysfs attribute? I don't believe so. I believe the filesystem has to opt into aio. In either case I don't believe we have to worry about aio because either it won't be supported or it will be a synchronous write to sysfs. > Additionally, can a process enter the exiting state after writing into the > sysfs > delete attribute started and before task_work_add() has been called? I'm > asking > this because in that case task_work_add() will fail. Not before task_work_add has been called because task_work_add is what will be called synchronously from the write function of the delete attribute. Furthermore task_work_run is guaranteed to be called before you return to userspace. It is remotely possible that someone sends the process a SIGKILL while the delete attribute is being written. In that case task_work_run should run before the task gets too deep into the exiting state. But the SIGKILL will not be processed until the write syscall finishes and the kernel returns to the edge of userspace. So I can't imagine anything that would make task_work_add be a poor choice in these circumstances. And basically that fix for just the scsi bits I think is as simple as: drivers/scsi/scsi_sysfs.c | 20 +--- include/scsi/scsi_device.h | 1 + 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 07349270535d..fee88cde7c12 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include @@ -705,13 +706,26 @@ store_rescan_field (struct device *dev, struct device_attribute *attr, } static DEVICE_ATTR(rescan, S_IWUSR, NULL, store_rescan_field); +static void sdev_remove_self(struct callback_head *cb) +{ + struct scsi_device *sdev = + container_of(cb, struct scsi_device, delete_work); + + scsi_remove_device(sdev); +} + static ssize_t sdev_store_delete(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) { - if (device_remove_file_self(dev, attr)) - scsi_remove_device(to_scsi_device(dev)); - return count; + struct scsi_device *sdev = to_scsi_device(dev); + ssize_t err; + + init_task_work(>delete_work, sdev_remove_self); + err = task_work_add(current, >delete_work, false); + if (!err) + err =
Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
On 11/08/2016 05:20 PM, Kashyap Desai wrote: -Original Message- From: Martin K. Petersen [mailto:martin.peter...@oracle.com] Sent: Wednesday, November 09, 2016 4:45 AM To: Jens Axboe Cc: linux-scsi; Kashyap Desai; Martin K. Petersen Subject: Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box "Jens" == Jens Axboewrites: Jens> I wasted half a day on this, thinking it was something in my Jens> 4.10 branches. But it turns out it is not, the regression is in Jens> mainline. Jens - Sorry for trouble. I did not validated this single patch. I validated complete patch set. Issue is improper MACRO usage MEGASAS_IS_LOGICAL, which gives incorrect check condition in qcmd Path. Below is proposed fix. diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 74c7b44..0d2625b 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -2236,7 +2236,7 @@ struct megasas_instance_template { }; #define MEGASAS_IS_LOGICAL(scp) \ - (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1 Ugh... So we're completing everything immediately. Martin - I validated whole series. Apologies for this. Please help me to know how to fix this ? Do I need to send only fix on top of latest commit (as posted above - MACRO definition) for this issue ? Send a fix on top of current -git asap. The current tree is completely broken for any megaraid user. -rc4 is no time to send in untested patches, especially not something that claims to fix a 9 year old bug and is marked for stable as well. -- Jens Axboe -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
James Bottomleywrites: > On Tue, 2016-11-08 at 13:13 -0600, Eric W. Biederman wrote: >> James Bottomley writes: >> >> > On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: >> > > On 11/08/2016 07:28 AM, James Bottomley wrote: >> > > > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: >> > > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c >> > > > > index cf4c636..44ec536 100644 >> > > > > --- a/fs/kernfs/dir.c >> > > > > +++ b/fs/kernfs/dir.c >> > > > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct >> > > > > kernfs_node >> > > > > *parent, const char *name, >> > > > > mutex_lock(_mutex); >> > > > > >> > > > > kn = kernfs_find_ns(parent, name, ns); >> > > > > -if (kn) >> > > > > +if (kn && !(kn->flags & KERNFS_SUICIDED)) >> > > > >> > > > Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is >> > > > that >> > > > kernfs_mutex is actually dropped half way through >> > > > __kernfs_remove, >> > > > so KERNFS_SUICIDED is not set atomically with this mutex. >> > > >> > > Hello James, >> > > >> > > Sorry but what you wrote is not correct. >> > >> > I think you agree it is dropped. I don't need to add the bit about >> > the reacquisition because the race is mediated by the first >> > acquisition not the second one, if you mediate on KERNFS_SUICIDAL, >> > you only need to worry about this because the mediation is in the >> > first acquisition. If you mediate on KERNFS_SUICIDED, you need to >> > explain that the final thing that means the race can't happen is >> > the unbreak in the sysfs delete path re-acquiring s_active ... the >> > explanation of what's going on and why gets about 2x more complex. >> >> Is it really the dropping of the lock that is causing this? >> I don't see that when I read those traces. > > No, it's an ABBA lock inversion that causes this. The traces are > somewhat dense, but they say it here: > > Possible unsafe locking scenario: >CPU0CPU1 > > lock(s_active#336); >lock(>scan_mutex); >lock(s_active#336); > lock(>scan_mutex); > > *** DEADLOCK *** > > The detailed explanation of this is here: > > http://marc.info/?l=linux-scsi=147855187425596 > > The fix is ensuring that the CPU1 thread doesn't get into taking > s_active if CPU0 already has it using the KERNFS_SUICIDED/AL flag as an > indicator. So. The kernfs code does not look safe to have kernfs_remove_self and kernfs_remove_by_name_ns racing against each other I agree. The kernfs_remove_self path turns KERNFS_SUICIDAL into another blocking lock by another name, and without lockdep annotations so I don't know that it is safe. The relevant bit from kernfs_remove_self is: if (!(kn->flags & KERNFS_SUICIDAL)) { kn->flags |= KERNFS_SUICIDAL; __kernfs_remove(kn); kn->flags |= KERNFS_SUICIDED; ret = true; } else { wait_queue_head_t *waitq = _root(kn)->deactivate_waitq; DEFINE_WAIT(wait); while (true) { prepare_to_wait(waitq, , TASK_UNINTERRUPTIBLE); if ((kn->flags & KERNFS_SUICIDED) && atomic_read(>active) == KN_DEACTIVATED_BIAS) break; mutex_unlock(_mutex); schedule(); mutex_lock(_mutex); } finish_wait(waitq, ); WARN_ON_ONCE(!RB_EMPTY_NODE(>rb)); ret = false; } I am pretty certain that if you are going to make kernfs_remove_self and kernfs_remove_by_name_ns to be safe to race against each other, not just the KERNFS_SUICIDAL check, but the wait when KERNFS_SUICIDAL is set needs to be added ot kernfs_remove_by_name_ns. And I suspect if you add the appropriate lockdep annotations to that mess you will find yourself in a similar but related ABBA deadlock. Which is why I would like a simpler easier to understand mechanism if we can. Eric -- 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: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Wednesday, November 09, 2016 4:45 AM > To: Jens Axboe > Cc: linux-scsi; Kashyap Desai; Martin K. Petersen > Subject: Re: [REGRESSION] 4.9-rc4+ doesn't boot on my test box > > > "Jens" == Jens Axboewrites: > > Jens> I wasted half a day on this, thinking it was something in my > Jens> 4.10 branches. But it turns out it is not, the regression is in > Jens> mainline. Jens - Sorry for trouble. I did not validated this single patch. I validated complete patch set. Issue is improper MACRO usage MEGASAS_IS_LOGICAL, which gives incorrect check condition in qcmd Path. Below is proposed fix. diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 74c7b44..0d2625b 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -2236,7 +2236,7 @@ struct megasas_instance_template { }; #define MEGASAS_IS_LOGICAL(scp) \ - (scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1 + ((scp->device->channel < MEGASAS_MAX_PD_CHANNELS) ? 0 : 1) #define MEGASAS_DEV_INDEX(scp) \ (((scp->device->channel % 2) * MEGASAS_MAX_DEV_PER_CHANNEL) + \ > > Kashyap, have you tested the stable fix without the remainder of the driver > update in place? Martin - I validated whole series. Apologies for this. Please help me to know how to fix this ? Do I need to send only fix on top of latest commit (as posted above - MACRO definition) for this issue ? > > -- > Martin K. PetersenOracle Linux Engineering -- 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 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
> "Chanwoo" == Chanwoo Choiwrites: Chanwoo> This patch uses the resource-managed to add the devfreq device. Chanwoo> This function will make it easy to handle the devfreq device. Applied to 4.10/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- 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 0/2] qla2xxx: fix errors in PCI device remove with ongoing I/O
> "Mauricio" == Mauricio Faria de Oliveira> writes: Mauricio> This patchset addresses a couple of errors that might happen Mauricio> during PCI device remove (e.g., PCI hotplug, PowerVM DLPAR), Mauricio> which prevent the successful removal and re-addition of the Mauricio> adapter to the system, and cause an oops and/or invalid DMA Mauricio> access (triggers an EEH event). Applied to 4.9/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering -- 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 4.9/scsi-fixes] libcxgbi: fix incorrect DDP resource cleanup
> "Varun" == Varun Prakashwrites: Varun> Before calling task_release_itt() task data is memset to zero Varun> because of which DDP context information is lost resulting in Varun> incorrect DDP resource cleanup, to fix this call Varun> task_release_itt() before memset. Applied to 4.9/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering -- 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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
> "Arun" == Arun Easiwrites: >> It's fine to post the patches split up to ease the review >> process. But whatever we commit must obviously be bisectable. Arun> If it is alright with you, we would like to have all of our Arun> initial patches for the driver (qedi) squashed as a single commit Arun> to the tree. We will ensure that this single combined commit Arun> compiles clean. That's fine with me. -- Martin K. Petersen Oracle Linux Engineering -- 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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
Martin, On Tue, 8 Nov 2016, 3:49pm -, Martin K. Petersen wrote: > > "Arun" == Arun Easiwrites: > > Arun, > > Arun> qedi is the new iSCSI driver, which we are trying to submit, for > Arun> our 41000 series CNA. This patch series were broken up into > Arun> logical blocks for review purpose, but were not made to compile > Arun> individually. It is our impression that this is acceptable for > Arun> SCSI and all the initial "qedi" patches will be squashed and > Arun> committed as a single commit. Please let us know if we are > Arun> mistaken, and if so, we will post another series with this taken > Arun> care of. > > It's fine to post the patches split up to ease the review process. But > whatever we commit must obviously be bisectable. > If it is alright with you, we would like to have all of our initial patches for the driver (qedi) squashed as a single commit to the tree. We will ensure that this single combined commit compiles clean. Regards, -Arun -- 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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
> "Arun" == Arun Easiwrites: Arun, Arun> qedi is the new iSCSI driver, which we are trying to submit, for Arun> our 41000 series CNA. This patch series were broken up into Arun> logical blocks for review purpose, but were not made to compile Arun> individually. It is our impression that this is acceptable for Arun> SCSI and all the initial "qedi" patches will be squashed and Arun> committed as a single commit. Please let us know if we are Arun> mistaken, and if so, we will post another series with this taken Arun> care of. It's fine to post the patches split up to ease the review process. But whatever we commit must obviously be bisectable. -- Martin K. Petersen Oracle Linux Engineering -- 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: Ordering problems with 3ware controller
> "Paul" == Paul Menzelwrites: Paul, Paul> Updating from Linux 4.4.X to Linux 4.8.4, we noticed that the Paul> 3ware devices under `/dev` – `/dev/twa0`, `/dev/twa1`, … – map to Paul> the controllers differently. Paul> This unfortunately breaks quite a lot of our scripts, as we depend Paul> on the fact that the first controller is also in the front. It's not the 3ware drivers since they have not been updated in a long time (since way before 4.4). Linux does not provide device discovery ordering guarantees. You need to fix your scripts to use UUIDs, filesystem labels, or DM devices to get stable naming. -- Martin K. Petersen Oracle Linux Engineering -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On Tue, 2016-11-08 at 13:13 -0600, Eric W. Biederman wrote: > James Bottomleywrites: > > > On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: > > > On 11/08/2016 07:28 AM, James Bottomley wrote: > > > > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: > > > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > > > > > index cf4c636..44ec536 100644 > > > > > --- a/fs/kernfs/dir.c > > > > > +++ b/fs/kernfs/dir.c > > > > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct > > > > > kernfs_node > > > > > *parent, const char *name, > > > > > mutex_lock(_mutex); > > > > > > > > > > kn = kernfs_find_ns(parent, name, ns); > > > > > - if (kn) > > > > > + if (kn && !(kn->flags & KERNFS_SUICIDED)) > > > > > > > > Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is > > > > that > > > > kernfs_mutex is actually dropped half way through > > > > __kernfs_remove, > > > > so KERNFS_SUICIDED is not set atomically with this mutex. > > > > > > Hello James, > > > > > > Sorry but what you wrote is not correct. > > > > I think you agree it is dropped. I don't need to add the bit about > > the reacquisition because the race is mediated by the first > > acquisition not the second one, if you mediate on KERNFS_SUICIDAL, > > you only need to worry about this because the mediation is in the > > first acquisition. If you mediate on KERNFS_SUICIDED, you need to > > explain that the final thing that means the race can't happen is > > the unbreak in the sysfs delete path re-acquiring s_active ... the > > explanation of what's going on and why gets about 2x more complex. > > Is it really the dropping of the lock that is causing this? > I don't see that when I read those traces. No, it's an ABBA lock inversion that causes this. The traces are somewhat dense, but they say it here: Possible unsafe locking scenario: CPU0CPU1 lock(s_active#336); lock(>scan_mutex); lock(s_active#336); lock(>scan_mutex); *** DEADLOCK *** The detailed explanation of this is here: http://marc.info/?l=linux-scsi=147855187425596 The fix is ensuring that the CPU1 thread doesn't get into taking s_active if CPU0 already has it using the KERNFS_SUICIDED/AL flag as an indicator. James -- 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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
[ Sending on behalf of Manish to cover for the time difference. ] Hi Martin, James, I would like to request your input on this kbuild test error on the series, where they compile fine together, but is not bisectable. qedi is the new iSCSI driver, which we are trying to submit, for our 41000 series CNA. This patch series were broken up into logical blocks for review purpose, but were not made to compile individually. It is our impression that this is acceptable for SCSI and all the initial "qedi" patches will be squashed and committed as a single commit. Please let us know if we are mistaken, and if so, we will post another series with this taken care of. FYI, this series accompany additions to the common core module, "qed", that goes under drivers/net/. The patches for the qed module compiles fine individually and so is bisectable. In regards to the additional warnings brought out by kbuild test on "PATCH v2 6/6" and "PATCH v2 3/6", we will post a v3 with the fixes. Regards, -Arun On Tue, 8 Nov 2016, 2:52am -, kbuild test robot wrote: > Hi Manish, > > [auto build test ERROR on net-next/master] > [also build test ERROR on v4.9-rc4] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 > config: ia64-allmodconfig (attached as .config) > compiler: ia64-linux-gcc (GCC) 6.2.0 > reproduce: > wget > https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross > -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=ia64 > > Note: the > linux-review/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 > HEAD dd4d1d0e0785d20cdcfdf9b2c792c564a79b2de2 builds fine. > It only hurts bisectibility. > -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On 11/08/2016 11:15 AM, Eric W. Biederman wrote: James Bottomleywrites: On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: On 11/08/2016 07:28 AM, James Bottomley wrote: On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index cf4c636..44ec536 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name, mutex_lock(_mutex); kn = kernfs_find_ns(parent, name, ns); - if (kn) + if (kn && !(kn->flags & KERNFS_SUICIDED)) Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that kernfs_mutex is actually dropped half way through __kernfs_remove, so KERNFS_SUICIDED is not set atomically with this mutex. Hello James, Sorry but what you wrote is not correct. I think you agree it is dropped. I don't need to add the bit about the reacquisition because the race is mediated by the first acquisition not the second one, if you mediate on KERNFS_SUICIDAL, you only need to worry about this because the mediation is in the first acquisition. If you mediate on KERNFS_SUICIDED, you need to explain that the final thing that means the race can't happen is the unbreak in the sysfs delete path re-acquiring s_active ... the explanation of what's going on and why gets about 2x more complex. Is it really the dropping of the lock that is causing this? I don't see that when I read those traces. I am going to put my vote in for doing all of the self removal sem-asynchronously by using task_work_add, and killing device_remove_self, sysfs_remove_self, kernfs_remove_self. Using task_work_add remains synchronous with userspace so userspace should not care, and we get the benefit of not having two different variants of the same code racing with each other. It might take a little more work but will leave code that is much more maintainable in the long run. Hello Eric, I'm completely in favor of keeping code maintainable. But what's not clear to me is whether asynchronous I/O can be submitted to a sysfs attribute? If so, on what context will task work queued through task_work_add() be executed if an aio write is used to write into a sysfs attribute? Additionally, can a process enter the exiting state after writing into the sysfs delete attribute started and before task_work_add() has been called? I'm asking this because in that case task_work_add() will fail. Thanks, Bart. -- 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: fix spelling mistake in error message
> "Colin" == Colin Kingwrites: Colin> From: Colin Ian King Trivial fix to Colin> spelling mistake "operatio" to "operation" in critical error Colin> message Applied to 4.10/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- 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: [REGRESSION] 4.9-rc4+ doesn't boot on my test box
> "Jens" == Jens Axboewrites: Jens> I wasted half a day on this, thinking it was something in my Jens> 4.10 branches. But it turns out it is not, the regression is in Jens> mainline. Kashyap, have you tested the stable fix without the remainder of the driver update in place? -- Martin K. Petersen Oracle Linux Engineering -- 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 v5 00/12] ufs-qcom: phy/hcd: Clean up qcom-ufs phy and ufs-qcom hcd
> "Vivek" == Vivek Gautamwrites: Vivek> Here's the rebased version of patches based on 4.10/scsi-queue Vivek> branch as requested. The patches can now be applied and Vivek> pulled-in. Thanks! Applied to 4.10/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- 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 8/8] megaraid_sas: driver version upgrade
> "Kashyap" == Kashyap Desaiwrites: Rebased on top of Linus' tree to get the JBOD SYNCHRONIZE CACHE fix pulled in and applied patches 5 through 8 to 4.10/scsi-queue. -- Martin K. Petersen Oracle Linux Engineering -- 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 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
On 2016-11-08 01:16, Chanwoo Choi wrote: This patch uses the resource-managed to add the devfreq device. This function will make it easy to handle the devfreq device. - struct devfreq *devm_devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile, const char *governor_name, void *data); Cc: Vinayak HolikattiCc: James E.J. Bottomley Cc: Martin K. Petersen Cc: linux-scsi@vger.kernel.org Signed-off-by: Chanwoo Choi --- drivers/scsi/ufs/ufshcd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e491c4bda32f..e8c5ba274830 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6250,8 +6250,6 @@ void ufshcd_remove(struct ufs_hba *hba) ufshcd_hba_stop(hba, true); ufshcd_exit_clk_gating(hba); - if (ufshcd_is_clkscaling_enabled(hba)) - devfreq_remove_device(hba->devfreq); ufshcd_hba_exit(hba); } EXPORT_SYMBOL_GPL(ufshcd_remove); @@ -6579,7 +6577,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) } if (ufshcd_is_clkscaling_enabled(hba)) { - hba->devfreq = devfreq_add_device(dev, _devfreq_profile, + hba->devfreq = devm_devfreq_add_device(dev, _devfreq_profile, "simple_ondemand", NULL); if (IS_ERR(hba->devfreq)) { dev_err(hba->dev, "Unable to register with devfreq %ld\n", LGTM. Reviewed-by: Subhash Jadavani -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: add a variety of definitions decribed in UFS spec
On 2016-11-08 00:00, Kiwoong Kim wrote: These things are defined to be used by some UFS Host controllers. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/mphy.h | 38 ++ drivers/scsi/ufs/ufshci.h | 14 +++--- drivers/scsi/ufs/unipro.h | 24 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 drivers/scsi/ufs/mphy.h diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new file mode 100644 index 000..c431f49 --- /dev/null +++ b/drivers/scsi/ufs/mphy.h @@ -0,0 +1,38 @@ +/* + * drivers/scsi/ufs/mphy.h + * + * Copyright (C) 2014 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#ifndef _MPHY_H_ +#define _MPHY_H_ Do we really need separate file for MPHY? May be we can have these accomodated in unipro.h itself? + +#define TX_HIBERN8TIME_CAP 0x0f +#define TX_MIN_ACTIVATE_TIME 0x33 + +#define RX_HS_G1_SYNC_LEN_CAP 0x8b +#define RX_HS_G1_PREP_LEN_CAP 0x8c +#define RX_HS_G2_SYNC_LEN_CAP 0x94 +#define RX_HS_G3_SYNC_LEN_CAP 0x95 +#define RX_HS_G2_PREP_LEN_CAP 0x96 +#define RX_HS_G3_PREP_LEN_CAP 0x97 + #define SYNC_RANGE_FINE (0 << 6) + #define SYNC_RANGE_COARSE (1 << 6) + #define SYNC_LEN(x) ((x) & 0x3f) + #define PREP_LEN(x) ((x) & 0xf) +#define RX_ADV_GRANULARITY_CAP 0x98 + #define RX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) +#define TX_ADV_GRANULARITY_CAP 0x10 + #define TX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) +#define RX_MIN_ACTIVATETIME_CAP0x8f +#define RX_HIBERN8TIME_CAP 0x92 +#define RX_ADV_HIBERN8TIME_CAP 0x99 +#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a + +#endif /* _MPHY_H_ */ + diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h index 9599741..26dc340 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -170,17 +170,25 @@ enum { /* UECDL - Host UIC Error Code Data Link Layer 3Ch */ #define UIC_DATA_LINK_LAYER_ERROR UFS_BIT(31) #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT 0x2000 -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001 -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002 +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0) +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT UFS_BIT(1) +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5) why don't we just add macros for all the bits in UECDL ? This makes it easy in future. +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT UFS_BIT(13) /* UECN - Host UIC Error Code Network Layer 40h */ #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31) #define UIC_NETWORK_LAYER_ERROR_CODE_MASK 0x7 +#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0) +#define UIC_NETWORK_BAD_DEVICEID_ENC BIT(1) +#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING BIT(2) /* UECT - Host UIC Error Code Transport Layer 44h */ #define UIC_TRANSPORT_LAYER_ERROR UFS_BIT(31) #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F +#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE BIT(0) +#define UIC_TRANSPORT_UNKNOWN_CPORTID BIT(1) +#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2) +#define UIC_TRANSPORT_BAD_TC BIT(4) May be add definition for bit-5 and 6 as well. /* UECDME - Host UIC Error Code DME 48h */ #define UIC_DME_ERROR UFS_BIT(31) diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h index eff8b56..e47e2c2 100644 --- a/drivers/scsi/ufs/unipro.h +++ b/drivers/scsi/ufs/unipro.h @@ -127,6 +127,7 @@ #define PA_PACPREQEOBTIMEOUT 0x1591 #define PA_HIBERN8TIME 0x15A7 #define PA_LOCALVERINFO0x15A9 +#define PA_GRANULARITY 0x15AA #define PA_TACTIVATE 0x15A8 #define PA_PACPFRAMECOUNT 0x15C0 #define PA_PACPERRORCOUNT 0x15C1 @@ -170,6 +171,9 @@ enum { UNCHANGED = 7, }; +#define IS_PWR_MODE_HS(m)(((m) == FAST_MODE) || ((m) == FASTAUTO_MODE)) s/IS_PWR_MODE_HS/IS_HS_PWR_MODE ? +#define IS_PWR_MODE_PWM(m) (((m) == SLOW_MODE) || ((m) == SLOWAUTO_MODE)) s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE ? + /* PA TX/RX Frequency Series */ enum { PA_HS_MODE_A= 1, @@ -231,6 +235,11 @@ enum ufs_unipro_ver { #define DL_PEERTC1PRESENT 0x2066 #define DL_PEERTC1RXINITCREVAL 0x2067 +/* Default value of L2 Timer */ +#define FC0PROTTIMEOUTVAL 8191 +#define TC0REPLAYTIMEOUTVAL65535 +#define AFC0REQTIMEOUTVAL 32767 + /* * Network Layer Attributes */ @@ -259,6 +268,21 @@ enum ufs_unipro_ver { #define T_TC0TXMAXSDUSIZE
Re: [PATCH v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk
On 2016-11-07 23:59, Kiwoong Kim wrote: If UFS driver resets interrupt aggregation timer and counter when there is a pending doorbell, an interrupt of IO completion of a corresponding task may be missed. That would cause a command timeout. If UFS driver resets interrupt aggregation timer and counter when there is a pending doorbell, a competion interrupt of a corresponing task may be issued. That would casue a command timeout. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 4 +++- drivers/scsi/ufs/ufshcd.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c904854..65bbf59 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3730,7 +3730,9 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba) * false interrupt if device completes another request after resetting * aggregation and before reading the DB. */ - if (ufshcd_is_intr_aggr_allowed(hba)) + if ((ufshcd_is_intr_aggr_allowed(hba)) + && !(hba->quirks & UFSHCI_QUIRK_SKIP_INTR_AGGR)) Why do we need this new quirk? If controller has the issue with interrupt aggregation, you can remove this UFSHCD_CAP_INTR_AGGR to disable the aggregation altogether. + ufshcd_reset_intr_aggr(hba); ufshcd_reset_intr_aggr(hba); tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 6a96f24..6a9c6e9 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -497,6 +497,7 @@ struct ufs_hba { #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9) + #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10) unsigned int quirks;/* Deviations from standard UFSHCI spec. */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: introduce UFSHCD_QUIRK_USE_OF_HCE quirk
On 2016-11-07 23:57, Kiwoong Kim wrote: Some host controller might not initialize itself by setting "Host Controller Enable" to 1. They should do this to reset UIC. I am not sure if i understood this statment. can you give more details? In such cases, 'DME reset' and 'DME enable' are required for normal subsequent operations. This means HCE implementation is broken, you should name the quirk as UFSHCD_QUIRK_BROKEN_HCE . Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 44 +++- drivers/scsi/ufs/ufshcd.h | 1 + 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 24d6ea7..c904854 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2496,6 +2496,37 @@ static inline void ufshcd_add_delay_before_dme_cmd(struct ufs_hba *hba) usleep_range(min_sleep_time_us, min_sleep_time_us + 50); } +static int ufshcd_dme_reset(struct ufs_hba *hba) +{ + struct uic_command uic_cmd = {0}; + int ret; + + uic_cmd.command = UIC_CMD_DME_RESET; + uic_cmd.argument1 = 0x1; + + ret = ufshcd_send_uic_cmd(hba, _cmd); + if (ret) + dev_err(hba->dev, + "dme-reset: error code %d\n", ret); This error message doesn't say which DME command failed, do you want to add that? + + return ret; +} + +static int ufshcd_dme_enable(struct ufs_hba *hba) +{ + struct uic_command uic_cmd = {0}; + int ret; + + uic_cmd.command = UIC_CMD_DME_ENABLE; + + ret = ufshcd_send_uic_cmd(hba, _cmd); + if (ret) + dev_err(hba->dev, + "dme-enable: error code %d\n", ret); This error message doesn't say which DME command failed, do you want to add that? + + return ret; +} + /** * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET * @hba: per adapter instance @@ -3101,6 +3132,7 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba, bool can_sleep) static int ufshcd_hba_enable(struct ufs_hba *hba) { int retry; + int ret = 0; /* * msleep of 1 and 5 used in this function might result in msleep(20), @@ -3117,6 +3149,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); + if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) + goto use_dme; + /* start controller initialization sequence */ ufshcd_hba_start(hba); @@ -3145,12 +3180,19 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) msleep(5); } +use_dme: /* enable UIC related interrupts */ ufshcd_enable_intr(hba, UFSHCD_UIC_MASK); + if (hba->quirks & UFSHCD_QUIRK_USE_OF_HCE) { + ret = ufshcd_dme_reset(hba); + if (!ret) + ret = ufshcd_dme_enable(hba); + } + ufshcd_vops_hce_enable_notify(hba, POST_CHANGE); - return 0; + return ret; } static int ufshcd_disable_tx_lcc(struct ufs_hba *hba, bool peer) diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index c4abd76..6a96f24 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -496,6 +496,7 @@ struct ufs_hba { #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) + #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9) unsigned int quirks;/* Deviations from standard UFSHCI spec. */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
On 2016-11-07 23:50, Kiwoong Kim wrote: Some UFS host controllers may clear a transfer request slot by setting an associated bit in UTLRCLR/UTMLRCLR to 1, not 0. s/UTLRCLR/UTRLCLR s/UTMLRCLR/UTMRLCLR That's opposite to what UFS spec decribes. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 32 drivers/scsi/ufs/ufshcd.h | 1 + 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 549e3e8..24d6ea7 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -392,7 +392,31 @@ static inline void ufshcd_put_tm_slot(struct ufs_hba *hba, int slot) */ static inline void ufshcd_utrl_clear(struct ufs_hba *hba, u32 pos) { - ufshcd_writel(hba, ~(1 << pos), REG_UTP_TRANSFER_REQ_LIST_CLEAR); + u32 clear; + + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR) + clear = (1 << pos); + else + clear = ~(1 << pos); + + ufshcd_writel(hba, clear, REG_UTP_TRANSFER_REQ_LIST_CLEAR); +} + +/** + * ufshcd_utmrl_clear - Clear a bit in UTRMLCLR register + * @hba: per adapter instance + * @pos: position of the bit to be cleared + */ +static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos) +{ + u32 clear; + + if (hba->quirks & UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR) + clear = (1 << pos); + else + clear = ~(1 << pos); + + ufshcd_writel(hba, clear, REG_UTP_TASK_REQ_LIST_CLEAR); } /** @@ -1147,7 +1171,7 @@ ufshcd_send_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) * * Returns 0 in case of success, non-zero value in case of failure */ -static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) +static int ufshcd_map_sg(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) unrelated change. Please remove it. { struct ufshcd_sg_entry *prd_table; struct scatterlist *sg; @@ -1529,7 +1553,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) ufshcd_comp_scsi_upiu(hba, lrbp); - err = ufshcd_map_sg(lrbp); + err = ufshcd_map_sg(hba, lrbp); unrelated change. Please remove it. if (err) { lrbp->cmd = NULL; clear_bit_unlock(tag, >lrb_in_use); @@ -4329,7 +4353,7 @@ static int ufshcd_clear_tm_cmd(struct ufs_hba *hba, int tag) goto out; spin_lock_irqsave(hba->host->host_lock, flags); - ufshcd_writel(hba, ~(1 << tag), REG_UTP_TASK_REQ_LIST_CLEAR); + ufshcd_utmrl_clear(hba, tag); spin_unlock_irqrestore(hba->host->host_lock, flags); /* poll for max. 1 sec to clear door bell register by h/w */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 565f005..c4abd76 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -495,6 +495,7 @@ struct ufs_hba { #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) unsigned int quirks;/* Deviations from standard UFSHCI spec. */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: introduce UFSHCD_QUIRK_BROKEN_DWORD_UTRD quirk
On 2016-11-07 23:49, Kiwoong Kim wrote: Some UFS host controllers may think granularitys of PRDT length and offset as bytes, not double words. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 24 +++- drivers/scsi/ufs/ufshcd.h | 2 ++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8e19631..549e3e8 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1161,8 +1161,13 @@ static int ufshcd_map_sg(struct ufshcd_lrb *lrbp) return sg_segments; if (sg_segments) { - lrbp->utr_descriptor_ptr->prd_table_length = - cpu_to_le16((u16) (sg_segments)); + if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) This might sound more specific: s/UFSHCD_QUIRK_BROKEN_DWORD_UTRD/UFSHCD_QUIRK_PRDT_BYTE_GRAN . + lrbp->utr_descriptor_ptr->prd_table_length = + cpu_to_le16((u16)(sg_segments * + sizeof(struct ufshcd_sg_entry))); + else + lrbp->utr_descriptor_ptr->prd_table_length = + cpu_to_le16((u16) (sg_segments)); prd_table = (struct ufshcd_sg_entry *)lrbp->ucd_prdt_ptr; @@ -2385,12 +2390,21 @@ static void ufshcd_host_memory_configure(struct ufs_hba *hba) cpu_to_le32(upper_32_bits(cmd_desc_element_addr)); /* Response upiu and prdt offset should be in double words */ - utrdlp[i].response_upiu_offset = + if (hba->quirks & UFSHCD_QUIRK_BROKEN_DWORD_UTRD) { + utrdlp[i].response_upiu_offset = + cpu_to_le16(response_offset); + utrdlp[i].prd_table_offset = + cpu_to_le16(prdt_offset); + utrdlp[i].response_upiu_length = + cpu_to_le16(ALIGNED_UPIU_SIZE); + } else { + utrdlp[i].response_upiu_offset = cpu_to_le16((response_offset >> 2)); - utrdlp[i].prd_table_offset = + utrdlp[i].prd_table_offset = cpu_to_le16((prdt_offset >> 2)); - utrdlp[i].response_upiu_length = + utrdlp[i].response_upiu_length = cpu_to_le16(ALIGNED_UPIU_SIZE >> 2); + } hba->lrb[i].utr_descriptor_ptr = (utrdlp + i); hba->lrb[i].ucd_req_ptr = diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 8cf3991..565f005 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -494,6 +494,8 @@ struct ufs_hba { #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) + #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) + This extra line space isn't needed. unsigned int quirks;/* Deviations from standard UFSHCI spec. */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH] scsi_devinfo: remove synchronous ALUA for NETAPP/RDAC devices
NetApp did confirm this is only required for ONTAP(LUN C-Mode). Cc: Martin GeorgeCc: Robert Stankey Cc: Steven Schremmer Cc: Sean Stewart Cc: Hannes Reinecke Cc: Christophe Varoqui Cc: James E.J. Bottomley Cc: Martin K. Petersen Cc: SCSI ML Cc: device-mapper development Signed-off-by: Xose Vazquez Perez --- drivers/scsi/scsi_devinfo.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/scsi_devinfo.c b/drivers/scsi/scsi_devinfo.c index 2464569..1fb7964 100644 --- a/drivers/scsi/scsi_devinfo.c +++ b/drivers/scsi/scsi_devinfo.c @@ -221,7 +221,6 @@ static struct { {"NEC", "PD-1 ODX654P", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"NEC", "iStorage", NULL, BLIST_REPORTLUN2}, {"NETAPP", "LUN C-Mode", NULL, BLIST_SYNC_ALUA}, - {"NETAPP", "INF-01-00", NULL, BLIST_SYNC_ALUA}, {"NRC", "MBR-7", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"NRC", "MBR-7.4", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, {"PIONEER", "CD-ROM DRM-600", NULL, BLIST_FORCELUN | BLIST_SINGLELUN}, -- 2.10.2 -- 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 v1] ufs: introcude UFSHCD_QUIRK_GET_VS_RESULT quirk
On 2016-11-07 23:48, Kiwoong Kim wrote: Some UFS host controllers may not report a result of UIC command completion properly. In such cases, they should get the result from UIC directly if their architectures support that. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 27 +++ drivers/scsi/ufs/ufshcd.h | 20 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index d4a5a9c..8e19631 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1011,7 +1011,10 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba *hba) */ static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba) { - return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7; + if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT) + return (u8)ufshcd_vops_get_vs_info(hba, VS_OP_03); + else + return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7; } /** @@ -1038,6 +1041,17 @@ ufshcd_dispatch_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) REG_UIC_COMMAND); } +static inline enum vs_opcode ufshcd_get_vs_opcode(struct uic_command *uic_cmd) +{ + if (uic_cmd->command == UIC_CMD_DME_LINK_STARTUP) + return VS_OP_00; + else if ((uic_cmd->command == UIC_CMD_DME_HIBER_ENTER)) + return VS_OP_01; + else if ((uic_cmd->command == UIC_CMD_DME_HIBER_EXIT)) + return VS_OP_02; + else + return VS_OP_INVALID; +} /** * ufshcd_wait_for_uic_cmd - Wait complectioin of UIC command * @hba: per adapter instance @@ -1051,11 +1065,16 @@ ufshcd_wait_for_uic_cmd(struct ufs_hba *hba, struct uic_command *uic_cmd) { int ret; unsigned long flags; + enum vs_opcode vs_op = ufshcd_get_vs_opcode(uic_cmd); Please move this after we check the quirk below. if (wait_for_completion_timeout(_cmd->done, - msecs_to_jiffies(UIC_CMD_TIMEOUT))) - ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT; - else + msecs_to_jiffies(UIC_CMD_TIMEOUT))) { + if (hba->quirks & UFSHCD_QUIRK_GET_VS_RESULT && + vs_op != VS_OP_INVALID) + ret = ufshcd_vops_get_vs_info(hba, vs_op); + else + ret = uic_cmd->argument2 & MASK_UIC_COMMAND_RESULT; + } else ret = -ETIMEDOUT; spin_lock_irqsave(hba->host->host_lock, flags); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 13504b4..8cf3991 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -245,6 +245,14 @@ struct ufs_pwr_mode_info { struct ufs_pa_layer_attr info; }; +enum vs_opcode { + VS_OP_00 = 0x00, + VS_OP_01, + VS_OP_02, + VS_OP_03, + VS_OP_INVALID = 0xFF, +}; + How do we interpret these codes? and how is this useful for non-exynos UFS host controller? Please make it generic which can be used by other controllers in future (if required). /** * struct ufs_hba_variant_ops - variant specific callbacks * @name: variant name @@ -297,6 +305,7 @@ struct ufs_hba_variant_ops { int (*resume)(struct ufs_hba *, enum ufs_pm_op); void(*dbg_register_dump)(struct ufs_hba *hba); int (*phy_initialization)(struct ufs_hba *); + int (*get_vs_info)(struct ufs_hba *hba, enum vs_opcode); }; /* clock gating state */ @@ -484,6 +493,8 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) + #define UFSHCD_QUIRK_GET_VS_RESULT UFS_BIT(6) Name is weird, can we name it to reflect the actual purpose of this quirk? something like UFSHCD_QUIRK_BROKEN_UIC_COMPL ? + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device spec. */ @@ -853,4 +864,13 @@ static inline void ufshcd_vops_dbg_register_dump(struct ufs_hba *hba) hba->vops->dbg_register_dump(hba); } +static inline int ufshcd_vops_get_vs_info(struct ufs_hba *hba, You may name it as *get_uic_coml_info() ? + enum vs_opcode op) +{ + if (hba->vops && hba->vops->get_vs_info) + return hba->vops->get_vs_info(hba, op); + + return -ENOTSUPP; +} + #endif /* End of Header */ -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
James Bottomleywrites: > On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: >> On 11/08/2016 07:28 AM, James Bottomley wrote: >> > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: >> > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c >> > > index cf4c636..44ec536 100644 >> > > --- a/fs/kernfs/dir.c >> > > +++ b/fs/kernfs/dir.c >> > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct >> > > kernfs_node >> > > *parent, const char *name, >> > > mutex_lock(_mutex); >> > > >> > > kn = kernfs_find_ns(parent, name, ns); >> > > -if (kn) >> > > +if (kn && !(kn->flags & KERNFS_SUICIDED)) >> > >> > Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that >> > kernfs_mutex is actually dropped half way through __kernfs_remove, >> > so KERNFS_SUICIDED is not set atomically with this mutex. >> >> Hello James, >> >> Sorry but what you wrote is not correct. > > I think you agree it is dropped. I don't need to add the bit about the > reacquisition because the race is mediated by the first acquisition not > the second one, if you mediate on KERNFS_SUICIDAL, you only need to > worry about this because the mediation is in the first acquisition. If > you mediate on KERNFS_SUICIDED, you need to explain that the final > thing that means the race can't happen is the unbreak in the sysfs > delete path re-acquiring s_active ... the explanation of what's going > on and why gets about 2x more complex. Is it really the dropping of the lock that is causing this? I don't see that when I read those traces. I am going to put my vote in for doing all of the self removal sem-asynchronously by using task_work_add, and killing device_remove_self, sysfs_remove_self, kernfs_remove_self. Using task_work_add remains synchronous with userspace so userspace should not care, and we get the benefit of not having two different variants of the same code racing with each other. It might take a little more work but will leave code that is much more maintainable in the long run. Eric -- 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 2/4] qla2xxx: Fix mailbox command timeout due to starvation
Hi Ewan, On 11/7/16, 7:53 AM, "Ewan D. Milne"wrote: >On Fri, 2016-11-04 at 09:33 -0700, himanshu.madh...@cavium.com wrote: >... >> @@ -2349,6 +2349,17 @@ uint32_t qla2x00_isp_reg_stat(struct qla_hw_data *ha) >> return atomic_read(>loop_state) == LOOP_READY; >> } >> >> +static void qla2x00_destroy_mbx_wq(struct qla_hw_data *ha) >> +{ >> +struct workqueue_struct *wq = ha->mbx_wq; >> + >> +if (wq) { >> +ha->mbx_wq = NULL; >> +flush_workqueue(wq); >> +destroy_workqueue(wq); >> +} >> +} >> + >> /* >> * PCI driver interface >> */ > >There is already a function qla2x00_destroy_deferred_work() that >destroys 3 other workqueues. > >... > >> @@ -3059,6 +3079,8 @@ uint32_t qla2x00_isp_reg_stat(struct qla_hw_data *ha) >> >> qla2x00_free_fw_dump(ha); >> >> +qla2x00_destroy_mbx_wq(ha); >> + >> pci_disable_pcie_error_reporting(pdev); >> pci_disable_device(pdev); >> } > >This code path (pci_driver->shutdown) does not appear to destroy the >other workqueues created by the driver. ??? > >> @@ -5011,6 +5033,8 @@ void qla2x00_relogin(struct scsi_qla_host *vha) >> */ >> qla2x00_free_sysfs_attr(base_vha, false); >> >> +qla2x00_destroy_mbx_wq(ha); >> + >> fc_remove_host(base_vha->host); >> >> scsi_remove_host(base_vha->host); > >See above. Ack. will fix up patch to address these comments. > >-Ewan Thanks, Himanshu
Re: [PATCH 3/4] qla2xxx: Add Block Multi Queue functionality.
Hi Ewan, On 11/7/16, 8:43 AM, "Ewan D. Milne"wrote: >On Fri, 2016-11-04 at 09:33 -0700, himanshu.madh...@cavium.com wrote: >> From: Michael Hernandez >> >> Tell the SCSI layer how many hardware queues we have based on the >> number of max queue pairs created. The number of max queue pairs >> created will depend on number of MSI X vector count or number of CPU's >> in a system. >> >> This feature can be turned on via CONFIG_SCSI_MQ_DEFAULT or passing >> scsi_mod.use_blk_mq=Y as a parameter to the kernel >> Queue pair creation depend on module parameter "ql2xmqsupport", which >> need to be enabled to create queue pair. >> > >I don't understand this change at all. Setting ->nr_hw_queues causes >the block layer to allocate a number of queues for that Scsi_Host >object, but it does not appear as if this code uses that functionality. >There is nothing in the patch that examines the tag to see which queue >the request came in on, in order to map it to a hardware queue. > >Instead, this patch seems to be reworking the mechanism involved in >NPIV vport creation, which creates an entirely separate Scsi_Host >object. The driver was already creating separate request queues to >the card for vports, so what does this have to do with Block-MQ? Thanks for the review comments. We are reworking patch series to address your review comments. > >-Ewan > > Thanks, Himanshu N�r��yb�X��ǧv�^�){.n�+{���"�{ay�ʇڙ�,j��f���h���z��w��� ���j:+v���w�j�mzZ+�ݢj"��!�i
Re: [PATCH v1] ufs: introduce setup_hibern8 callback
On 2016-11-07 23:48, Kiwoong Kim wrote: Some UFS host controller may need to configure some things around hibern8 enter/exit Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 10 -- drivers/scsi/ufs/ufshcd.h | 10 ++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index fdb0502..d4a5a9c 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2697,6 +2697,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba) int ret; struct uic_command uic_cmd = {0}; + ufshcd_vops_setup_hibern8(hba, true, PRE_CHANGE); + uic_cmd.command = UIC_CMD_DME_HIBER_ENTER; ret = ufshcd_uic_pwr_ctrl(hba, _cmd); @@ -2710,7 +2712,8 @@ static int __ufshcd_uic_hibern8_enter(struct ufs_hba *hba) */ if (ufshcd_link_recovery(hba)) ret = -ENOLINK; - } + } else + ufshcd_vops_setup_hibern8(hba, true, POST_CHANGE); return ret; } @@ -2733,13 +2736,16 @@ static int ufshcd_uic_hibern8_exit(struct ufs_hba *hba) struct uic_command uic_cmd = {0}; int ret; + ufshcd_vops_setup_hibern8(hba, false, PRE_CHANGE); + uic_cmd.command = UIC_CMD_DME_HIBER_EXIT; ret = ufshcd_uic_pwr_ctrl(hba, _cmd); if (ret) { dev_err(hba->dev, "%s: hibern8 exit failed. ret = %d\n", __func__, ret); ret = ufshcd_link_recovery(hba); - } + } else + ufshcd_vops_setup_hibern8(hba, false, POST_CHANGE); return ret; } diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index b084d89..13504b4 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -265,6 +265,8 @@ struct ufs_pwr_mode_info { * to set some things * @setup_task_mgmt: called before any task management request is issued * to set some things + * @setup_hibern8: called around hibern8 enter/exit + * to configure some things * @suspend: called during host controller PM callback * @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information @@ -290,6 +292,7 @@ struct ufs_hba_variant_ops { struct ufs_pa_layer_attr *); void(*setup_xfer_req)(struct ufs_hba *, int, bool); void(*setup_task_mgmt)(struct ufs_hba *, int, u8); + void(*setup_hibern8)(struct ufs_hba *, bool, bool); Can we change the name to "hibern8_notify" ? You may check other ufs_hba_variant_ops for reference. int (*suspend)(struct ufs_hba *, enum ufs_pm_op); int (*resume)(struct ufs_hba *, enum ufs_pm_op); void(*dbg_register_dump)(struct ufs_hba *hba); @@ -821,6 +824,13 @@ static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba, return hba->vops->setup_task_mgmt(hba, tag, tm_function); } +static inline void ufshcd_vops_setup_hibern8(struct ufs_hba *hba, + bool enter, bool notify) Using bool to specify whether it is hibern8 enter or hibern8 exit doesn't seem to be readable. May be you can pass the UIC_CMD_DME_HIBER_ENTER or UIC_CMD_DME_HIBER_EXIT (in other words use "enum uic_cmd_dme" type). also "notify" type should be changed from "bool" to "enum ufs_notify_change_status". +{ + if (hba->vops && hba->vops->setup_hibern8) + return hba->vops->setup_hibern8(hba, enter, notify); +} + static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op) { if (hba->vops && hba->vops->suspend) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: introduce setup_task_mgmt
On 2016-11-07 23:47, Kiwoong Kim wrote: Some UFS host controller may need to configure some things before any task management request is issued Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 1 + drivers/scsi/ufs/ufshcd.h | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index bf78321..fdb0502 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id, task_req_upiup->input_param2 = cpu_to_be32(task_id); /* send command to the controller */ May be you need to move this comment just above __set_bit() call. Rest all looks good in this patch. + ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function); __set_bit(free_slot, >outstanding_tasks); /* Make sure descriptors are ready before ringing the task doorbell */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index f2a69c0..b084d89 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -263,6 +263,8 @@ struct ufs_pwr_mode_info { * to be set. * @setup_xfer_req: called before any transfer request is issued * to set some things + * @setup_task_mgmt: called before any task management request is issued + * to set some things * @suspend: called during host controller PM callback * @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information @@ -287,6 +289,7 @@ struct ufs_hba_variant_ops { struct ufs_pa_layer_attr *, struct ufs_pa_layer_attr *); void(*setup_xfer_req)(struct ufs_hba *, int, bool); + void(*setup_task_mgmt)(struct ufs_hba *, int, u8); int (*suspend)(struct ufs_hba *, enum ufs_pm_op); int (*resume)(struct ufs_hba *, enum ufs_pm_op); void(*dbg_register_dump)(struct ufs_hba *hba); @@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag, return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null); } +static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba, + int tag, u8 tm_function) +{ + if (hba->vops && hba->vops->setup_task_mgmt) + return hba->vops->setup_task_mgmt(hba, tag, tm_function); +} + static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op) { if (hba->vops && hba->vops->suspend) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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 v1] ufs: introduce setup_xfer_req callback
On 2016-11-07 23:45, Kiwoong Kim wrote: Some UFS host controller may need to configure some things before any transfer request is issued. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 2 ++ drivers/scsi/ufs/ufshcd.h | 10 ++ 2 files changed, 12 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8cf5d8f..bf78321 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1516,6 +1516,7 @@ 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_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false)); ufshcd_send_command(hba, tag); out_unlock: spin_unlock_irqrestore(hba->host->host_lock, flags); @@ -1727,6 +1728,7 @@ static int ufshcd_exec_dev_cmd(struct ufs_hba *hba, /* Make sure descriptors are ready before ringing the doorbell */ wmb(); spin_lock_irqsave(hba->host->host_lock, flags); + ufshcd_vops_setup_xfer_req(hba, tag, (lrbp->cmd ? true : false)); ufshcd_send_command(hba, tag); spin_unlock_irqrestore(hba->host->host_lock, flags); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index afff7f4..f2a69c0 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -261,6 +261,8 @@ struct ufs_pwr_mode_info { * @pwr_change_notify: called before and after a power mode change * is carried out to allow vendor spesific capabilities * to be set. + * @setup_xfer_req: called before any transfer request is issued + * to set some things * @suspend: called during host controller PM callback * @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information @@ -284,6 +286,7 @@ struct ufs_hba_variant_ops { enum ufs_notify_change_status status, struct ufs_pa_layer_attr *, struct ufs_pa_layer_attr *); + void(*setup_xfer_req)(struct ufs_hba *, int, bool); int (*suspend)(struct ufs_hba *, enum ufs_pm_op); int (*resume)(struct ufs_hba *, enum ufs_pm_op); void(*dbg_register_dump)(struct ufs_hba *hba); @@ -801,6 +804,13 @@ static inline int ufshcd_vops_pwr_change_notify(struct ufs_hba *hba, return -ENOTSUPP; } +static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag, + bool is_cmd_not_null) This might be more readable: s/is_cmd_not_null/is_scsi_cmd , rest looks good in this patch. +{ + if (hba->vops && hba->vops->setup_xfer_req) + return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null); +} + static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op) { if (hba->vops && hba->vops->suspend) -- The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On Tue, 2016-11-08 at 08:52 -0800, Bart Van Assche wrote: > On 11/08/2016 07:28 AM, James Bottomley wrote: > > On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > > > index cf4c636..44ec536 100644 > > > --- a/fs/kernfs/dir.c > > > +++ b/fs/kernfs/dir.c > > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct > > > kernfs_node > > > *parent, const char *name, > > > mutex_lock(_mutex); > > > > > > kn = kernfs_find_ns(parent, name, ns); > > > - if (kn) > > > + if (kn && !(kn->flags & KERNFS_SUICIDED)) > > > > Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that > > kernfs_mutex is actually dropped half way through __kernfs_remove, > > so KERNFS_SUICIDED is not set atomically with this mutex. > > Hello James, > > Sorry but what you wrote is not correct. I think you agree it is dropped. I don't need to add the bit about the reacquisition because the race is mediated by the first acquisition not the second one, if you mediate on KERNFS_SUICIDAL, you only need to worry about this because the mediation is in the first acquisition. If you mediate on KERNFS_SUICIDED, you need to explain that the final thing that means the race can't happen is the unbreak in the sysfs delete path re-acquiring s_active ... the explanation of what's going on and why gets about 2x more complex. James > __kernfs_remove() calls kernfs_drain(). That last function not only > drops but also reacquires kernfs_mutex. So both KERNFS_SUICIDAL and > KERNFS_SUICIDED are set while holding kernfs_mutex. -- 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 0/2] qla2xxx: fix errors in PCI device remove with ongoing I/O
On 11/7/16, 11:53 AM, "Mauricio Faria de Oliveira"wrote: >This patchset addresses a couple of errors that might happen during >PCI device remove (e.g., PCI hotplug, PowerVM DLPAR), which prevent >the successful removal and re-addition of the adapter to the system, >and cause an oops and/or invalid DMA access (triggers an EEH event). > >It allowed several cycles of PCI device add/remove with ongoing I/O, >to complete successfully without triggering oopses or EEH events. > >Verified on v4.9-rc3. > >Test-case: >--- ># lspci ><...> >001d:70:00.0 Fibre Channel: QLogic Corp. ISP2532-based ... >001d:70:00.1 Fibre Channel: QLogic Corp. ISP2532-based ... ><...> > ># for sd in $(find /sys/bus/pci/devices/001d:70:00.*/ \ > -name 'sd*' -printf "%f\n"); do \ >dd if=/dev/$sd of=/dev/null iflag=nocache & done > ># echo 1 | tee /sys/bus/pci/devices/001d:70:00.*/remove >(this either works or not) > ># echo 1 > /sys/bus/pci/rescan > >Before: >--- ><...> >EEH: Frozen PHB#1d-PE#70 detected >qla2xxx [001d:70:00.1]-8042:2: PCI/Register disconnect, exiting. ><...> >EEH: Detected PCI bus error on PHB#29-PE#70 ><...> >(and/or) >Unable to handle kernel paging request for data at address 0x0138 ><...> >NIP [d4700a40] qla2xxx_queuecommand+0x80/0x3f0 [qla2xxx] >LR [d4700a10] qla2xxx_queuecommand+0x50/0x3f0 [qla2xxx] > >(command does not return; adapter cannot be re-added) > >After: >--- ><...> >qla2xxx [001d:70:00.0]-801c:1: Abort command issued nexus=1:0:0 -- 1 2003. ><...> >qla2xxx [001d:70:00.1]-801c:2: Abort command issued nexus=2:3:0 -- 1 2003. ><...> > >(command does return; adapter can be re-added correctly) > > >Mauricio Faria de Oliveira (2): > qla2xxx: do not queue commands when unloading > qla2xxx: fix invalid DMA access after command aborts in PCI device >remove > > drivers/scsi/qla2xxx/qla_os.c | 14 ++ > 1 file changed, 14 insertions(+) > >-- >1.8.3.1 > Thanks for the patches. Series Looks Good. Acked-by: Himanshu Madhani >
[REGRESSION] 4.9-rc4+ doesn't boot on my test box
Hi, I wasted half a day on this, thinking it was something in my 4.10 branches. But it turns out it is not, the regression is in mainline. Looking at the recent fixes, turns out to be this one: commit 1e793f6fc0db920400574211c48f9157a37e3945 Author: Kashyap DesaiDate: Fri Oct 21 06:33:32 2016 -0700 scsi: megaraid_sas: Fix data integrity failure for JBOD (passthrough) devices If I revert that, box boots fine again. The effect of the regression is that my box detects tons of SCSI devices (literally, thousands): [ 11.675834] sd 0:2:0:0: [sda] Sector size 0 reported, assuming 512. [ 11.675840] sd 0:2:0:0: Attached scsi generic sg0 type 0 [ 11.676096] sd 0:2:1:0: Attached scsi generic sg1 type 0 [ 11.676099] sd 0:2:1:0: [sdb] Sector size 0 reported, assuming 512. [ 11.676105] sd 0:2:1:0: [sdb] 1 512-byte logical blocks: (512 B/512 B) [ 11.676107] sd 0:2:1:0: [sdb] 0-byte physical blocks [ 11.676132] sd 0:2:1:0: [sdb] Write Protect is off [ 11.676170] sd 0:2:1:0: [sdb] Asking for cache data failed [ 11.676173] sd 0:2:1:0: [sdb] Assuming drive cache: write through [ 11.676410] sd 0:2:2:0: Attached scsi generic sg2 type 0 [ 11.676432] sd 0:2:2:0: [sdc] Sector size 0 reported, assuming 512. [ 11.676435] sd 0:2:2:0: [sdc] 1 512-byte logical blocks: (512 B/512 B) [ 11.676438] sd 0:2:2:0: [sdc] 0-byte physical blocks [ 11.676442] sd 0:2:1:0: [sdb] Sector size 0 reported, assuming 512. [ 11.676454] sd 0:2:2:0: [sdc] Write Protect is off [ 11.676476] sd 0:2:2:0: [sdc] Asking for cache data failed [ 11.676478] sd 0:2:2:0: [sdc] Assuming drive cache: write through [...] [ 11.717641] sd 0:3:63:0: Attached scsi generic sg127 type 0 [ 11.717757] sd 0:3:62:0: [sddw] Sector size 0 reported, assuming 512. [ 11.717801] sd 0:3:63:0: [sddx] Sector size 0 reported, assuming 512. [ 11.717802] sd 0:3:62:0: [sddw] Attached SCSI disk [ 11.718011] sd 0:3:63:0: [sddx] Sector size 0 reported, assuming 512. where a normal boots just detects sda and the CD-ROM: [ 10.398120] sd 0:2:0:0: Attached scsi generic sg0 type 0 [ 10.398369] sd 0:2:0:0: [sda] 975699968 512-byte logical blocks: (500 GB/465 GiB) [ 10.398684] sd 0:2:0:0: [sda] Write Protect is off [ 10.398872] sd 0:2:0:0: [sda] Write cache: disabled, read cache: enabled, supports DPO and FUA [ 10.435084] sda: sda1 sda2 < sda5 sda6 > sda3 [ 10.437522] scsi 5:0:0:0: CD-ROMTEAC DVD-ROM DV-28SW R.2B PQ: 0 ANSI: 5 [ 10.456582] sd 0:2:0:0: [sda] Attached SCSI disk [ 10.500499] sr 5:0:0:0: [sr0] scsi3-mmc drive: 24x/24x cd/rw xa/form2 cdda tray [ 10.508830] cdrom: Uniform CD-ROM driver Revision: 3.20 [ 10.515186] sr 5:0:0:0: Attached scsi generic sg1 type 5 -- Jens Axboe -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On 11/08/2016 07:28 AM, James Bottomley wrote: On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index cf4c636..44ec536 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct kernfs_node *parent, const char *name, mutex_lock(_mutex); kn = kernfs_find_ns(parent, name, ns); - if (kn) + if (kn && !(kn->flags & KERNFS_SUICIDED)) Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that kernfs_mutex is actually dropped half way through __kernfs_remove, so KERNFS_SUICIDED is not set atomically with this mutex. Hello James, Sorry but what you wrote is not correct. __kernfs_remove() calls kernfs_drain(). That last function not only drops but also reacquires kernfs_mutex. So both KERNFS_SUICIDAL and KERNFS_SUICIDED are set while holding kernfs_mutex. Bart. -- 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
[Bug 187231] kernel panic during hpsa MSI plus tg3 MSI
https://bugzilla.kernel.org/show_bug.cgi?id=187231 --- Comment #4 from Patrick Schaaf--- That problematic box, which showed the kernel panic with 4.8.6, and the resetting/reset-up-to-20-seconds pauses several times a day with both 4.8 and 4.4.x, has now been running on 3.14.79 (with the same kvm load as before), for 30 hours, without any such HPSA resetting symptoms, or untoward pauses in the VMs that I could otherwise notice in monitoring. So somehow 3.14 does not trigger these episodes, or so it seems. -- You are receiving this mail because: You are the assignee for the bug. -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On Tue, 2016-11-08 at 08:01 +0100, Greg KH wrote: > On Mon, Nov 07, 2016 at 04:32:18PM -0800, Bart Van Assche wrote: > > The SCSI core holds scan_mutex around SCSI device addition and > > removal operations. sysfs serializes attribute read and write > > operations against attribute removal through s_active. Avoid that > > grabbing scan_mutex during self-removal of a SCSI device triggers > > a deadlock by not calling __kernfs_remove() from > > kernfs_remove_by_name_ns() in case of self-removal. This patch > > avoids that self-removal triggers the following deadlock: > > > > === > > [ INFO: possible circular locking dependency detected ] > > 4.9.0-rc1-dbg+ #4 Not tainted > > --- > > test_02_sdev_de/12586 is trying to acquire lock: > > (>scan_mutex){+.+.+.}, at: [] > > scsi_remove_device+0x1e/0x40 > > but task is already holding lock: > > (s_active#336){.+}, at: [] > > kernfs_remove_self+0xde/0x140 > > which lock already depends on the new lock. > > > > the existing dependency chain (in reverse order) is: > > -> #1 (s_active#336){.+}: > > [] lock_acquire+0xe9/0x1d0 > > [] __kernfs_remove+0x24a/0x310 > > [] kernfs_remove_by_name_ns+0x40/0x90 > > [] remove_files.isra.1+0x30/0x70 > > [] sysfs_remove_group+0x3f/0x90 > > [] sysfs_remove_groups+0x29/0x40 > > [] device_remove_attrs+0x59/0x80 > > [] device_del+0x125/0x240 > > [] __scsi_remove_device+0x143/0x180 > > [] scsi_forget_host+0x64/0x70 > > [] scsi_remove_host+0x75/0x120 > > [] 0xa035dbbb > > [] process_one_work+0x1f5/0x690 > > [] worker_thread+0x49/0x490 > > [] kthread+0xeb/0x110 > > [] ret_from_fork+0x27/0x40 > > > > -> #0 (>scan_mutex){+.+.+.}: > > [] __lock_acquire+0x10fc/0x1270 > > [] lock_acquire+0xe9/0x1d0 > > [] mutex_lock_nested+0x5f/0x360 > > [] scsi_remove_device+0x1e/0x40 > > [] sdev_store_delete+0x22/0x30 > > [] dev_attr_store+0x13/0x20 > > [] sysfs_kf_write+0x40/0x50 > > [] kernfs_fop_write+0x137/0x1c0 > > [] __vfs_write+0x23/0x140 > > [] vfs_write+0xb0/0x190 > > [] SyS_write+0x44/0xa0 > > [] entry_SYSCALL_64_fastpath+0x18/0xad > > > > other info that might help us debug this: > > > > Possible unsafe locking scenario: > >CPU0CPU1 > > > > lock(s_active#336); > >lock(>scan_mutex); > >lock(s_active#336); > > lock(>scan_mutex); > > > > *** DEADLOCK *** > > 3 locks held by test_02_sdev_de/12586: > > #0: (sb_writers#4){.+.+.+}, at: [] > > vfs_write+0x178/0x190 > > #1: (>mutex){+.+.+.}, at: [] > > kernfs_fop_write+0x101/0x1c0 > > #2: (s_active#336){.+}, at: [] > > kernfs_remove_self+0xde/0x140 > > > > stack backtrace: > > CPU: 4 PID: 12586 Comm: test_02_sdev_de Not tainted 4.9.0-rc1-dbg+ > > #4 > > Call Trace: > > [] dump_stack+0x68/0x93 > > [] print_circular_bug+0x1be/0x210 > > [] __lock_acquire+0x10fc/0x1270 > > [] lock_acquire+0xe9/0x1d0 > > [] mutex_lock_nested+0x5f/0x360 > > [] scsi_remove_device+0x1e/0x40 > > [] sdev_store_delete+0x22/0x30 > > [] dev_attr_store+0x13/0x20 > > [] sysfs_kf_write+0x40/0x50 > > [] kernfs_fop_write+0x137/0x1c0 > > [] __vfs_write+0x23/0x140 > > [] vfs_write+0xb0/0x190 > > [] SyS_write+0x44/0xa0 > > [] entry_SYSCALL_64_fastpath+0x18/0xad > > > > References: http://www.spinics.net/lists/linux-scsi/msg86551.html > > Signed-off-by: Bart Van Assche> > Cc: Greg Kroah-Hartman > > Cc: Eric Biederman > > Cc: Hannes Reinecke > > Cc: Johannes Thumshirn > > Cc: Sagi Grimberg > > Cc: > > --- > > fs/kernfs/dir.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > > index cf4c636..44ec536 100644 > > --- a/fs/kernfs/dir.c > > +++ b/fs/kernfs/dir.c > > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct > > kernfs_node *parent, const char *name, > > mutex_lock(_mutex); > > > > kn = kernfs_find_ns(parent, name, ns); > > - if (kn) > > + if (kn && !(kn->flags & KERNFS_SUICIDED)) > > __kernfs_remove(kn); > > Really? What changed recently to require this? I thought we fixed > these issues a long time ago in kernfs :( It's a rare but obvious race between the self delete and non self delete paths. You can trigger it in SCSI by racing device delete with HBA delete. This is the detailed explanation: http://marc.info/?l=linux-scsi=147855187425596 Which we should probably have in a condensed form in the changelog James -- 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] Avoid that SCSI device removal through sysfs triggers a deadlock
On Mon, 2016-11-07 at 16:32 -0800, Bart Van Assche wrote: > The SCSI core holds scan_mutex around SCSI device addition and > removal operations. sysfs serializes attribute read and write > operations against attribute removal through s_active. Avoid that > grabbing scan_mutex during self-removal of a SCSI device triggers > a deadlock by not calling __kernfs_remove() from > kernfs_remove_by_name_ns() in case of self-removal. This patch > avoids that self-removal triggers the following deadlock: > > === > [ INFO: possible circular locking dependency detected ] > 4.9.0-rc1-dbg+ #4 Not tainted > --- > test_02_sdev_de/12586 is trying to acquire lock: > (>scan_mutex){+.+.+.}, at: [] > scsi_remove_device+0x1e/0x40 > but task is already holding lock: > (s_active#336){.+}, at: [] > kernfs_remove_self+0xde/0x140 > which lock already depends on the new lock. > > the existing dependency chain (in reverse order) is: > -> #1 (s_active#336){.+}: > [] lock_acquire+0xe9/0x1d0 > [] __kernfs_remove+0x24a/0x310 > [] kernfs_remove_by_name_ns+0x40/0x90 > [] remove_files.isra.1+0x30/0x70 > [] sysfs_remove_group+0x3f/0x90 > [] sysfs_remove_groups+0x29/0x40 > [] device_remove_attrs+0x59/0x80 > [] device_del+0x125/0x240 > [] __scsi_remove_device+0x143/0x180 > [] scsi_forget_host+0x64/0x70 > [] scsi_remove_host+0x75/0x120 > [] 0xa035dbbb > [] process_one_work+0x1f5/0x690 > [] worker_thread+0x49/0x490 > [] kthread+0xeb/0x110 > [] ret_from_fork+0x27/0x40 > > -> #0 (>scan_mutex){+.+.+.}: > [] __lock_acquire+0x10fc/0x1270 > [] lock_acquire+0xe9/0x1d0 > [] mutex_lock_nested+0x5f/0x360 > [] scsi_remove_device+0x1e/0x40 > [] sdev_store_delete+0x22/0x30 > [] dev_attr_store+0x13/0x20 > [] sysfs_kf_write+0x40/0x50 > [] kernfs_fop_write+0x137/0x1c0 > [] __vfs_write+0x23/0x140 > [] vfs_write+0xb0/0x190 > [] SyS_write+0x44/0xa0 > [] entry_SYSCALL_64_fastpath+0x18/0xad > > other info that might help us debug this: > > Possible unsafe locking scenario: >CPU0CPU1 > > lock(s_active#336); >lock(>scan_mutex); >lock(s_active#336); > lock(>scan_mutex); > > *** DEADLOCK *** > 3 locks held by test_02_sdev_de/12586: > #0: (sb_writers#4){.+.+.+}, at: [] > vfs_write+0x178/0x190 > #1: (>mutex){+.+.+.}, at: [] > kernfs_fop_write+0x101/0x1c0 > #2: (s_active#336){.+}, at: [] > kernfs_remove_self+0xde/0x140 > > stack backtrace: > CPU: 4 PID: 12586 Comm: test_02_sdev_de Not tainted 4.9.0-rc1-dbg+ #4 > Call Trace: > [] dump_stack+0x68/0x93 > [] print_circular_bug+0x1be/0x210 > [] __lock_acquire+0x10fc/0x1270 > [] lock_acquire+0xe9/0x1d0 > [] mutex_lock_nested+0x5f/0x360 > [] scsi_remove_device+0x1e/0x40 > [] sdev_store_delete+0x22/0x30 > [] dev_attr_store+0x13/0x20 > [] sysfs_kf_write+0x40/0x50 > [] kernfs_fop_write+0x137/0x1c0 > [] __vfs_write+0x23/0x140 > [] vfs_write+0xb0/0x190 > [] SyS_write+0x44/0xa0 > [] entry_SYSCALL_64_fastpath+0x18/0xad > > References: http://www.spinics.net/lists/linux-scsi/msg86551.html > Signed-off-by: Bart Van Assche> Cc: Greg Kroah-Hartman > Cc: Eric Biederman > Cc: Hannes Reinecke > Cc: Johannes Thumshirn > Cc: Sagi Grimberg > Cc: > --- > fs/kernfs/dir.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c > index cf4c636..44ec536 100644 > --- a/fs/kernfs/dir.c > +++ b/fs/kernfs/dir.c > @@ -1410,7 +1410,7 @@ int kernfs_remove_by_name_ns(struct kernfs_node > *parent, const char *name, > mutex_lock(_mutex); > > kn = kernfs_find_ns(parent, name, ns); > - if (kn) > + if (kn && !(kn->flags & KERNFS_SUICIDED)) Actually, wrong flag, you need KERNFS_SUICIDAL. The reason is that kernfs_mutex is actually dropped half way through __kernfs_remove, so KERNFS_SUICIDED is not set atomically with this mutex. James -- 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] hpsa: switch to pci_alloc_irq_vectors
On 11/08/2016 03:58 PM, Christoph Hellwig wrote: On Tue, Nov 08, 2016 at 08:12:25AM +0100, Hannes Reinecke wrote: Use pci_alloc_irq_vectors and drop the hand-crafted interrupt affinity routines. There are a couple more things we can do here. I actually have a patch in my tree that goes a little further, I'll post it in a bit. Right. I'll wait for it. If you also happen to have patches for megaraid and mpt3sas I'd be very much interested in looking into them; I'm currently working on converting them, too. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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] hpsa: switch to pci_alloc_irq_vectors
On Tue, Nov 08, 2016 at 04:02:54PM +0100, Hannes Reinecke wrote: >> I've also started playing with lpfc, something I'll need to send to >> you and James for testing and feedback soon. >> > Bah. Another duplicate then. > Let's see who is faster :-) lpfc depends on the series adding the post_vetors support, so I can't send it until we have that merged. -- 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] aacraid: switch to pci_alloc_irq_vectors
> @@ -1246,7 +1246,6 @@ struct aac_dev > u32 max_msix; /* max. MSI-X vectors */ > u32 vector_cap; /* MSI-X vector capab.*/ > int msi_enabled;/* MSI/MSI-X enabled */ > - struct msix_entry msixentry[AAC_MAX_MSIX]; > struct aac_msix_ctx aac_msix[AAC_MAX_MSIX]; /* context */ With a bit more work we should be able to get rid of the aac_msix array as well. I had actually started looking into that but got dragged away. -- 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] libfc: fix seconds_since_last_reset miscalculation
On 11/08/16 00:45, Johannes Thumshirn wrote: > Commit 540eb1eef 'scsi: libfc: fix seconds_since_last_reset calculation' > removed the use of 'struct timespec' from fc_get_host_stats(). This broke the > output of 'fcoeadm -s' after kernel 4.8-rc1 as lport->boot_time - jiffies > could become negative as in this example: > > $ cat /sys/class/fc_host/host8/statistics/seconds_since_last_reset > 0x10624dd2f1977b4 > > Take this into account so > /sys/class/fc_host/hostX/statistics/seconds_since_last_reset is sane again. > > Fixes: 540eb1eef ('scsi: libfc: fix seconds_since_last_reset calculation') > Signed-off-by: Johannes Thumshirn> Tested-by: Holger Schranz > --- > drivers/scsi/libfc/fc_lport.c | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c > index 04ce7cf..475c0a9 100644 > --- a/drivers/scsi/libfc/fc_lport.c > +++ b/drivers/scsi/libfc/fc_lport.c > @@ -304,11 +304,15 @@ struct fc_host_statistics *fc_get_host_stats(struct > Scsi_Host *shost) > unsigned int cpu; > u64 fcp_in_bytes = 0; > u64 fcp_out_bytes = 0; > + unsigned long boot_time = lport->boot_time; > > fc_stats = >host_stats; > memset(fc_stats, 0, sizeof(struct fc_host_statistics)); > > - fc_stats->seconds_since_last_reset = (lport->boot_time - jiffies) / HZ; > + if (boot_time > jiffies) > + fc_stats->seconds_since_last_reset = (boot_time - jiffies) / HZ; > + else > + fc_stats->seconds_since_last_reset = (jiffies - boot_time) / HZ; > > for_each_possible_cpu(cpu) { > struct fc_stats *stats; Hello Johannes, I think the above code will miscalculate seconds_since_last_reset if 'jiffies' wraps around after an lport has been created and before seconds_since_last_reset is computed. Shouldn't seconds_since_last_reset be computed as follows? fc_stats->seconds_since_last_reset = (jiffies - boot_time) / HZ; Bart. -- 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] hpsa: switch to pci_alloc_irq_vectors
On 11/08/2016 04:01 PM, Christoph Hellwig wrote: On Tue, Nov 08, 2016 at 04:00:46PM +0100, Hannes Reinecke wrote: Right. I'll wait for it. If you also happen to have patches for megaraid and mpt3sas I'd be very much interested in looking into them; I'm currently working on converting them, too. I've looked at the them but haven't started work, so feel free to go ahead. The MSI-X handling in mpt3sas is pretty nasty, so be careful. I've also started playing with lpfc, something I'll need to send to you and James for testing and feedback soon. Bah. Another duplicate then. Let's see who is faster :-) Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- 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] hpsa: switch to pci_alloc_irq_vectors
On Tue, Nov 08, 2016 at 04:00:46PM +0100, Hannes Reinecke wrote: > Right. I'll wait for it. > > If you also happen to have patches for megaraid and mpt3sas I'd be very > much interested in looking into them; I'm currently working on converting > them, too. I've looked at the them but haven't started work, so feel free to go ahead. The MSI-X handling in mpt3sas is pretty nasty, so be careful. I've also started playing with lpfc, something I'll need to send to you and James for testing and feedback soon. -- 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] hpsa: switch to pci_alloc_irq_vectors
On Tue, Nov 08, 2016 at 08:12:25AM +0100, Hannes Reinecke wrote: > Use pci_alloc_irq_vectors and drop the hand-crafted > interrupt affinity routines. There are a couple more things we can do here. I actually have a patch in my tree that goes a little further, I'll post it in a bit. -- 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 6/8] MAINTAINERS: Update megaraid maintainers list
On 21.10.2016 15:33, Kashyap Desai wrote: > Update MEGARAID drivers maintainers list. > > Signed-off-by: Sumit Saxena> Reviewed-by: Hannes Reinecke > --- > MAINTAINERS | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index f0ee7a6..8b9117f 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -7612,12 +7612,12 @@ S:Maintained > F: drivers/net/wireless/mediatek/mt7601u/ > > MEGARAID SCSI/SAS DRIVERS > -M: Kashyap Desai > -M: Sumit Saxena > -M: Uday Lingala > -L: megaraidlinux@avagotech.com > +M: Kashyap Desai > +M: Sumit Saxena > +M: Shivasharan S > +L: megaraidlinux@broadcom.com > L: linux-scsi@vger.kernel.org > -W: http://www.lsi.com > +W: http://www.avagotech.com/support/ I like the phase delay, everything moved to Broadcom, so it's good time to switch the support page to from LSI to Avago.. Reviewed-by: Tomas Henzl -- 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: Ordering problems with 3ware controller
Dear Linux SCSI folks, On 11/08/16 11:07, Paul Menzel wrote: Updating from Linux 4.4.X to Linux 4.8.4, we noticed that the 3ware devices under `/dev` – `/dev/twa0`, `/dev/twa1`, … – map to the controllers differently. This unfortunately breaks quite a lot of our scripts, as we depend on the fact that the first controller is also in the front. $ dmesg | grep 3ware [ 14.509238] 3ware 9000 Storage Controller device driver for Linux v2.26.02.014. [ 14.824274] scsi host8: 3ware 9000 Storage Controller [ 14.824537] 3w-9xxx: scsi8: Found a 3ware 9000 Storage Controller at 0xd020, IRQ: 17. [ 15.508310] scsi host9: 3ware 9000 Storage Controller [ 15.508569] 3w-9xxx: scsi9: Found a 3ware 9000 Storage Controller at 0xda10, IRQ: 17. Tracing `twi_cli` it looks like the ordering of the devices in `/sys/class/scsi_host` might have changed, as `getdents64` seems to be used for the ordering of creating `/dev/twaX`. $ find /sys/class/scsi_host/ -ls 6033 0 drwxr-xr-x 2 root system 0 Nov 8 10:58 /sys/class/scsi_host/ 23125 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host0 -> ../../devices/pci:00/:00:07.0/ata1/host0/scsi_host/host0 29893 0 lrwxrwxrwx 1 root system 0 Oct 27 18:03 /sys/class/scsi_host/host9 -> ../../devices/pci:80/:80:0e.0/:90:00.0/host9/scsi_host/host9 23878 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host7 -> ../../devices/pci:80/:80:08.0/ata8/host7/scsi_host/host7 23640 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host5 -> ../../devices/pci:80/:80:07.0/ata6/host5/scsi_host/host5 23402 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host3 -> ../../devices/pci:00/:00:08.0/ata4/host3/scsi_host/host3 23164 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host1 -> ../../devices/pci:00/:00:07.0/ata2/host1/scsi_host/host1 29851 0 lrwxrwxrwx 1 root system 0 Oct 27 18:03 /sys/class/scsi_host/host8 -> ../../devices/pci:00/:00:0e.0/:05:00.0/host8/scsi_host/host8 23839 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host6 -> ../../devices/pci:80/:80:08.0/ata7/host6/scsi_host/host6 23601 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host4 -> ../../devices/pci:80/:80:07.0/ata5/host4/scsi_host/host4 23363 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host2 -> ../../devices/pci:00/:00:08.0/ata3/host2/scsi_host/host2 $ sudo -i tw_cli show Ctl Model(V)Ports Drives Units NotOpt RRate VRate BBU c89650SE-8LPML 8 81 0 5 1 OK c99690SA-8E0 00 0 5 1 OK Enclosure Slots Drives Fans TSUnits PSUnits Alarms -- /c9/e016 0 3 121 So in this case `c8` is mapped to `/dev/twa1`, and `c9` to `/dev/twa0`. As we do not know of a way, to use `tw_cli` to find the correct mapping, or another place, we rely on the implicit ordering, which – according to my colleagues – has worked for over 15 years [1]. Here is the excerpt from the manual page for smartctl [2]. > --- end of manual page excerpt --- 3ware,N - [FreeBSD and Linux only] the device consists of one or more ATA disks con‐ nected to a 3ware RAID controller. The non-negative integer N (in the range from 0 to 127 inclusive) denotes which disk on the controller is monitored. Use syntax such as: smartctl -a -d 3ware,2 /dev/sda smartctl -a -d 3ware,0 /dev/twe0 smartctl -a -d 3ware,1 /dev/twa0 smartctl -a -d 3ware,1 /dev/twl0 The first two forms, which refer to devices /dev/sda-z and /dev/twe0-15, may be used with 3ware series 6000, 7000, and 8000 series controllers that use the 3x- driver. Note that the /dev/sda-z form is deprecated starting with the Linux 2.6 kernel series and may not be supported by the Linux kernel in the near future. The final form, which refers to devices /dev/twa0-15, must be used with 3ware 9000 series controllers, which use the 3w-9xxx driver. The devices /dev/twl0-15 must be used with the 3ware/LSI 9750 series controllers which use the 3w-sas driver. Note that if the special character device nodes /dev/twl?, /dev/twa? and /dev/twe? do not exist, or exist with the incorrect major or minor numbers, smartctl will recreate them on the fly. Typically /dev/twa0 refers to the first 9000-series controller, /dev/twa1 refers to the second 9000 series controller, and so on. The /dev/twl0 devices refers to the first
Re: [PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
Hi Manish, [auto build test ERROR on net-next/master] [also build test ERROR on v4.9-rc4 next-20161028] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 config: parisc-allyesconfig (attached as .config) compiler: hppa-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=parisc All errors (new ones prefixed by >>): drivers/scsi/qedi/qedi_main.c: In function 'qedi_iscsi_event_cb': drivers/scsi/qedi/qedi_main.c:87:14: error: dereferencing pointer to incomplete type 'struct qedi_endpoint' if (qedi_ep->state == EP_STATE_OFLDCONN_START) ^~ drivers/scsi/qedi/qedi_main.c:87:25: error: 'EP_STATE_OFLDCONN_START' undeclared (first use in this function) if (qedi_ep->state == EP_STATE_OFLDCONN_START) ^~~ drivers/scsi/qedi/qedi_main.c:87:25: note: each undeclared identifier is reported only once for each function it appears in drivers/scsi/qedi/qedi_main.c:88:21: error: 'EP_STATE_OFLDCONN_COMPL' undeclared (first use in this function) qedi_ep->state = EP_STATE_OFLDCONN_COMPL; ^~~ drivers/scsi/qedi/qedi_main.c:93:20: error: 'EP_STATE_DISCONN_COMPL' undeclared (first use in this function) qedi_ep->state = EP_STATE_DISCONN_COMPL; ^~ drivers/scsi/qedi/qedi_main.c:97:3: error: implicit declaration of function 'qedi_process_iscsi_error' [-Werror=implicit-function-declaration] qedi_process_iscsi_error(qedi_ep, data); ^~~~ drivers/scsi/qedi/qedi_main.c:106:3: error: implicit declaration of function 'qedi_process_tcp_error' [-Werror=implicit-function-declaration] qedi_process_tcp_error(qedi_ep, data); ^~ drivers/scsi/qedi/qedi_main.c: In function 'qedi_host_alloc': drivers/scsi/qedi/qedi_main.c:414:28: error: 'qedi_host_template' undeclared (first use in this function) shost = iscsi_host_alloc(_host_template, ^~ drivers/scsi/qedi/qedi_main.c:433:27: error: 'ISCSI_MAX_SESS_PER_HBA' undeclared (first use in this function) qedi->max_active_conns = ISCSI_MAX_SESS_PER_HBA; ^~ drivers/scsi/qedi/qedi_main.c: In function 'qedi_set_iscsi_pf_param': >> drivers/scsi/qedi/qedi_main.c:463:4: error: passing argument 3 of >> 'pci_alloc_consistent' from incompatible pointer type >> [-Werror=incompatible-pointer-types] >hw_p_cpuq); ^ In file included from include/linux/pci.h:2131:0, from drivers/scsi/qedi/qedi_main.c:11: include/linux/pci-dma-compat.h:16:1: note: expected 'dma_addr_t * {aka unsigned int *}' but argument is of type 'u64 * {aka long long unsigned int *}' pci_alloc_consistent(struct pci_dev *hwdev, size_t size, ^~~~ drivers/scsi/qedi/qedi_main.c: In function 'qedi_queue_cqe': drivers/scsi/qedi/qedi_main.c:571:15: error: dereferencing pointer to incomplete type 'struct qedi_conn' conn = q_conn->cls_conn->dd_data; ^~ drivers/scsi/qedi/qedi_main.c:581:27: error: dereferencing pointer to incomplete type 'struct qedi_cmd' INIT_LIST_HEAD(_cmd->cqe_work.list); ^~ drivers/scsi/qedi/qedi_main.c: At top level: drivers/scsi/qedi/qedi_main.c:1095:15: error: variable 'qedi_ll2_cb_ops' has initializer but incomplete type static struct qed_ll2_cb_ops qedi_ll2_cb_ops = { ^~ drivers/scsi/qedi/qedi_main.c:1096:2: error: unknown field 'rx_cb' specified in initializer .rx_cb = qedi_ll2_rx, ^ drivers/scsi/qedi/qedi_main.c:1096:11: error: 'qedi_ll2_rx' undeclared here (not in a function) .rx_cb = qedi_ll2_rx, ^~~ drivers/scsi/qedi/qedi_main.c:1096:11: warning: excess elements in struct initializer drivers/scsi/qedi/qedi_main.c:1096:11: note: (near initialization for 'qedi_ll2_cb_ops') drivers/scsi/qedi/qedi_main.c:1097:2: error: unknown field 'tx_cb' specified in initializer .tx_cb = NULL, ^ In file included from include/uapi/linux/posix_types.h:4:0, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/list.h:4, from include/linux/module.h:9, from drivers/scsi/qedi/qedi_main.c:10: include/linux/stddef.h:7:14: warning: exc
Re: [PATCH v2 6/6] qedi: Add support for data path.
Hi Manish, [auto build test WARNING on net-next/master] [also build test WARNING on v4.9-rc4 next-20161028] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) include/linux/compiler.h:253:8: sparse: attribute 'no_sanitize_address': unknown attribute >> drivers/scsi/qedi/qedi_fw.c:2179:31: sparse: incompatible types in >> comparison expression (different base types) vim +2179 drivers/scsi/qedi/qedi_fw.c 2163 fw_task_ctx = qedi_get_task_mem(>tasks, tid); 2164 2165 memset(fw_task_ctx, 0, sizeof(struct iscsi_task_context)); 2166 cmd->task_id = tid; 2167 2168 /* Ystorm context */ 2169 fw_cmd = _task_ctx->ystorm_st_context.pdu_hdr.cmd; 2170 SET_FIELD(fw_cmd->flags_attr, ISCSI_CMD_HDR_ATTR, ISCSI_ATTR_SIMPLE); 2171 2172 if (sc->sc_data_direction == DMA_TO_DEVICE) { 2173 if (conn->session->initial_r2t_en) { 2174 fw_task_ctx->ustorm_ag_context.exp_data_acked = 2175 min((conn->session->imm_data_en * 2176 conn->max_xmit_dlength), 2177 conn->session->first_burst); 2178 fw_task_ctx->ustorm_ag_context.exp_data_acked = > 2179 > min(fw_task_ctx->ustorm_ag_context.exp_data_acked, 2180scsi_bufflen(sc)); 2181 } else { 2182 fw_task_ctx->ustorm_ag_context.exp_data_acked = 2183min(conn->session->first_burst, scsi_bufflen(sc)); 2184 } 2185 2186 SET_FIELD(fw_cmd->flags_attr, ISCSI_CMD_HDR_WRITE, 1); 2187 task_type = ISCSI_TASK_TYPE_INITIATOR_WRITE; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation -- 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 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
Hi Manish, [auto build test ERROR on net-next/master] [also build test ERROR on v4.9-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 6.2.0 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=ia64 Note: the linux-review/Manish-Rangankar/qed-Add-support-for-hardware-offloaded-iSCSI/20161108-180027 HEAD dd4d1d0e0785d20cdcfdf9b2c792c564a79b2de2 builds fine. It only hurts bisectibility. All error/warnings (new ones prefixed by >>): drivers/scsi/qedi/qedi_main.c: In function 'qedi_iscsi_event_cb': >> drivers/scsi/qedi/qedi_main.c:87:14: error: dereferencing pointer to >> incomplete type 'struct qedi_endpoint' if (qedi_ep->state == EP_STATE_OFLDCONN_START) ^~ >> drivers/scsi/qedi/qedi_main.c:87:25: error: 'EP_STATE_OFLDCONN_START' >> undeclared (first use in this function) if (qedi_ep->state == EP_STATE_OFLDCONN_START) ^~~ drivers/scsi/qedi/qedi_main.c:87:25: note: each undeclared identifier is reported only once for each function it appears in >> drivers/scsi/qedi/qedi_main.c:88:21: error: 'EP_STATE_OFLDCONN_COMPL' >> undeclared (first use in this function) qedi_ep->state = EP_STATE_OFLDCONN_COMPL; ^~~ >> drivers/scsi/qedi/qedi_main.c:93:20: error: 'EP_STATE_DISCONN_COMPL' >> undeclared (first use in this function) qedi_ep->state = EP_STATE_DISCONN_COMPL; ^~ >> drivers/scsi/qedi/qedi_main.c:97:3: error: implicit declaration of function >> 'qedi_process_iscsi_error' [-Werror=implicit-function-declaration] qedi_process_iscsi_error(qedi_ep, data); ^~~~ >> drivers/scsi/qedi/qedi_main.c:106:3: error: implicit declaration of function >> 'qedi_process_tcp_error' [-Werror=implicit-function-declaration] qedi_process_tcp_error(qedi_ep, data); ^~ drivers/scsi/qedi/qedi_main.c: In function 'qedi_host_alloc': >> drivers/scsi/qedi/qedi_main.c:414:28: error: 'qedi_host_template' undeclared >> (first use in this function) shost = iscsi_host_alloc(_host_template, ^~ >> drivers/scsi/qedi/qedi_main.c:433:27: error: 'ISCSI_MAX_SESS_PER_HBA' >> undeclared (first use in this function) qedi->max_active_conns = ISCSI_MAX_SESS_PER_HBA; ^~ drivers/scsi/qedi/qedi_main.c: In function 'qedi_queue_cqe': >> drivers/scsi/qedi/qedi_main.c:571:15: error: dereferencing pointer to >> incomplete type 'struct qedi_conn' conn = q_conn->cls_conn->dd_data; ^~ >> drivers/scsi/qedi/qedi_main.c:581:27: error: dereferencing pointer to >> incomplete type 'struct qedi_cmd' INIT_LIST_HEAD(_cmd->cqe_work.list); ^~ drivers/scsi/qedi/qedi_main.c: At top level: drivers/scsi/qedi/qedi_main.c:1095:15: error: variable 'qedi_ll2_cb_ops' has initializer but incomplete type static struct qed_ll2_cb_ops qedi_ll2_cb_ops = { ^~ drivers/scsi/qedi/qedi_main.c:1096:2: error: unknown field 'rx_cb' specified in initializer .rx_cb = qedi_ll2_rx, ^ drivers/scsi/qedi/qedi_main.c:1096:11: error: 'qedi_ll2_rx' undeclared here (not in a function) .rx_cb = qedi_ll2_rx, ^~~ drivers/scsi/qedi/qedi_main.c:1096:11: warning: excess elements in struct initializer drivers/scsi/qedi/qedi_main.c:1096:11: note: (near initialization for 'qedi_ll2_cb_ops') drivers/scsi/qedi/qedi_main.c:1097:2: error: unknown field 'tx_cb' specified in initializer .tx_cb = NULL, ^ In file included from include/uapi/linux/posix_types.h:4:0, from include/uapi/linux/types.h:13, from include/linux/types.h:5, from include/linux/list.h:4, from include/linux/module.h:9, from drivers/scsi/qedi/qedi_main.c:10: include/linux/stddef.h:7:14: warning: excess elements in struct initializer #define NULL ((void *)0) ^ drivers/scsi/qedi/qedi_main.c:1097:11: note: in expansion of macro 'NULL' .tx_cb = NULL, ^~~~ include/linux/stddef.h:7:14: note: (near initialization for 'qedi_ll2_cb_ops') #define NULL ((void *)0)
[PATCH v2 0/6] Add QLogic FastLinQ iSCSI (qedi) driver.
This series introduces hardware offload iSCSI initiator driver for the 41000 Series Converged Network Adapters (579xx chip) by Qlogic. The overall driver design includes a common module ('qed') and protocol specific dependent modules ('qedi' for iSCSI). This is an open iSCSI driver, modifications to open iSCSI user components 'iscsid', 'iscsiuio', etc. are required for the solution to work. The user space changes are also in the process of being submitted. https://groups.google.com/forum/#!forum/open-iscsi The 'qed' common module, under drivers/net/ethernet/qlogic/qed/, is enhanced with functionality required for the iSCSI support. This series is based on: net tree base: Merge of net and net-next as of 10/31/2016 Changes from RFC v1: RFC v1 comments reference: http://marc.info/?l=linux-scsi=2=1=qed=b The following comments were incorporated:- 1. MAC copying to use unaligned macro in qed_sp_iscsi_conn_offload() 2. Contain scattered IS_ENABLED() check inside header file. 3. Reduce indent in qed_iscsi_start() by changing logic slightly. 4. Use kernel-doc style documentation. 5. Shorten qed_ll2_lb_rxq_completion() by removing queue handling as a separate function. 6. Reduce code duplication by creating qed_ooo_submit_[tr]x_buffers(). 7. Remove dynamic memory allocation in ISR for solicited packets. 8. Rearrange qedi_process_iscsi_error() code to use mapping values onto strings. 9. Use kernel-doc style documentation in qedi_hsi.h 10. Change pr_crit to pr_err. 11. Remove unnecessary typecasting. 12. Change kmalloc to kmalloc_array in qedi_setup_cid_que(). 13. Rearrange firmware and driver version information print. 14. Change variable names from scsi_lun to other, as scsi_lun struct exists. 15. Introduce QEDI_U64_HI and QEDI_U64_LO macros for code readability. 16. Move scsi_dma_unmap above sge_valid in qedi_iscsi_unmap_sg_list(). 17. Introduce QEDI_OFLD_WAIT_STATE for code readability. 18. Add cond_resched() in per_cpu_io_thread to avoid soft-lockups. 19. Change trace_io to accomodate all the scsi commands. 20. Spell correction in comments section. The following comments were not incorporated:- 1. Remove "QLogic qed NIC Driver" comment - Applies to drivers/net/ethernet/qlogic/qed/*.[ch] - This will be submitted as a separate patch, as there are multiple files (files that are not part of this series) need to be updated and this change is not related to this series. 2. Update copyright - Applies to drivers/net/ethernet/qlogic/qed/*.[ch] - Same as above, will submit a separate patch, as multiple files need to be updated and this change is not related to this series. 3. Remove adding and removing of QEDI Kconfig entries. Added by "qed" patch and removed and re-added by "qedi" patch. - QEDI driver is composed of two parts, the common module (qed) and the protocol module (qedi), which goes in net and scsi trees respectively. The qed module conditionally enables certain functionality for the QEDI module in qed; hence the Kconfig entry was added for QEDI by qed; otherwise it looks odd to use CONFIG_QEDI in qed. So, the remove and re-add is left as is. 4. Workqueue implementation in completion path. - This change will be submitted as a separate patch, as it requires complete performance validation cycle. 5. IRQ handling changes from Christoph H. irq work. - This change will also be subitted as a separate patch, as it also require change in qed.ko module which will affect other protocol drivers (qedr, qede) as well. - This change will be integral part of MQ/MQ-TAG infrastructure that we want to employ in our qedi driver. 6. Using multiple memory barriers. - We kept both memory barriers as a failsafe, as for some architectures the call is the same but on others they are two different assembly operations. We have updated patch with same information in comments. Manish Rangankar (4): qedi: Add QLogic FastLinQ offload iSCSI driver framework. qedi: Add LL2 iSCSI interface for offload iSCSI. qedi: Add support for iSCSI session management. qedi: Add support for data path. Yuval Mintz (2): qed: Add support for hardware offloaded iSCSI. qed: Add iSCSI out of order packet handling. MAINTAINERS|6 + drivers/net/ethernet/qlogic/Kconfig| 15 + drivers/net/ethernet/qlogic/qed/Makefile |1 + drivers/net/ethernet/qlogic/qed/qed.h |8 +- drivers/net/ethernet/qlogic/qed/qed_dev.c | 22 + drivers/net/ethernet/qlogic/qed/qed_int.h |1 - drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1276 + drivers/net/ethernet/qlogic/qed/qed_iscsi.h| 52 + drivers/net/ethernet/qlogic/qed/qed_l2.c |1 - drivers/net/ethernet/qlogic/qed/qed_ll2.c | 507 -
Ordering problems with 3ware controller
Dear Linux SCSI folks, Updating from Linux 4.4.X to Linux 4.8.4, we noticed that the 3ware devices under `/dev` – `/dev/twa0`, `/dev/twa1`, … – map to the controllers differently. This unfortunately breaks quite a lot of our scripts, as we depend on the fact that the first controller is also in the front. $ dmesg | grep 3ware [ 14.509238] 3ware 9000 Storage Controller device driver for Linux v2.26.02.014. [ 14.824274] scsi host8: 3ware 9000 Storage Controller [ 14.824537] 3w-9xxx: scsi8: Found a 3ware 9000 Storage Controller at 0xd020, IRQ: 17. [ 15.508310] scsi host9: 3ware 9000 Storage Controller [ 15.508569] 3w-9xxx: scsi9: Found a 3ware 9000 Storage Controller at 0xda10, IRQ: 17. Tracing `twi_cli` it looks like the ordering of the devices in `/sys/class/scsi_host` might have changed, as `getdents64` seems to be used for the ordering of creating `/dev/twaX`. $ find /sys/class/scsi_host/ -ls 6033 0 drwxr-xr-x 2 root system 0 Nov 8 10:58 /sys/class/scsi_host/ 23125 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host0 -> ../../devices/pci:00/:00:07.0/ata1/host0/scsi_host/host0 29893 0 lrwxrwxrwx 1 root system 0 Oct 27 18:03 /sys/class/scsi_host/host9 -> ../../devices/pci:80/:80:0e.0/:90:00.0/host9/scsi_host/host9 23878 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host7 -> ../../devices/pci:80/:80:08.0/ata8/host7/scsi_host/host7 23640 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host5 -> ../../devices/pci:80/:80:07.0/ata6/host5/scsi_host/host5 23402 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host3 -> ../../devices/pci:00/:00:08.0/ata4/host3/scsi_host/host3 23164 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host1 -> ../../devices/pci:00/:00:07.0/ata2/host1/scsi_host/host1 29851 0 lrwxrwxrwx 1 root system 0 Oct 27 18:03 /sys/class/scsi_host/host8 -> ../../devices/pci:00/:00:0e.0/:05:00.0/host8/scsi_host/host8 23839 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host6 -> ../../devices/pci:80/:80:08.0/ata7/host6/scsi_host/host6 23601 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host4 -> ../../devices/pci:80/:80:07.0/ata5/host4/scsi_host/host4 23363 0 lrwxrwxrwx 1 root system 0 Oct 27 17:41 /sys/class/scsi_host/host2 -> ../../devices/pci:00/:00:08.0/ata3/host2/scsi_host/host2 $ sudo -i tw_cli show Ctl Model(V)Ports Drives Units NotOpt RRate VRate BBU c89650SE-8LPML 8 81 0 5 1 OK c99690SA-8E0 00 0 5 1 OK Enclosure Slots Drives Fans TSUnits PSUnits Alarms -- /c9/e016 0 3 121 So in this case `c8` is mapped to `/dev/twa1`, and `c9` to `/dev/twa0`. As we do not know of a way, to use `tw_cli` to find the correct mapping, or another place, we rely on the implicit ordering, which – according to my colleagues – has worked for over 15 years [1]. Do you know of a way, to either get the mapping “over an API” so we don’t have to rely on the implicit ordering? Otherwise, do you know, why the ordering has changed, and can this be reverted? Kind regards, Paul Menzel [1] https://www.thomas-krenn.com/de/wiki/Smartmontools_mit_3ware_RAID_Controller (German) -- 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
[PATCH v5 08/12] phy: qcom-ufs-qmp-xx: Move clock and regulator init out of phy init
The phy init is meant to do phy initialization rather than just getting the clock and regulator. Move these clock and regulator get to probe(), to make room for actual phy initialization sequence. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs-qmp-14nm.c | 52 ++--- drivers/phy/phy-qcom-ufs-qmp-20nm.c | 46 +++- 2 files changed, 46 insertions(+), 52 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/phy-qcom-ufs-qmp-14nm.c index 560f272..ae74614 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-14nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-14nm.c @@ -44,30 +44,7 @@ void ufs_qcom_phy_qmp_14nm_advertise_quirks(struct ufs_qcom_phy *phy_common) static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy) { - struct ufs_qcom_phy_qmp_14nm *phy = phy_get_drvdata(generic_phy); - struct ufs_qcom_phy *phy_common = >common_cfg; - int err; - - err = ufs_qcom_phy_init_clks(phy_common); - if (err) { - dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_clks() failed %d\n", - __func__, err); - goto out; - } - - err = ufs_qcom_phy_init_vregulators(phy_common); - if (err) { - dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_vregulators() failed %d\n", - __func__, err); - goto out; - } - phy_common->vdda_phy.max_uV = UFS_PHY_VDDA_PHY_UV; - phy_common->vdda_phy.min_uV = UFS_PHY_VDDA_PHY_UV; - - ufs_qcom_phy_qmp_14nm_advertise_quirks(phy_common); - -out: - return err; + return 0; } static @@ -136,6 +113,7 @@ static int ufs_qcom_phy_qmp_14nm_probe(struct platform_device *pdev) struct device *dev = >dev; struct phy *generic_phy; struct ufs_qcom_phy_qmp_14nm *phy; + struct ufs_qcom_phy *phy_common; int err = 0; phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL); @@ -143,8 +121,9 @@ static int ufs_qcom_phy_qmp_14nm_probe(struct platform_device *pdev) err = -ENOMEM; goto out; } + phy_common = >common_cfg; - generic_phy = ufs_qcom_phy_generic_probe(pdev, >common_cfg, + generic_phy = ufs_qcom_phy_generic_probe(pdev, phy_common, _qcom_phy_qmp_14nm_phy_ops, _14nm_ops); if (!generic_phy) { @@ -154,10 +133,29 @@ static int ufs_qcom_phy_qmp_14nm_probe(struct platform_device *pdev) goto out; } + err = ufs_qcom_phy_init_clks(phy_common); + if (err) { + dev_err(phy_common->dev, + "%s: ufs_qcom_phy_init_clks() failed %d\n", + __func__, err); + goto out; + } + + err = ufs_qcom_phy_init_vregulators(phy_common); + if (err) { + dev_err(phy_common->dev, + "%s: ufs_qcom_phy_init_vregulators() failed %d\n", + __func__, err); + goto out; + } + phy_common->vdda_phy.max_uV = UFS_PHY_VDDA_PHY_UV; + phy_common->vdda_phy.min_uV = UFS_PHY_VDDA_PHY_UV; + + ufs_qcom_phy_qmp_14nm_advertise_quirks(phy_common); + phy_set_drvdata(generic_phy, phy); - strlcpy(phy->common_cfg.name, UFS_PHY_NAME, - sizeof(phy->common_cfg.name)); + strlcpy(phy_common->name, UFS_PHY_NAME, sizeof(phy_common->name)); out: return err; diff --git a/drivers/phy/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/phy-qcom-ufs-qmp-20nm.c index 9a2f53d..dfc5175 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-20nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-20nm.c @@ -63,28 +63,7 @@ void ufs_qcom_phy_qmp_20nm_advertise_quirks(struct ufs_qcom_phy *phy_common) static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy) { - struct ufs_qcom_phy_qmp_20nm *phy = phy_get_drvdata(generic_phy); - struct ufs_qcom_phy *phy_common = >common_cfg; - int err = 0; - - err = ufs_qcom_phy_init_clks(phy_common); - if (err) { - dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_clks() failed %d\n", - __func__, err); - goto out; - } - - err = ufs_qcom_phy_init_vregulators(phy_common); - if (err) { - dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_vregulators() failed %d\n", - __func__, err); - goto out; - } - - ufs_qcom_phy_qmp_20nm_advertise_quirks(phy_common); - -out: - return err; + return 0; } static @@ -192,6 +171,7 @@ static int ufs_qcom_phy_qmp_20nm_probe(struct platform_device *pdev) struct device *dev = >dev; struct phy *generic_phy; struct ufs_qcom_phy_qmp_20nm *phy; + struct ufs_qcom_phy *phy_common; int err = 0; phy =
[PATCH v5 10/12] phy: qcom-ufs: Remove common layer phy exit callback
The common layer phy exit callback ufs_qcom_phy_exit() calls phy_power_off() that has no meaning when phy_power_off() callback is already registered with the phy provider and the consumer makes use of the same. Instead, add a no-op specific phy_exit() callback for now to add the exit sequence at a later point. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs-i.h| 1 - drivers/phy/phy-qcom-ufs-qmp-14nm.c | 7 ++- drivers/phy/phy-qcom-ufs-qmp-20nm.c | 7 ++- drivers/phy/phy-qcom-ufs.c | 17 ++--- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h index 69e836d..d505d98 100644 --- a/drivers/phy/phy-qcom-ufs-i.h +++ b/drivers/phy/phy-qcom-ufs-i.h @@ -141,7 +141,6 @@ struct ufs_qcom_phy_specific_ops { struct ufs_qcom_phy *get_ufs_qcom_phy(struct phy *generic_phy); int ufs_qcom_phy_power_on(struct phy *generic_phy); int ufs_qcom_phy_power_off(struct phy *generic_phy); -int ufs_qcom_phy_exit(struct phy *generic_phy); int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common); int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common); int ufs_qcom_phy_remove(struct phy *generic_phy, diff --git a/drivers/phy/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/phy-qcom-ufs-qmp-14nm.c index ae74614..c71c847 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-14nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-14nm.c @@ -47,6 +47,11 @@ static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy) return 0; } +static int ufs_qcom_phy_qmp_14nm_exit(struct phy *generic_phy) +{ + return 0; +} + static void ufs_qcom_phy_qmp_14nm_power_control(struct ufs_qcom_phy *phy, bool val) { @@ -94,7 +99,7 @@ static int ufs_qcom_phy_qmp_14nm_is_pcs_ready(struct ufs_qcom_phy *phy_common) static const struct phy_ops ufs_qcom_phy_qmp_14nm_phy_ops = { .init = ufs_qcom_phy_qmp_14nm_init, - .exit = ufs_qcom_phy_exit, + .exit = ufs_qcom_phy_qmp_14nm_exit, .power_on = ufs_qcom_phy_power_on, .power_off = ufs_qcom_phy_power_off, .owner = THIS_MODULE, diff --git a/drivers/phy/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/phy-qcom-ufs-qmp-20nm.c index dfc5175..1a26a64 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-20nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-20nm.c @@ -66,6 +66,11 @@ static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy) return 0; } +static int ufs_qcom_phy_qmp_20nm_exit(struct phy *generic_phy) +{ + return 0; +} + static void ufs_qcom_phy_qmp_20nm_power_control(struct ufs_qcom_phy *phy, bool val) { @@ -152,7 +157,7 @@ static int ufs_qcom_phy_qmp_20nm_is_pcs_ready(struct ufs_qcom_phy *phy_common) static const struct phy_ops ufs_qcom_phy_qmp_20nm_phy_ops = { .init = ufs_qcom_phy_qmp_20nm_init, - .exit = ufs_qcom_phy_exit, + .exit = ufs_qcom_phy_qmp_20nm_exit, .power_on = ufs_qcom_phy_power_on, .power_off = ufs_qcom_phy_power_off, .owner = THIS_MODULE, diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index fdd9b90..c69568b 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -602,17 +602,6 @@ int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B) } EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy); -int ufs_qcom_phy_exit(struct phy *generic_phy) -{ - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy); - - if (ufs_qcom_phy->is_powered_on) - phy_power_off(generic_phy); - - return 0; -} -EXPORT_SYMBOL_GPL(ufs_qcom_phy_exit); - int ufs_qcom_phy_is_pcs_ready(struct phy *generic_phy) { struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy); @@ -634,6 +623,9 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy) struct device *dev = phy_common->dev; int err; + if (phy_common->is_powered_on) + return 0; + err = ufs_qcom_phy_enable_vreg(dev, _common->vdda_phy); if (err) { dev_err(dev, "%s enable vdda_phy failed, err=%d\n", @@ -696,6 +688,9 @@ int ufs_qcom_phy_power_off(struct phy *generic_phy) { struct ufs_qcom_phy *phy_common = get_ufs_qcom_phy(generic_phy); + if (!phy_common->is_powered_on) + return 0; + phy_common->phy_spec_ops->power_control(phy_common, false); if (phy_common->vddp_ref_clk.reg) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 07/12] phy: qcom-ufs: Remove unnecessary function declarations
Move the functions' definitions to remove unnecessary declarations. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs.c | 131 + 1 file changed, 62 insertions(+), 69 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index b85f882..c5c29fe 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -22,13 +22,6 @@ #define VDDP_REF_CLK_MIN_UV120 #define VDDP_REF_CLK_MAX_UV120 -static int __ufs_qcom_phy_init_vreg(struct device *, struct ufs_qcom_phy_vreg *, - const char *, bool); -static int ufs_qcom_phy_init_vreg(struct device *, struct ufs_qcom_phy_vreg *, - const char *); -static int ufs_qcom_phy_base_init(struct platform_device *pdev, - struct ufs_qcom_phy *phy_common); - int ufs_qcom_phy_calibrate(struct ufs_qcom_phy *ufs_qcom_phy, struct ufs_qcom_phy_calibration *tbl_A, int tbl_size_A, @@ -75,45 +68,6 @@ int ufs_qcom_phy_calibrate(struct ufs_qcom_phy *ufs_qcom_phy, } EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate); -struct phy *ufs_qcom_phy_generic_probe(struct platform_device *pdev, - struct ufs_qcom_phy *common_cfg, - const struct phy_ops *ufs_qcom_phy_gen_ops, - struct ufs_qcom_phy_specific_ops *phy_spec_ops) -{ - int err; - struct device *dev = >dev; - struct phy *generic_phy = NULL; - struct phy_provider *phy_provider; - - err = ufs_qcom_phy_base_init(pdev, common_cfg); - if (err) { - dev_err(dev, "%s: phy base init failed %d\n", __func__, err); - goto out; - } - - phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); - if (IS_ERR(phy_provider)) { - err = PTR_ERR(phy_provider); - dev_err(dev, "%s: failed to register phy %d\n", __func__, err); - goto out; - } - - generic_phy = devm_phy_create(dev, NULL, ufs_qcom_phy_gen_ops); - if (IS_ERR(generic_phy)) { - err = PTR_ERR(generic_phy); - dev_err(dev, "%s: failed to create phy %d\n", __func__, err); - generic_phy = NULL; - goto out; - } - - common_cfg->phy_spec_ops = phy_spec_ops; - common_cfg->dev = dev; - -out: - return generic_phy; -} -EXPORT_SYMBOL_GPL(ufs_qcom_phy_generic_probe); - /* * This assumes the embedded phy structure inside generic_phy is of type * struct ufs_qcom_phy. In order to function properly it's crucial @@ -154,6 +108,45 @@ int ufs_qcom_phy_base_init(struct platform_device *pdev, return 0; } +struct phy *ufs_qcom_phy_generic_probe(struct platform_device *pdev, + struct ufs_qcom_phy *common_cfg, + const struct phy_ops *ufs_qcom_phy_gen_ops, + struct ufs_qcom_phy_specific_ops *phy_spec_ops) +{ + int err; + struct device *dev = >dev; + struct phy *generic_phy = NULL; + struct phy_provider *phy_provider; + + err = ufs_qcom_phy_base_init(pdev, common_cfg); + if (err) { + dev_err(dev, "%s: phy base init failed %d\n", __func__, err); + goto out; + } + + phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate); + if (IS_ERR(phy_provider)) { + err = PTR_ERR(phy_provider); + dev_err(dev, "%s: failed to register phy %d\n", __func__, err); + goto out; + } + + generic_phy = devm_phy_create(dev, NULL, ufs_qcom_phy_gen_ops); + if (IS_ERR(generic_phy)) { + err = PTR_ERR(generic_phy); + dev_err(dev, "%s: failed to create phy %d\n", __func__, err); + generic_phy = NULL; + goto out; + } + + common_cfg->phy_spec_ops = phy_spec_ops; + common_cfg->dev = dev; + +out: + return generic_phy; +} +EXPORT_SYMBOL_GPL(ufs_qcom_phy_generic_probe); + static int __ufs_qcom_phy_clk_get(struct device *dev, const char *name, struct clk **clk_out, bool err_print) { @@ -217,29 +210,6 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common) } EXPORT_SYMBOL_GPL(ufs_qcom_phy_init_clks); -int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common) -{ - int err; - - err = ufs_qcom_phy_init_vreg(phy_common->dev, _common->vdda_pll, - "vdda-pll"); - if (err) - goto out; - - err = ufs_qcom_phy_init_vreg(phy_common->dev, _common->vdda_phy, - "vdda-phy"); - - if (err) - goto out; - - /* vddp-ref-clk-* properties are
[PATCH v5 12/12] scsi/ufs: qcom: Don't free resource-managed kmalloc element
Host is allocated by managed kmalloc (devm_kmalloc). The memory allocated with this function is automatically freed on driver detach. So, no need to make an exclusive free call over it. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/scsi/ufs/ufs-qcom.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 31df783..804f4e7 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -1268,7 +1268,6 @@ static int ufs_qcom_init(struct ufs_hba *hba) out_unregister_bus: phy_exit(host->generic_phy); out_host_free: - devm_kfree(dev, host); ufshcd_set_variant(hba, NULL); out: return err; -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 09/12] ufs-qcom: phy/hcd: Refactoring phy clock handling
Add phy clock enable code to phy_power_on/off callbacks, and remove explicit calls to enable these phy clocks from the ufs-qcom hcd driver. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs.c | 36 ++-- drivers/scsi/ufs/ufs-qcom.c | 21 ++--- include/linux/phy/phy-qcom-ufs.h | 18 -- 3 files changed, 24 insertions(+), 51 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index c5c29fe..fdd9b90 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -361,10 +361,9 @@ static int ufs_qcom_phy_enable_vreg(struct device *dev, return ret; } -int ufs_qcom_phy_enable_ref_clk(struct phy *generic_phy) +static int ufs_qcom_phy_enable_ref_clk(struct ufs_qcom_phy *phy) { int ret = 0; - struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy); if (phy->is_ref_clk_enabled) goto out; @@ -411,7 +410,6 @@ int ufs_qcom_phy_enable_ref_clk(struct phy *generic_phy) out: return ret; } -EXPORT_SYMBOL_GPL(ufs_qcom_phy_enable_ref_clk); static int ufs_qcom_phy_disable_vreg(struct device *dev, struct ufs_qcom_phy_vreg *vreg) @@ -435,10 +433,8 @@ static int ufs_qcom_phy_disable_vreg(struct device *dev, return ret; } -void ufs_qcom_phy_disable_ref_clk(struct phy *generic_phy) +static void ufs_qcom_phy_disable_ref_clk(struct ufs_qcom_phy *phy) { - struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy); - if (phy->is_ref_clk_enabled) { clk_disable_unprepare(phy->ref_clk); /* @@ -451,7 +447,6 @@ 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) @@ -504,9 +499,8 @@ void ufs_qcom_phy_disable_dev_ref_clk(struct phy *generic_phy) 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) +static int ufs_qcom_phy_enable_iface_clk(struct ufs_qcom_phy *phy) { - struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy); int ret = 0; if (phy->is_iface_clk_enabled) @@ -530,20 +524,16 @@ 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) +void ufs_qcom_phy_disable_iface_clk(struct ufs_qcom_phy *phy) { - struct ufs_qcom_phy *phy = get_ufs_qcom_phy(generic_phy); - if (phy->is_iface_clk_enabled) { clk_disable_unprepare(phy->tx_iface_clk); clk_disable_unprepare(phy->rx_iface_clk); 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) { @@ -661,13 +651,20 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy) goto out_disable_phy; } - err = ufs_qcom_phy_enable_ref_clk(generic_phy); + err = ufs_qcom_phy_enable_iface_clk(phy_common); if (err) { - dev_err(dev, "%s enable phy ref clock failed, err=%d\n", + dev_err(dev, "%s enable phy iface clock failed, err=%d\n", __func__, err); goto out_disable_pll; } + err = ufs_qcom_phy_enable_ref_clk(phy_common); + if (err) { + dev_err(dev, "%s enable phy ref clock failed, err=%d\n", + __func__, err); + goto out_disable_iface_clk; + } + /* enable device PHY ref_clk pad rail */ if (phy_common->vddp_ref_clk.reg) { err = ufs_qcom_phy_enable_vreg(dev, @@ -683,7 +680,9 @@ int ufs_qcom_phy_power_on(struct phy *generic_phy) goto out; out_disable_ref_clk: - ufs_qcom_phy_disable_ref_clk(generic_phy); + ufs_qcom_phy_disable_ref_clk(phy_common); +out_disable_iface_clk: + ufs_qcom_phy_disable_iface_clk(phy_common); out_disable_pll: ufs_qcom_phy_disable_vreg(dev, _common->vdda_pll); out_disable_phy: @@ -702,7 +701,8 @@ int ufs_qcom_phy_power_off(struct phy *generic_phy) if (phy_common->vddp_ref_clk.reg) ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vddp_ref_clk); - ufs_qcom_phy_disable_ref_clk(generic_phy); + ufs_qcom_phy_disable_ref_clk(phy_common); + ufs_qcom_phy_disable_iface_clk(phy_common); ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_pll); ufs_qcom_phy_disable_vreg(phy_common->dev, _common->vdda_phy); diff --git a/drivers/scsi/ufs/ufs-qcom.c
[PATCH v5 11/12] scsi/ufs: qcom: Add phy_exit call in hcd exit path
Do a phy_exit() over the ufs phy in the ufs qcom exit path to de-initialize the phy. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/scsi/ufs/ufs-qcom.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/scsi/ufs/ufs-qcom.c b/drivers/scsi/ufs/ufs-qcom.c index 5f70a35..31df783 100644 --- a/drivers/scsi/ufs/ufs-qcom.c +++ b/drivers/scsi/ufs/ufs-qcom.c @@ -1280,6 +1280,7 @@ static void ufs_qcom_exit(struct ufs_hba *hba) ufs_qcom_disable_lane_clks(host); phy_power_off(host->generic_phy); + phy_exit(host->generic_phy); } static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba, -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 03/12] phy: qcom-ufs: Cleanup clock and regulator initialization
Different menthods pass around generic phy pointer to extract device pointer. Instead, pass the device pointer directly between function calls. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs-i.h| 6 +-- drivers/phy/phy-qcom-ufs-qmp-14nm.c | 4 +- drivers/phy/phy-qcom-ufs-qmp-20nm.c | 4 +- drivers/phy/phy-qcom-ufs.c | 80 ++--- 4 files changed, 37 insertions(+), 57 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs-i.h b/drivers/phy/phy-qcom-ufs-i.h index 2bd5ce4..69e836d 100644 --- a/drivers/phy/phy-qcom-ufs-i.h +++ b/drivers/phy/phy-qcom-ufs-i.h @@ -142,10 +142,8 @@ struct ufs_qcom_phy_specific_ops { int ufs_qcom_phy_power_on(struct phy *generic_phy); int ufs_qcom_phy_power_off(struct phy *generic_phy); int ufs_qcom_phy_exit(struct phy *generic_phy); -int ufs_qcom_phy_init_clks(struct phy *generic_phy, - struct ufs_qcom_phy *phy_common); -int ufs_qcom_phy_init_vregulators(struct phy *generic_phy, - struct ufs_qcom_phy *phy_common); +int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common); +int ufs_qcom_phy_init_vregulators(struct ufs_qcom_phy *phy_common); int ufs_qcom_phy_remove(struct phy *generic_phy, struct ufs_qcom_phy *ufs_qcom_phy); struct phy *ufs_qcom_phy_generic_probe(struct platform_device *pdev, diff --git a/drivers/phy/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/phy-qcom-ufs-qmp-14nm.c index 6ee5149..e3bede7 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-14nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-14nm.c @@ -48,14 +48,14 @@ static int ufs_qcom_phy_qmp_14nm_init(struct phy *generic_phy) struct ufs_qcom_phy *phy_common = >common_cfg; int err; - err = ufs_qcom_phy_init_clks(generic_phy, phy_common); + err = ufs_qcom_phy_init_clks(phy_common); if (err) { dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_clks() failed %d\n", __func__, err); goto out; } - err = ufs_qcom_phy_init_vregulators(generic_phy, phy_common); + err = ufs_qcom_phy_init_vregulators(phy_common); if (err) { dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_vregulators() failed %d\n", __func__, err); diff --git a/drivers/phy/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/phy-qcom-ufs-qmp-20nm.c index 770087a..e09ecb8c 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-20nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-20nm.c @@ -67,14 +67,14 @@ static int ufs_qcom_phy_qmp_20nm_init(struct phy *generic_phy) struct ufs_qcom_phy *phy_common = >common_cfg; int err = 0; - err = ufs_qcom_phy_init_clks(generic_phy, phy_common); + err = ufs_qcom_phy_init_clks(phy_common); if (err) { dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_clks() failed %d\n", __func__, err); goto out; } - err = ufs_qcom_phy_init_vregulators(generic_phy, phy_common); + err = ufs_qcom_phy_init_vregulators(phy_common); if (err) { dev_err(phy_common->dev, "%s: ufs_qcom_phy_init_vregulators() failed %d\n", __func__, err); diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index 5dc24d8..b0171fe 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -22,9 +22,9 @@ #define VDDP_REF_CLK_MIN_UV120 #define VDDP_REF_CLK_MAX_UV120 -static int __ufs_qcom_phy_init_vreg(struct phy *, struct ufs_qcom_phy_vreg *, +static int __ufs_qcom_phy_init_vreg(struct device *, struct ufs_qcom_phy_vreg *, const char *, bool); -static int ufs_qcom_phy_init_vreg(struct phy *, struct ufs_qcom_phy_vreg *, +static int ufs_qcom_phy_init_vreg(struct device *, struct ufs_qcom_phy_vreg *, const char *); static int ufs_qcom_phy_base_init(struct platform_device *pdev, struct ufs_qcom_phy *phy_common); @@ -154,13 +154,11 @@ int ufs_qcom_phy_base_init(struct platform_device *pdev, return 0; } -static int __ufs_qcom_phy_clk_get(struct phy *phy, +static int __ufs_qcom_phy_clk_get(struct device *dev, const char *name, struct clk **clk_out, bool err_print) { struct clk *clk; int err = 0; - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(phy); - struct device *dev = ufs_qcom_phy->dev; clk = devm_clk_get(dev, name); if (IS_ERR(clk)) { @@ -174,30 +172,27 @@ static int __ufs_qcom_phy_clk_get(struct phy *phy, return err; } -static -int ufs_qcom_phy_clk_get(struct phy *phy, +static int ufs_qcom_phy_clk_get(struct device *dev, const char *name, struct clk **clk_out) { - return __ufs_qcom_phy_clk_get(phy,
[PATCH v5 04/12] phy: qcom-ufs-14nm: Add new compatible for msm8996 based phy
Add a new compatible string for 14nm ufs phy present on msm8996 chipset. This phy is bit different from the legacy 14nm ufs phy in terms of the clocks that are needed to be handled in the driver. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- Documentation/devicetree/bindings/ufs/ufs-qcom.txt | 7 +-- drivers/phy/phy-qcom-ufs-qmp-14nm.c| 1 + 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/ufs/ufs-qcom.txt b/Documentation/devicetree/bindings/ufs/ufs-qcom.txt index 070baf4..b6b5130 100644 --- a/Documentation/devicetree/bindings/ufs/ufs-qcom.txt +++ b/Documentation/devicetree/bindings/ufs/ufs-qcom.txt @@ -7,8 +7,11 @@ 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. +- compatible: compatible list, contains one of the following - + "qcom,ufs-phy-qmp-20nm" for 20nm ufs phy, + "qcom,ufs-phy-qmp-14nm" for legacy 14nm ufs phy, + "qcom,msm8996-ufs-phy-qmp-14nm" for 14nm ufs phy +present on MSM8996 chipset. - 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". diff --git a/drivers/phy/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/phy-qcom-ufs-qmp-14nm.c index e3bede7..b3d2612 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-14nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-14nm.c @@ -180,6 +180,7 @@ static int ufs_qcom_phy_qmp_14nm_remove(struct platform_device *pdev) static const struct of_device_id ufs_qcom_phy_qmp_14nm_of_match[] = { {.compatible = "qcom,ufs-phy-qmp-14nm"}, + {.compatible = "qcom,msm8996-ufs-phy-qmp-14nm"}, {}, }; MODULE_DEVICE_TABLE(of, ufs_qcom_phy_qmp_14nm_of_match); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 02/12] phy: qcom-ufs: Use devm sibling of kstrdup for regulator names
This helps us in avoiding any requirement for kfree() operation to be called exclusively over the allocated string pointer. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index 455064c..5dc24d8 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -251,7 +251,7 @@ static int __ufs_qcom_phy_init_vreg(struct phy *phy, char prop_name[MAX_PROP_NAME]; - vreg->name = kstrdup(name, GFP_KERNEL); + vreg->name = devm_kstrdup(dev, name, GFP_KERNEL); if (!vreg->name) { err = -ENOMEM; goto out; @@ -637,9 +637,6 @@ int ufs_qcom_phy_remove(struct phy *generic_phy, { phy_power_off(generic_phy); - kfree(ufs_qcom_phy->vdda_pll.name); - kfree(ufs_qcom_phy->vdda_phy.name); - return 0; } EXPORT_SYMBOL_GPL(ufs_qcom_phy_remove); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 05/12] phy: qcom-ufs: Skip obtaining rx/tx_iface_clk for msm8996 based phy
The tx_iface_clk and rx_iface_clk no longer exist with UFS Phy present on msm8996. So skip obtaining these clocks using compatible match. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index b0171fe..3fa7b07 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -182,6 +182,10 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common) { int err; + if (of_device_is_compatible(phy_common->dev->of_node, + "qcom,msm8996-ufs-phy-qmp-14nm")) + goto skip_txrx_clk; + err = ufs_qcom_phy_clk_get(phy_common->dev, "tx_iface_clk", _common->tx_iface_clk); if (err) @@ -197,6 +201,7 @@ int ufs_qcom_phy_init_clks(struct ufs_qcom_phy *phy_common) if (err) goto out; +skip_txrx_clk: /* * "ref_clk_parent" is optional hence don't abort init if it's not * found. -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 01/12] phy: qcom-ufs: Remove unnecessary BUG_ON
BUG_ON() are not preferred in the driver, plus the variable on which BUG_ON is asserted is already checked in the code before passing. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index 18a5b49..455064c 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -322,8 +322,6 @@ int ufs_qcom_phy_cfg_vreg(struct phy *phy, struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(phy); struct device *dev = ufs_qcom_phy->dev; - BUG_ON(!vreg); - if (regulator_count_voltages(reg) > 0) { min_uV = on ? vreg->min_uV : 0; ret = regulator_set_voltage(reg, min_uV, vreg->max_uV); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 06/12] phy: qcom-ufs-qmp-xx: Discard remove callback for drivers.
remove() callback does a phy_power_off() only over the phy, and nothing else now. The phy_power_off() over the generic phy is called from the phy consumer, and phy provider driver should not explicitly need to call any phy ops. So discard the remove callback for qcom-ufs phy platform drivers. Signed-off-by: Vivek GautamReviewed-by: Subhash Jadavani --- drivers/phy/phy-qcom-ufs-qmp-14nm.c | 16 drivers/phy/phy-qcom-ufs-qmp-20nm.c | 16 drivers/phy/phy-qcom-ufs.c | 9 - 3 files changed, 41 deletions(-) diff --git a/drivers/phy/phy-qcom-ufs-qmp-14nm.c b/drivers/phy/phy-qcom-ufs-qmp-14nm.c index b3d2612..560f272 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-14nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-14nm.c @@ -163,21 +163,6 @@ static int ufs_qcom_phy_qmp_14nm_probe(struct platform_device *pdev) return err; } -static int ufs_qcom_phy_qmp_14nm_remove(struct platform_device *pdev) -{ - struct device *dev = >dev; - struct phy *generic_phy = to_phy(dev); - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy); - int err = 0; - - err = ufs_qcom_phy_remove(generic_phy, ufs_qcom_phy); - if (err) - dev_err(dev, "%s: ufs_qcom_phy_remove failed = %d\n", - __func__, err); - - return err; -} - static const struct of_device_id ufs_qcom_phy_qmp_14nm_of_match[] = { {.compatible = "qcom,ufs-phy-qmp-14nm"}, {.compatible = "qcom,msm8996-ufs-phy-qmp-14nm"}, @@ -187,7 +172,6 @@ static int ufs_qcom_phy_qmp_14nm_remove(struct platform_device *pdev) static struct platform_driver ufs_qcom_phy_qmp_14nm_driver = { .probe = ufs_qcom_phy_qmp_14nm_probe, - .remove = ufs_qcom_phy_qmp_14nm_remove, .driver = { .of_match_table = ufs_qcom_phy_qmp_14nm_of_match, .name = "ufs_qcom_phy_qmp_14nm", diff --git a/drivers/phy/phy-qcom-ufs-qmp-20nm.c b/drivers/phy/phy-qcom-ufs-qmp-20nm.c index e09ecb8c..9a2f53d 100644 --- a/drivers/phy/phy-qcom-ufs-qmp-20nm.c +++ b/drivers/phy/phy-qcom-ufs-qmp-20nm.c @@ -219,21 +219,6 @@ static int ufs_qcom_phy_qmp_20nm_probe(struct platform_device *pdev) return err; } -static int ufs_qcom_phy_qmp_20nm_remove(struct platform_device *pdev) -{ - struct device *dev = >dev; - struct phy *generic_phy = to_phy(dev); - struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy); - int err = 0; - - err = ufs_qcom_phy_remove(generic_phy, ufs_qcom_phy); - if (err) - dev_err(dev, "%s: ufs_qcom_phy_remove failed = %d\n", - __func__, err); - - return err; -} - static const struct of_device_id ufs_qcom_phy_qmp_20nm_of_match[] = { {.compatible = "qcom,ufs-phy-qmp-20nm"}, {}, @@ -242,7 +227,6 @@ static int ufs_qcom_phy_qmp_20nm_remove(struct platform_device *pdev) static struct platform_driver ufs_qcom_phy_qmp_20nm_driver = { .probe = ufs_qcom_phy_qmp_20nm_probe, - .remove = ufs_qcom_phy_qmp_20nm_remove, .driver = { .of_match_table = ufs_qcom_phy_qmp_20nm_of_match, .name = "ufs_qcom_phy_qmp_20nm", diff --git a/drivers/phy/phy-qcom-ufs.c b/drivers/phy/phy-qcom-ufs.c index 3fa7b07..b85f882 100644 --- a/drivers/phy/phy-qcom-ufs.c +++ b/drivers/phy/phy-qcom-ufs.c @@ -619,15 +619,6 @@ int ufs_qcom_phy_calibrate_phy(struct phy *generic_phy, bool is_rate_B) } EXPORT_SYMBOL_GPL(ufs_qcom_phy_calibrate_phy); -int ufs_qcom_phy_remove(struct phy *generic_phy, - struct ufs_qcom_phy *ufs_qcom_phy) -{ - phy_power_off(generic_phy); - - return 0; -} -EXPORT_SYMBOL_GPL(ufs_qcom_phy_remove); - int ufs_qcom_phy_exit(struct phy *generic_phy) { struct ufs_qcom_phy *ufs_qcom_phy = get_ufs_qcom_phy(generic_phy); -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v5 00/12] ufs-qcom: phy/hcd: Clean up qcom-ufs phy and ufs-qcom hcd
Hi Martin, Here's the rebased version of patches based on 4.10/scsi-queue branch as requested. The patches can now be applied and pulled-in. Thanks These patches cleanup the ufs phy driver to an extent. Subsequent patches will target to clean the phy_init() of these qcom-ufs phy drivers in order to get rid of a number of exported APIs that phy drivers expose for ufs-qcom hcd driver to use. These patches are based on linux-phy next branch, and have been tested with on db820c hardware with integration branch - 'integration-linux-qcomlt' of qualcomm linaro lt tree [1]. Changes since v4: - Rebased on top of 4.10/scsi-queue branch. Changes since v3: - Addressed review comment to move phy_power_off() under *link not active* check during aggressive clock gating from ufs hcd driver (in ufs_qcom_setup_clocks() API). Changes since v2: 1) Addressed review comment for the patch making tx/rx_iface clocks as optional. Added a new compatible string for 14nm ufs phy present on msm8996 chips, and skipping the tx/r_iface clock fetching based on this compatible string. 2) Added phy_power_off() and phy_power_on() calls in setup_clock() callback of ufs-qcom platform driver. This is to follow the turning off of the clocks during aggressive clock gating. 3) Addressed review comment for fixing commit message for patch: phy: qcom-ufs-qmp-xx: Discard remove callback for drivers. 4) Added a patch to remove the call to devm_free() for resource allocated through devm_kzalloc(). Changes since v1: 1) Added a patch to the series to remove following unnecessary function declarations by moving the code: - __ufs_qcom_phy_init_vreg(), - ufs_qcom_phy_init_vreg(), - ufs_qcom_phy_base_init() 2) Cleaned up following functions further for patch [2]: - ufs_qcom_phy_enable(/disable)_ref_clk() - ufs_qcom_phy_enable(/disable)_iface_clk() 3) Added patch to add phy_exit() call to ufs-qcom exit path. 4) Added a patch to remove ufs_qcom_phy_exit() from 'phy-qcom-ufs' driver, since this api just powers off the phy. [1] https://git.linaro.org/landing-teams/working/qualcomm/kernel.git [2] ufs-qcom: phy/hcd: Refactoring phy clock handling Vivek Gautam (12): phy: qcom-ufs: Remove unnecessary BUG_ON phy: qcom-ufs: Use devm sibling of kstrdup for regulator names phy: qcom-ufs: Cleanup clock and regulator initialization phy: qcom-ufs-14nm: Add new compatible for msm8996 based phy phy: qcom-ufs: Skip obtaining rx/tx_iface_clk for msm8996 based phy phy: qcom-ufs-qmp-xx: Discard remove callback for drivers. phy: qcom-ufs: Remove unnecessary function declarations phy: qcom-ufs-qmp-xx: Move clock and regulator init out of phy init ufs-qcom: phy/hcd: Refactoring phy clock handling phy: qcom-ufs: Remove common layer phy exit callback scsi/ufs: qcom: Add phy_exit call in hcd exit path scsi/ufs: qcom: Don't free resource-managed kmalloc element Documentation/devicetree/bindings/ufs/ufs-qcom.txt | 7 +- drivers/phy/phy-qcom-ufs-i.h | 7 +- drivers/phy/phy-qcom-ufs-qmp-14nm.c| 72 +++--- drivers/phy/phy-qcom-ufs-qmp-20nm.c| 65 ++--- drivers/phy/phy-qcom-ufs.c | 273 + drivers/scsi/ufs/ufs-qcom.c| 23 +- include/linux/phy/phy-qcom-ufs.h | 18 -- 7 files changed, 186 insertions(+), 279 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project -- 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
[PATCH v2 2/6] qed: Add iSCSI out of order packet handling.
From: Yuval MintzThis patch adds out of order packet handling for hardware offloaded iSCSI. Out of order packet handling requires driver buffer allocation and assistance. Signed-off-by: Arun Easi Signed-off-by: Yuval Mintz --- drivers/net/ethernet/qlogic/qed/Makefile | 2 +- drivers/net/ethernet/qlogic/qed/qed.h | 1 + drivers/net/ethernet/qlogic/qed/qed_dev.c | 14 +- drivers/net/ethernet/qlogic/qed/qed_ll2.c | 503 - drivers/net/ethernet/qlogic/qed/qed_ll2.h | 9 + drivers/net/ethernet/qlogic/qed/qed_ooo.c | 501 drivers/net/ethernet/qlogic/qed/qed_ooo.h | 173 ++ drivers/net/ethernet/qlogic/qed/qed_roce.c | 1 + drivers/net/ethernet/qlogic/qed/qed_spq.c | 9 + 9 files changed, 1201 insertions(+), 12 deletions(-) create mode 100644 drivers/net/ethernet/qlogic/qed/qed_ooo.c create mode 100644 drivers/net/ethernet/qlogic/qed/qed_ooo.h diff --git a/drivers/net/ethernet/qlogic/qed/Makefile b/drivers/net/ethernet/qlogic/qed/Makefile index 597e15c..729e437 100644 --- a/drivers/net/ethernet/qlogic/qed/Makefile +++ b/drivers/net/ethernet/qlogic/qed/Makefile @@ -6,4 +6,4 @@ qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o qed_init_ops.o \ qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o qed-$(CONFIG_QED_LL2) += qed_ll2.o qed-$(CONFIG_QED_RDMA) += qed_roce.o -qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o +qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o qed_ooo.o diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h index 15286c1..85c2c6a 100644 --- a/drivers/net/ethernet/qlogic/qed/qed.h +++ b/drivers/net/ethernet/qlogic/qed/qed.h @@ -392,6 +392,7 @@ struct qed_hwfn { /* Protocol related */ boolusing_ll2; struct qed_ll2_info *p_ll2_info; + struct qed_ooo_info *p_ooo_info; struct qed_rdma_info*p_rdma_info; struct qed_iscsi_info *p_iscsi_info; struct qed_pf_paramspf_params; diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index e3612be..896429c 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -32,6 +32,7 @@ #include "qed_iscsi.h" #include "qed_ll2.h" #include "qed_mcp.h" +#include "qed_ooo.h" #include "qed_reg_addr.h" #include "qed_sp.h" #include "qed_sriov.h" @@ -156,8 +157,10 @@ void qed_resc_free(struct qed_dev *cdev) #ifdef CONFIG_QED_LL2 qed_ll2_free(p_hwfn, p_hwfn->p_ll2_info); #endif - if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) + if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) { qed_iscsi_free(p_hwfn, p_hwfn->p_iscsi_info); + qed_ooo_free(p_hwfn, p_hwfn->p_ooo_info); + } qed_iov_free(p_hwfn); qed_dmae_info_free(p_hwfn); qed_dcbx_info_free(p_hwfn, p_hwfn->p_dcbx_info); @@ -415,6 +418,7 @@ int qed_qm_reconf(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt) int qed_resc_alloc(struct qed_dev *cdev) { struct qed_iscsi_info *p_iscsi_info; + struct qed_ooo_info *p_ooo_info; #ifdef CONFIG_QED_LL2 struct qed_ll2_info *p_ll2_info; #endif @@ -542,6 +546,10 @@ int qed_resc_alloc(struct qed_dev *cdev) if (!p_iscsi_info) goto alloc_no_mem; p_hwfn->p_iscsi_info = p_iscsi_info; + p_ooo_info = qed_ooo_alloc(p_hwfn); + if (!p_ooo_info) + goto alloc_no_mem; + p_hwfn->p_ooo_info = p_ooo_info; } /* DMA info initialization */ @@ -596,8 +604,10 @@ void qed_resc_setup(struct qed_dev *cdev) if (p_hwfn->using_ll2) qed_ll2_setup(p_hwfn, p_hwfn->p_ll2_info); #endif - if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) + if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) { qed_iscsi_setup(p_hwfn, p_hwfn->p_iscsi_info); + qed_ooo_setup(p_hwfn, p_hwfn->p_ooo_info); + } } } diff --git a/drivers/net/ethernet/qlogic/qed/qed_ll2.c b/drivers/net/ethernet/qlogic/qed/qed_ll2.c index e5da1b2..c7a9bbd 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_ll2.c +++ b/drivers/net/ethernet/qlogic/qed/qed_ll2.c @@ -36,6 +36,7 @@ #include "qed_int.h" #include "qed_ll2.h" #include "qed_mcp.h" +#include "qed_ooo.h" #include "qed_reg_addr.h" #include "qed_sp.h" #include "qed_roce.h" @@ -540,13 +541,455 @@ static void qed_ll2_rxq_flush(struct qed_hwfn *p_hwfn, u8 connection_handle) list_move_tail(_pkt->list_entry, _rx->free_descq); - rx_buf_addr
[PATCH v2 3/6] qedi: Add QLogic FastLinQ offload iSCSI driver framework.
The QLogic FastLinQ Driver for iSCSI (qedi) is the iSCSI specific module for 41000 Series Converged Network Adapters by QLogic. This patch consists of following changes: - MAINTAINERS Makefile and Kconfig changes for qedi, - PCI driver registration, - iSCSI host level initialization, - Debugfs and log level infrastructure. Signed-off-by: Nilesh JavaliSigned-off-by: Adheer Chandravanshi Signed-off-by: Chad Dupuis Signed-off-by: Saurav Kashyap Signed-off-by: Arun Easi Signed-off-by: Manish Rangankar --- MAINTAINERS |6 + drivers/net/ethernet/qlogic/Kconfig | 12 - drivers/scsi/Kconfig|1 + drivers/scsi/Makefile |1 + drivers/scsi/qedi/Kconfig | 10 + drivers/scsi/qedi/Makefile |5 + drivers/scsi/qedi/qedi.h| 291 +++ drivers/scsi/qedi/qedi_dbg.c| 143 drivers/scsi/qedi/qedi_dbg.h| 144 drivers/scsi/qedi/qedi_debugfs.c| 244 ++ drivers/scsi/qedi/qedi_hsi.h| 52 ++ drivers/scsi/qedi/qedi_main.c | 1616 +++ drivers/scsi/qedi/qedi_sysfs.c | 52 ++ drivers/scsi/qedi/qedi_version.h| 14 + 14 files changed, 2579 insertions(+), 12 deletions(-) create mode 100644 drivers/scsi/qedi/Kconfig create mode 100644 drivers/scsi/qedi/Makefile create mode 100644 drivers/scsi/qedi/qedi.h create mode 100644 drivers/scsi/qedi/qedi_dbg.c create mode 100644 drivers/scsi/qedi/qedi_dbg.h create mode 100644 drivers/scsi/qedi/qedi_debugfs.c create mode 100644 drivers/scsi/qedi/qedi_hsi.h create mode 100644 drivers/scsi/qedi/qedi_main.c create mode 100644 drivers/scsi/qedi/qedi_sysfs.c create mode 100644 drivers/scsi/qedi/qedi_version.h diff --git a/MAINTAINERS b/MAINTAINERS index e5c17a9..04eec14 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -9934,6 +9934,12 @@ F: drivers/net/ethernet/qlogic/qed/ F: include/linux/qed/ F: drivers/net/ethernet/qlogic/qede/ +QLOGIC QL41xxx ISCSI DRIVER +M: qlogic-storage-upstr...@cavium.com +L: linux-scsi@vger.kernel.org +S: Supported +F: drivers/scsi/qedi/ + QNX4 FILESYSTEM M: Anders Larsen W: http://www.alarsen.net/linux/qnx4fs/ diff --git a/drivers/net/ethernet/qlogic/Kconfig b/drivers/net/ethernet/qlogic/Kconfig index 2832570..3cfd105 100644 --- a/drivers/net/ethernet/qlogic/Kconfig +++ b/drivers/net/ethernet/qlogic/Kconfig @@ -113,16 +113,4 @@ config QED_RDMA config QED_ISCSI bool -config QEDI - tristate "QLogic QED 25/40/100Gb iSCSI driver" - depends on QED - select QED_LL2 - select QED_ISCSI - default n - ---help--- - This provides a temporary node that allows the compilation - and logical testing of the hardware offload iSCSI support - for QLogic QED. This would be replaced by the 'real' option - once the QEDI driver is added [+relocated]. - endif # NET_VENDOR_QLOGIC diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 3e2bdb9..5cf03db 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -1254,6 +1254,7 @@ config SCSI_QLOGICPTI source "drivers/scsi/qla2xxx/Kconfig" source "drivers/scsi/qla4xxx/Kconfig" +source "drivers/scsi/qedi/Kconfig" config SCSI_LPFC tristate "Emulex LightPulse Fibre Channel Support" diff --git a/drivers/scsi/Makefile b/drivers/scsi/Makefile index 38d938d..da9e312 100644 --- a/drivers/scsi/Makefile +++ b/drivers/scsi/Makefile @@ -132,6 +132,7 @@ obj-$(CONFIG_PS3_ROM) += ps3rom.o obj-$(CONFIG_SCSI_CXGB3_ISCSI) += libiscsi.o libiscsi_tcp.o cxgbi/ obj-$(CONFIG_SCSI_CXGB4_ISCSI) += libiscsi.o libiscsi_tcp.o cxgbi/ obj-$(CONFIG_SCSI_BNX2_ISCSI) += libiscsi.o bnx2i/ +obj-$(CONFIG_QEDI) += libiscsi.o qedi/ obj-$(CONFIG_BE2ISCSI) += libiscsi.o be2iscsi/ obj-$(CONFIG_SCSI_ESAS2R) += esas2r/ obj-$(CONFIG_SCSI_PMCRAID) += pmcraid.o diff --git a/drivers/scsi/qedi/Kconfig b/drivers/scsi/qedi/Kconfig new file mode 100644 index 000..23ca8a2 --- /dev/null +++ b/drivers/scsi/qedi/Kconfig @@ -0,0 +1,10 @@ +config QEDI + tristate "QLogic QEDI 25/40/100Gb iSCSI Initiator Driver Support" + depends on PCI && SCSI + depends on QED + select SCSI_ISCSI_ATTRS + select QED_LL2 + select QED_ISCSI + ---help--- + This driver supports iSCSI offload for the QLogic FastLinQ + 41000 Series Converged Network Adapters. diff --git a/drivers/scsi/qedi/Makefile b/drivers/scsi/qedi/Makefile new file mode 100644 index 000..2b3e16b --- /dev/null +++ b/drivers/scsi/qedi/Makefile @@ -0,0 +1,5 @@ +obj-$(CONFIG_QEDI) := qedi.o +qedi-y := qedi_main.o qedi_iscsi.o qedi_fw.o qedi_sysfs.o \ + qedi_dbg.o + +qedi-$(CONFIG_DEBUG_FS) +=
[PATCH v2 4/6] qedi: Add LL2 iSCSI interface for offload iSCSI.
This patch adds support for iscsiuio interface using Light L2 (LL2) qed interface. Signed-off-by: Nilesh JavaliSigned-off-by: Adheer Chandravanshi Signed-off-by: Chad Dupuis Signed-off-by: Saurav Kashyap Signed-off-by: Arun Easi Signed-off-by: Manish Rangankar --- drivers/scsi/qedi/qedi.h | 73 + drivers/scsi/qedi/qedi_main.c | 357 ++ 2 files changed, 430 insertions(+) diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h index 92136a3..e00c141 100644 --- a/drivers/scsi/qedi/qedi.h +++ b/drivers/scsi/qedi/qedi.h @@ -21,6 +21,7 @@ #include #include "qedi_dbg.h" #include +#include #include "qedi_version.h" #define QEDI_MODULE_NAME "qedi" @@ -54,6 +55,78 @@ #define QEDI_LOCAL_PORT_MAX 61024 #define QEDI_LOCAL_PORT_RANGE (QEDI_LOCAL_PORT_MAX - QEDI_LOCAL_PORT_MIN) #define QEDI_LOCAL_PORT_INVALID0x +#define TX_RX_RING 16 +#define RX_RING(TX_RX_RING - 1) +#define LL2_SINGLE_BUF_SIZE0x400 +#define QEDI_PAGE_SIZE 4096 +#define QEDI_PAGE_ALIGN(addr) ALIGN(addr, QEDI_PAGE_SIZE) +#define QEDI_PAGE_MASK (~((QEDI_PAGE_SIZE) - 1)) + +#define QEDI_PAGE_SIZE 4096 +#define QEDI_PATH_HANDLE 0xFE000UL + +struct qedi_uio_ctrl { + /* meta data */ + u32 uio_hsi_version; + + /* user writes */ + u32 host_tx_prod; + u32 host_rx_cons; + u32 host_rx_bd_cons; + u32 host_tx_pkt_len; + u32 host_rx_cons_cnt; + + /* driver writes */ + u32 hw_tx_cons; + u32 hw_rx_prod; + u32 hw_rx_bd_prod; + u32 hw_rx_prod_cnt; + + /* other */ + u8 mac_addr[6]; + u8 reserve[2]; +}; + +struct qedi_rx_bd { + u32 rx_pkt_index; + u32 rx_pkt_len; + u16 vlan_id; +}; + +#define QEDI_RX_DESC_CNT (QEDI_PAGE_SIZE / sizeof(struct qedi_rx_bd)) +#define QEDI_MAX_RX_DESC_CNT (QEDI_RX_DESC_CNT - 1) +#define QEDI_NUM_RX_BD (QEDI_RX_DESC_CNT * 1) +#define QEDI_MAX_RX_BD (QEDI_NUM_RX_BD - 1) + +#define QEDI_NEXT_RX_IDX(x)x) & (QEDI_MAX_RX_DESC_CNT)) == \ + (QEDI_MAX_RX_DESC_CNT - 1)) ? \ +(x) + 2 : (x) + 1) + +struct qedi_uio_dev { + struct uio_info qedi_uinfo; + u32 uio_dev; + struct list_headlist; + + u32 ll2_ring_size; + void*ll2_ring; + + u32 ll2_buf_size; + void*ll2_buf; + + void*rx_pkt; + void*tx_pkt; + + struct qedi_ctx *qedi; + struct pci_dev *pdev; + void*uctrl; +}; + +/* List to maintain the skb pointers */ +struct skb_work_list { + struct list_head list; + struct sk_buff *skb; + u16 vlan_id; +}; /* Queue sizes in number of elements */ #define QEDI_SQ_SIZE MAX_OUSTANDING_TASKS_PER_CON diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c index 5845dc9..ef309c6 100644 --- a/drivers/scsi/qedi/qedi_main.c +++ b/drivers/scsi/qedi/qedi_main.c @@ -45,10 +45,13 @@ static struct scsi_transport_template *qedi_scsi_transport; static struct pci_driver qedi_pci_driver; static DEFINE_PER_CPU(struct qedi_percpu_s, qedi_percpu); +static LIST_HEAD(qedi_udev_list); /* Static function declaration */ static int qedi_alloc_global_queues(struct qedi_ctx *qedi); static void qedi_free_global_queues(struct qedi_ctx *qedi); static struct qedi_cmd *qedi_get_cmd_from_tid(struct qedi_ctx *qedi, u32 tid); +static void qedi_reset_uio_rings(struct qedi_uio_dev *udev); +static void qedi_ll2_free_skbs(struct qedi_ctx *qedi); static int qedi_iscsi_event_cb(void *context, u8 fw_event_code, void *fw_handle) { @@ -113,6 +116,224 @@ static int qedi_iscsi_event_cb(void *context, u8 fw_event_code, void *fw_handle) return rval; } +static int qedi_uio_open(struct uio_info *uinfo, struct inode *inode) +{ + struct qedi_uio_dev *udev = uinfo->priv; + struct qedi_ctx *qedi = udev->qedi; + + if (!capable(CAP_NET_ADMIN)) + return -EPERM; + + if (udev->uio_dev != -1) + return -EBUSY; + + rtnl_lock(); + udev->uio_dev = iminor(inode); + qedi_reset_uio_rings(udev); + set_bit(UIO_DEV_OPENED, >flags); + rtnl_unlock(); + + return 0; +} + +static int qedi_uio_close(struct uio_info *uinfo, struct inode *inode) +{ + struct qedi_uio_dev *udev = uinfo->priv; + struct qedi_ctx *qedi = udev->qedi; + + udev->uio_dev = -1; + clear_bit(UIO_DEV_OPENED, >flags); + qedi_ll2_free_skbs(qedi); + return 0;
[PATCH v2 1/6] qed: Add support for hardware offloaded iSCSI.
From: Yuval MintzThis adds the backbone required for the various HW initalizations which are necessary for the iSCSI driver (qedi) for QLogic FastLinQ 4 line of adapters - FW notification, resource initializations, etc. Signed-off-by: Arun Easi Signed-off-by: Yuval Mintz --- drivers/net/ethernet/qlogic/Kconfig| 15 + drivers/net/ethernet/qlogic/qed/Makefile |1 + drivers/net/ethernet/qlogic/qed/qed.h |7 +- drivers/net/ethernet/qlogic/qed/qed_dev.c | 12 + drivers/net/ethernet/qlogic/qed/qed_int.h |1 - drivers/net/ethernet/qlogic/qed/qed_iscsi.c| 1276 drivers/net/ethernet/qlogic/qed/qed_iscsi.h| 52 + drivers/net/ethernet/qlogic/qed/qed_l2.c |1 - drivers/net/ethernet/qlogic/qed/qed_ll2.c |4 +- drivers/net/ethernet/qlogic/qed/qed_reg_addr.h |2 + drivers/net/ethernet/qlogic/qed/qed_spq.c | 15 + include/linux/qed/qed_if.h |2 + include/linux/qed/qed_iscsi_if.h | 229 + 13 files changed, 1613 insertions(+), 4 deletions(-) create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.c create mode 100644 drivers/net/ethernet/qlogic/qed/qed_iscsi.h create mode 100644 include/linux/qed/qed_iscsi_if.h diff --git a/drivers/net/ethernet/qlogic/Kconfig b/drivers/net/ethernet/qlogic/Kconfig index 32f2a45..2832570 100644 --- a/drivers/net/ethernet/qlogic/Kconfig +++ b/drivers/net/ethernet/qlogic/Kconfig @@ -110,4 +110,19 @@ config QEDE config QED_RDMA bool +config QED_ISCSI + bool + +config QEDI + tristate "QLogic QED 25/40/100Gb iSCSI driver" + depends on QED + select QED_LL2 + select QED_ISCSI + default n + ---help--- + This provides a temporary node that allows the compilation + and logical testing of the hardware offload iSCSI support + for QLogic QED. This would be replaced by the 'real' option + once the QEDI driver is added [+relocated]. + endif # NET_VENDOR_QLOGIC diff --git a/drivers/net/ethernet/qlogic/qed/Makefile b/drivers/net/ethernet/qlogic/qed/Makefile index 967acf3..597e15c 100644 --- a/drivers/net/ethernet/qlogic/qed/Makefile +++ b/drivers/net/ethernet/qlogic/qed/Makefile @@ -6,3 +6,4 @@ qed-y := qed_cxt.o qed_dev.o qed_hw.o qed_init_fw_funcs.o qed_init_ops.o \ qed-$(CONFIG_QED_SRIOV) += qed_sriov.o qed_vf.o qed-$(CONFIG_QED_LL2) += qed_ll2.o qed-$(CONFIG_QED_RDMA) += qed_roce.o +qed-$(CONFIG_QED_ISCSI) += qed_iscsi.o diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h index 50b8a01..15286c1 100644 --- a/drivers/net/ethernet/qlogic/qed/qed.h +++ b/drivers/net/ethernet/qlogic/qed/qed.h @@ -35,6 +35,7 @@ #define QED_WFQ_UNIT 100 +#define ISCSI_BDQ_ID(_port_id) (_port_id) #define QED_WID_SIZE(1024) #define QED_PF_DEMS_SIZE(4) @@ -392,6 +393,7 @@ struct qed_hwfn { boolusing_ll2; struct qed_ll2_info *p_ll2_info; struct qed_rdma_info*p_rdma_info; + struct qed_iscsi_info *p_iscsi_info; struct qed_pf_paramspf_params; bool b_rdma_enabled_in_prs; @@ -593,6 +595,8 @@ struct qed_dev { /* Linux specific here */ struct qede_dev*edev; struct pci_dev *pdev; + u32 flags; +#define QED_FLAG_STORAGE_STARTED (BIT(0)) int msg_enable; struct pci_params pci_params; @@ -606,6 +610,7 @@ struct qed_dev { union { struct qed_common_cb_ops*common; struct qed_eth_cb_ops *eth; + struct qed_iscsi_cb_ops *iscsi; } protocol_ops; void*ops_cookie; @@ -615,7 +620,7 @@ struct qed_dev { struct qed_cb_ll2_info *ll2; u8 ll2_mac_address[ETH_ALEN]; #endif - + DECLARE_HASHTABLE(connections, 10); const struct firmware *firmware; u32 rdma_max_sge; diff --git a/drivers/net/ethernet/qlogic/qed/qed_dev.c b/drivers/net/ethernet/qlogic/qed/qed_dev.c index 5be7b8a..e3612be 100644 --- a/drivers/net/ethernet/qlogic/qed/qed_dev.c +++ b/drivers/net/ethernet/qlogic/qed/qed_dev.c @@ -29,6 +29,7 @@ #include "qed_hw.h" #include "qed_init_ops.h" #include "qed_int.h" +#include "qed_iscsi.h" #include "qed_ll2.h" #include "qed_mcp.h" #include "qed_reg_addr.h" @@ -155,6 +156,8 @@ void qed_resc_free(struct qed_dev *cdev) #ifdef CONFIG_QED_LL2 qed_ll2_free(p_hwfn, p_hwfn->p_ll2_info); #endif + if (p_hwfn->hw_info.personality == QED_PCI_ISCSI) + qed_iscsi_free(p_hwfn, p_hwfn->p_iscsi_info); qed_iov_free(p_hwfn);
[PATCH v2 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
This patch uses the resource-managed to add the devfreq device. This function will make it easy to handle the devfreq device. - struct devfreq *devm_devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile, const char *governor_name, void *data); Cc: Vinayak HolikattiCc: James E.J. Bottomley Cc: Martin K. Petersen Cc: linux-scsi@vger.kernel.org Signed-off-by: Chanwoo Choi --- drivers/scsi/ufs/ufshcd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e491c4bda32f..e8c5ba274830 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6250,8 +6250,6 @@ void ufshcd_remove(struct ufs_hba *hba) ufshcd_hba_stop(hba, true); ufshcd_exit_clk_gating(hba); - if (ufshcd_is_clkscaling_enabled(hba)) - devfreq_remove_device(hba->devfreq); ufshcd_hba_exit(hba); } EXPORT_SYMBOL_GPL(ufshcd_remove); @@ -6579,7 +6577,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) } if (ufshcd_is_clkscaling_enabled(hba)) { - hba->devfreq = devfreq_add_device(dev, _devfreq_profile, + hba->devfreq = devm_devfreq_add_device(dev, _devfreq_profile, "simple_ondemand", NULL); if (IS_ERR(hba->devfreq)) { dev_err(hba->dev, "Unable to register with devfreq %ld\n", -- 1.9.1 -- 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
[PATCH v2 2/2] scsi: ufs: Use the resource-managed function to add devfreq device
This patch uses the resource-managed to add the devfreq device. This function will make it easy to handle the devfreq device. - struct devfreq *devm_devfreq_add_device(struct device *dev, struct devfreq_dev_profile *profile, const char *governor_name, void *data); Cc: Vinayak HolikattiCc: James E.J. Bottomley Cc: Martin K. Petersen Cc: linux-scsi@vger.kernel.org Signed-off-by: Chanwoo Choi --- drivers/scsi/ufs/ufshcd.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e491c4bda32f..e8c5ba274830 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -6250,8 +6250,6 @@ void ufshcd_remove(struct ufs_hba *hba) ufshcd_hba_stop(hba, true); ufshcd_exit_clk_gating(hba); - if (ufshcd_is_clkscaling_enabled(hba)) - devfreq_remove_device(hba->devfreq); ufshcd_hba_exit(hba); } EXPORT_SYMBOL_GPL(ufshcd_remove); @@ -6579,7 +6577,7 @@ int ufshcd_init(struct ufs_hba *hba, void __iomem *mmio_base, unsigned int irq) } if (ufshcd_is_clkscaling_enabled(hba)) { - hba->devfreq = devfreq_add_device(dev, _devfreq_profile, + hba->devfreq = devm_devfreq_add_device(dev, _devfreq_profile, "simple_ondemand", NULL); if (IS_ERR(hba->devfreq)) { dev_err(hba->dev, "Unable to register with devfreq %ld\n", -- 1.9.1 -- 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
[PATCH] libfc: fix seconds_since_last_reset miscalculation
Commit 540eb1eef 'scsi: libfc: fix seconds_since_last_reset calculation' removed the use of 'struct timespec' from fc_get_host_stats(). This broke the output of 'fcoeadm -s' after kernel 4.8-rc1 as lport->boot_time - jiffies could become negative as in this example: $ cat /sys/class/fc_host/host8/statistics/seconds_since_last_reset 0x10624dd2f1977b4 Take this into account so /sys/class/fc_host/hostX/statistics/seconds_since_last_reset is sane again. Fixes: 540eb1eef ('scsi: libfc: fix seconds_since_last_reset calculation') Signed-off-by: Johannes ThumshirnTested-by: Holger Schranz --- drivers/scsi/libfc/fc_lport.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 04ce7cf..475c0a9 100644 --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -304,11 +304,15 @@ struct fc_host_statistics *fc_get_host_stats(struct Scsi_Host *shost) unsigned int cpu; u64 fcp_in_bytes = 0; u64 fcp_out_bytes = 0; + unsigned long boot_time = lport->boot_time; fc_stats = >host_stats; memset(fc_stats, 0, sizeof(struct fc_host_statistics)); - fc_stats->seconds_since_last_reset = (lport->boot_time - jiffies) / HZ; + if (boot_time > jiffies) + fc_stats->seconds_since_last_reset = (boot_time - jiffies) / HZ; + else + fc_stats->seconds_since_last_reset = (jiffies - boot_time) / HZ; for_each_possible_cpu(cpu) { struct fc_stats *stats; -- 1.8.5.6 -- 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
[RESEND][PATCH v1] ufs: introduce setup_task_mgmt
Some UFS host controller may need to configure some things before any task management request is issued. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 1 + drivers/scsi/ufs/ufshcd.h | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index bf78321..fdb0502 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -4358,6 +4358,7 @@ static int ufshcd_issue_tm_cmd(struct ufs_hba *hba, int lun_id, int task_id, task_req_upiup->input_param2 = cpu_to_be32(task_id); /* send command to the controller */ + ufshcd_vops_setup_task_mgmt(hba, free_slot, tm_function); __set_bit(free_slot, >outstanding_tasks); /* Make sure descriptors are ready before ringing the task doorbell */ diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index f2a69c0..b084d89 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -263,6 +263,8 @@ struct ufs_pwr_mode_info { * to be set. * @setup_xfer_req: called before any transfer request is issued * to set some things + * @setup_task_mgmt: called before any task management request is issued + * to set some things * @suspend: called during host controller PM callback * @resume: called during host controller PM callback * @dbg_register_dump: used to dump controller debug information @@ -287,6 +289,7 @@ struct ufs_hba_variant_ops { struct ufs_pa_layer_attr *, struct ufs_pa_layer_attr *); void(*setup_xfer_req)(struct ufs_hba *, int, bool); + void(*setup_task_mgmt)(struct ufs_hba *, int, u8); int (*suspend)(struct ufs_hba *, enum ufs_pm_op); int (*resume)(struct ufs_hba *, enum ufs_pm_op); void(*dbg_register_dump)(struct ufs_hba *hba); @@ -811,6 +814,13 @@ static inline void ufshcd_vops_setup_xfer_req(struct ufs_hba *hba, int tag, return hba->vops->setup_xfer_req(hba, tag, is_cmd_not_null); } +static inline void ufshcd_vops_setup_task_mgmt(struct ufs_hba *hba, + int tag, u8 tm_function) +{ + if (hba->vops && hba->vops->setup_task_mgmt) + return hba->vops->setup_task_mgmt(hba, tag, tm_function); +} + static inline int ufshcd_vops_suspend(struct ufs_hba *hba, enum ufs_pm_op op) { if (hba->vops && hba->vops->suspend) -- 2.1.4 -- 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
[Bug 151631] "Synchronizing SCSI cache" fails during(and delays) reboot/shutdown
https://bugzilla.kernel.org/show_bug.cgi?id=151631 Gianpaolochanged: What|Removed |Added CC||gianpao...@gmail.com --- Comment #5 from Gianpaolo --- Created attachment 243891 --> https://bugzilla.kernel.org/attachment.cgi?id=243891=edit Patch reverting commit 2c85025c75dfe7ddc2bb33363a998dad59383f94 This patch solved bug https://bugzilla.kernel.org/show_bug.cgi?id=187061 which I suspect being a duuplicate of this bug. If someone affected by this bug could test it and confirm it works (it should work both with v4.8.6 and v4.9-rc4). -- You are receiving this mail because: You are the assignee for the bug. -- 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
[PATCH v1] ufs: add a variety of definitions decribed in UFS spec
These things are defined to be used by some UFS Host controllers. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/mphy.h | 38 ++ drivers/scsi/ufs/ufshci.h | 14 +++--- drivers/scsi/ufs/unipro.h | 24 3 files changed, 73 insertions(+), 3 deletions(-) create mode 100644 drivers/scsi/ufs/mphy.h diff --git a/drivers/scsi/ufs/mphy.h b/drivers/scsi/ufs/mphy.h new file mode 100644 index 000..c431f49 --- /dev/null +++ b/drivers/scsi/ufs/mphy.h @@ -0,0 +1,38 @@ +/* + * drivers/scsi/ufs/mphy.h + * + * Copyright (C) 2014 Samsung Electronics Co., Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + */ + +#ifndef _MPHY_H_ +#define _MPHY_H_ + +#define TX_HIBERN8TIME_CAP 0x0f +#define TX_MIN_ACTIVATE_TIME 0x33 + +#define RX_HS_G1_SYNC_LEN_CAP 0x8b +#define RX_HS_G1_PREP_LEN_CAP 0x8c +#define RX_HS_G2_SYNC_LEN_CAP 0x94 +#define RX_HS_G3_SYNC_LEN_CAP 0x95 +#define RX_HS_G2_PREP_LEN_CAP 0x96 +#define RX_HS_G3_PREP_LEN_CAP 0x97 + #define SYNC_RANGE_FINE (0 << 6) + #define SYNC_RANGE_COARSE (1 << 6) + #define SYNC_LEN(x) ((x) & 0x3f) + #define PREP_LEN(x) ((x) & 0xf) +#define RX_ADV_GRANULARITY_CAP 0x98 + #define RX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) +#define TX_ADV_GRANULARITY_CAP 0x10 + #define TX_ADV_GRAN_STEP(x) x) & 0x3) << 1) | 0x1) +#define RX_MIN_ACTIVATETIME_CAP0x8f +#define RX_HIBERN8TIME_CAP 0x92 +#define RX_ADV_HIBERN8TIME_CAP 0x99 +#define RX_ADV_MIN_ACTIVATETIME_CAP0x9a + +#endif /* _MPHY_H_ */ + diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h index 9599741..26dc340 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -170,17 +170,25 @@ enum { /* UECDL - Host UIC Error Code Data Link Layer 3Ch */ #define UIC_DATA_LINK_LAYER_ERROR UFS_BIT(31) #define UIC_DATA_LINK_LAYER_ERROR_CODE_MASK0x7FFF -#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT 0x2000 -#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED 0x0001 -#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT 0x0002 +#define UIC_DATA_LINK_LAYER_ERROR_NAC_RECEIVED UFS_BIT(0) +#define UIC_DATA_LINK_LAYER_ERROR_TCx_REPLAY_TIMEOUT UFS_BIT(1) +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5) +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT UFS_BIT(13) /* UECN - Host UIC Error Code Network Layer 40h */ #define UIC_NETWORK_LAYER_ERRORUFS_BIT(31) #define UIC_NETWORK_LAYER_ERROR_CODE_MASK 0x7 +#define UIC_NETWORK_UNSUPPORTED_HEADER_TYPEBIT(0) +#define UIC_NETWORK_BAD_DEVICEID_ENC BIT(1) +#define UIC_NETWORK_LHDR_TRAP_PACKET_DROPPING BIT(2) /* UECT - Host UIC Error Code Transport Layer 44h */ #define UIC_TRANSPORT_LAYER_ERROR UFS_BIT(31) #define UIC_TRANSPORT_LAYER_ERROR_CODE_MASK0x7F +#define UIC_TRANSPORT_UNSUPPORTED_HEADER_TYPE BIT(0) +#define UIC_TRANSPORT_UNKNOWN_CPORTID BIT(1) +#define UIC_TRANSPORT_NO_CONNECTION_RX BIT(2) +#define UIC_TRANSPORT_BAD_TC BIT(4) /* UECDME - Host UIC Error Code DME 48h */ #define UIC_DME_ERROR UFS_BIT(31) diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h index eff8b56..e47e2c2 100644 --- a/drivers/scsi/ufs/unipro.h +++ b/drivers/scsi/ufs/unipro.h @@ -127,6 +127,7 @@ #define PA_PACPREQEOBTIMEOUT 0x1591 #define PA_HIBERN8TIME 0x15A7 #define PA_LOCALVERINFO0x15A9 +#define PA_GRANULARITY 0x15AA #define PA_TACTIVATE 0x15A8 #define PA_PACPFRAMECOUNT 0x15C0 #define PA_PACPERRORCOUNT 0x15C1 @@ -170,6 +171,9 @@ enum { UNCHANGED = 7, }; +#define IS_PWR_MODE_HS(m)(((m) == FAST_MODE) || ((m) == FASTAUTO_MODE)) +#define IS_PWR_MODE_PWM(m) (((m) == SLOW_MODE) || ((m) == SLOWAUTO_MODE)) + /* PA TX/RX Frequency Series */ enum { PA_HS_MODE_A= 1, @@ -231,6 +235,11 @@ enum ufs_unipro_ver { #define DL_PEERTC1PRESENT 0x2066 #define DL_PEERTC1RXINITCREVAL 0x2067 +/* Default value of L2 Timer */ +#define FC0PROTTIMEOUTVAL 8191 +#define TC0REPLAYTIMEOUTVAL65535 +#define AFC0REQTIMEOUTVAL 32767 + /* * Network Layer Attributes */ @@ -259,6 +268,21 @@ enum ufs_unipro_ver { #define T_TC0TXMAXSDUSIZE 0x4060 #define T_TC1TXMAXSDUSIZE 0x4061 +/* CPort setting */ +#define E2EFC_ON (1 << 0) +#define E2EFC_OFF (0 << 0) +#define CSD_N_ON (0 << 1) +#define CSD_N_OFF (1 << 1) +#define CSV_N_ON (0 << 2) +#define CSV_N_OFF (1 << 2) +#define CPORT_DEF_FLAGS(CSV_N_OFF | CSD_N_OFF | E2EFC_OFF) + +/* CPort connection state */
[PATCH v1] ufs: introduce UFSHCI_QUIRK_SKIP_INTR_AGGR quirk
If UFS driver resets interrupt aggregation timer and counter when there is a pending doorbell, an interrupt of IO completion of a corresponding task may be missed. That would cause a command timeout. If UFS driver resets interrupt aggregation timer and counter when there is a pending doorbell, a competion interrupt of a corresponing task may be issued. That would casue a command timeout. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 4 +++- drivers/scsi/ufs/ufshcd.h | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c904854..65bbf59 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3730,7 +3730,9 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba) * false interrupt if device completes another request after resetting * aggregation and before reading the DB. */ - if (ufshcd_is_intr_aggr_allowed(hba)) + if ((ufshcd_is_intr_aggr_allowed(hba)) + && !(hba->quirks & UFSHCI_QUIRK_SKIP_INTR_AGGR)) + ufshcd_reset_intr_aggr(hba); ufshcd_reset_intr_aggr(hba); tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index 6a96f24..6a9c6e9 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -497,6 +497,7 @@ struct ufs_hba { #define UFSHCD_QUIRK_BROKEN_DWORD_UTRD UFS_BIT(7) #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) #define UFSHCD_QUIRK_USE_OF_HCE UFS_BIT(9) + #define UFSHCI_QUIRK_SKIP_INTR_AGGR UFS_BIT(10) unsigned int quirks;/* Deviations from standard UFSHCI spec. */ -- 2.1.4 -- 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