Re: [PATCH] aacraid: switch to pci_alloc_irq_vectors
On 11/15/2016 08:33 PM, Raghava Aditya Renukunta wrote: > Hi Hannes, > Thank you for the patch > >> -Original Message- >> From: Hannes Reinecke [mailto:h...@suse.de] >> Sent: Monday, November 7, 2016 11:12 PM >> To: Martin K. Petersen >> Cc: Christoph Hellwig; James Bottomley; linux-scsi@vger.kernel.org; Hannes >> Reinecke; Hannes Reinecke; dl-esc-Aacraid Linux Driver >> Subject: [PATCH] aacraid: switch to pci_alloc_irq_vectors >> >> EXTERNAL EMAIL >> >> >> Use pci_alloc_irq_vectors and drop the hand-crafted >> interrupt affinity routines. >> >> Signed-off-by: Hannes Reinecke>> Cc: Adaptec OEM Raid Solutions >> --- >> drivers/scsi/aacraid/aacraid.h | 1 - >> drivers/scsi/aacraid/comminit.c | 10 +++--- >> drivers/scsi/aacraid/commsup.c | 25 + >> drivers/scsi/aacraid/linit.c| 20 >> 4 files changed, 12 insertions(+), 44 deletions(-) >> >> diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h >> index 969c312..f059c14 100644 >> --- a/drivers/scsi/aacraid/aacraid.h >> +++ b/drivers/scsi/aacraid/aacraid.h >> @@ -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 */ >> u8 adapter_shutdown; >> u32 handle_pci_error; >> diff --git a/drivers/scsi/aacraid/comminit.c >> b/drivers/scsi/aacraid/comminit.c >> index 341ea32..4f56b10 100644 >> --- a/drivers/scsi/aacraid/comminit.c >> +++ b/drivers/scsi/aacraid/comminit.c >> @@ -378,16 +378,12 @@ void aac_define_int_mode(struct aac_dev *dev) >> if (msi_count > AAC_MAX_MSIX) >> msi_count = AAC_MAX_MSIX; >> >> - for (i = 0; i < msi_count; i++) >> - dev->msixentry[i].entry = i; >> - >> if (msi_count > 1 && >> pci_find_capability(dev->pdev, PCI_CAP_ID_MSIX)) { >> min_msix = 2; >> - i = pci_enable_msix_range(dev->pdev, >> - dev->msixentry, >> - min_msix, >> - msi_count); >> + i = pci_alloc_irq_vectors(dev->pdev, >> + min_msix, msi_count, >> + PCI_IRQ_MSIX | PCI_IRQ_AFFINITY); >> if (i > 0) { >> dev->msi_enabled = 1; >> msi_count = i; >> diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c >> index 0aeecec..9e7551f 100644 >> --- a/drivers/scsi/aacraid/commsup.c >> +++ b/drivers/scsi/aacraid/commsup.c >> @@ -2043,30 +2043,22 @@ int aac_acquire_irq(struct aac_dev *dev) >> int i; >> int j; >> int ret = 0; >> - int cpu; >> >> - cpu = cpumask_first(cpu_online_mask); >> if (!dev->sync_mode && dev->msi_enabled && dev->max_msix > 1) { >> for (i = 0; i < dev->max_msix; i++) { >> dev->aac_msix[i].vector_no = i; >> dev->aac_msix[i].dev = dev; >> - if (request_irq(dev->msixentry[i].vector, >> + if (request_irq(pci_irq_vector(dev->pdev, i), >> dev->a_ops.adapter_intr, >> 0, "aacraid", &(dev->aac_msix[i]))) { >> printk(KERN_ERR "%s%d: Failed to register >> IRQ for vector >> %d.\n", >> dev->name, dev->id, i); >> for (j = 0 ; j < i ; j++) >> - free_irq(dev->msixentry[j].vector, >> + free_irq(pci_irq_vector(dev->pdev, >> j), >> &(dev->aac_msix[j])); >> pci_disable_msix(dev->pdev); >> ret = -1; >> } >> - if (irq_set_affinity_hint(dev->msixentry[i].vector, >> - get_cpu_mask(cpu))) { >> - printk(KERN_ERR "%s%d: Failed to set IRQ >> affinity for cpu >> %d\n", >> - dev->name, dev->id, cpu); >> - } >> - cpu = cpumask_next(cpu, cpu_online_mask); > > Is there a reason why the irq_set_affinity_hint was removed? Will the > PCI_IRQ_AFFINITY flag enable hints? > Precisely. Setting the PCI_IRQ_AFFINITY flag will instruct the PCI subsystem to set the interrupt affinity. It will
[Bug 187221] HPSA resetting logical / reset logical
https://bugzilla.kernel.org/show_bug.cgi?id=187221 --- Comment #1 from Patrick Schaaf--- Some more info on my problematic machine / further diagnosing is in https://bugzilla.kernel.org/show_bug.cgi?id=187231 Summary: at least with the P440ar controllers, such 10-30 second "logical reset" episodes eventually reveal an underlying faulty drive, and go away when that is drive is replaced. But there is no up-front information in the "logical reset" that would permit pinpointing the drive on the first round. -- 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
[Bug 187231] kernel panic during hpsa MSI plus tg3 MSI
https://bugzilla.kernel.org/show_bug.cgi?id=187231 Patrick Schaafchanged: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |UNREPRODUCIBLE --- Comment #6 from Patrick Schaaf --- After several more such Abort request / reset sequences with 3.14.79, two days ago the box _finally_ announced that one of its 8 drives has a SMART "predictive failure"; after swapping that drive for a spare, the symptoms are no longer seen. This is the third or fourth time, over the last year, that I've seen Gen9 servers with P440ar cards behave that way. Anyway, my immediate test case is gone, so I'll close this as RESOLVED / unreproducible... -- 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 v3] ufs: add a variety of definitions decribed in UFS spec
These things are defined to be used by some UFS Host controllers. And a new file for some declarations of mphy standard is added Signed-off-by: Kiwoong Kim--- V3: add new macros of all bits of UECDL --- drivers/scsi/ufs/mphy.h | 38 ++ drivers/scsi/ufs/ufshci.h | 28 +--- drivers/scsi/ufs/unipro.h | 26 ++ 3 files changed, 89 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..799cb26 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -170,17 +170,39 @@ 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_AFCx_REQUEST_TIMEOUT UFS_BIT(2) +#define UIC_DATA_LINK_LAYER_ERROR_FCx_PROTECTION_TIMEOUT UFS_BIT(3) +#define UIC_DATA_LINK_LAYER_ERROR_CRC_ERRORUFS_BIT(4) +#define UIC_DATA_LINK_LAYER_ERROR_RX_BUF_OFUFS_BIT(5) +#define UIC_DATA_LINK_LAYER_ERROR_MAX_FRAME_LENGTH_EXCEEDEDUFS_BIT(6) +#define UIC_DATA_LINK_LAYER_ERROR_WRONG_SEQUENCE_NUMBER UFS_BIT(7) +#define UIC_DATA_LINK_LAYER_ERROR_AFC_FRAME_SYNTAX_ERROR UFS_BIT(8) +#define UIC_DATA_LINK_LAYER_ERROR_NAC_FRAME_SYNTAX_ERROR UFS_BIT(9) +#define UIC_DATA_LINK_LAYER_ERROR_EOF_SYNTAX_ERROR UFS_BIT(10) +#define UIC_DATA_LINK_LAYER_ERROR_FRAME_SYNTAX_ERROR UFS_BIT(11) +#define UIC_DATA_LINK_LAYER_ERROR_BAD_CTRL_SYMBOL_TYPE UFS_BIT(12) +#define UIC_DATA_LINK_LAYER_ERROR_PA_INIT UFS_BIT(13) +#define UIC_DATA_LINK_LAYER_ERROR_PA_ERROR_IND_RECEIVED UFS_BIT(14) + /* 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) +#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW BIT(5) +#define UIC_TRANSPORT_SAFETY_VALVE_DROPPINGBIT(6) /* 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..490d867 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
RE: [PATCH v2] ufs: add a variety of definitions decribed in UFS spec
> On 2016-11-15 03:24, Kiwoong Kim wrote: > > These things are defined to be used by some UFS Host controllers. > > And a new file for some declarations of mphy standard is added > > > > V2 > > - modify the commit message > > - add a new macro for UECDL > > - add two definitions about UECDL > > - change the names of two macros > > (s/IS_PWR_MODE_HS/IS_HS_PWR_MODE, s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE) > > > > Signed-off-by: Kiwoong Kim> > --- > > drivers/scsi/ufs/mphy.h | 38 ++ > > drivers/scsi/ufs/ufshci.h | 17 ++--- > > drivers/scsi/ufs/unipro.h | 26 ++ > > 3 files changed, 78 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..a1a06ac9 100644 > > --- a/drivers/scsi/ufs/ufshci.h > > +++ b/drivers/scsi/ufs/ufshci.h > > @@ -170,17 +170,28 @@ 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) > > +#define UIC_DATA_LINK_LAYER_ERROR_CASE(r, x) (r & x) > > > I has this comment on Patch set #1 : "why don't we just add macros for all > the bits in UECDL ? This makes it easy in future." and you had replied > "Okay. I'll think about it and apply new macros on new version of this > patch." but i don't see these macros added. Do you want to add them? Also, > i am not sure why we are adding > UIC_DATA_LINK_LAYER_ERROR_CASE() macro? I misunderstood what you said. Sorry for bothering you. I'll update this patch. > > One more thing, > As Martin mentioned in other email, please separate this version history > from commit text with line having "" before the start of version > history. > Rest all looks good but i will wait for updated patch fixing above before > giving Reviewed-By. > > > > > > /* 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) > > +#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW
[PATCH v3] ufs: introduce UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR quirk
If UFS driver resets interrupt aggregation timer and counter when there are some pended tasks, an IO competion interrupt of any corresponing task may be issued. That would casue a command timeout. One thing you should mind to use interrupt aggreation with this quirk is that the host controller should be able to refresh interrupt aggreation counter or timer in other way, such as doing it automatically when receiving any response. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 3 ++- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8aac98f..7b62d8b 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3713,7 +3713,8 @@ 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_RESET_INTR_AGGR)) 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 dfa17ac..d6861ed 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -505,6 +505,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9) + /* +* This quirk is only not to reset interrupt aggregation logic +* in ISR. The reset can make the host controller miss an event +* of previously completed IO. +*/ + #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR UFS_BIT(10) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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
[PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_HCE quirk
Some UFS host controllers might not be able to reset UIC by setting HCE to 1. Those controllers should invoke 'DME reset' and 'DME enable' in order instead. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 44 +++- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c9cf011..8aac98f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2477,6 +2477,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); + + 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); + + return ret; +} + /** * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET * @hba: per adapter instance @@ -3084,6 +3115,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), @@ -3100,6 +3132,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); + if (hba->quirks & UFSHCD_QUIRK_BROKEN_HCE) + goto use_dme; + /* start controller initialization sequence */ ufshcd_hba_start(hba); @@ -3128,12 +3163,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_BROKEN_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 9838598..dfa17ac 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -498,6 +498,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) + /* +* This quirk needs to be enabled if the host contoller can't reset +* UIC by setting HCE to 1. Those controllers should invoke +* DME reset and DME enable in order. +*/ + #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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
[PATCH v3] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
Some UFS host controllers may clear a transfer request slot by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0. That's opposite to what UFS spec describes. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 28 ++-- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index d6e3112..c9cf011 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); } /** @@ -4312,7 +4336,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 7d9ff22..9838598 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -491,6 +491,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7) + /* +* This quirk needs to be enabled if the host contoller has to set +* the bit corresponding the slot to be cleared to 1, not 0 as +* described in UFS spec. +*/ + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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
[PATCH] scsi: ufs: introduce a new ufshcd_statea UFSHCD_STATE_EH_SCHEDULED
Add a new ufshcd_state, indicats that an err handler may get to run immediately. Use UFSHCD_STATE_ERROR here looks not literaly correct. Signed-off-by: Zang Leigangdiff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 05c7456..59d4130 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -123,6 +123,7 @@ enum { UFSHCD_STATE_RESET, UFSHCD_STATE_ERROR, UFSHCD_STATE_OPERATIONAL, + UFSHCD_STATE_EH_SCHEDULED, }; /* UFSHCD error handling flags */ @@ -1410,6 +1411,7 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) switch (hba->ufshcd_state) { case UFSHCD_STATE_OPERATIONAL: break; + case UFSHCD_STATE_EH_SCHEDULED: case UFSHCD_STATE_RESET: err = SCSI_MLQUEUE_HOST_BUSY; goto out_unlock; @@ -4158,7 +4160,7 @@ static void ufshcd_check_errors(struct ufs_hba *hba) /* block commands from scsi mid-layer */ scsi_block_requests(hba->host); - hba->ufshcd_state = UFSHCD_STATE_ERROR; + hba->ufshcd_state = UFSHCD_STATE_EH_SCHEDULED; schedule_work(>eh_work); } } -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: ufs: remove unnecessary UFSHCD_STATE_ERROR set
Ok, got it. I will resend a new patch. On Tue, Nov 15, 2016 at 11:00:12AM -0800, Subhash Jadavani wrote: > On 2016-11-15 04:25, Zang Leigang wrote: > >We set ufshcd_state to UFSHCD_STATE_ERROR when an err handler > >fails. So I think UFSHCD_STATE_ERROR means host not works any more. > >Set it before schedule eh_work is not correct. > > > >Signed-off-by: Zang Leigang> > > >diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > >index 05c7456..a7724ad 100644 > >--- a/drivers/scsi/ufs/ufshcd.c > >+++ b/drivers/scsi/ufs/ufshcd.c > >@@ -4158,7 +4158,6 @@ static void ufshcd_check_errors(struct > >ufs_hba *hba) > > /* block commands from scsi mid-layer */ > > scsi_block_requests(hba->host); > > > >-hba->ufshcd_state = UFSHCD_STATE_ERROR; > > But we may still want to stop the new requests from being issued > (via ufshcd_queuecommand) until error handler (ufshcd_err_handler()) > gets chance to run. May be a new intermediate state like > UFSHCD_STATE_EH_SCHEDULED should be added and set here, we have to > check the same in ufshcd_queuecommand() to return busy if > ufshcd_state is UFSHCD_STATE_EH_SCHEDULED. > > > schedule_work(>eh_work); > > } > > } > > -- > 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 v2] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
>> > Some UFS host controllers may clear a transfer request slot by setting >> > an associated bit in UTRLCLR/UTMRLCLR to 1, not 0. >> > That's opposite to what UFS spec describes. >> > >> > v2: modify the commit message, remove unrelated changes >> >> As Martin mentioned in other email, please separate this version history >> from commit text with line having "" before the start of version >> history. I didn't understand clearly what Martin said. Thank you for your comment. >> Rest all looks good but i will wait for updated patch fixing above before >> giving Reviewed-By. >> >> > >> > Signed-off-by: Kiwoong Kim>> > --- >> > drivers/scsi/ufs/ufshcd.c | 28 ++-- >> > drivers/scsi/ufs/ufshcd.h | 7 +++ >> > 2 files changed, 33 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c >> > index d6e3112..c9cf011 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); >> > } >> > >> > /** >> > @@ -4312,7 +4336,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 7d9ff22..9838598 100644 >> > --- a/drivers/scsi/ufs/ufshcd.h >> > +++ b/drivers/scsi/ufs/ufshcd.h >> > @@ -491,6 +491,13 @@ struct ufs_hba { >> > */ >> >#define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7) >> > >> > + /* >> > + * This quirk needs to be enabled if the host contoller has to set >> > + * the bit corresponding the slot to be cleared to 1, not 0 as >> > + * described in UFS spec. >> > + */ >> > + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) >> > + >> >unsigned int quirks;/* Deviations from standard UFSHCI spec. >> */ >> > >> >/* Device deviations from standard UFS device 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 06/11] hisi_sas: modify return value of hisi_sas_query_task()
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > sas_scsi_find_task() only deals with return value > TMF_RESP_FUNC_FAILED/TMF_RESP_FUNC_SUCC/TMF_RESP_FUNC_COMPLETE of > query task. So for LLDD errors just return TMF_RESP_FUNC_FAILED. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 09/11] hisi_sas: check SATA FIS when directly attaching SATA device
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > Check ERR bit of status to decide whether there is something wrong with > initial register-D2H FIS. If error exists, PHY reset the channel to > restart OOB. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 11/11] hisi_sas: add PHY set linkrate support for v1 and v2 hw
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > Add the function to set PHY min and max linkrate through > sysfs interface. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 03/11] hisi_sas: only process broadcast change in phy_bcast_v2_hw()
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > There are many BROADCAST primitives generated by the host. > We are only interested in BROADCAST (CHANGE) primitives currently, > so only process this. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 01/11] hisi_sas: add v2 hw support for ECC and AXI bus fatal error
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > For ECC 1bit error, logic can recover it, so we only print > a warning. > For ECC multi-bit and AXI bus fatal error, we panic. Is it possible to recover via resetting phy and device etc instead of panic? Thanks -- 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 08/11] hisi_sas: modify some values in get_ata_protocol()
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > Modify and add some SATA commands according to SATA protocol. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 10/11] hisi_sas: use atomic64_t for hisi_sas_device.running_req
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > Sometimes the value of hisi_sas_device.running_req > would go negative unless we have the check for > running_req >= 0 before trying to decrement. > > This is because using running_req is not thread-safe. > > As such, the value for running_req may be actually incorrect, > so use atomic64_t instead. > > Signed-off-by: John Garry > Reviewed-by: Xiang Chen Reviewed-by: Zhangfei Gao -- 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 07/11] hisi_sas: delete repeated configuration in free_device_v2_hw()
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > Delete repeated configuration items for hisi_sas_device() when > we free a device. These items are now only set in > hisi_sas_dev_gone(). > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 04/11] hisi_sas: fix port form bug in hisi_sas_port_notify_formed()
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > When we form a wideport, we should use hardware PHY port_id instead > of sas_phy->id. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 02/11] hisi_sas: alloc queue id of slot according to device id
On Mon, Nov 7, 2016 at 8:48 PM, John Garrywrote: > From: Xiang Chen > > Currently slots are allocated from queues in a round-robin fashion. > This causes a problem for internal commands in device mode. For this > mode, we should ensure that the internal abort command is the last > command seen in the host for that device. We can only ensure this when > we place the internal abort command after the preceding commands for > device that in the same queue, as there is no order in which the host > will select a queue to execute the next command. Is there performance penalty, since only one queue is supported for a device. > > This queue restriction makes supporting scsi mq more tricky in > the future, but should not be a blocker. > > Note: Even though v1 hw does not support internal abort, the > allocation method is chosen to be the same for consistency. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao -- 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 13/14] cxlflash: Migrate IOARRIN specific routines to function pointers
From: "Matthew R. Ochs"As staging for supporting hardware with a different queuing mechanism, move the send_cmd() and context_reset() routines to function pointers that are configured when the AFU is initialized. In addition, rename the existing routines to better reflect the queue model they support. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 3 +++ drivers/scsi/cxlflash/main.c | 21 +++-- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index c4a7014..30c3cdb 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -161,6 +161,9 @@ struct afu { * fields after this point */ + int (*send_cmd)(struct afu *, struct afu_cmd *); + void (*context_reset)(struct afu_cmd *); + /* AFU HW */ struct cxl_ioctl_start_work work; struct cxlflash_afu_map __iomem *afu_map; /* entire MMIO map */ diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index d5b355c..7fc8843 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -188,12 +188,10 @@ static void cmd_complete(struct afu_cmd *cmd) } /** - * context_reset() - timeout handler for AFU commands + * context_reset_ioarrin() - reset command owner context via IOARRIN register * @cmd: AFU command that timed out. - * - * Sends a reset to the AFU. */ -static void context_reset(struct afu_cmd *cmd) +static void context_reset_ioarrin(struct afu_cmd *cmd) { int nretry = 0; u64 rrin = 0x1; @@ -217,14 +215,14 @@ static void context_reset(struct afu_cmd *cmd) } /** - * send_cmd() - sends an AFU command + * send_cmd_ioarrin() - sends an AFU command via IOARRIN register * @afu: AFU associated with the host. * @cmd: AFU command to send. * * Return: * 0 on success, SCSI_MLQUEUE_HOST_BUSY on failure */ -static int send_cmd(struct afu *afu, struct afu_cmd *cmd) +static int send_cmd_ioarrin(struct afu *afu, struct afu_cmd *cmd) { struct cxlflash_cfg *cfg = afu->parent; struct device *dev = >dev->dev; @@ -298,7 +296,7 @@ static int wait_resp(struct afu *afu, struct afu_cmd *cmd) timeout = wait_for_completion_timeout(>cevent, timeout); if (!timeout) { - context_reset(cmd); + afu->context_reset(cmd); rc = -1; } @@ -355,7 +353,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) SISL_REQ_FLAGS_TMF_CMD); memcpy(cmd->rcb.cdb, , sizeof(tmfcmd)); - rc = send_cmd(afu, cmd); + rc = afu->send_cmd(afu, cmd); if (unlikely(rc)) { spin_lock_irqsave(>tmf_slock, lock_flags); cfg->tmf_active = false; @@ -486,7 +484,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) cmd->rcb.req_flags = req_flags; memcpy(cmd->rcb.cdb, scp->cmnd, sizeof(cmd->rcb.cdb)); - rc = send_cmd(afu, cmd); + rc = afu->send_cmd(afu, cmd); if (unlikely(rc)) scsi_dma_unmap(scp); out: @@ -1656,6 +1654,9 @@ static int init_afu(struct cxlflash_cfg *cfg) goto err2; } + afu->send_cmd = send_cmd_ioarrin; + afu->context_reset = context_reset_ioarrin; + pr_debug("%s: afu version %s, interface version 0x%llX\n", __func__, afu->version, afu->interface_version); @@ -1747,7 +1748,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, *((__be16 *)>rcb.cdb[2]) = cpu_to_be16(ctx_hndl_u); *((__be32 *)>rcb.cdb[4]) = cpu_to_be32(res_hndl_u); - rc = send_cmd(afu, cmd); + rc = afu->send_cmd(afu, cmd); if (unlikely(rc)) goto out; -- 2.1.0 -- 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 14/14] cxlflash: Migrate scsi command pointer to AFU command
From: "Matthew R. Ochs"Currently, when sending a SCSI command, the pointer is stored in a reserved field of the AFU command descriptor for retrieval once the SCSI command has completed. In order to support new descriptor formats that make use of the reserved field, the pointer is migrated to outside the descriptor where it can still be found during completion processing. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 1 + drivers/scsi/cxlflash/main.c| 10 +- drivers/scsi/cxlflash/sislite.h | 2 +- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 30c3cdb..7a892c5 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -129,6 +129,7 @@ struct afu_cmd { struct sisl_ioarcb rcb; /* IOARCB (cache line aligned) */ struct sisl_ioasa sa; /* IOASA must follow IOARCB */ struct afu *parent; + struct scsi_cmnd *scp; struct completion cevent; u8 cmd_tmf:1; diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 7fc8843..08ef68a 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -151,7 +151,7 @@ static void process_cmd_err(struct afu_cmd *cmd, struct scsi_cmnd *scp) * * Prepares and submits command that has either completed or timed out to * the SCSI stack. Checks AFU command back into command pool for non-internal - * (rcb.scp populated) commands. + * (cmd->scp populated) commands. */ static void cmd_complete(struct afu_cmd *cmd) { @@ -161,8 +161,8 @@ static void cmd_complete(struct afu_cmd *cmd) struct cxlflash_cfg *cfg = afu->parent; bool cmd_is_tmf; - if (cmd->rcb.scp) { - scp = cmd->rcb.scp; + if (cmd->scp) { + scp = cmd->scp; if (unlikely(cmd->sa.ioasc)) process_cmd_err(cmd, scp); else @@ -340,7 +340,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) cfg->tmf_active = true; spin_unlock_irqrestore(>tmf_slock, lock_flags); - cmd->rcb.scp = scp; + cmd->scp = scp; cmd->parent = afu; cmd->cmd_tmf = true; @@ -470,7 +470,7 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) cmd->rcb.data_ea = sg_dma_address(sg); } - cmd->rcb.scp = scp; + cmd->scp = scp; cmd->parent = afu; cmd->rcb.ctx_id = afu->ctx_hndl; diff --git a/drivers/scsi/cxlflash/sislite.h b/drivers/scsi/cxlflash/sislite.h index 347fc16..1a2d09c 100644 --- a/drivers/scsi/cxlflash/sislite.h +++ b/drivers/scsi/cxlflash/sislite.h @@ -72,7 +72,7 @@ struct sisl_ioarcb { u16 timeout;/* in units specified by req_flags */ u32 rsvd1; u8 cdb[16]; /* must be in big endian */ - struct scsi_cmnd *scp; + u64 reserved; /* Reserved area */ } __packed; struct sisl_rc { -- 2.1.0 -- 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 12/14] cxlflash: Cleanup queuecommand()
From: "Matthew R. Ochs"The queuecommand routine is disorganized where it populates the private command and also contains some logic/statements that are not needed given that cxlflash devices do not (and likely never will) support scatter-gather. Restructure the code to remove the unnecessary logic and create an organized flow: handle state -> DMA map -> populate command -> send command Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/main.c | 50 ++-- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 5acc3fe..d5b355c 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -413,11 +413,11 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) struct afu *afu = cfg->afu; struct device *dev = >dev->dev; struct afu_cmd *cmd = sc_to_afucz(scp); + struct scatterlist *sg = scsi_sglist(scp); u32 port_sel = scp->device->channel + 1; - int nseg, i, ncount; - struct scatterlist *sg; + u16 req_flags = SISL_REQ_FLAGS_SUP_UNDERRUN; ulong lock_flags; - short lflag = 0; + int nseg = 0; int rc = 0; int kref_got = 0; @@ -460,45 +460,35 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) kref_get(>afu->mapcount); kref_got = 1; - cmd->rcb.ctx_id = afu->ctx_hndl; - cmd->rcb.msi = SISL_MSI_RRQ_UPDATED; - cmd->rcb.port_sel = port_sel; - cmd->rcb.lun_id = lun_to_lunid(scp->device->lun); - - if (scp->sc_data_direction == DMA_TO_DEVICE) - lflag = SISL_REQ_FLAGS_HOST_WRITE; - else - lflag = SISL_REQ_FLAGS_HOST_READ; + if (likely(sg)) { + nseg = scsi_dma_map(scp); + if (unlikely(nseg < 0)) { + dev_err(dev, "%s: Fail DMA map!\n", __func__); + rc = SCSI_MLQUEUE_HOST_BUSY; + goto out; + } - cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID | - SISL_REQ_FLAGS_SUP_UNDERRUN | lflag); + cmd->rcb.data_len = sg_dma_len(sg); + cmd->rcb.data_ea = sg_dma_address(sg); + } - /* Stash the scp in the reserved field, for reuse during interrupt */ cmd->rcb.scp = scp; cmd->parent = afu; - nseg = scsi_dma_map(scp); - if (unlikely(nseg < 0)) { - dev_err(dev, "%s: Fail DMA map! nseg=%d\n", - __func__, nseg); - rc = SCSI_MLQUEUE_HOST_BUSY; - goto out; - } + cmd->rcb.ctx_id = afu->ctx_hndl; + cmd->rcb.msi = SISL_MSI_RRQ_UPDATED; + cmd->rcb.port_sel = port_sel; + cmd->rcb.lun_id = lun_to_lunid(scp->device->lun); - ncount = scsi_sg_count(scp); - scsi_for_each_sg(scp, sg, ncount, i) { - cmd->rcb.data_len = sg_dma_len(sg); - cmd->rcb.data_ea = sg_dma_address(sg); - } + if (scp->sc_data_direction == DMA_TO_DEVICE) + req_flags |= SISL_REQ_FLAGS_HOST_WRITE; - /* Copy the CDB from the scsi_cmnd passed in */ + cmd->rcb.req_flags = req_flags; memcpy(cmd->rcb.cdb, scp->cmnd, sizeof(cmd->rcb.cdb)); - /* Send the command */ rc = send_cmd(afu, cmd); if (unlikely(rc)) scsi_dma_unmap(scp); - out: if (kref_got) kref_put(>mapcount, afu_unmap); -- 2.1.0 -- 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 10/14] cxlflash: Remove AFU command lock
From: "Matthew R. Ochs"The original design of the cxlflash driver required AFU commands to convey state information across multiple threads. The IOASA "host use" byte was used to track if a command was done, errored, or timed out. A per-command spin lock was used to serialize access to this byte. As this is no longer required with the introduction of completions and various refactoring over time, the spin lock, state tracking, and associated code can be removed. To support the simplification, the wait_resp() routine is refactored to return a success or failure. Additionally, as the simplification to the AFU internal command routine, explicit assignments of AFU command fields to zero are removed as the memory is zeroed upon allocation. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 8 +--- drivers/scsi/cxlflash/main.c | 46 ++ 2 files changed, 16 insertions(+), 38 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 5e5651ba..c4a7014 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -63,11 +63,6 @@ static inline void check_sizes(void) /* AFU defines a fixed size of 4K for command buffers (borrow 4K page define) */ #define CMD_BUFSIZE SIZE_4K -/* flags in IOA status area for host use */ -#define B_DONE 0x01 -#define B_ERROR 0x02 /* set with B_DONE */ -#define B_TIMEOUT0x04 /* set with B_DONE & B_ERROR */ - enum cxlflash_lr_state { LINK_RESET_INVALID, LINK_RESET_REQUIRED, @@ -133,9 +128,8 @@ struct cxlflash_cfg { struct afu_cmd { struct sisl_ioarcb rcb; /* IOARCB (cache line aligned) */ struct sisl_ioasa sa; /* IOASA must follow IOARCB */ - spinlock_t slock; - struct completion cevent; struct afu *parent; + struct completion cevent; u8 cmd_tmf:1; diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index f192b1c..e5b5d8f 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -161,10 +161,6 @@ static void cmd_complete(struct afu_cmd *cmd) struct cxlflash_cfg *cfg = afu->parent; bool cmd_is_tmf; - spin_lock_irqsave(>slock, lock_flags); - cmd->sa.host_use_b[0] |= B_DONE; - spin_unlock_irqrestore(>slock, lock_flags); - if (cmd->rcb.scp) { scp = cmd->rcb.scp; if (unlikely(cmd->sa.ioasc)) @@ -204,21 +200,9 @@ static void context_reset(struct afu_cmd *cmd) struct afu *afu = cmd->parent; struct cxlflash_cfg *cfg = afu->parent; struct device *dev = >dev->dev; - ulong lock_flags; pr_debug("%s: cmd=%p\n", __func__, cmd); - spin_lock_irqsave(>slock, lock_flags); - - /* Already completed? */ - if (cmd->sa.host_use_b[0] & B_DONE) { - spin_unlock_irqrestore(>slock, lock_flags); - return; - } - - cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT); - spin_unlock_irqrestore(>slock, lock_flags); - writeq_be(rrin, >host_map->ioarrin); do { rrin = readq_be(>host_map->ioarrin); @@ -303,20 +287,30 @@ static int send_cmd(struct afu *afu, struct afu_cmd *cmd) * wait_resp() - polls for a response or timeout to a sent AFU command * @afu: AFU associated with the host. * @cmd: AFU command that was sent. + * + * Return: + * 0 on success, -1 on timeout/error */ -static void wait_resp(struct afu *afu, struct afu_cmd *cmd) +static int wait_resp(struct afu *afu, struct afu_cmd *cmd) { + int rc = 0; ulong timeout = msecs_to_jiffies(cmd->rcb.timeout * 2 * 1000); timeout = wait_for_completion_timeout(>cevent, timeout); - if (!timeout) + if (!timeout) { context_reset(cmd); + rc = -1; + } - if (unlikely(cmd->sa.ioasc != 0)) + if (unlikely(cmd->sa.ioasc != 0)) { pr_err("%s: CMD 0x%X failed, IOASC: flags 0x%X, afu_rc 0x%X, " "scsi_rc 0x%X, fc_rc 0x%X\n", __func__, cmd->rcb.cdb[0], cmd->sa.rc.flags, cmd->sa.rc.afu_rc, cmd->sa.rc.scsi_rc, cmd->sa.rc.fc_rc); + rc = -1; + } + + return rc; } /** @@ -364,7 +358,6 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) /* Stash the scp in the command, for reuse during interrupt */ cmd->rcb.scp = scp; cmd->parent = afu; - spin_lock_init(>slock); /* Copy the CDB from the cmd passed in */ memcpy(cmd->rcb.cdb, , sizeof(tmfcmd)); @@ -491,7 +484,6 @@ static int cxlflash_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *scp) /* Stash the scp in the reserved field, for reuse during interrupt */ cmd->rcb.scp = scp; cmd->parent =
[PATCH 11/14] cxlflash: Cleanup send_tmf()
From: "Matthew R. Ochs"The send_tmf() routine includes some copy/paste cruft that can be removed as well as the setting of an AFU command-specific while holding the tmf_slock. While not a bug, it is out of place and should be shifted down alongside the other command initialization statements for clarity. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/main.c | 22 +++--- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index e5b5d8f..5acc3fe 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -324,12 +324,10 @@ static int wait_resp(struct afu *afu, struct afu_cmd *cmd) */ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) { - struct afu_cmd *cmd = sc_to_afucz(scp); - u32 port_sel = scp->device->channel + 1; - short lflag = 0; struct Scsi_Host *host = scp->device->host; struct cxlflash_cfg *cfg = (struct cxlflash_cfg *)host->hostdata; + struct afu_cmd *cmd = sc_to_afucz(scp); struct device *dev = >dev->dev; ulong lock_flags; int rc = 0; @@ -342,27 +340,21 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) !cfg->tmf_active, cfg->tmf_slock); cfg->tmf_active = true; - cmd->cmd_tmf = true; spin_unlock_irqrestore(>tmf_slock, lock_flags); + cmd->rcb.scp = scp; + cmd->parent = afu; + cmd->cmd_tmf = true; + cmd->rcb.ctx_id = afu->ctx_hndl; cmd->rcb.msi = SISL_MSI_RRQ_UPDATED; cmd->rcb.port_sel = port_sel; cmd->rcb.lun_id = lun_to_lunid(scp->device->lun); - - lflag = SISL_REQ_FLAGS_TMF_CMD; - cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID | - SISL_REQ_FLAGS_SUP_UNDERRUN | lflag); - - /* Stash the scp in the command, for reuse during interrupt */ - cmd->rcb.scp = scp; - cmd->parent = afu; - - /* Copy the CDB from the cmd passed in */ + SISL_REQ_FLAGS_SUP_UNDERRUN | + SISL_REQ_FLAGS_TMF_CMD); memcpy(cmd->rcb.cdb, , sizeof(tmfcmd)); - /* Send the command */ rc = send_cmd(afu, cmd); if (unlikely(rc)) { spin_lock_irqsave(>tmf_slock, lock_flags); -- 2.1.0 -- 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 09/14] cxlflash: Wait for active AFU commands to timeout upon tear down
From: "Matthew R. Ochs"With the removal of the static private command pool, the ability to 'complete' outstanding commands was lost. While not an issue for the commands originating outside the driver, internal AFU commands are synchronous and therefore have a timeout associated with them. To avoid a stale memory access, the tear down sequence needs to ensure that there are not any active commands before proceeding. As these internal AFU commands are rare events, the simplest way to accomplish this is detecting the activity and waiting for it to timeout. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 1 + drivers/scsi/cxlflash/main.c | 6 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 465330f..5e5651ba 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -181,6 +181,7 @@ struct afu { u64 *hrrq_curr; bool toggle; bool read_room; + atomic_t cmds_active; /* Number of currently active AFU commands */ atomic64_t room; u64 hb; u32 internal_lun; /* User-desired LUN mode for this AFU */ diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index a17bff2..f192b1c 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -556,7 +556,7 @@ static void free_mem(struct cxlflash_cfg *cfg) * * Safe to call with AFU in a partially allocated/initialized state. * - * Cleans up all state associated with the command queue, and unmaps + * Waits for any active internal AFU commands to timeout and then unmaps * the MMIO space. */ static void stop_afu(struct cxlflash_cfg *cfg) @@ -564,6 +564,8 @@ static void stop_afu(struct cxlflash_cfg *cfg) struct afu *afu = cfg->afu; if (likely(afu)) { + while (atomic_read(>cmds_active)) + ssleep(1); if (likely(afu->afu_map)) { cxl_psa_unmap((void __iomem *)afu->afu_map); afu->afu_map = NULL; @@ -1745,6 +1747,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, } mutex_lock(_active); + atomic_inc(>cmds_active); buf = kzalloc(sizeof(*cmd) + __alignof__(*cmd) - 1, GFP_KERNEL); if (unlikely(!buf)) { dev_err(dev, "%s: no memory for command\n", __func__); @@ -1786,6 +1789,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, (cmd->sa.host_use_b[0] & B_ERROR))) rc = -1; out: + atomic_dec(>cmds_active); mutex_unlock(_active); kfree(buf); pr_debug("%s: returning rc=%d\n", __func__, rc); -- 2.1.0 -- 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 07/14] cxlflash: Use cmd_size for private commands
From: "Matthew R. Ochs"Instead of using a private pool of AFU commands, use cmd_size to prime the private pool of SCSI commands such that they are allocated with a size large enough to contain an aligned AFU command. Use scsi_cmd_priv() to derive the aligned/zeroed private command on queuecommand and TMF paths. Remove cmd_checkout() as it is no longer required. The remaining AFU private command infrastructure will be removed in a cleanup commit. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 14 + drivers/scsi/cxlflash/main.c | 65 +++--- 2 files changed, 25 insertions(+), 54 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 0744546..40507a9 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -19,6 +19,7 @@ #include #include #include +#include #include extern const struct file_operations cxlflash_cxl_fops; @@ -146,6 +147,19 @@ struct afu_cmd { */ } __aligned(cache_line_size()); +static inline struct afu_cmd *sc_to_afuc(struct scsi_cmnd *sc) +{ + return PTR_ALIGN(scsi_cmd_priv(sc), __alignof__(struct afu_cmd)); +} + +static inline struct afu_cmd *sc_to_afucz(struct scsi_cmnd *sc) +{ + struct afu_cmd *afuc = sc_to_afuc(sc); + + memset(afuc, 0, sizeof(*afuc)); + return afuc; +} + struct afu { /* Stuff requiring alignment go first. */ diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 816d85c..7fa27db 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -35,38 +35,6 @@ MODULE_AUTHOR("Matthew R. Ochs "); MODULE_LICENSE("GPL"); /** - * cmd_checkout() - checks out an AFU command - * @afu: AFU to checkout from. - * - * Commands are checked out in a round-robin fashion. Note that since - * the command pool is larger than the hardware queue, the majority of - * times we will only loop once or twice before getting a command. The - * CDB within the command is initialized (zeroed) prior to returning. - * - * Return: The checked out command or NULL when command pool is empty. - */ -static struct afu_cmd *cmd_checkout(struct afu *afu) -{ - int k, dec = CXLFLASH_NUM_CMDS; - struct afu_cmd *cmd; - - while (dec--) { - k = (afu->cmd_couts++ & (CXLFLASH_NUM_CMDS - 1)); - - cmd = >cmd[k]; - - if (!atomic_dec_if_positive(>free)) { - pr_devel("%s: returning found index=%d cmd=%p\n", -__func__, cmd->slot, cmd); - memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb)); - return cmd; - } - } - - return NULL; -} - -/** * cmd_checkin() - checks in an AFU command * @cmd: AFU command to checkin. * @@ -232,7 +200,6 @@ static void cmd_complete(struct afu_cmd *cmd) scp->result = (DID_OK << 16); cmd_is_tmf = cmd->cmd_tmf; - cmd_checkin(cmd); /* Don't use cmd after here */ pr_debug_ratelimited("%s: calling scsi_done scp=%p result=%X " "ioasc=%d\n", __func__, scp, scp->result, @@ -390,7 +357,7 @@ static void wait_resp(struct afu *afu, struct afu_cmd *cmd) */ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) { - struct afu_cmd *cmd; + struct afu_cmd *cmd = sc_to_afucz(scp); u32 port_sel = scp->device->channel + 1; short lflag = 0; @@ -401,13 +368,6 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) int rc = 0; ulong to; - cmd = cmd_checkout(afu); - if (unlikely(!cmd)) { - dev_err(dev, "%s: could not get a free command\n", __func__); - rc = SCSI_MLQUEUE_HOST_BUSY; - goto out; - } - /* When Task Management Function is active do not send another */ spin_lock_irqsave(>tmf_slock, lock_flags); if (cfg->tmf_active) @@ -419,6 +379,7 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) spin_unlock_irqrestore(>tmf_slock, lock_flags); cmd->rcb.ctx_id = afu->ctx_hndl; + cmd->rcb.msi = SISL_MSI_RRQ_UPDATED; cmd->rcb.port_sel = port_sel; cmd->rcb.lun_id = lun_to_lunid(scp->device->lun); @@ -427,8 +388,10 @@ static int send_tmf(struct afu *afu, struct scsi_cmnd *scp, u64 tmfcmd) cmd->rcb.req_flags = (SISL_REQ_FLAGS_PORT_LUN_ID | SISL_REQ_FLAGS_SUP_UNDERRUN | lflag); - /* Stash the scp in the reserved field, for reuse during interrupt */ + /* Stash the scp in the command, for reuse during interrupt */ cmd->rcb.scp = scp; + cmd->parent = afu; + spin_lock_init(>slock); /* Copy the CDB from
[PATCH 06/14] cxlflash: Allocate memory instead of using command pool for AFU sync
From: "Matthew R. Ochs"As staging for the removal of the AFU command pool, remove the reliance upon the pool for the internal AFU sync command. Instead of obtaining an AFU command from the pool, dynamically allocate memory with the appropriate alignment requirements. Since the AFU sync service is only executed from the process environment, blocking is acceptable. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/main.c | 25 - 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 51ee9ef..816d85c 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -1847,8 +1847,8 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, struct cxlflash_cfg *cfg = afu->parent; struct device *dev = >dev->dev; struct afu_cmd *cmd = NULL; + char *buf = NULL; int rc = 0; - int retry_cnt = 0; static DEFINE_MUTEX(sync_active); if (cfg->state != STATE_NORMAL) { @@ -1857,23 +1857,23 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, } mutex_lock(_active); -retry: - cmd = cmd_checkout(afu); - if (unlikely(!cmd)) { - retry_cnt++; - udelay(1000 * retry_cnt); - if (retry_cnt < MC_RETRY_CNT) - goto retry; - dev_err(dev, "%s: could not get a free command\n", __func__); + buf = kzalloc(sizeof(*cmd) + __alignof__(*cmd) - 1, GFP_KERNEL); + if (unlikely(!buf)) { + dev_err(dev, "%s: no memory for command\n", __func__); rc = -1; goto out; } - pr_debug("%s: afu=%p cmd=%p %d\n", __func__, afu, cmd, ctx_hndl_u); + cmd = (struct afu_cmd *)PTR_ALIGN(buf, __alignof__(*cmd)); + init_completion(>cevent); + spin_lock_init(>slock); + cmd->parent = afu; - memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb)); + pr_debug("%s: afu=%p cmd=%p %d\n", __func__, afu, cmd, ctx_hndl_u); cmd->rcb.req_flags = SISL_REQ_FLAGS_AFU_CMD; + cmd->rcb.ctx_id = afu->ctx_hndl; + cmd->rcb.msi = SISL_MSI_RRQ_UPDATED; cmd->rcb.port_sel = 0x0;/* NA */ cmd->rcb.lun_id = 0x0; /* NA */ cmd->rcb.data_len = 0x0; @@ -1899,8 +1899,7 @@ int cxlflash_afu_sync(struct afu *afu, ctx_hndl_t ctx_hndl_u, rc = -1; out: mutex_unlock(_active); - if (cmd) - cmd_checkin(cmd); + kfree(buf); pr_debug("%s: returning rc=%d\n", __func__, rc); return rc; } -- 2.1.0 -- 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 08/14] cxlflash: Remove private command pool
From: "Matthew R. Ochs"Clean up and remove the remaining private command pool infrastructure that is no longer required. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 7 - drivers/scsi/cxlflash/main.c | 68 -- 2 files changed, 75 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 40507a9..465330f 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -136,8 +136,6 @@ struct afu_cmd { spinlock_t slock; struct completion cevent; struct afu *parent; - int slot; - atomic_t free; u8 cmd_tmf:1; @@ -164,10 +162,6 @@ struct afu { /* Stuff requiring alignment go first. */ u64 rrq_entry[NUM_RRQ_ENTRY]; /* 2K RRQ */ - /* -* Command & data for AFU commands. -*/ - struct afu_cmd cmd[CXLFLASH_NUM_CMDS]; /* Beware of alignment till here. Preferably introduce new * fields after this point @@ -189,7 +183,6 @@ struct afu { bool read_room; atomic64_t room; u64 hb; - u32 cmd_couts; /* Number of command checkouts */ u32 internal_lun; /* User-desired LUN mode for this AFU */ char version[16]; diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 7fa27db..a17bff2 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -35,33 +35,6 @@ MODULE_AUTHOR("Matthew R. Ochs "); MODULE_LICENSE("GPL"); /** - * cmd_checkin() - checks in an AFU command - * @cmd: AFU command to checkin. - * - * Safe to pass commands that have already been checked in. Several - * internal tracking fields are reset as part of the checkin. Note - * that these are intentionally reset prior to toggling the free bit - * to avoid clobbering values in the event that the command is checked - * out right away. - */ -static void cmd_checkin(struct afu_cmd *cmd) -{ - cmd->rcb.scp = NULL; - cmd->rcb.timeout = 0; - cmd->sa.ioasc = 0; - cmd->cmd_tmf = false; - cmd->sa.host_use[0] = 0; /* clears both completion and retry bytes */ - - if (unlikely(atomic_inc_return(>free) != 1)) { - pr_err("%s: Freeing cmd (%d) that is not in use!\n", - __func__, cmd->slot); - return; - } - - pr_devel("%s: released cmd %p index=%d\n", __func__, cmd, cmd->slot); -} - -/** * process_cmd_err() - command error handler * @cmd: AFU command that experienced the error. * @scp: SCSI command associated with the AFU command in error. @@ -585,28 +558,12 @@ static void free_mem(struct cxlflash_cfg *cfg) * * Cleans up all state associated with the command queue, and unmaps * the MMIO space. - * - * - complete() will take care of commands we initiated (they'll be checked - * in as part of the cleanup that occurs after the completion) - * - * - cmd_checkin() will take care of entries that we did not initiate and that - * have not (and will not) complete because they are sitting on a [now stale] - * hardware queue */ static void stop_afu(struct cxlflash_cfg *cfg) { - int i; struct afu *afu = cfg->afu; - struct afu_cmd *cmd; if (likely(afu)) { - for (i = 0; i < CXLFLASH_NUM_CMDS; i++) { - cmd = >cmd[i]; - complete(>cevent); - if (!atomic_read(>free)) - cmd_checkin(cmd); - } - if (likely(afu->afu_map)) { cxl_psa_unmap((void __iomem *)afu->afu_map); afu->afu_map = NULL; @@ -819,7 +776,6 @@ static void cxlflash_remove(struct pci_dev *pdev) static int alloc_mem(struct cxlflash_cfg *cfg) { int rc = 0; - int i; struct device *dev = >dev->dev; /* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */ @@ -833,12 +789,6 @@ static int alloc_mem(struct cxlflash_cfg *cfg) } cfg->afu->parent = cfg; cfg->afu->afu_map = NULL; - - for (i = 0; i < CXLFLASH_NUM_CMDS; i++) { - atomic_set(>afu->cmd[i].free, 1); - cfg->afu->cmd[i].slot = i; - } - out: return rc; } @@ -1468,13 +1418,6 @@ static void init_pcr(struct cxlflash_cfg *cfg) /* Program the Endian Control for the master context */ writeq_be(SISL_ENDIAN_CTRL, >host_map->endian_ctrl); - - /* Initialize cmd fields that never change */ - for (i = 0; i < CXLFLASH_NUM_CMDS; i++) { - afu->cmd[i].rcb.ctx_id = afu->ctx_hndl; - afu->cmd[i].rcb.msi = SISL_MSI_RRQ_UPDATED; - afu->cmd[i].rcb.rrq = 0x0; - } } /** @@ -1563,19 +1506,8 @@ static int init_global(struct cxlflash_cfg *cfg) static
[PATCH 05/14] cxlflash: Remove unused buffer from AFU command
From: "Matthew R. Ochs"The cxlflash driver originally required a per-command 4K buffer that hosted data passed to the AFU. When the routines that initiate AFU and internal SCSI commands were refactored to use scsi_execute(), the need for this buffer became obsolete. As it is no longer necessary, the buffer is removed. Signed-off-by: Matthew R. Ochs --- drivers/scsi/cxlflash/common.h | 1 - drivers/scsi/cxlflash/main.c | 28 ++-- 2 files changed, 2 insertions(+), 27 deletions(-) diff --git a/drivers/scsi/cxlflash/common.h b/drivers/scsi/cxlflash/common.h index 6e68155..0744546 100644 --- a/drivers/scsi/cxlflash/common.h +++ b/drivers/scsi/cxlflash/common.h @@ -134,7 +134,6 @@ struct afu_cmd { struct sisl_ioasa sa; /* IOASA must follow IOARCB */ spinlock_t slock; struct completion cevent; - char *buf; /* per command buffer */ struct afu *parent; int slot; atomic_t free; diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 1a32e8b..51ee9ef 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -41,8 +41,7 @@ MODULE_LICENSE("GPL"); * Commands are checked out in a round-robin fashion. Note that since * the command pool is larger than the hardware queue, the majority of * times we will only loop once or twice before getting a command. The - * buffer and CDB within the command are initialized (zeroed) prior to - * returning. + * CDB within the command is initialized (zeroed) prior to returning. * * Return: The checked out command or NULL when command pool is empty. */ @@ -59,7 +58,6 @@ static struct afu_cmd *cmd_checkout(struct afu *afu) if (!atomic_dec_if_positive(>free)) { pr_devel("%s: returning found index=%d cmd=%p\n", __func__, cmd->slot, cmd); - memset(cmd->buf, 0, CMD_BUFSIZE); memset(cmd->rcb.cdb, 0, sizeof(cmd->rcb.cdb)); return cmd; } @@ -615,17 +613,9 @@ static void cxlflash_wait_for_pci_err_recovery(struct cxlflash_cfg *cfg) */ static void free_mem(struct cxlflash_cfg *cfg) { - int i; - char *buf = NULL; struct afu *afu = cfg->afu; if (cfg->afu) { - for (i = 0; i < CXLFLASH_NUM_CMDS; i++) { - buf = afu->cmd[i].buf; - if (!((u64)buf & (PAGE_SIZE - 1))) - free_page((ulong)buf); - } - free_pages((ulong)afu, get_order(sizeof(struct afu))); cfg->afu = NULL; } @@ -874,7 +864,6 @@ static int alloc_mem(struct cxlflash_cfg *cfg) { int rc = 0; int i; - char *buf = NULL; struct device *dev = >dev->dev; /* AFU is ~12k, i.e. only one 64k page or up to four 4k pages */ @@ -889,20 +878,7 @@ static int alloc_mem(struct cxlflash_cfg *cfg) cfg->afu->parent = cfg; cfg->afu->afu_map = NULL; - for (i = 0; i < CXLFLASH_NUM_CMDS; buf += CMD_BUFSIZE, i++) { - if (!((u64)buf & (PAGE_SIZE - 1))) { - buf = (void *)__get_free_page(GFP_KERNEL | __GFP_ZERO); - if (unlikely(!buf)) { - dev_err(dev, - "%s: Allocate command buffers fail!\n", - __func__); - rc = -ENOMEM; - free_mem(cfg); - goto out; - } - } - - cfg->afu->cmd[i].buf = buf; + for (i = 0; i < CXLFLASH_NUM_CMDS; i++) { atomic_set(>afu->cmd[i].free, 1); cfg->afu->cmd[i].slot = i; } -- 2.1.0 -- 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 04/14] cxlflash: Avoid command room violation
During test, a command room violation interrupt is occasionally seen for the master context when the CXL flash devices are stressed. After studying the code, there could be gaps in the way command room value is being cached in cxlflash. When the cached command room is zero the thread attempting to send becomes burdened with updating the cached value with the actual value from the AFU. Today, this is handled with an atomic set operation of the raw value read. Following the atomic update, the thread proceeds to send. This behavior is incorrect on two counts: - The update fails to take into account the current thread and its consumption of one of the hardware commands. - The update does not take into account other threads also atomically updating. Per design, a worker thread updates the cached value when a send thread times out. By not performing an atomic compare/exchange, the cached value can be incorrectly clobbered. To correct these issues, the runtime updates of the cached command room are updated to use atomic64_cmpxchg() and the send routine is updated to take into account the current thread consuming a hardware command. Signed-off-by: Uma Krishnan--- drivers/scsi/cxlflash/main.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 6d33d8c..1a32e8b 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -322,9 +322,10 @@ static int send_cmd(struct afu *afu, struct afu_cmd *cmd) if (!newval) { do { room = readq_be(>host_map->cmd_room); - atomic64_set(>room, room); - if (room) - goto write_ioarrin; + if (room) { + atomic64_cmpxchg(>room, 0, room); + goto retry; + } udelay(1 << nretry); } while (nretry++ < MC_ROOM_RETRY_CNT); @@ -346,7 +347,6 @@ static int send_cmd(struct afu *afu, struct afu_cmd *cmd) goto no_room; } -write_ioarrin: writeq_be((u64)>rcb, >host_map->ioarrin); out: pr_devel("%s: cmd=%p len=%d ea=%p rc=%d\n", __func__, cmd, @@ -2409,6 +2409,7 @@ static void cxlflash_worker_thread(struct work_struct *work) struct afu *afu = cfg->afu; struct device *dev = >dev->dev; int port; + u64 room; ulong lock_flags; /* Avoid MMIO if the device has failed */ @@ -2437,8 +2438,11 @@ static void cxlflash_worker_thread(struct work_struct *work) } if (afu->read_room) { - atomic64_set(>room, readq_be(>host_map->cmd_room)); - afu->read_room = false; + room = readq_be(>host_map->cmd_room); + if (room) { + atomic64_cmpxchg(>room, 0, room); + afu->read_room = false; + } } spin_unlock_irqrestore(cfg->host->host_lock, lock_flags); -- 2.1.0 -- 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 01/14] cxlflash: Set sg_tablesize to 1 instead of SG_NONE
The following Oops is encountered when blk_mq is enabled with the cxlflash driver: [ 2960.817172] Oops: Kernel access of bad area, sig: 11 [#5] [ 2960.817309] NIP __blk_mq_run_hw_queue+0x278/0x4c0 [ 2960.817313] LR __blk_mq_run_hw_queue+0x2bc/0x4c0 [ 2960.817314] Call Trace: [ 2960.817320] __blk_mq_run_hw_queue+0x2bc/0x4c0 (unreliable) [ 2960.817324] blk_mq_run_hw_queue+0xd8/0x100 [ 2960.817329] blk_mq_insert_requests+0x14c/0x1f0 [ 2960.817333] blk_mq_flush_plug_list+0x150/0x190 [ 2960.817338] blk_flush_plug_list+0x11c/0x2b0 [ 2960.817344] blk_finish_plug+0x58/0x80 [ 2960.817348] __do_page_cache_readahead+0x1c0/0x2e0 [ 2960.817352] force_page_cache_readahead+0x68/0xd0 [ 2960.817356] generic_file_read_iter+0x43c/0x6a0 [ 2960.817359] blkdev_read_iter+0x68/0xa0 [ 2960.817361] __vfs_read+0x11c/0x180 [ 2960.817364] vfs_read+0xa4/0x1c0 [ 2960.817366] SyS_read+0x6c/0x110 [ 2960.817369] system_call+0x38/0xb4 The SCSI blk_mq stack assumes that sg_tablesize is always a non-zero value with scsi_mq_setup_tags() allocating tags using sg_tablesize. The cxlflash driver currently uses SG_NONE (0) for the sg_tablesize as the devices it supports are not capable of scatter gather. This mismatch of values results in the Oops above. To resolve this issue, sg_tablesize for cxlflash can simply be set to 1, a value which satisfies the constraints in cxlflash and the lack of support of SG_NONE in SCSI blk_mq. Signed-off-by: Uma Krishnan--- drivers/scsi/cxlflash/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index b301655..6004860 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -2377,7 +2377,7 @@ static struct scsi_host_template driver_template = { .cmd_per_lun = CXLFLASH_MAX_CMDS_PER_LUN, .can_queue = CXLFLASH_MAX_CMDS, .this_id = -1, - .sg_tablesize = SG_NONE,/* No scatter gather support */ + .sg_tablesize = 1, /* No scatter gather support */ .max_sectors = CXLFLASH_MAX_SECTORS, .use_clustering = ENABLE_CLUSTERING, .shost_attrs = cxlflash_host_attrs, -- 2.1.0 -- 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 03/14] cxlflash: Improve context_reset() logic
Currently, the context reset routine waits for command room to be available before sending the reset request. Per review of the SISLite specification and clarifications from the CXL Flash AFU designers, this wait is unnecessary. The reset request can be sent anytime regardless of command room, so long as only a single reset request is active at any one point in time. This commit simplifies the reset routine by removing the wait for command room. Additionally it adds a debug trace to help pinpoint hardware errors when a context reset does not complete. Signed-off-by: Uma Krishnan--- drivers/scsi/cxlflash/main.c | 23 +-- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/drivers/scsi/cxlflash/main.c b/drivers/scsi/cxlflash/main.c index 6004860..6d33d8c 100644 --- a/drivers/scsi/cxlflash/main.c +++ b/drivers/scsi/cxlflash/main.c @@ -263,8 +263,9 @@ static void context_reset(struct afu_cmd *cmd) { int nretry = 0; u64 rrin = 0x1; - u64 room = 0; struct afu *afu = cmd->parent; + struct cxlflash_cfg *cfg = afu->parent; + struct device *dev = >dev->dev; ulong lock_flags; pr_debug("%s: cmd=%p\n", __func__, cmd); @@ -280,23 +281,6 @@ static void context_reset(struct afu_cmd *cmd) cmd->sa.host_use_b[0] |= (B_DONE | B_ERROR | B_TIMEOUT); spin_unlock_irqrestore(>slock, lock_flags); - /* -* We really want to send this reset at all costs, so spread -* out wait time on successive retries for available room. -*/ - do { - room = readq_be(>host_map->cmd_room); - atomic64_set(>room, room); - if (room) - goto write_rrin; - udelay(1 << nretry); - } while (nretry++ < MC_ROOM_RETRY_CNT); - - pr_err("%s: no cmd_room to send reset\n", __func__); - return; - -write_rrin: - nretry = 0; writeq_be(rrin, >host_map->ioarrin); do { rrin = readq_be(>host_map->ioarrin); @@ -305,6 +289,9 @@ static void context_reset(struct afu_cmd *cmd) /* Double delay each time */ udelay(1 << nretry); } while (nretry++ < MC_ROOM_RETRY_CNT); + + dev_dbg(dev, "%s: returning rrin=0x%016llX nretry=%d\n", + __func__, rrin, nretry); } /** -- 2.1.0 -- 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 02/14] cxlflash: Fix crash in cxlflash_restore_luntable()
During test, the following crash was observed: [34538.981505] Faulting instruction address: 0xd7c9c870 cpu 0x9: Vector: 300 (Data Access) at [c007f1e8f590] pc: d7c9c870: cxlflash_restore_luntable+0x70/0x1d0 [cxlflash] lr: d7c9c84c: cxlflash_restore_luntable+0x4c/0x1d0 [cxlflash] sp: c007f1e8f810 msr: 90019033 dar: c0171d637438 dsisr: 4000 current = 0xc007f1e43f90 paca= 0xc7b25100 softe: 0irq_happened: 0x01 pid = 493, comm = eehd enter ? for help [c007f1e8f8a0] d7c940b0 init_afu+0xd60/0x1200 [cxlflash] [c007f1e8f9a0] d7c945a8 cxlflash_pci_slot_reset+0x58/0xe0 [cxlflash] [c007f1e8fa20] d715f790 cxl_pci_slot_reset+0x230/0x340 [cxl] [c007f1e8fae0] c0040dd4 eeh_report_reset+0x144/0x180 [c007f1e8fb20] c003f708 eeh_pe_dev_traverse+0x98/0x170 [c007f1e8fbb0] c0041618 eeh_handle_normal_event+0x328/0x410 [c007f1e8fc30] c0041db8 eeh_handle_event+0x178/0x330 [c007f1e8fce0] c0042118 eeh_event_handler+0x1a8/0x1b0 [c007f1e8fd80] c011420c kthread+0xec/0x100 [c007f1e8fe30] c000a47c ret_from_kernel_thread+0x5c/0xe0 When superpipe mode is disabled for a LUN, the references for the local lun are deleted but the LUN is still identified as being present in the LUN table. This mismatched state can result in the above crash when the LUN table is restored during an error recovery operation. To fix this issue, the local LUN information structure is updated to reflect the LUN is no longer in the LUN table once all references to the LUN are gone. Signed-off-by: Uma Krishnan--- drivers/scsi/cxlflash/lunmgt.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/drivers/scsi/cxlflash/lunmgt.c b/drivers/scsi/cxlflash/lunmgt.c index a0923ca..6c318db9 100644 --- a/drivers/scsi/cxlflash/lunmgt.c +++ b/drivers/scsi/cxlflash/lunmgt.c @@ -254,8 +254,14 @@ int cxlflash_manage_lun(struct scsi_device *sdev, if (lli->parent->mode != MODE_NONE) rc = -EBUSY; else { + /* +* Clean up local LUN for this port and reset table +* tracking when no more references exist. +*/ sdev->hostdata = NULL; lli->port_sel &= ~CHAN2PORT(chan); + if (lli->port_sel == 0U) + lli->in_table = false; } } -- 2.1.0 -- 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 00/14] cxlflash: Fixes, enhancements, cleanup and staging
The first four patches in this patch series include fixes for command room violation and lun table management. The remaining patches remove the reliance upon an internally maintained private command pool in favor of private commands being allocated alongside the SCSI commands. Several cleanup opportunities were noticed while removing the private command pool infrastructure and have been included as well. Lastly, the final two patches provide staging for supporting hardware with a different queuing model. The series is based upon 4.9-rc5, intended for 4.10 and is bisectable. Matthew R. Ochs (10): cxlflash: Remove unused buffer from AFU command cxlflash: Allocate memory instead of using command pool for AFU sync cxlflash: Use cmd_size for private commands cxlflash: Remove private command pool cxlflash: Wait for active AFU commands to timeout upon tear down cxlflash: Remove AFU command lock cxlflash: Cleanup send_tmf() cxlflash: Cleanup queuecommand() cxlflash: Migrate IOARRIN specific routines to function pointers cxlflash: Migrate scsi command pointer to AFU command Uma Krishnan (4): cxlflash: Set sg_tablesize to 1 instead of SG_NONE cxlflash: Fix crash in cxlflash_restore_luntable() cxlflash: Improve context_reset() logic cxlflash: Avoid command room violation drivers/scsi/cxlflash/common.h | 35 ++-- drivers/scsi/cxlflash/lunmgt.c | 6 + drivers/scsi/cxlflash/main.c| 360 +++- drivers/scsi/cxlflash/sislite.h | 2 +- 4 files changed, 120 insertions(+), 283 deletions(-) -- 2.1.0 -- 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/2] blk-mq: Avoid memory reclaim when remapping queues
On 11/14/2016 11:24 AM, Gabriel Krisman Bertazi wrote: --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -1597,7 +1597,7 @@ static struct blk_mq_tags *blk_mq_init_rq_map(struct blk_mq_tag_set *set, INIT_LIST_HEAD(>page_list); tags->rqs = kzalloc_node(set->queue_depth * sizeof(struct request *), -GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY, +GFP_NOWAIT | __GFP_NOWARN | __GFP_NORETRY, set->numa_node); if (!tags->rqs) { blk_mq_free_tags(tags); Hello Gabriel, I don't think that GFP_NOWAIT is acceptable in this context. Have you tried GFP_NOIO instead of GFP_NOWAIT? 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
[PATCH 1/1] [fnic, snic] Updating maintainers list for Cisco FNI and SNIC drivers
Signed-off-by: Satish KharatSigned-off-by: Sesidhar Baddela --- MAINTAINERS | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index b3a7774..11fdf45 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3149,15 +3149,15 @@ S: Supported F: drivers/clocksource CISCO FCOE HBA DRIVER -M: Hiral Patel -M: Suma Ramars -M: Brian Uchino +M: Satish Kharat +M: Sesidhar Baddela +M: Karan Tilak Kumar L: linux-scsi@vger.kernel.org S: Supported F: drivers/scsi/fnic/ CISCO SCSI HBA DRIVER -M: Narsimhulu Musini +M: Karan Tilak Kumar M: Sesidhar Baddela L: linux-scsi@vger.kernel.org S: Supported -- 2.5.5 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V2] hpsa: correct logical resets
- driver was not calling done in some cases which causes the volume to be offlined. - avoid doing rescan during a reset. Reviewed-by: Scott TeelReviewed-by: Kevin Barnett Signed-off-by: Don Brace --- drivers/scsi/hpsa.c | 23 ++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index b924f6a..5156cde 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -276,6 +276,9 @@ static int hpsa_find_cfg_addrs(struct pci_dev *pdev, void __iomem *vaddr, static int hpsa_pci_find_memory_BAR(struct pci_dev *pdev, unsigned long *memory_bar); static int hpsa_lookup_board_id(struct pci_dev *pdev, u32 *board_id); +static int wait_for_device_to_become_ready(struct ctlr_info *h, + unsigned char lunaddr[], + int reply_queue); static int hpsa_wait_for_board_state(struct pci_dev *pdev, void __iomem *vaddr, int wait_for_ready); static inline void finish_cmd(struct CommandList *c); @@ -2540,7 +2543,7 @@ static void complete_scsi_command(struct CommandList *cp) if ((unlikely(hpsa_is_pending_event(cp { if (cp->reset_pending) - return hpsa_cmd_resolve_and_free(h, cp); + return hpsa_cmd_free_and_done(h, cp, cmd); if (cp->abort_pending) return hpsa_cmd_abort_and_free(h, cp, cmd); } @@ -3079,6 +3082,8 @@ static int hpsa_do_reset(struct ctlr_info *h, struct hpsa_scsi_dev_t *dev, if (unlikely(rc)) atomic_set(>reset_cmds_out, 0); + else + wait_for_device_to_become_ready(h, scsi3addr, 0); mutex_unlock(>reset_mutex); return rc; @@ -5563,6 +5568,14 @@ static void hpsa_scan_start(struct Scsi_Host *sh) if (unlikely(lockup_detected(h))) return hpsa_scan_complete(h); + /* +* Do the scan after a reset completion +*/ + if (h->reset_in_progress) { + h->drv_req_rescan = 1; + return; + } + hpsa_update_scsi_devices(h); hpsa_scan_complete(h); @@ -8599,6 +8612,14 @@ static void hpsa_rescan_ctlr_worker(struct work_struct *work) if (h->remove_in_progress) return; + /* +* Do the scan after the reset +*/ + if (h->reset_in_progress) { + h->drv_req_rescan = 1; + return; + } + if (hpsa_ctlr_needs_rescan(h) || hpsa_offline_devices_ready(h)) { scsi_host_get(h->scsi_host); hpsa_ack_ctlr_events(h); -- 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
Hi Hannes, Thank you for the patch > -Original Message- > From: Hannes Reinecke [mailto:h...@suse.de] > Sent: Monday, November 7, 2016 11:12 PM > To: Martin K. Petersen > Cc: Christoph Hellwig; James Bottomley; linux-scsi@vger.kernel.org; Hannes > Reinecke; Hannes Reinecke; dl-esc-Aacraid Linux Driver > Subject: [PATCH] aacraid: switch to pci_alloc_irq_vectors > > EXTERNAL EMAIL > > > Use pci_alloc_irq_vectors and drop the hand-crafted > interrupt affinity routines. > > Signed-off-by: Hannes Reinecke> Cc: Adaptec OEM Raid Solutions > --- > drivers/scsi/aacraid/aacraid.h | 1 - > drivers/scsi/aacraid/comminit.c | 10 +++--- > drivers/scsi/aacraid/commsup.c | 25 + > drivers/scsi/aacraid/linit.c| 20 > 4 files changed, 12 insertions(+), 44 deletions(-) > > diff --git a/drivers/scsi/aacraid/aacraid.h b/drivers/scsi/aacraid/aacraid.h > index 969c312..f059c14 100644 > --- a/drivers/scsi/aacraid/aacraid.h > +++ b/drivers/scsi/aacraid/aacraid.h > @@ -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 */ > u8 adapter_shutdown; > u32 handle_pci_error; > diff --git a/drivers/scsi/aacraid/comminit.c b/drivers/scsi/aacraid/comminit.c > index 341ea32..4f56b10 100644 > --- a/drivers/scsi/aacraid/comminit.c > +++ b/drivers/scsi/aacraid/comminit.c > @@ -378,16 +378,12 @@ void aac_define_int_mode(struct aac_dev *dev) > if (msi_count > AAC_MAX_MSIX) > msi_count = AAC_MAX_MSIX; > > - for (i = 0; i < msi_count; i++) > - dev->msixentry[i].entry = i; > - > if (msi_count > 1 && > pci_find_capability(dev->pdev, PCI_CAP_ID_MSIX)) { > min_msix = 2; > - i = pci_enable_msix_range(dev->pdev, > - dev->msixentry, > - min_msix, > - msi_count); > + i = pci_alloc_irq_vectors(dev->pdev, > + min_msix, msi_count, > + PCI_IRQ_MSIX | PCI_IRQ_AFFINITY); > if (i > 0) { > dev->msi_enabled = 1; > msi_count = i; > diff --git a/drivers/scsi/aacraid/commsup.c b/drivers/scsi/aacraid/commsup.c > index 0aeecec..9e7551f 100644 > --- a/drivers/scsi/aacraid/commsup.c > +++ b/drivers/scsi/aacraid/commsup.c > @@ -2043,30 +2043,22 @@ int aac_acquire_irq(struct aac_dev *dev) > int i; > int j; > int ret = 0; > - int cpu; > > - cpu = cpumask_first(cpu_online_mask); > if (!dev->sync_mode && dev->msi_enabled && dev->max_msix > 1) { > for (i = 0; i < dev->max_msix; i++) { > dev->aac_msix[i].vector_no = i; > dev->aac_msix[i].dev = dev; > - if (request_irq(dev->msixentry[i].vector, > + if (request_irq(pci_irq_vector(dev->pdev, i), > dev->a_ops.adapter_intr, > 0, "aacraid", &(dev->aac_msix[i]))) { > printk(KERN_ERR "%s%d: Failed to register IRQ > for vector > %d.\n", > dev->name, dev->id, i); > for (j = 0 ; j < i ; j++) > - free_irq(dev->msixentry[j].vector, > + free_irq(pci_irq_vector(dev->pdev, j), > &(dev->aac_msix[j])); > pci_disable_msix(dev->pdev); > ret = -1; > } > - if (irq_set_affinity_hint(dev->msixentry[i].vector, > - get_cpu_mask(cpu))) { > - printk(KERN_ERR "%s%d: Failed to set IRQ > affinity for cpu > %d\n", > - dev->name, dev->id, cpu); > - } > - cpu = cpumask_next(cpu, cpu_online_mask); Is there a reason why the irq_set_affinity_hint was removed? Will the PCI_IRQ_AFFINITY flag enable hints? > } > } else { > dev->aac_msix[0].vector_no = 0; > @@ -2096,16 +2088,9 @@ void aac_free_irq(struct aac_dev *dev) > dev->pdev->device == PMC_DEVICE_S8 || > dev->pdev->device == PMC_DEVICE_S9) { > if
Re: [PATCH 1/2] nvme: untangle 0 and BLK_MQ_RQ_QUEUE_OK
On 11/15/2016 12:11 PM, Omar Sandoval wrote: From: Omar SandovalLet's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having specific values. No functional change. Thanks Omar, applied both for 4.10. -- 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 1/2] nvme: untangle 0 and BLK_MQ_RQ_QUEUE_OK
On Tue, Nov 15, 2016 at 11:11:58AM -0800, Omar Sandoval wrote: > From: Omar Sandoval> > Let's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having > specific values. No functional change. > > Signed-off-by: Omar Sandoval Yeah, we've been depending on the values of BLK_MQ_RQ_QUEUE_[ERROR|BUSY] not being zero without this. Looks good. Reviewed-by: Keith Busch > --- > drivers/nvme/host/core.c | 4 ++-- > drivers/nvme/host/pci.c| 8 > drivers/nvme/host/rdma.c | 2 +- > drivers/nvme/target/loop.c | 6 +++--- > 4 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c > index 53584d2..e54bb10 100644 > --- a/drivers/nvme/host/core.c > +++ b/drivers/nvme/host/core.c > @@ -269,7 +269,7 @@ static inline int nvme_setup_discard(struct nvme_ns *ns, > struct request *req, >*/ > req->__data_len = nr_bytes; > > - return 0; > + return BLK_MQ_RQ_QUEUE_OK; > } > > static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req, > @@ -317,7 +317,7 @@ static inline void nvme_setup_rw(struct nvme_ns *ns, > struct request *req, > int nvme_setup_cmd(struct nvme_ns *ns, struct request *req, > struct nvme_command *cmd) > { > - int ret = 0; > + int ret = BLK_MQ_RQ_QUEUE_OK; > > if (req->cmd_type == REQ_TYPE_DRV_PRIV) > memcpy(cmd, nvme_req(req)->cmd, sizeof(*cmd)); > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c > index 51d13d5..d58f8e4 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -328,7 +328,7 @@ static int nvme_init_iod(struct request *rq, unsigned > size, > rq->retries = 0; > rq->rq_flags |= RQF_DONTPREP; > } > - return 0; > + return BLK_MQ_RQ_QUEUE_OK; > } > > static void nvme_free_iod(struct nvme_dev *dev, struct request *req) > @@ -598,17 +598,17 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx, > > map_len = nvme_map_len(req); > ret = nvme_init_iod(req, map_len, dev); > - if (ret) > + if (ret != BLK_MQ_RQ_QUEUE_OK) > return ret; > > ret = nvme_setup_cmd(ns, req, ); > - if (ret) > + if (ret != BLK_MQ_RQ_QUEUE_OK) > goto out; > > if (req->nr_phys_segments) > ret = nvme_map_data(dev, req, map_len, ); > > - if (ret) > + if (ret != BLK_MQ_RQ_QUEUE_OK) > goto out; > > cmnd.common.command_id = req->tag; > diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c > index c4700ef..ff1b34060 100644 > --- a/drivers/nvme/host/rdma.c > +++ b/drivers/nvme/host/rdma.c > @@ -1395,7 +1395,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx > *hctx, > sizeof(struct nvme_command), DMA_TO_DEVICE); > > ret = nvme_setup_cmd(ns, rq, c); > - if (ret) > + if (ret != BLK_MQ_RQ_QUEUE_OK) > return ret; > > c->common.command_id = rq->tag; > diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c > index 26aa3a5..be56d05 100644 > --- a/drivers/nvme/target/loop.c > +++ b/drivers/nvme/target/loop.c > @@ -169,7 +169,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, > int ret; > > ret = nvme_setup_cmd(ns, req, >cmd); > - if (ret) > + if (ret != BLK_MQ_RQ_QUEUE_OK) > return ret; > > iod->cmd.common.flags |= NVME_CMD_SGL_METABUF; > @@ -179,7 +179,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, > nvme_cleanup_cmd(req); > blk_mq_start_request(req); > nvme_loop_queue_response(>req); > - return 0; > + return BLK_MQ_RQ_QUEUE_OK; > } > > if (blk_rq_bytes(req)) { > @@ -198,7 +198,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, > blk_mq_start_request(req); > > schedule_work(>work); > - return 0; > + return BLK_MQ_RQ_QUEUE_OK; > } > > static void nvme_loop_submit_async_event(struct nvme_ctrl *arg, int aer_idx) > -- > 2.10.2 > > > ___ > Linux-nvme mailing list > linux-n...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-nvme -- 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 2/2] scsi_lib: untangle 0 and BLK_MQ_RQ_QUEUE_OK
From: Omar SandovalLet's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having specific values. No functional change. Signed-off-by: Omar Sandoval --- drivers/scsi/scsi_lib.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index 2e35132..47a5c87 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -1810,7 +1810,7 @@ static inline int prep_to_mq(int ret) { switch (ret) { case BLKPREP_OK: - return 0; + return BLK_MQ_RQ_QUEUE_OK; case BLKPREP_DEFER: return BLK_MQ_RQ_QUEUE_BUSY; default: @@ -1897,7 +1897,7 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx, int reason; ret = prep_to_mq(scsi_prep_state_check(sdev, req)); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) goto out; ret = BLK_MQ_RQ_QUEUE_BUSY; @@ -1914,7 +1914,7 @@ static int scsi_queue_rq(struct blk_mq_hw_ctx *hctx, if (!(req->rq_flags & RQF_DONTPREP)) { ret = prep_to_mq(scsi_mq_prep_fn(req)); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) goto out_dec_host_busy; req->rq_flags |= RQF_DONTPREP; } else { -- 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
[PATCH 1/2] nvme: untangle 0 and BLK_MQ_RQ_QUEUE_OK
From: Omar SandovalLet's not depend on any of the BLK_MQ_RQ_QUEUE_* constants having specific values. No functional change. Signed-off-by: Omar Sandoval --- drivers/nvme/host/core.c | 4 ++-- drivers/nvme/host/pci.c| 8 drivers/nvme/host/rdma.c | 2 +- drivers/nvme/target/loop.c | 6 +++--- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 53584d2..e54bb10 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -269,7 +269,7 @@ static inline int nvme_setup_discard(struct nvme_ns *ns, struct request *req, */ req->__data_len = nr_bytes; - return 0; + return BLK_MQ_RQ_QUEUE_OK; } static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req, @@ -317,7 +317,7 @@ static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req, int nvme_setup_cmd(struct nvme_ns *ns, struct request *req, struct nvme_command *cmd) { - int ret = 0; + int ret = BLK_MQ_RQ_QUEUE_OK; if (req->cmd_type == REQ_TYPE_DRV_PRIV) memcpy(cmd, nvme_req(req)->cmd, sizeof(*cmd)); diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 51d13d5..d58f8e4 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -328,7 +328,7 @@ static int nvme_init_iod(struct request *rq, unsigned size, rq->retries = 0; rq->rq_flags |= RQF_DONTPREP; } - return 0; + return BLK_MQ_RQ_QUEUE_OK; } static void nvme_free_iod(struct nvme_dev *dev, struct request *req) @@ -598,17 +598,17 @@ static int nvme_queue_rq(struct blk_mq_hw_ctx *hctx, map_len = nvme_map_len(req); ret = nvme_init_iod(req, map_len, dev); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) return ret; ret = nvme_setup_cmd(ns, req, ); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) goto out; if (req->nr_phys_segments) ret = nvme_map_data(dev, req, map_len, ); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) goto out; cmnd.common.command_id = req->tag; diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c index c4700ef..ff1b34060 100644 --- a/drivers/nvme/host/rdma.c +++ b/drivers/nvme/host/rdma.c @@ -1395,7 +1395,7 @@ static int nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx, sizeof(struct nvme_command), DMA_TO_DEVICE); ret = nvme_setup_cmd(ns, rq, c); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) return ret; c->common.command_id = rq->tag; diff --git a/drivers/nvme/target/loop.c b/drivers/nvme/target/loop.c index 26aa3a5..be56d05 100644 --- a/drivers/nvme/target/loop.c +++ b/drivers/nvme/target/loop.c @@ -169,7 +169,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, int ret; ret = nvme_setup_cmd(ns, req, >cmd); - if (ret) + if (ret != BLK_MQ_RQ_QUEUE_OK) return ret; iod->cmd.common.flags |= NVME_CMD_SGL_METABUF; @@ -179,7 +179,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, nvme_cleanup_cmd(req); blk_mq_start_request(req); nvme_loop_queue_response(>req); - return 0; + return BLK_MQ_RQ_QUEUE_OK; } if (blk_rq_bytes(req)) { @@ -198,7 +198,7 @@ static int nvme_loop_queue_rq(struct blk_mq_hw_ctx *hctx, blk_mq_start_request(req); schedule_work(>work); - return 0; + return BLK_MQ_RQ_QUEUE_OK; } static void nvme_loop_submit_async_event(struct nvme_ctrl *arg, int aer_idx) -- 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] scsi: ufs: remove unnecessary UFSHCD_STATE_ERROR set
On 2016-11-15 04:25, Zang Leigang wrote: We set ufshcd_state to UFSHCD_STATE_ERROR when an err handler fails. So I think UFSHCD_STATE_ERROR means host not works any more. Set it before schedule eh_work is not correct. Signed-off-by: Zang Leigangdiff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 05c7456..a7724ad 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -4158,7 +4158,6 @@ static void ufshcd_check_errors(struct ufs_hba *hba) /* block commands from scsi mid-layer */ scsi_block_requests(hba->host); - hba->ufshcd_state = UFSHCD_STATE_ERROR; But we may still want to stop the new requests from being issued (via ufshcd_queuecommand) until error handler (ufshcd_err_handler()) gets chance to run. May be a new intermediate state like UFSHCD_STATE_EH_SCHEDULED should be added and set here, we have to check the same in ufshcd_queuecommand() to return busy if ufshcd_state is UFSHCD_STATE_EH_SCHEDULED. schedule_work(>eh_work); } } -- 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 v2] ufs: add a variety of definitions decribed in UFS spec
On 2016-11-15 03:24, Kiwoong Kim wrote: These things are defined to be used by some UFS Host controllers. And a new file for some declarations of mphy standard is added V2 - modify the commit message - add a new macro for UECDL - add two definitions about UECDL - change the names of two macros (s/IS_PWR_MODE_HS/IS_HS_PWR_MODE, s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE) Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/mphy.h | 38 ++ drivers/scsi/ufs/ufshci.h | 17 ++--- drivers/scsi/ufs/unipro.h | 26 ++ 3 files changed, 78 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..a1a06ac9 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -170,17 +170,28 @@ 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) +#define UIC_DATA_LINK_LAYER_ERROR_CASE(r, x) (r & x) I has this comment on Patch set #1 : "why don't we just add macros for all the bits in UECDL ? This makes it easy in future." and you had replied "Okay. I'll think about it and apply new macros on new version of this patch." but i don't see these macros added. Do you want to add them? Also, i am not sure why we are adding UIC_DATA_LINK_LAYER_ERROR_CASE() macro? One more thing, As Martin mentioned in other email, please separate this version history from commit text with line having "" before the start of version history. Rest all looks good but i will wait for updated patch fixing above before giving Reviewed-By. /* 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) +#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW BIT(5) +#define UIC_TRANSPORT_SAFETY_VALVE_DROPPINGBIT(6) /* 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..490d867 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
Re: [PATCH v2] ufs: introduce UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR quirk
On 2016-11-15 02:57, Kiwoong Kim wrote: If UFS driver resets interrupt aggregation timer and counter when there are some pended tasks, an IO competion interrupt of any corresponing task may be issued. That would casue a command timeout. One thing you should mind to use interrupt aggreation with this quirk is that the host controller should be able to refresh interrupt aggreation counter or timer in other way, such as doing it automatically when receiving any response. V2 As Martin mentioned in other email, please separate this version history from commit text with line having "" before the start of version history. Rest all looks good but i will wait for updated patch fixing above before giving Reviewed-By. - modify the commit message - change the name of the quirk (s/ UFSHCI_QUIRK_SKIP_INTR_AGGR/UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR) Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 3 ++- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8aac98f..7b62d8b 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3713,7 +3713,8 @@ 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_RESET_INTR_AGGR)) 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 dfa17ac..d6861ed 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -505,6 +505,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9) + /* +* This quirk is only not to reset interrupt aggregation logic +* in ISR. The reset can make the host controller miss an event +* of previously completed IO. +*/ + #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR UFS_BIT(10) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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 v2] ufs: introduce UFSHCD_QUIRK_BROKEN_HCE quirk
On 2016-11-15 02:55, Kiwoong Kim wrote: Some UFS host controllers might not be able to reset UIC by setting HCE to 1. Those controllers should invoke 'DME reset' and 'DME enable' in order instead. V2 As Martin mentioned in other email, please separate this version history from commit text with line having "" before the start of version history. Rest all looks good but i will wait for updated patch fixing above before giving Reviewed-By. - modify the commit message - change the name of the quirk (s/UFSHCD_QUIRK_USE_OF_HCE/UFSHCD_QUIRK_BROKEN_HCE) Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 44 +++- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index c9cf011..8aac98f 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2477,6 +2477,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); + + 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); + + return ret; +} + /** * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET * @hba: per adapter instance @@ -3084,6 +3115,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), @@ -3100,6 +3132,9 @@ static int ufshcd_hba_enable(struct ufs_hba *hba) ufshcd_vops_hce_enable_notify(hba, PRE_CHANGE); + if (hba->quirks & UFSHCD_QUIRK_BROKEN_HCE) + goto use_dme; + /* start controller initialization sequence */ ufshcd_hba_start(hba); @@ -3128,12 +3163,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_BROKEN_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 9838598..dfa17ac 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -498,6 +498,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) + /* +* This quirk needs to be enabled if the host contoller can't reset +* UIC by setting HCE to 1. Those controllers should invoke +* DME reset and DME enable in order. +*/ + #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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 v2] ufs: introduce UFSHCD_QUIRK_BROKEN_REQ_LIST_CLR quirk
On 2016-11-15 02:52, Kiwoong Kim wrote: Some UFS host controllers may clear a transfer request slot by setting an associated bit in UTRLCLR/UTMRLCLR to 1, not 0. That's opposite to what UFS spec describes. v2: modify the commit message, remove unrelated changes As Martin mentioned in other email, please separate this version history from commit text with line having "" before the start of version history. Rest all looks good but i will wait for updated patch fixing above before giving Reviewed-By. Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 28 ++-- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index d6e3112..c9cf011 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); } /** @@ -4312,7 +4336,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 7d9ff22..9838598 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -491,6 +491,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7) + /* +* This quirk needs to be enabled if the host contoller has to set +* the bit corresponding the slot to be cleared to 1, not 0 as +* described in UFS spec. +*/ + #define UFSHCD_QUIRK_BROKEN_REQ_LIST_CLRUFS_BIT(8) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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 v2] ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN quirk
On 2016-11-15 02: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 | 28 +--- drivers/scsi/ufs/ufshcd.h | 6 ++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e75fbb3..d6e3112 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1128,7 +1128,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) { struct ufshcd_sg_entry *prd_table; struct scatterlist *sg; @@ -1142,8 +1142,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_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; @@ -1505,7 +1510,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); if (err) { lrbp->cmd = NULL; clear_bit_unlock(tag, >lrb_in_use); @@ -2366,12 +2371,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_PRDT_BYTE_GRAN) { + 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 8e76501..7d9ff22 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -485,6 +485,12 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) + /* +* This quirk needs to be enabled if the host contoller regards +* resolution of the values of PRDTO and PRDTL in UTRD as byte. +*/ + #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device spec. */ 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
Back to Synopsys
Hi all, I am back to Synopsys, so please don't apply the patches I sent a few weeks ago with the goal of changing the UFS drivers author e-mail. I will be available to help in anything you need. Thanks, Joao -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] ata: xgene: Enable NCQ support for APM X-Gene SATA controller hardware v1.1
Hello, Rameshwar. On Fri, Nov 11, 2016 at 01:36:28PM +0530, Rameshwar Sahu wrote: > Hi Tejun, > > On Wed, Nov 9, 2016 at 10:15 PM, Tejun Heowrote: > > Hello, > > > > On Wed, Sep 14, 2016 at 04:15:00PM +0530, Rameshwar Sahu wrote: > >> > @@ -821,8 +823,6 @@ static int xgene_ahci_probe(struct platform_device > >> > *pdev) > >> > dev_warn(>dev, "%s: Error reading > >> > device info. Assume version1\n", > >> > __func__); > >> > version = XGENE_AHCI_V1; > >> > - } else if (info->valid & ACPI_VALID_CID) { > >> > - version = XGENE_AHCI_V2; > > > > Can you please explain this part a bit? Everything else looks good to > > me. > > Here we should not assume XGENE_AHCI_V2 always in case of having valid > _CID in ACPI table. > I need to remove this assumption because V1_1 has also valid _CID for > backward compatibly with v1. Can you please repost with the above explanation added to the commit message? Thanks! -- tejun -- 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: correct logical resets
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Monday, November 14, 2016 5:44 PM > To: Don Brace > Cc: j...@linux.vnet.ibm.com; John Hall; Kevin Barnett; Mahesh > Rajashekhara; Bader Ali-saleh; h...@infradead.org; Scott Teel; Viswas G; > Justin Lindley; Scott Benesh; elli...@hpe.com; posw...@suse.com; linux- > s...@vger.kernel.org > Subject: Re: [PATCH] hpsa: correct logical resets > > EXTERNAL EMAIL > > > > "Don" == Don Bracewrites: > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > index 0b6eb5a..a296537 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -300,6 +300,10 @@ static bool hpsa_cmd_dev_match(struct ctlr_info *h, > struct CommandList *c, >struct hpsa_scsi_dev_t *dev, >unsigned char *scsi3addr); > > +static int wait_for_device_to_become_ready(struct ctlr_info *h, > + unsigned char lunaddr[], > + int reply_queue); > + > static inline struct ctlr_info *sdev_to_hba(struct scsi_device *sdev) > { > unsigned long *priv = shost_priv(sdev->host); > > Wouldn't it be nicer to put this with the rest of the function > prototypes at the beginning of the file? > > -- > Martin K. Petersen Oracle Linux Engineering Ok, I can move it up above static int hpsa_wait_for_board_state(struct pci_dev *pdev, void __iomem *vaddr, int wait_for_ready); Thanks, Don Brace ESC - Smart Storage Microsemi 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 v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Nov 15, 2016 at 03:31:27PM +0100, Steffen Maier wrote: > Hi Johannes, > > On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: > > On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > > > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > > > Hm, still behaves for me like I reported for v2: > > > > > http://marc.info/?l=linux-scsi=147637177902937=2 > > > > [...] > > > > > > > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > > > job->request as req->cmd and job->request_len = req->cmd_len. But > > > > without > > > > checkinf job->request_len we don't know whether we're save to touch > > > > job->request (a.k.a. bsg_request). > > > > > > Hi Steffen, > > > Did you have any chance testing this? I hacked fcping to work with > > > non-FCoE > > > and rports as well and tested with FCoE and lpfc. No problems seen from my > > > side. I've also pused the series (With this change folded in) to my git > > > tree at [1] if this helps you in any way. > > > > > > [1] > > > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > > > > > > > So I finally have a test system up and running. I have good and bad news. > > The > > good news is, I can't get the system crashing with my patches, the bad news > > is > > I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR > > with my patches and without. Please ignore my last mails, apparently it's a wise idea to check which user id one has before running zfcp_ping... The good news for this is, I can now recreate the crashes you have and thus have a chance to fix them :-) Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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 Tue, Nov 15, 2016 at 02:50:17PM +, Bart Van Assche wrote: > On Tue, 2016-11-15 at 10:18 +0100, Johannes Thumshirn wrote: > > On Tue, Nov 08, 2016 at 03:04:43PM +, Bart Van Assche wrote: > > > 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; > > > > But what happens when jiffies - boot_time becomes negative? Then we > > reintroduce the bug again and have 'fcoeadm -s' show weird values. > > Hello Johannes, > > If your concern is about 'jiffies' wrapping around on 32-bit systems > then you should use get_jiffies_64(). get_jiffies_64() - boot_time > can't become negative. It namely takes several million years before a > 64-bit HZ counter wraps around. You're right. I'll respin using get_jiffies_64() and resent once it is tested. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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 Tue, 2016-11-15 at 10:18 +0100, Johannes Thumshirn wrote: > On Tue, Nov 08, 2016 at 03:04:43PM +, Bart Van Assche wrote: > > 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; > > But what happens when jiffies - boot_time becomes negative? Then we > reintroduce the bug again and have 'fcoeadm -s' show weird values. Hello Johannes, If your concern is about 'jiffies' wrapping around on 32-bit systems then you should use get_jiffies_64(). get_jiffies_64() - boot_time can't become negative. It namely takes several million years before a 64-bit HZ counter wraps around. Bart.
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Nov 15, 2016 at 03:31:27PM +0100, Steffen Maier wrote: > Hi Johannes, > > On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: > > On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > > > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > > > Hm, still behaves for me like I reported for v2: > > > > > http://marc.info/?l=linux-scsi=147637177902937=2 > > > > [...] > > > > > > > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > > > job->request as req->cmd and job->request_len = req->cmd_len. But > > > > without > > > > checkinf job->request_len we don't know whether we're save to touch > > > > job->request (a.k.a. bsg_request). > > > > > > Hi Steffen, > > > Did you have any chance testing this? I hacked fcping to work with > > > non-FCoE > > > and rports as well and tested with FCoE and lpfc. No problems seen from my > > > side. I've also pused the series (With this change folded in) to my git > > > tree at [1] if this helps you in any way. > > > > > > [1] > > > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > > > > > > > So I finally have a test system up and running. I have good and bad news. > > The > > good news is, I can't get the system crashing with my patches, the bad news > > is > > I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR > > with my patches and without. > > Assuming you run the latest package version on s390x: > > Do steps 2 and 3 of the procedure in > http://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lhdd/lhdd_t_fcp_api_runappl.html > help? No, I do have a correct /etc/hba.conf: $ grep libzfcphbaapi /etc/hba.conf com.ibm.libzfcphbaapi /usr/lib64/libzfcphbaapi.so.0 and bumping the log level via export LIB_ZFCP_HBAAPI_LOG_LEVEL=1 just adds '(vlib.c:105)_initvlib: libzfcphbaapi.so loaded at Nov 15 15:43:23' to the 1st line of zfcp_ping output. Here's the full output: $ zfcp_ping -v -a 0.0.fc00 -d 0x50050763051b473a (vlib.c:105)_initvlib: libzfcphbaapi.so loaded at Nov 15 15:45:33 Sending PNG from BUS_ID=0.0.fc00 WWPN=0xc05076e0f3002344 ID=0x760c1c dev=/dev/bsg/fc_host1 speed=8 GBit/s --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 --- REQUEST cmd = 0x0401 --- 03 00 00 00 fa 01 00 00 04 01 00 01 00 00 00 00 00 00 00 01 00 02 00 08 50 05 07 63 05 1b 47 3a 00 00 00 00 --- RESPONSE rc = 0x1 --- 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Error received for FPNG request, aborting. -- ping statistics --- min/avg/max = 0.000/0.000/0.000 ms -- I agree this looks a bit suspicious and I'll add some debugs in the kernel to see what's going on. > > The only other thing I can think of from the top of my head is that BSG > ioctls are sensitive regarding ABI and I once had the kernel ioctl return > EINVAL due to unmatching kernel-headers and libzfcphbaapi maps this EINVAL > to HBA_STATUS_ERROR because there is no more specifically suitable HBA > constant [old SUSE bugs 834498 and 834500]. > > > And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone > > from it (it has patch 2/16 changed to the v3 submission). > > > > Can you please have a look with your setup? > > I'm going to re-test hopefully within the next few days. That'll be great, thanks. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
Hi Johannes, On 11/15/2016 12:56 PM, Johannes Thumshirn wrote: On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: Hm, still behaves for me like I reported for v2: http://marc.info/?l=linux-scsi=147637177902937=2 [...] The rational behind this is, in fc_req_to_bsgjob() we're assigning job->request as req->cmd and job->request_len = req->cmd_len. But without checkinf job->request_len we don't know whether we're save to touch job->request (a.k.a. bsg_request). Hi Steffen, Did you have any chance testing this? I hacked fcping to work with non-FCoE and rports as well and tested with FCoE and lpfc. No problems seen from my side. I've also pused the series (With this change folded in) to my git tree at [1] if this helps you in any way. [1] https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 So I finally have a test system up and running. I have good and bad news. The good news is, I can't get the system crashing with my patches, the bad news is I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR with my patches and without. Assuming you run the latest package version on s390x: Do steps 2 and 3 of the procedure in http://www.ibm.com/support/knowledgecenter/linuxonibm/com.ibm.linux.z.lhdd/lhdd_t_fcp_api_runappl.html help? The only other thing I can think of from the top of my head is that BSG ioctls are sensitive regarding ABI and I once had the kernel ioctl return EINVAL due to unmatching kernel-headers and libzfcphbaapi maps this EINVAL to HBA_STATUS_ERROR because there is no more specifically suitable HBA constant [old SUSE bugs 834498 and 834500]. And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone from it (it has patch 2/16 changed to the v3 submission). Can you please have a look with your setup? I'm going to re-test hopefully within the next few days. -- Mit freundlichen Grüßen / Kind regards Steffen Maier Linux on z Systems Development IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschaeftsfuehrung: Dirk Wittkopp Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- 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
[RFC PATCH] scsi:fc: avoid a permanent stop of the scsi device's request queue
A scan work can run simultaneously with fc_remote_port_delete(). If a scsi device is added to the ->__devices list in the scan work, it can be touched and will be blocked in scsi_target_block(), and QUEUE_FLAG_STOPPED will be setted to the scsi device's request queue. But unfortunately, this flag will not be cleared when the scsi device is being setted to the SDEV_RUNNING state in the end of the scan work. After that, all the requests sending to the device will be blocked. Fix this by flushing the scan work before blocking the device. Reported-by: Zengxi ChenSigned-off-by: Wei Fang --- drivers/scsi/scsi_transport_fc.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c index e05c07f..0d476df 100644 --- a/drivers/scsi/scsi_transport_fc.c +++ b/drivers/scsi/scsi_transport_fc.c @@ -2978,14 +2978,6 @@ fc_remote_port_delete(struct fc_rport *rport) unsigned long timeout = rport->dev_loss_tmo; unsigned long flags; - /* -* No need to flush the fc_host work_q's, as all adds are synchronous. -* -* We do need to reclaim the rport scan work element, so eventually -* (in fc_rport_final_delete()) we'll flush the scsi host work_q if -* there's still a scan pending. -*/ - spin_lock_irqsave(shost->host_lock, flags); if (rport->port_state != FC_PORTSTATE_ONLINE) { @@ -3012,6 +3004,14 @@ fc_remote_port_delete(struct fc_rport *rport) spin_unlock_irqrestore(shost->host_lock, flags); + /* +* make sure no scan is pending before blocking it, otherwise +* simultaneously scan may cause a permanent QUEUE_FLAG_STOPPED +* flag set of the device's request queue. +*/ + if (rport->flags & FC_RPORT_SCAN_PENDING) + scsi_flush_work(shost); + scsi_target_block(>dev); /* see if we need to kill io faster than waiting for device loss */ -- 2.4.11 -- 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: ufs: remove unnecessary UFSHCD_STATE_ERROR set
We set ufshcd_state to UFSHCD_STATE_ERROR when an err handler fails. So I think UFSHCD_STATE_ERROR means host not works any more. Set it before schedule eh_work is not correct. Signed-off-by: Zang Leigangdiff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 05c7456..a7724ad 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -4158,7 +4158,6 @@ static void ufshcd_check_errors(struct ufs_hba *hba) /* block commands from scsi mid-layer */ scsi_block_requests(hba->host); - hba->ufshcd_state = UFSHCD_STATE_ERROR; schedule_work(>eh_work); } } -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 02/16] scsi: don't use fc_bsg_job::request and fc_bsg_job::reply directly
On Tue, Oct 25, 2016 at 09:43:14AM +0200, Johannes Thumshirn wrote: > On Fri, Oct 14, 2016 at 09:38:21AM +0200, Johannes Thumshirn wrote: > > On Thu, Oct 13, 2016 at 05:55:11PM +0200, Steffen Maier wrote: > > > Hm, still behaves for me like I reported for v2: > > > http://marc.info/?l=linux-scsi=147637177902937=2 [...] > > > > The rational behind this is, in fc_req_to_bsgjob() we're assigning > > job->request as req->cmd and job->request_len = req->cmd_len. But without > > checkinf job->request_len we don't know whether we're save to touch > > job->request (a.k.a. bsg_request). > > Hi Steffen, > Did you have any chance testing this? I hacked fcping to work with non-FCoE > and rports as well and tested with FCoE and lpfc. No problems seen from my > side. I've also pused the series (With this change folded in) to my git > tree at [1] if this helps you in any way. > > [1] > https://git.kernel.org/cgit/linux/kernel/git/jth/linux.git/log/?h=scsi-bsg-rewrite-v4 > So I finally have a test system up and running. I have good and bad news. The good news is, I can't get the system crashing with my patches, the bad news is I can't get zfcp_ping and zfcp_show to output something but HBA_STATUS_ERROR with my patches and without. And btw, I renamed the branch to fc-bsg-rewrite-v4 in case you want to clone from it (it has patch 2/16 changed to the v3 submission). Can you please have a look with your setup? Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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] ufs: add a variety of definitions decribed in UFS spec
These things are defined to be used by some UFS Host controllers. And a new file for some declarations of mphy standard is added V2 - modify the commit message - add a new macro for UECDL - add two definitions about UECDL - change the names of two macros (s/IS_PWR_MODE_HS/IS_HS_PWR_MODE, s/IS_PWR_MODE_PWM/IS_PWM_PWR_MODE) Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/mphy.h | 38 ++ drivers/scsi/ufs/ufshci.h | 17 ++--- drivers/scsi/ufs/unipro.h | 26 ++ 3 files changed, 78 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..a1a06ac9 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -170,17 +170,28 @@ 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) +#define UIC_DATA_LINK_LAYER_ERROR_CASE(r, x) (r & x) /* 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) +#define UIC_TRANSPORT_E2E_CREDIT_OVERFLOW BIT(5) +#define UIC_TRANSPORT_SAFETY_VALVE_DROPPINGBIT(6) /* 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..490d867 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_HS_PWR_MODE(m)(((m) == FAST_MODE) || ((m) == FASTAUTO_MODE)) +#define IS_PWM_PWR_MODE(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 */ @@
[PATCH v2] ufs: introduce UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR quirk
If UFS driver resets interrupt aggregation timer and counter when there are some pended tasks, an IO competion interrupt of any corresponing task may be issued. That would casue a command timeout. One thing you should mind to use interrupt aggreation with this quirk is that the host controller should be able to refresh interrupt aggreation counter or timer in other way, such as doing it automatically when receiving any response. V2 - modify the commit message - change the name of the quirk (s/ UFSHCI_QUIRK_SKIP_INTR_AGGR/UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR) Signed-off-by: Kiwoong Kim--- drivers/scsi/ufs/ufshcd.c | 3 ++- drivers/scsi/ufs/ufshcd.h | 7 +++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 8aac98f..7b62d8b 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -3713,7 +3713,8 @@ 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_RESET_INTR_AGGR)) 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 dfa17ac..d6861ed 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -505,6 +505,13 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_HCE UFS_BIT(9) + /* +* This quirk is only not to reset interrupt aggregation logic +* in ISR. The reset can make the host controller miss an event +* of previously completed IO. +*/ + #define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR UFS_BIT(10) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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
[PATCH v2] ufs: introduce UFSHCD_QUIRK_PRDT_BYTE_GRAN 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 | 28 +--- drivers/scsi/ufs/ufshcd.h | 6 ++ 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index e75fbb3..d6e3112 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -1128,7 +1128,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) { struct ufshcd_sg_entry *prd_table; struct scatterlist *sg; @@ -1142,8 +1142,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_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; @@ -1505,7 +1510,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); if (err) { lrbp->cmd = NULL; clear_bit_unlock(tag, >lrb_in_use); @@ -2366,12 +2371,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_PRDT_BYTE_GRAN) { + 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 8e76501..7d9ff22 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -485,6 +485,12 @@ struct ufs_hba { */ #define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION UFS_BIT(5) + /* +* This quirk needs to be enabled if the host contoller regards +* resolution of the values of PRDTO and PRDTL in UTRD as byte. +*/ + #define UFSHCD_QUIRK_PRDT_BYTE_GRAN UFS_BIT(7) + unsigned int quirks;/* Deviations from standard UFSHCI spec. */ /* Device deviations from standard UFS device 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
RE: [PATCH] megaraid-sas: request irqs later
>-Original Message- >From: Martin K. Petersen [mailto:martin.peter...@oracle.com] >Sent: Tuesday, November 15, 2016 5:18 AM >To: Tomas Henzl >Cc: linux-scsi@vger.kernel.org; sumit.sax...@broadcom.com; >kashyap.de...@broadcom.com >Subject: Re: [PATCH] megaraid-sas: request irqs later > >> "Tomas" == Tomas Henzlwrites: > >Tomas> It is not good when an irq arrives before driver structures are >Tomas> allocated. > >Sumit, Kashyap: Please review! Looks good.. I think I have acked this patch earlier also. Acked-by: Sumit Saxena > >-- >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] libfc: fix seconds_since_last_reset miscalculation
On Tue, Nov 08, 2016 at 03:04:43PM +, Bart Van Assche wrote: [...] > 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; But what happens when jiffies - boot_time becomes negative? Then we reintroduce the bug again and have 'fcoeadm -s' show weird values. Byte, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- 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