Re: targetcli -fb now also Apache 2.0 licensed
On Friday 26 July 2013 09:01 AM, Andy Grover wrote: > > It's one thing to claim a prerogative of "upstream", but for this to > make sense, there needs to be an actual community around the upstream. > And if there's going to be submissions for review, then there needs to > be someone in charge of the community with a high degree of skill in > the code base. Nick you have a great deal of technical expertise > around the C code in drivers/target, but Jerome was the one who wrote > rtslib, targetcli, and configshell. I believe you can assess the > technical aspects of how the user library interacts with the kernel > code, but the maintainer should also be extremely conversant in the > language the library is written in. In this case, Python. So it's not > clear to me if submitting the code would actually result in meaningful > code improvements. > > Also, there has been no effort to sustain a community around this > code. There is no bug tracking, no separate mailing list, no regular > releases. Debian is running ancient, broken code because nothing's > been tagged in over two years. > > I would love to have you maintain and improve this code, but if you > aren't then you can't just say "I'm the upstream bow to me!". We're > shipping this code in Fedora and it needs active maintenance. Now that > we're all on an even licensing footing, code can flow both ways, and > even into your commercial version. Yes. Apart from reaching out the developers directly, I am not aware of other communication channels. There is a git and wiki though. Nick, if you guys have the same license now, nothing should stop you to pull the changes from the targetcli-fb repo. You can start afresh now and comply with what the community guidelines are. Licensing is just one part of it. And regarding this whole licensing dispute in the open, the kernel maintainers should have resolved this long ago, when it was decided to have LIO as the in-kernel target for Linux. You might say that the target component complied by the kernel's licensing requirements. But for a subsystem inclusion, you don't look just at the code, but also the maintainer. Their support tools. And their long term plans. These rules have been applied, in the past, for other features in the kernel. -- Ritesh Raj Sarraf | http://people.debian.org/~rrs Debian - The Universal Operating System signature.asc Description: OpenPGP digital signature
Re: [PATCHv2 0/7] Limit overall SCSI EH runtime
Hi, Hannes: On 07/15/2013 06:33 PM, Ren Mingxin wrote: I noticed that the dd time had been reduced from 6m+ to 2m+ when the 'eh_deadline' was set as 30s, but the dd time was 6m+(nearly the same as default - 'eh_deadline' was 0) when the 'eh_deadline' was set as 10s. I havn't been able to dig further, but I guess there is some restriction when setting this 'eh_deadline' interface. Maybe should not less than some timeout, otherwise 'eh_deadline' setting will not work? I've retried and confirmed that the exception above is caused by misoperation - for I had two fc hosts to build a failover multipath, but I just set 'eh_deadline' on one host. When I tested with 10s, the 'eh_deadline' on the host of the active path wasn't set :-( Sorry for my mistake. So: Tested-by: Ren Mingxin Thanks, Ren -- 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: hpsa - BUG: using smp_processor_id() in preemptible [00000000 00000000] code: kworker/u:0/6
- Original Message - > [Adding missing cc to linux-scsi] > On Thu, 2013-07-25 at 23:33 +0200, John Kacur wrote: > > Hi > > > > We're seeing this on a 3.6 kernel with the real-time patch applied, but it > > looks like it is relevant with the real-time patch in the latest kernel This should read, "it looks like it is relevant WITHOUT the real-time patch in the latest kernel". > > too. > > > > [ 49.688847] hpsa :03:00.0: hpsa0: <0x323a> at IRQ 67 using DAC > > [ 49.749928] scsi0 : hpsa > > [ 49.784437] BUG: using smp_processor_id() in preemptible [ > > ] code: kworker/u:0/6 > > [ 49.784465] caller is enqueue_cmd_and_start_io+0x5a/0x100 [hpsa] > > [ 49.784468] Pid: 6, comm: kworker/u:0 Not tainted > > 3.6.11.5-rt37.52.el6rt.x86_64.debug #1 > > [ 49.784471] Call Trace: > > [ 49.784512] [] debug_smp_processor_id+0x123/0x150 > > [ 49.784520] [] enqueue_cmd_and_start_io+0x5a/0x100 > > [hpsa] > > [ 49.784529] [] > > hpsa_scsi_do_simple_cmd_core+0xeb/0x110 [hpsa] > > [ 49.784537] [] ? swiotlb_dma_mapping_error+0x18/0x30 > > [ 49.784544] [] ? swiotlb_dma_mapping_error+0x18/0x30 > > [ 49.784553] [] > > hpsa_scsi_do_simple_cmd_with_retry+0x91/0x280 [hpsa] > > [ 49.784562] [] > > hpsa_scsi_do_report_luns.clone.2+0xd8/0x130 [hpsa] > > [ 49.784571] [] > > hpsa_gather_lun_info.clone.3+0x3a/0x1a0 [hpsa] > > [ 49.784580] [] hpsa_update_scsi_devices+0x11f/0x4f0 > > [hpsa] > > [ 49.784592] [] ? sub_preempt_count+0xa9/0xe0 > > [ 49.784601] [] hpsa_scan_start+0xfd/0x150 [hpsa] > > [ 49.784613] [] ? rt_spin_lock_slowunlock+0x78/0x90 > > [ 49.784626] [] do_scsi_scan_host+0x37/0xa0 > > [ 49.784632] [] do_scan_async+0x1a/0x30 > > [ 49.784643] [] async_run_entry_fn+0x9b/0x1d0 > > [ 49.784655] [] process_one_work+0x1f2/0x620 > > [ 49.784661] [] ? process_one_work+0x180/0x620 > > [ 49.784668] [] ? worker_thread+0x5e/0x3a0 > > [ 49.784674] [] ? async_schedule+0x20/0x20 > > [ 49.784681] [] worker_thread+0x133/0x3a0 > > [ 49.784688] [] ? manage_workers+0x190/0x190 > > [ 49.784696] [] kthread+0xa6/0xb0 > > [ 49.784707] [] kernel_thread_helper+0x4/0x10 > > [ 49.784715] [] ? finish_task_switch+0x8c/0x110 > > [ 49.784721] [] ? _raw_spin_unlock_irq+0x3b/0x70 > > [ 49.784727] [] ? retint_restore_args+0xe/0xe > > [ 49.784734] [] ? kthreadd+0x1e0/0x1e0 > > [ 49.784739] [] ? gs_change+0xb/0xb > > > > --- > > > > When I look at the code I see this call chain > > enqueue_cmd_and_start_io()-> > > set_performant_mode()-> > > smp_processor_id() > > Which if you have debugging enabled calls debug_processor_id() and > > triggers the warning. > > > > I'm not very familiar with the hpsa code, so I'm not entirely sure what > > the purpose of this line is > > > > c->Header.ReplyQueue = smp_processor_id() % h->nreply_queues; > > > > Is the purpose to simply try to get a range of ReplyQueue numbers, but > > somewhat arbitrary? Or is it necessary that the current processor_id > > is used? If it is the former, and you're not accessing per cpu structures, > > or pinning a cpu, or anything like that then I would think it is safe to > > change this to a raw_smp_processor_id() to get rid of a false positive > > warning. > > > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > > index 7f4f790..4e19267 100644 > > --- a/drivers/scsi/hpsa.c > > +++ b/drivers/scsi/hpsa.c > > @@ -583,7 +583,7 @@ static void set_performant_mode(struct ctlr_info *h, > > struct CommandList *c) > > c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1); > > if (likely(h->msix_vector)) > > c->Header.ReplyQueue = > > - smp_processor_id() % h->nreply_queues; > > + raw_smp_processor_id() % h->nreply_queues; > > } > > } > > > > > > Thanks > > > > John Kacur > > > -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 0/2] block: Fix regression since 46081b166415acb66d4b3150ecefcd9460bb48a1 (was: Allow merging of tail pages into the last segment)
On 12/07/13 17:52, Jan Vesely wrote: > Hi > > These patches modify __bio_add_page to accept pages that extent the last bio > segment. some drivers craft their buffers and rely on this behavior (see > message in patch 2 for details) > > > jan > > v4: whitespace fixes to make checkpatch happy > > v3: Use code from __blk_recalc_rq_segments to decide whether the page is > mergeable, > > v2: modify a comment ping and a bit more info from patch 2/2: The original behavior was to refuse all pages after the maximum number of segments has been reached. However, some drivers (like st) craft their buffers to potentially require exactly max segments and multiple pages in the last segment. This patch modifies the check to allow pages that can be merged into the last segment. Fixes EBUSY failures when using large tape block size in high memory fragmentation condition. This regression was introduced by commit 46081b166415acb66d4b3150ecefcd9460bb48a1 st: Increase success probability in driver buffer allocation Jan -- Jan Vesely -- 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: [RFC PATCH 2/2] ata: acpi: rework the ata acpi bind support
On Friday, July 26, 2013 09:37:09 AM Aaron Lu wrote: > On 07/25/2013 10:52 PM, Tejun Heo wrote: > > On Thu, Jul 25, 2013 at 01:47:03PM +0800, Aaron Lu wrote: > >> Binding ACPI handle to SCSI device has several drawbacks, namely: > >> 1 During ATA device initialization time, ACPI handle will be needed > >> while SCSI devices are not created yet. So each time ACPI handle is > >> needed, instead of retrieving the handle by ACPI_HANDLE macro, > >> a namespace scan is performed to find the handle for the corresponding > >> ATA device. This is inefficient, and also expose a restriction on > >> calling path not holding any lock. > >> 2 The binding to SCSI device tree makes code complex, while at the same > >> time doesn't bring us any benefit. All ACPI handlings are still done > >> in ATA module, not in SCSI. > >> > >> Rework the ATA ACPI binding code to bind ACPI handle to ATA transport > >> devices(ATA port and ATA device). The binding needs to be done only once, > >> since the ATA transport devices do not go away with hotplug. And due to > >> this, the flush_work call in hotplug handler for ATA bay is no longer > >> needed. > > > > I like it but am wondering why we weren't doing this before. Was the > > acpi support added before we made ata objects proper devices? > > I think so. But since I didn't do the original binding, I can only guess > :-) > > At the time of the original binding, ACPI binding logic requires the > binding device has a bus type, which these ATA transport devices don't > have. > > Later, the ACPI binding code evolves and no such limitation exists > anymore. As you can see, we can simply set the ACPI handle before this > device is added in driver core, and the binding will be done. Yeah, we made that change in the ACPI core around 3.8 IIRC. Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: hpsa - BUG: using smp_processor_id() in preemptible [00000000 00000000] code: kworker/u:0/6
On Fri, Jul 26, 2013 at 06:28:02AM -0400, John Kacur wrote: > > > - Original Message - > > [Adding missing cc to linux-scsi] > > On Thu, 2013-07-25 at 23:33 +0200, John Kacur wrote: > > > Hi > > > > > > We're seeing this on a 3.6 kernel with the real-time patch applied, but it > > > looks like it is relevant with the real-time patch in the latest kernel > > This should read, "it looks like it is relevant WITHOUT the real-time patch > in the latest kernel". > > > > > too. > > > > > > [ 49.688847] hpsa :03:00.0: hpsa0: <0x323a> at IRQ 67 using DAC > > > [ 49.749928] scsi0 : hpsa > > > [ 49.784437] BUG: using smp_processor_id() in preemptible [ > > > ] code: kworker/u:0/6 > > > [ 49.784465] caller is enqueue_cmd_and_start_io+0x5a/0x100 [hpsa] > > > [ 49.784468] Pid: 6, comm: kworker/u:0 Not tainted > > > 3.6.11.5-rt37.52.el6rt.x86_64.debug #1 > > > [ 49.784471] Call Trace: > > > [ 49.784512] [] debug_smp_processor_id+0x123/0x150 > > > [ 49.784520] [] enqueue_cmd_and_start_io+0x5a/0x100 > > > [hpsa] > > > [ 49.784529] [] > > > hpsa_scsi_do_simple_cmd_core+0xeb/0x110 [hpsa] > > > [ 49.784537] [] ? swiotlb_dma_mapping_error+0x18/0x30 > > > [ 49.784544] [] ? swiotlb_dma_mapping_error+0x18/0x30 > > > [ 49.784553] [] > > > hpsa_scsi_do_simple_cmd_with_retry+0x91/0x280 [hpsa] > > > [ 49.784562] [] > > > hpsa_scsi_do_report_luns.clone.2+0xd8/0x130 [hpsa] > > > [ 49.784571] [] > > > hpsa_gather_lun_info.clone.3+0x3a/0x1a0 [hpsa] > > > [ 49.784580] [] hpsa_update_scsi_devices+0x11f/0x4f0 > > > [hpsa] > > > [ 49.784592] [] ? sub_preempt_count+0xa9/0xe0 > > > [ 49.784601] [] hpsa_scan_start+0xfd/0x150 [hpsa] > > > [ 49.784613] [] ? rt_spin_lock_slowunlock+0x78/0x90 > > > [ 49.784626] [] do_scsi_scan_host+0x37/0xa0 > > > [ 49.784632] [] do_scan_async+0x1a/0x30 > > > [ 49.784643] [] async_run_entry_fn+0x9b/0x1d0 > > > [ 49.784655] [] process_one_work+0x1f2/0x620 > > > [ 49.784661] [] ? process_one_work+0x180/0x620 > > > [ 49.784668] [] ? worker_thread+0x5e/0x3a0 > > > [ 49.784674] [] ? async_schedule+0x20/0x20 > > > [ 49.784681] [] worker_thread+0x133/0x3a0 > > > [ 49.784688] [] ? manage_workers+0x190/0x190 > > > [ 49.784696] [] kthread+0xa6/0xb0 > > > [ 49.784707] [] kernel_thread_helper+0x4/0x10 > > > [ 49.784715] [] ? finish_task_switch+0x8c/0x110 > > > [ 49.784721] [] ? _raw_spin_unlock_irq+0x3b/0x70 > > > [ 49.784727] [] ? retint_restore_args+0xe/0xe > > > [ 49.784734] [] ? kthreadd+0x1e0/0x1e0 > > > [ 49.784739] [] ? gs_change+0xb/0xb > > > > > > --- > > > > > > When I look at the code I see this call chain > > > enqueue_cmd_and_start_io()-> > > > set_performant_mode()-> > > > smp_processor_id() > > > Which if you have debugging enabled calls debug_processor_id() and > > > triggers the warning. > > > > > > I'm not very familiar with the hpsa code, so I'm not entirely sure what > > > the purpose of this line is > > > > > > c->Header.ReplyQueue = smp_processor_id() % h->nreply_queues; > > > > > > Is the purpose to simply try to get a range of ReplyQueue numbers, but > > > somewhat arbitrary? Or is it necessary that the current processor_id > > > is used? If it is the former, and you're not accessing per cpu structures, > > > or pinning a cpu, or anything like that then I would think it is safe to > > > change this to a raw_smp_processor_id() to get rid of a false positive > > > warning. It's not critical that they match (will work if they don't) but for certain workloads you can get more performance if you pin processes to cpus and arrange msix interrupt vectors so that commands are likely to complete on the same cpu they originated from. In any case, I think your analysis is correct. Thanks. > > > > > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > > > index 7f4f790..4e19267 100644 > > > --- a/drivers/scsi/hpsa.c > > > +++ b/drivers/scsi/hpsa.c > > > @@ -583,7 +583,7 @@ static void set_performant_mode(struct ctlr_info *h, > > > struct CommandList *c) > > > c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1); > > > if (likely(h->msix_vector)) > > > c->Header.ReplyQueue = > > > - smp_processor_id() % h->nreply_queues; > > > + raw_smp_processor_id() % h->nreply_queues; > > > } > > > } Ack. -- steve > > > > > > > > > Thanks > > > > > > John Kacur > > > > > > -- 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: Questions about your scsi-mq prototype
On Thu, Jul 25, 2013 at 07:16:39PM -0700, Nicholas A. Bellinger wrote: > On Tue, 2013-07-16 at 10:39 -0700, Nicholas A. Bellinger wrote: > > On Mon, 2013-07-15 at 16:41 -0500, scame...@beardog.cce.hp.com wrote: > > > Hi, > > > > > > I have some questions about your scsi-mq prototype. I have > > > CC'ed Chayan Biswas from Sandisk who is the other guy working > > > with me on the scsi-over-pcie driver. BTW, if you would prefer > > > I CC linux-kernel or linux-scsi or other people with such > > > questions (or refrain from bothering you at all) just let me know. > > > > > > > Thanks for asking. Adding CC's to linux-scsi + Jens now.. > > > > > I wonder if you can provide a summary of how you envision > > > a scsi LLD is expected to interact with the scsi-mq code. I > > > gather that: > > > > > > 1. scsi_mq in scsi_host_template should be set to true. > > > > Correct. > > > > > 2. scsi LLD should provide queuecommand_mq function > > >(in addition to normal queuecommand? or instead of? > > > virtio_scsi.c seems to provide both.) > > > > In addition to for the moment, correct. > > > > > 3. LLD should tell scsi mid layer size of its commands via > > >scsi_host_template cmd_size field. > > >I guess midlayer will pre-allocate per queue to avoid > > >the midlayer threads contending to allocate commands maybe? > > >and hands this space to LLD to use via sc->SCp.ptr (sc is struct > > > scsi_cmnd) > > >(right? or am I off in the weeds here?) > > > > This is all exactly correct. > > > > The one extra point here wrt ->queuecommand_mq() is that it's using the > > same parameters for the moment, but once nr_hw_queues > 1 is > > implemented, we will expect to pass down the associated blk_mq_hw_ctx in > > order for the LLD to determine which hardware context the struct > > scsi_cmnd is being dispatched upon. > > > > > 4. Wondering about how the multiple queue thing will work. > > >I see this: > > > > > >#warning FIXME: Need to pass nr_hw_queues into scsi-mq > > > > > >and I don't see any code setting sdev_mq_reg.nr_hw_queues to > > >anything other than 1. > > > > > >So maybe I'm just looking too early and that part isn't done yet? > > > > > > > Correct, not done yet.. > > > > However, so far with a single SCSI LUN with nr_hw_queues=1, 4k blocksize > > randrw performance is on the order of 1M, vs. ~250K using legacy > > scsi_request_fn(). > > > > > In any case, since we switched away from a scsi driver for the > > > scsi-over-pcie > > > driver to a block driver, the original scsi version of the driver has lain > > > fallow, and the first thing we would need to do before we could begin to > > > try > > > to use your code is to update that driver to work with our current > > > hardware, and > > > take advantage of other code improvements made in the block driver. > > > > > > > Hi Stephen & Robert, > > Just curious if you've been able to make any progress here..? Only just started to try to make the scsi variant of the sop driver work again yesterday and start bringing it up to date since it was last touched in December, and so I haven't done anything with the scsi-mq code yet. Too many other things demanding my attention at work lately, but at least I was able to spend nearly all day yesterday working on the scsi SOP driver. > > Also as per our earlier off-list discussion, you should be able to > safely run existing scsi_request_fn() based LLDs along side new > SHT->scsi_mq=true enabled code for prototyping purposes. Thanks, good to know. > > Btw, any chance to get an SOP sample to hack on..? I'd like to connect > one of these to IvyBridge-EP silicon for various scsi-mq and target mode > performance testing. ;) You mean hardware? Not for me to answer, but I'd say it's not very likely at the moment. -- steve -- 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 0/7] scsi: ufs: some fixes and updates
This path series contain driver's fixes and updates. Seungwon Jeon (7): scsi: ufs: amend the ocs handling with fatal error scsi: ufs: find out sense data over scsi status values scsi: ufs: fix the setting interrupt aggregation counter scsi: ufs: add dme configuration primitives scsi: ufs: add unipro attribute IDs scsi: ufs: add operation for the uic power mode change scsi: ufs: configure the attribute for power mode drivers/scsi/ufs/ufs.h|1 + drivers/scsi/ufs/ufshcd.c | 325 +++-- drivers/scsi/ufs/ufshcd.h | 54 drivers/scsi/ufs/ufshci.h | 22 +++- drivers/scsi/ufs/unipro.h | 151 + 5 files changed, 507 insertions(+), 46 deletions(-) Thanks, Seungwon Jeon -- 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/7] scsi: ufs: amend the ocs handling with fatal error
Fatal error in OCS(overall command status) field indicates
error conditions which is not covered by UFSHCI.
It means that host cannot define the result of command status
and therefore host may need to check transfer response UPIU's
response and status field.
It was actually found that 'CHECK CONDITION' is stored in status
field of UPIU where OCS is 'FATAL ERROR'.
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufshcd.c |3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index b743bd6..4cf3a2d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -1199,7 +1199,7 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
switch (ocs) {
case OCS_SUCCESS:
-
+ case OCS_FATAL_ERROR:
/* check if the returned transfer response is valid */
result = ufshcd_is_valid_req_rsp(lrbp->ucd_rsp_ptr);
if (result) {
@@ -1229,7 +1229,6 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
case OCS_MISMATCH_DATA_BUF_SIZE:
case OCS_MISMATCH_RESP_UPIU_SIZE:
case OCS_PEER_COMM_FAILURE:
- case OCS_FATAL_ERROR:
default:
result |= DID_ERROR << 16;
dev_err(hba->dev,
--
1.7.0.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 2/7] scsi: ufs: find out sense data over scsi status values
Except for 'GOOD' and 'CHECK CONDITION', other status value
in Response UPIU may or may contain sense data. If a non-zero
value is in the Data Segment Length field, it means that UPIU
has Sense Data in the Data Segment area.
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufs.h|1 +
drivers/scsi/ufs/ufshcd.c | 27 +++
2 files changed, 20 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ufs/ufs.h b/drivers/scsi/ufs/ufs.h
index 139bc06..737c31b 100644
--- a/drivers/scsi/ufs/ufs.h
+++ b/drivers/scsi/ufs/ufs.h
@@ -114,6 +114,7 @@ enum {
MASK_SCSI_STATUS= 0xFF,
MASK_TASK_RESPONSE = 0xFF00,
MASK_RSP_UPIU_RESULT= 0x,
+ MASK_RSP_UPIU_DATA_SEG_LEN = 0x,
};
/* Task management service response */
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 4cf3a2d..688ae0e 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -218,6 +218,20 @@ ufshcd_get_rsp_upiu_result(struct utp_upiu_rsp
*ucd_rsp_ptr)
return be32_to_cpu(ucd_rsp_ptr->header.dword_1) & MASK_RSP_UPIU_RESULT;
}
+/*
+ * ufshcd_get_rsp_upiu_data_seg_len - Get the data segment length
+ * from response UPIU
+ * @ucd_rsp_ptr: pointer to response UPIU
+ *
+ * Return the data segment length.
+ */
+static inline int
+ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp *ucd_rsp_ptr)
+{
+ return be32_to_cpu(ucd_rsp_ptr->header.dword_2) &
+ MASK_RSP_UPIU_DATA_SEG_LEN;
+}
+
/**
* ufshcd_config_int_aggr - Configure interrupt aggregation values.
* Currently there is no use case where we want to configure
@@ -298,7 +312,8 @@ void ufshcd_send_command(struct ufs_hba *hba, unsigned int
task_tag)
static inline void ufshcd_copy_sense_data(struct ufshcd_lrb *lrbp)
{
int len;
- if (lrbp->sense_buffer) {
+ if (lrbp->sense_buffer &&
+ !ufshcd_get_rsp_upiu_data_seg_len(lrbp->ucd_rsp_ptr)) {
len = be16_to_cpu(lrbp->ucd_rsp_ptr->sense_data_len);
memcpy(lrbp->sense_buffer,
lrbp->ucd_rsp_ptr->sense_data,
@@ -1156,21 +1171,17 @@ ufshcd_scsi_cmd_status(struct ufshcd_lrb *lrbp, int
scsi_status)
SAM_STAT_CHECK_CONDITION;
ufshcd_copy_sense_data(lrbp);
break;
- case SAM_STAT_BUSY:
- result |= SAM_STAT_BUSY;
- break;
case SAM_STAT_TASK_SET_FULL:
-
/*
* If a LUN reports SAM_STAT_TASK_SET_FULL, then the LUN queue
* depth needs to be adjusted to the exact number of
* outstanding commands the LUN can handle at any given time.
*/
ufshcd_adjust_lun_qdepth(lrbp->cmd);
- result |= SAM_STAT_TASK_SET_FULL;
- break;
+ case SAM_STAT_BUSY:
case SAM_STAT_TASK_ABORTED:
- result |= SAM_STAT_TASK_ABORTED;
+ result |= scsi_status;
+ ufshcd_copy_sense_data(lrbp);
break;
default:
result |= DID_ERROR << 16;
--
1.7.0.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 3/7] scsi: ufs: fix the setting interrupt aggregation counter
LACTH(Interrupt aggregation counter threshold) value is allowed
up to 0x1F and current setting value is the maximum.
This value is related with NUTRS(max:0x20) of HCI's capability.
Considering HCI controller doesn't support the maximum, LATCH
setting should be adjusted with possible value.
For that, existing 'ufshcd_config_int_aggr' is split into two part
[reset, configure].
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufshcd.c | 53 +---
drivers/scsi/ufs/ufshci.h |4 +-
2 files changed, 27 insertions(+), 30 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 688ae0e..7152ec4 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -43,6 +43,9 @@
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT500
+/* Interrupt aggregation default timeout, unit: 40us */
+#define INT_AGGR_DEF_TO0x02
+
enum {
UFSHCD_MAX_CHANNEL = 0,
UFSHCD_MAX_ID = 1,
@@ -65,12 +68,6 @@ enum {
UFSHCD_INT_CLEAR,
};
-/* Interrupt aggregation options */
-enum {
- INT_AGGR_RESET,
- INT_AGGR_CONFIG,
-};
-
/**
* ufshcd_get_intr_mask - Get the interrupt bit mask
* @hba - Pointer to adapter instance
@@ -233,30 +230,30 @@ ufshcd_get_rsp_upiu_data_seg_len(struct utp_upiu_rsp
*ucd_rsp_ptr)
}
/**
- * ufshcd_config_int_aggr - Configure interrupt aggregation values.
- * Currently there is no use case where we want to configure
- * interrupt aggregation dynamically. So to configure interrupt
- * aggregation, #define INT_AGGR_COUNTER_THRESHOLD_VALUE and
- * INT_AGGR_TIMEOUT_VALUE are used.
+ * ufshcd_reset_intr_aggr - Reset interrupt aggregation values.
* @hba: per adapter instance
- * @option: Interrupt aggregation option
*/
static inline void
-ufshcd_config_int_aggr(struct ufs_hba *hba, int option)
+ufshcd_reset_intr_aggr(struct ufs_hba *hba)
{
- switch (option) {
- case INT_AGGR_RESET:
- ufshcd_writel(hba, INT_AGGR_ENABLE |
- INT_AGGR_COUNTER_AND_TIMER_RESET,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- case INT_AGGR_CONFIG:
- ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
- INT_AGGR_COUNTER_THRESHOLD_VALUE |
- INT_AGGR_TIMEOUT_VALUE,
- REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
- break;
- }
+ ufshcd_writel(hba, INT_AGGR_ENABLE |
+ INT_AGGR_COUNTER_AND_TIMER_RESET,
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
+}
+
+/**
+ * ufshcd_config_intr_aggr - Configure interrupt aggregation values.
+ * @hba: per adapter instance
+ * @cntr_thld: Interrupt aggregation counter threshold
+ * @timeout: Interrupt aggregation timeout value
+ */
+static inline void
+ufshcd_config_intr_aggr(struct ufs_hba *hba, u8 cnt, u8 timeout)
+{
+ ufshcd_writel(hba, INT_AGGR_ENABLE | INT_AGGR_PARAM_WRITE |
+ INT_AGGR_COUNTER_THLD_VAL(cnt) |
+ INT_AGGR_TIMEOUT_VAL(timeout),
+ REG_UTP_TRANSFER_REQ_INT_AGG_CONTROL);
}
/**
@@ -853,7 +850,7 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
ufshcd_enable_intr(hba, UFSHCD_ENABLE_INTRS);
/* Configure interrupt aggregation */
- ufshcd_config_int_aggr(hba, INT_AGGR_CONFIG);
+ ufshcd_config_intr_aggr(hba, hba->nutrs - 1, INT_AGGR_DEF_TO);
/* Configure UTRL and UTMRL base address registers */
ufshcd_writel(hba, lower_32_bits(hba->utrdl_dma_addr),
@@ -1299,7 +1296,7 @@ static void ufshcd_transfer_req_compl(struct ufs_hba *hba)
hba->outstanding_reqs ^= completed_reqs;
/* Reset interrupt aggregation counters */
- ufshcd_config_int_aggr(hba, INT_AGGR_RESET);
+ ufshcd_reset_intr_aggr(hba);
}
/**
diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h
index d5c5f14..a8f69cc 100644
--- a/drivers/scsi/ufs/ufshci.h
+++ b/drivers/scsi/ufs/ufshci.h
@@ -226,8 +226,8 @@ enum {
#define MASK_UIC_COMMAND_RESULT0xFF
-#define INT_AGGR_COUNTER_THRESHOLD_VALUE (0x1F << 8)
-#define INT_AGGR_TIMEOUT_VALUE (0x02)
+#define INT_AGGR_COUNTER_THLD_VAL(c) (((c) & 0x1F) << 8)
+#define INT_AGGR_TIMEOUT_VAL(t)(((t) & 0xFF) << 0)
/* Interrupt disable masks */
enum {
--
1.7.0.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 4/7] scsi: ufs: add dme configuration primitives
Implements to support GET and SET operations of the DME.
These operations are used to configure the behavior of
the UNIPRO. Along with basic operation, {Peer/AttrSetType}
can be mixed.
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufshcd.c | 88 +
drivers/scsi/ufs/ufshcd.h | 51 ++
drivers/scsi/ufs/ufshci.h |6 +++
3 files changed, 145 insertions(+), 0 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 7152ec4..8277c40 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -188,6 +188,18 @@ static inline int ufshcd_get_uic_cmd_result(struct ufs_hba
*hba)
}
/**
+ * ufshcd_get_dme_attr_val - Get the value of attribute returned by UIC command
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets UIC command argument3
+ * Returns 0 on success, non zero value on error
+ */
+static inline u32 ufshcd_get_dme_attr_val(struct ufs_hba *hba)
+{
+ return ufshcd_readl(hba, REG_UIC_COMMAND_ARG_3);
+}
+
+/**
* ufshcd_is_valid_req_rsp - checks if controller TR response is valid
* @ucd_rsp_ptr: pointer to response UPIU
*
@@ -821,6 +833,80 @@ static int ufshcd_dme_link_startup(struct ufs_hba *hba)
}
/**
+ * ufshcd_dme_set_attr - UIC command for DME_SET, DME_PEER_SET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @attr_set: attribute set type as uic command argument2
+ * @mib_val: setting value as uic command argument3
+ * @peer: indicate wherter peer or non-peer
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-set",
+ "dme-peer-set"
+ };
+ const char *set = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_SET : UIC_CMD_DME_SET;
+ uic_cmd.argument1 = attr_sel;
+ uic_cmd.argument2 = UIC_ARG_ATTR_TYPE(attr_set);
+ uic_cmd.argument3 = mib_val;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret)
+ dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
+ set, UIC_GET_ATTR_ID(attr_sel), ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_set_attr);
+
+/**
+ * ufshcd_dme_get_attr - UIC command for DME_GET, DME_PEER_GET
+ * @hba: per adapter instance
+ * @attr_sel: uic command argument1
+ * @mib_val: the value of the attribute as returned by the UIC command
+ * @peer: indicate wherter peer or non-peer
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer)
+{
+ struct uic_command uic_cmd = {0};
+ static const char *const action[] = {
+ "dme-get",
+ "dme-peer-get"
+ };
+ const char *get = action[!!peer];
+ int ret;
+
+ uic_cmd.command = peer ?
+ UIC_CMD_DME_PEER_GET : UIC_CMD_DME_GET;
+ uic_cmd.argument1 = attr_sel;
+
+ ret = ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev, "%s: attr-id 0x%x error code %d\n",
+ get, UIC_GET_ATTR_ID(attr_sel), ret);
+ goto out;
+ }
+
+ if (mib_val)
+ *mib_val = uic_cmd.argument3;
+out:
+ return ret;
+}
+EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1256,6 +1342,8 @@ static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
if (hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
+ hba->active_uic_cmd->argument3 =
+ ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
}
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 49590ee..50bcd29 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -199,6 +199,11 @@ int ufshcd_init(struct device *, struct ufs_hba ** , void
__iomem * ,
unsigned int);
void ufshcd_remove(struct ufs_hba *);
+extern int ufshcd_dme_set_attr(struct ufs_hba *hba, u32 attr_sel,
+ u8 attr_set, u32 mib_val, u8 peer);
+extern int ufshcd_dme_get_attr(struct ufs_hba *hba, u32 attr_sel,
+ u32 *mib_val, u8 peer);
+
/**
* ufshcd_hba_stop - Send controller to reset state
* @hba: per adapter instance
@@ -208,4 +213,50 @@ static inline void ufshcd_hba_stop(struct ufs_hba *hba)
ufshcd_writel(hba, CONTROLLER_DISABLE, REG_CONTROLLER_ENABLE);
}
+/* UIC command interfaces for DME primitives */
+#define DME_LOCAL
[PATCH 5/7] scsi: ufs: add unipro attribute IDs
'drivers/scsi/ufs/unipro.h' is added. Attributes in the layers of the UNIPRO stack can be read and written via the DME. Signed-off-by: Seungwon Jeon --- drivers/scsi/ufs/unipro.h | 130 + 1 files changed, 130 insertions(+), 0 deletions(-) create mode 100644 drivers/scsi/ufs/unipro.h diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h new file mode 100644 index 000..3a710eb --- /dev/null +++ b/drivers/scsi/ufs/unipro.h @@ -0,0 +1,130 @@ +/* + * drivers/scsi/ufs/unipro.h + * + * Copyright (C) 2013 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 _UNIPRO_H_ +#define _UNIPRO_H_ + +/* + * PHY Adpater attributes + */ +#define PA_ACTIVETXDATALANES 0x1560 +#define PA_ACTIVERXDATALANES 0x1580 +#define PA_TXTRAILINGCLOCKS0x1564 +#define PA_PHY_TYPE0x1500 +#define PA_AVAILTXDATALANES0x1520 +#define PA_AVAILRXDATALANES0x1540 +#define PA_MINRXTRAILINGCLOCKS 0x1543 +#define PA_TXPWRSTATUS 0x1567 +#define PA_RXPWRSTATUS 0x1582 +#define PA_TXFORCECLOCK0x1562 +#define PA_TXPWRMODE 0x1563 +#define PA_LEGACYDPHYESCDL 0x1570 +#define PA_MAXTXSPEEDFAST 0x1521 +#define PA_MAXTXSPEEDSLOW 0x1522 +#define PA_MAXRXSPEEDFAST 0x1541 +#define PA_MAXRXSPEEDSLOW 0x1542 +#define PA_TXLINKSTARTUPHS 0x1544 +#define PA_TXSPEEDFAST 0x1565 +#define PA_TXSPEEDSLOW 0x1566 +#define PA_REMOTEVERINFO 0x15A0 +#define PA_TXGEAR 0x1568 +#define PA_TXTERMINATION 0x1569 +#define PA_HSSERIES0x156A +#define PA_PWRMODE 0x1571 +#define PA_RXGEAR 0x1583 +#define PA_RXTERMINATION 0x1584 +#define PA_MAXRXPWMGEAR0x1586 +#define PA_MAXRXHSGEAR 0x1587 +#define PA_RXHSUNTERMCAP 0x15A5 +#define PA_RXLSTERMCAP 0x15A6 +#define PA_PACPREQTIMEOUT 0x1590 +#define PA_PACPREQEOBTIMEOUT 0x1591 +#define PA_HIBERN8TIME 0x15A7 +#define PA_LOCALVERINFO0x15A9 +#define PA_TACTIVATE 0x15A8 +#define PA_PACPFRAMECOUNT 0x15C0 +#define PA_PACPERRORCOUNT 0x15C1 +#define PA_PHYTESTCONTROL 0x15C2 +#define PA_PWRMODEUSERDATA00x15B0 +#define PA_PWRMODEUSERDATA10x15B1 +#define PA_PWRMODEUSERDATA20x15B2 +#define PA_PWRMODEUSERDATA30x15B3 +#define PA_PWRMODEUSERDATA40x15B4 +#define PA_PWRMODEUSERDATA50x15B5 +#define PA_PWRMODEUSERDATA60x15B6 +#define PA_PWRMODEUSERDATA70x15B7 +#define PA_PWRMODEUSERDATA80x15B8 +#define PA_PWRMODEUSERDATA90x15B9 +#define PA_PWRMODEUSERDATA10 0x15BA +#define PA_PWRMODEUSERDATA11 0x15BB +#define PA_CONNECTEDTXDATALANES0x1561 +#define PA_CONNECTEDRXDATALANES0x1581 +#define PA_LOGICALLANEMAP 0x15A1 +#define PA_SLEEPNOCONFIGTIME 0x15A2 +#define PA_STALLNOCONFIGTIME 0x15A3 +#define PA_SAVECONFIGTIME 0x15A4 + +/* + * Data Link Layer Attributes + */ +#define DL_TC0TXFCTHRESHOLD0x2040 +#define DL_FC0PROTTIMEOUTVAL 0x2041 +#define DL_TC0REPLAYTIMEOUTVAL 0x2042 +#define DL_AFC0REQTIMEOUTVAL 0x2043 +#define DL_AFC0CREDITTHRESHOLD 0x2044 +#define DL_TC0OUTACKTHRESHOLD 0x2045 +#define DL_TC1TXFCTHRESHOLD0x2060 +#define DL_FC1PROTTIMEOUTVAL 0x2061 +#define DL_TC1REPLAYTIMEOUTVAL 0x2062 +#define DL_AFC1REQTIMEOUTVAL 0x2063 +#define DL_AFC1CREDITTHRESHOLD 0x2064 +#define DL_TC1OUTACKTHRESHOLD 0x2065 +#define DL_TXPREEMPTIONCAP 0x2000 +#define DL_TC0TXMAXSDUSIZE 0x2001 +#define DL_TC0RXINITCREDITVAL 0x2002 +#define DL_TC0TXBUFFERSIZE 0x2005 +#define DL_PEERTC0PRESENT 0x2046 +#define DL_PEERTC0RXINITCREVAL 0x2047 +#define DL_TC1TXMAXSDUSIZE 0x2003 +#define DL_TC1RXINITCREDITVAL 0x2004 +#define DL_TC1TXBUFFERSIZE 0x2006 +#define DL_PEERTC1PRESENT 0x2066 +#define DL_PEERTC1RXINITCREVAL 0x2067 + +/* + * Network Layer Attributes + */ +#define N_DEVICEID 0x3000 +#define N_DEVICEID_VALID 0x3001 +#define N_TC0TXMAXSDUSIZE 0x3020 +#define N_TC1TXMAXSDUSIZE 0x3021 + +/* + * Transport Layer Attributes + */ +#define T_NUMCPORTS0x4000 +#define T_NUMTESTFEATURES 0x4001 +#define T_CONNECTIONSTATE 0x4020 +#define T_PEERDEVICEID 0x4021 +#define T_PEERCPORTID 0x4022 +#define T_TRAFFICCLASS 0x4023 +#define T_PROTOCOLID 0x4024 +#define T_CPORTFLAGS 0x4025 +#define T_TXTOKENVALUE 0x4026 +#define T_RXTOKENVALUE 0x4027 +#define T_LOCALBUFFERSPACE 0x4028 +#define T_PEERBUFFERSPACE 0x4029 +#define T_CREDITSTOSEND0x402A +#define T_CPORTMODE0x402B +#define T_TC0TXMAXSDUSIZE 0x4060 +#define T_TC1TXMAXSDUSIZE 0x4061 + +#endif /* _UNIPRO_
[PATCH 6/7] scsi: ufs: add operation for the uic power mode change
Setting PA_PWRMode using DME_SET triggers the power mode
change. And then the result will be given by the HCS.UPMCRS.
This operation should be done atomically.
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufshcd.c | 80 ++--
drivers/scsi/ufs/ufshcd.h |3 ++
drivers/scsi/ufs/ufshci.h | 12 +++
3 files changed, 91 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 8277c40..ffda72d 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -36,9 +36,11 @@
#include
#include "ufshcd.h"
+#include "unipro.h"
#define UFSHCD_ENABLE_INTRS(UTP_TRANSFER_REQ_COMPL |\
UTP_TASK_REQ_COMPL |\
+UIC_POWER_MODE |\
UFSHCD_ERROR_MASK)
/* UIC command timeout, unit: ms */
#define UIC_CMD_TIMEOUT500
@@ -359,6 +361,18 @@ static inline bool ufshcd_ready_for_uic_cmd(struct ufs_hba
*hba)
}
/**
+ * ufshcd_get_upmcrs - Get the power mode change request status
+ * @hba: Pointer to adapter instance
+ *
+ * This function gets the UPMCRS field of HCS register
+ * Returns value of UPMCRS field
+ */
+static inline u8 ufshcd_get_upmcrs(struct ufs_hba *hba)
+{
+ return (ufshcd_readl(hba, REG_CONTROLLER_STATUS) >> 8) & 0x7;
+}
+
+/**
* ufshcd_dispatch_uic_cmd - Dispatch UIC commands to unipro layers
* @hba: per adapter instance
* @uic_cmd: UIC command
@@ -907,6 +921,60 @@ out:
EXPORT_SYMBOL_GPL(ufshcd_dme_get_attr);
/**
+ * ufshcd_uic_change_pwr_mode - Perform the UIC power mode chage
+ * using DME_SET primitives.
+ * @hba: per adapter instance
+ * @mode: powr mode value
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+int ufshcd_uic_change_pwr_mode(struct ufs_hba *hba, u8 mode)
+{
+ struct uic_command uic_cmd = {0};
+ struct completion pwr_done;
+ unsigned long flags;
+ u8 status;
+ int ret;
+
+ uic_cmd.command = UIC_CMD_DME_SET;
+ uic_cmd.argument1 = UIC_ARG_MIB(PA_PWRMODE);
+ uic_cmd.argument3 = mode;
+ init_completion(&pwr_done);
+
+ mutex_lock(&hba->uic_cmd_mutex);
+
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = &pwr_done;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ ret = __ufshcd_send_uic_cmd(hba, &uic_cmd);
+ if (ret) {
+ dev_err(hba->dev, "pwr mode change uic error %d\n", ret);
+ goto out;
+ }
+
+ if (!wait_for_completion_timeout(hba->pwr_done,
+msecs_to_jiffies(UIC_CMD_TIMEOUT))) {
+ dev_err(hba->dev, "pwr mode change completion timeout\n");
+ ret = -ETIMEDOUT;
+ goto out;
+ }
+
+ status = ufshcd_get_upmcrs(hba);
+ if (status != PWR_LOCAL) {
+ dev_err(hba->dev,
+ "pwr mode change failed, host umpcrs:0x%x\n",
+ status);
+ ret = (status != PWR_OK) ? status : -1;
+ }
+out:
+ spin_lock_irqsave(hba->host->host_lock, flags);
+ hba->pwr_done = NULL;
+ spin_unlock_irqrestore(hba->host->host_lock, flags);
+ mutex_unlock(&hba->uic_cmd_mutex);
+ return ret;
+}
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1336,16 +1404,20 @@ ufshcd_transfer_rsp_status(struct ufs_hba *hba, struct
ufshcd_lrb *lrbp)
/**
* ufshcd_uic_cmd_compl - handle completion of uic command
* @hba: per adapter instance
+ * @intr_status: interrupt status generated by the controller
*/
-static void ufshcd_uic_cmd_compl(struct ufs_hba *hba)
+static void ufshcd_uic_cmd_compl(struct ufs_hba *hba, u32 intr_status)
{
- if (hba->active_uic_cmd) {
+ if ((intr_status & UIC_COMMAND_COMPL) && hba->active_uic_cmd) {
hba->active_uic_cmd->argument2 |=
ufshcd_get_uic_cmd_result(hba);
hba->active_uic_cmd->argument3 =
ufshcd_get_dme_attr_val(hba);
complete(&hba->active_uic_cmd->done);
}
+
+ if ((intr_status & UIC_POWER_MODE) && hba->pwr_done)
+ complete(hba->pwr_done);
}
/**
@@ -1447,8 +1519,8 @@ static void ufshcd_sl_intr(struct ufs_hba *hba, u32
intr_status)
if (hba->errors)
ufshcd_err_handler(hba);
- if (intr_status & UIC_COMMAND_COMPL)
- ufshcd_uic_cmd_compl(hba);
+ if (intr_status & UFSHCD_UIC_MASK)
+ ufshcd_uic_cmd_compl(hba, intr_status);
if (intr_status & UTP_TASK_REQ_COMPL)
ufshcd_tmc_handler(hba);
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 50bcd29..246c0e1 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -142,6 +142,7 @@ struct ufshcd_lrb {
* @uic_cmd_mutex: mu
[PATCH 7/7] scsi: ufs: configure the attribute for power mode
UIC attributes can be set with using DME_SET command for
power mode change. For configuration the link capability
attributes are used, which is updated after successful
link startup.
Signed-off-by: Seungwon Jeon
---
drivers/scsi/ufs/ufshcd.c | 74 +++-
drivers/scsi/ufs/unipro.h | 21 +
2 files changed, 93 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index ffda72d..ebdb9ff 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -975,6 +975,70 @@ out:
}
/**
+ * ufshcd_config_max_pwr_mode - Set & Change power mode with
+ * maximum capability attribute information.
+ * @hba: per adapter instance
+ *
+ * Returns 0 on success, non-zero value on failure
+ */
+static int ufshcd_config_max_pwr_mode(struct ufs_hba *hba)
+{
+ enum {RX = 0, TX = 1};
+ u32 lanes[] = {1, 1};
+ u32 gear[] = {1, 1};
+ u8 pwr[] = {FASTAUTO_MODE, FASTAUTO_MODE};
+ int i, ret;
+
+ /* Get the connected lane count */
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDRXDATALANES), &lanes[RX]);
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_CONNECTEDTXDATALANES), &lanes[TX]);
+
+ /*
+* First, get the maximum gears of HS speed.
+* If a zero value, it means there is no HSGEAR capability.
+* Then, get the maximum gears of PWM speed.
+*/
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[RX]);
+ if (!gear[RX]) {
+ ufshcd_dme_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR), &gear[RX]);
+ pwr[RX] = SLOWAUTO_MODE;
+ }
+
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXHSGEAR), &gear[TX]);
+ if (!gear[TX]) {
+ ufshcd_dme_peer_get(hba, UIC_ARG_MIB(PA_MAXRXPWMGEAR),
+ &gear[TX]);
+ pwr[TX] = SLOWAUTO_MODE;
+ }
+
+ /*
+* Configure attributes for power mode change with below.
+* - PA_RXGEAR, PA_ACTIVERXDATALANES, PA_RXTERMINATION,
+* - PA_TXGEAR, PA_ACTIVETXDATALANES, PA_TXTERMINATION,
+* - PA_HSSERIES
+*/
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXGEAR), gear[RX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVERXDATALANES), lanes[RX]);
+ if (pwr[RX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_RXTERMINATION), TRUE);
+
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXGEAR), gear[TX]);
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_ACTIVETXDATALANES), lanes[TX]);
+ if (pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_TXTERMINATION), TRUE);
+
+ if (pwr[RX] == FASTAUTO_MODE || pwr[TX] == FASTAUTO_MODE)
+ ufshcd_dme_set(hba, UIC_ARG_MIB(PA_HSSERIES), PA_HS_MODE_B);
+
+ ret = ufshcd_uic_change_pwr_mode(hba, pwr[RX] << 4 | pwr[TX]);
+ if (ret)
+ dev_err(hba->dev,
+ "pwr_mode: power mode change failed %d\n", ret);
+
+ return ret;
+}
+
+/**
* ufshcd_make_hba_operational - Make UFS controller operational
* @hba: per adapter instance
*
@@ -1754,8 +1818,14 @@ static void ufshcd_async_scan(void *data, async_cookie_t
cookie)
int ret;
ret = ufshcd_link_startup(hba);
- if (!ret)
- scsi_scan_host(hba->host);
+ if (ret)
+ goto out;
+
+ ufshcd_config_max_pwr_mode(hba);
+
+ scsi_scan_host(hba->host);
+out:
+ return;
}
static struct scsi_host_template ufshcd_driver_template = {
diff --git a/drivers/scsi/ufs/unipro.h b/drivers/scsi/ufs/unipro.h
index 3a710eb..0bb8041 100644
--- a/drivers/scsi/ufs/unipro.h
+++ b/drivers/scsi/ufs/unipro.h
@@ -72,6 +72,21 @@
#define PA_STALLNOCONFIGTIME 0x15A3
#define PA_SAVECONFIGTIME 0x15A4
+/* PA power modes */
+enum {
+ FAST_MODE = 1,
+ SLOW_MODE = 2,
+ FASTAUTO_MODE = 4,
+ SLOWAUTO_MODE = 5,
+ UNCHANGED = 7,
+};
+
+/* PA TX/RX Frequency Series */
+enum {
+ PA_HS_MODE_A= 1,
+ PA_HS_MODE_B= 2,
+};
+
/*
* Data Link Layer Attributes
*/
@@ -127,4 +142,10 @@
#define T_TC0TXMAXSDUSIZE 0x4060
#define T_TC1TXMAXSDUSIZE 0x4061
+/* Boolean attribute values */
+enum {
+ FALSE = 0,
+ TRUE,
+};
+
#endif /* _UNIPRO_H_ */
--
1.7.0.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: hpsa - BUG: using smp_processor_id() in preemptible [00000000 00000000] code: kworker/u:0/6
- Original Message - > On Fri, Jul 26, 2013 at 06:28:02AM -0400, John Kacur wrote: > > > > > > - Original Message - > > > [Adding missing cc to linux-scsi] > > > On Thu, 2013-07-25 at 23:33 +0200, John Kacur wrote: > > > > Hi > > > > > > > > We're seeing this on a 3.6 kernel with the real-time patch applied, but > > > > it > > > > looks like it is relevant with the real-time patch in the latest kernel > > > > This should read, "it looks like it is relevant WITHOUT the real-time patch > > in the latest kernel". > > > > > > > > too. > > > > > > > > [ 49.688847] hpsa :03:00.0: hpsa0: <0x323a> at IRQ 67 using DAC > > > > [ 49.749928] scsi0 : hpsa > > > > [ 49.784437] BUG: using smp_processor_id() in preemptible [ > > > > ] code: kworker/u:0/6 > > > > [ 49.784465] caller is enqueue_cmd_and_start_io+0x5a/0x100 [hpsa] > > > > [ 49.784468] Pid: 6, comm: kworker/u:0 Not tainted > > > > 3.6.11.5-rt37.52.el6rt.x86_64.debug #1 > > > > [ 49.784471] Call Trace: > > > > [ 49.784512] [] debug_smp_processor_id+0x123/0x150 > > > > [ 49.784520] [] > > > > enqueue_cmd_and_start_io+0x5a/0x100 > > > > [hpsa] > > > > [ 49.784529] [] > > > > hpsa_scsi_do_simple_cmd_core+0xeb/0x110 [hpsa] > > > > [ 49.784537] [] ? > > > > swiotlb_dma_mapping_error+0x18/0x30 > > > > [ 49.784544] [] ? > > > > swiotlb_dma_mapping_error+0x18/0x30 > > > > [ 49.784553] [] > > > > hpsa_scsi_do_simple_cmd_with_retry+0x91/0x280 [hpsa] > > > > [ 49.784562] [] > > > > hpsa_scsi_do_report_luns.clone.2+0xd8/0x130 [hpsa] > > > > [ 49.784571] [] > > > > hpsa_gather_lun_info.clone.3+0x3a/0x1a0 [hpsa] > > > > [ 49.784580] [] > > > > hpsa_update_scsi_devices+0x11f/0x4f0 > > > > [hpsa] > > > > [ 49.784592] [] ? sub_preempt_count+0xa9/0xe0 > > > > [ 49.784601] [] hpsa_scan_start+0xfd/0x150 [hpsa] > > > > [ 49.784613] [] ? > > > > rt_spin_lock_slowunlock+0x78/0x90 > > > > [ 49.784626] [] do_scsi_scan_host+0x37/0xa0 > > > > [ 49.784632] [] do_scan_async+0x1a/0x30 > > > > [ 49.784643] [] async_run_entry_fn+0x9b/0x1d0 > > > > [ 49.784655] [] process_one_work+0x1f2/0x620 > > > > [ 49.784661] [] ? process_one_work+0x180/0x620 > > > > [ 49.784668] [] ? worker_thread+0x5e/0x3a0 > > > > [ 49.784674] [] ? async_schedule+0x20/0x20 > > > > [ 49.784681] [] worker_thread+0x133/0x3a0 > > > > [ 49.784688] [] ? manage_workers+0x190/0x190 > > > > [ 49.784696] [] kthread+0xa6/0xb0 > > > > [ 49.784707] [] kernel_thread_helper+0x4/0x10 > > > > [ 49.784715] [] ? finish_task_switch+0x8c/0x110 > > > > [ 49.784721] [] ? _raw_spin_unlock_irq+0x3b/0x70 > > > > [ 49.784727] [] ? retint_restore_args+0xe/0xe > > > > [ 49.784734] [] ? kthreadd+0x1e0/0x1e0 > > > > [ 49.784739] [] ? gs_change+0xb/0xb > > > > > > > > --- > > > > > > > > When I look at the code I see this call chain > > > > enqueue_cmd_and_start_io()-> > > > > set_performant_mode()-> > > > > smp_processor_id() > > > > Which if you have debugging enabled calls debug_processor_id() and > > > > triggers the warning. > > > > > > > > I'm not very familiar with the hpsa code, so I'm not entirely sure what > > > > the purpose of this line is > > > > > > > > c->Header.ReplyQueue = smp_processor_id() % h->nreply_queues; > > > > > > > > Is the purpose to simply try to get a range of ReplyQueue numbers, but > > > > somewhat arbitrary? Or is it necessary that the current processor_id > > > > is used? If it is the former, and you're not accessing per cpu > > > > structures, > > > > or pinning a cpu, or anything like that then I would think it is safe > > > > to > > > > change this to a raw_smp_processor_id() to get rid of a false positive > > > > warning. > > It's not critical that they match (will work if they don't) but for certain > workloads you can get more performance if you pin processes to cpus and > arrange msix interrupt vectors so that commands are likely to complete on > the same cpu they originated from. > > In any case, I think your analysis is correct. Thanks. > > > > > > > > > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c > > > > index 7f4f790..4e19267 100644 > > > > --- a/drivers/scsi/hpsa.c > > > > +++ b/drivers/scsi/hpsa.c > > > > @@ -583,7 +583,7 @@ static void set_performant_mode(struct ctlr_info > > > > *h, > > > > struct CommandList *c) > > > > c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] > > > > << 1); > > > > if (likely(h->msix_vector)) > > > > c->Header.ReplyQueue = > > > > - smp_processor_id() % h->nreply_queues; > > > > + raw_smp_processor_id() % > > > > h->nreply_queues; > > > > } > > > > } > > Ack. > > -- steve > Ok, thanks, I'll put the patch in another mail with my sign-off. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.ke
[PATCH] hpsa: fix warning with smp_processor_id() in preemptible
>From 4c01ac362210c38cdbaddd0a75c24c7070e77dfa Mon Sep 17 00:00:00 2001 From: John Kacur Date: Fri, 26 Jul 2013 16:06:18 +0200 Subject: [PATCH] hpsa: fix warning with smp_processor_id() in preemptible section Signed-off-by: John Kacur On a 3.6-rt (real-time patch) kernel we are seeing the following BUG However, it appears to be relevant for non-realtime (mainline) as well. [ 49.688847] hpsa :03:00.0: hpsa0: <0x323a> at IRQ 67 using DAC [ 49.749928] scsi0 : hpsa [ 49.784437] BUG: using smp_processor_id() in preemptible [ ] code: kworker/u:0/6 [ 49.784465] caller is enqueue_cmd_and_start_io+0x5a/0x100 [hpsa] [ 49.784468] Pid: 6, comm: kworker/u:0 Not tainted 3.6.11.5-rt37.52.el6rt.x86_64.debug #1 [ 49.784471] Call Trace: [ 49.784512] [] debug_smp_processor_id+0x123/0x150 [ 49.784520] [] enqueue_cmd_and_start_io+0x5a/0x100 [hpsa] [ 49.784529] [] hpsa_scsi_do_simple_cmd_core+0xeb/0x110 [hpsa] [ 49.784537] [] ? swiotlb_dma_mapping_error+0x18/0x30 [ 49.784544] [] ? swiotlb_dma_mapping_error+0x18/0x30 [ 49.784553] [] hpsa_scsi_do_simple_cmd_with_retry+0x91/0x280 [hpsa] [ 49.784562] [] hpsa_scsi_do_report_luns.clone.2+0xd8/0x130 [hpsa] [ 49.784571] [] hpsa_gather_lun_info.clone.3+0x3a/0x1a0 [hpsa] [ 49.784580] [] hpsa_update_scsi_devices+0x11f/0x4f0 [hpsa] [ 49.784592] [] ? sub_preempt_count+0xa9/0xe0 [ 49.784601] [] hpsa_scan_start+0xfd/0x150 [hpsa] [ 49.784613] [] ? rt_spin_lock_slowunlock+0x78/0x90 [ 49.784626] [] do_scsi_scan_host+0x37/0xa0 [ 49.784632] [] do_scan_async+0x1a/0x30 [ 49.784643] [] async_run_entry_fn+0x9b/0x1d0 [ 49.784655] [] process_one_work+0x1f2/0x620 [ 49.784661] [] ? process_one_work+0x180/0x620 [ 49.784668] [] ? worker_thread+0x5e/0x3a0 [ 49.784674] [] ? async_schedule+0x20/0x20 [ 49.784681] [] worker_thread+0x133/0x3a0 [ 49.784688] [] ? manage_workers+0x190/0x190 [ 49.784696] [] kthread+0xa6/0xb0 [ 49.784707] [] kernel_thread_helper+0x4/0x10 [ 49.784715] [] ? finish_task_switch+0x8c/0x110 [ 49.784721] [] ? _raw_spin_unlock_irq+0x3b/0x70 [ 49.784727] [] ? retint_restore_args+0xe/0xe [ 49.784734] [] ? kthreadd+0x1e0/0x1e0 [ 49.784739] [] ? gs_change+0xb/0xb --- This is caused by enqueue_cmd_and_start_io()-> set_performant_mode()-> smp_processor_id() Which if you have debugging enabled calls debug_processor_id() and triggers the warning. The code here is c->Header.ReplyQueue = smp_processor_id() % h->nreply_queues; Since it is not critical that the code complete on the same processor, but the cpu is a hint used in generating the ReplyQueue and will still work if the cpu migrates or is preempted, it is safe to use the raw_smp_processor_id() to surpress the false positve warning. Signed-off-by: John Kacur Acked-by: Stephen --- drivers/scsi/hpsa.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 7f4f790..4e19267 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -583,7 +583,7 @@ static void set_performant_mode(struct ctlr_info *h, struct CommandList *c) c->busaddr |= 1 | (h->blockFetchTable[c->Header.SGList] << 1); if (likely(h->msix_vector)) c->Header.ReplyQueue = - smp_processor_id() % h->nreply_queues; + raw_smp_processor_id() % h->nreply_queues; } } -- 1.7.11.7 -- 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: [RFC/RFT PATCH 0/5] mm: ARM nobootmem and few dma_mask fixes
On Fri, Jul 12, 2013 at 05:48:09PM -0400, Santosh Shilimkar wrote: > The series is an attempt to move ARM port to NO_BOOTMEM. As discussed > on list NO_BOOTMEM move needed updates to max*pfn meaning to be maximum > PFNs but that breaks the dma_mask for few block layer drivers since > ARM start of physical memory is not PFN0 unlike most of the architectures. > Some more read on it is here: > http://lwn.net/Articles/543408/ > http://lwn.net/Articles/543424/ > > To address this issue, we introduce generic dma_max_pfn() helper which > can be overridden from the architectures. > > Another intention behind move to nobootmem is also to convert ARM to > switch to memblock and getting rid of bootmem allocator dependency which > don't work for LPAE machines which has physical memory starting beyond > 4 GB boundary. It needs changes to core kernel and also a new memblock > API. More on this can be found here: > https://lkml.org/lkml/2013/6/29/77 > > I have been trying to cook up these patches with kind help from Russell > and we know series don't solve all the dma_mask bad assumptions. But at > least I am hoping that it can get the ball rolling. > > Comments/testing help is welcome !! As this is related to some of the cleanup of dma_mask which I've been doing, I think it may make sense to roll this into one tree. Any objection to that? Can we get any acks on this stuff from Jens and Jejb etc - especially for the bits which touch block/ and for the scsi bits as these are touching other subsystems. (oddly, linux-scsi wasn't on the original mail for this series summary.) -- 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: targetcli -fb now also Apache 2.0 licensed
Ritesh, We have put immense labor and love in creating LIO and targetcli. Please just take a look at the code, the wiki, the manual, target-devel - we have poured years of our lives into this project, and we look forward to implementing many more exciting ideas. Asking us to just abandon this is - simply absurd. Of course, we'll continue to maintain one coherent software stack with extensive documentation (we'd love contributions here, too!) and provide support for all of it through a well-known forum (target-devel). If Andy prefers not helping us to pull his few changes upstream, we'll gladly just do it ourselves. Best regards, Marc Fleischmann DATERA | 650.384.6366 | @dateranews -Original Message- From: target-devel-ow...@vger.kernel.org [mailto:target-devel-ow...@vger.kernel.org] On Behalf Of Ritesh Raj Sarraf Sent: Friday, July 26, 2013 12:24 AM To: target-devel; linux-scsi Cc: Andy Grover; Nicholas A. Bellinger; James Bottomley; targetcli-fb-de...@lists.fedorahosted.org; linux-kernel Subject: Re: targetcli -fb now also Apache 2.0 licensed On Friday 26 July 2013 09:01 AM, Andy Grover wrote: > > It's one thing to claim a prerogative of "upstream", but for this to > make sense, there needs to be an actual community around the upstream. > And if there's going to be submissions for review, then there needs to > be someone in charge of the community with a high degree of skill in > the code base. Nick you have a great deal of technical expertise > around the C code in drivers/target, but Jerome was the one who wrote > rtslib, targetcli, and configshell. I believe you can assess the > technical aspects of how the user library interacts with the kernel > code, but the maintainer should also be extremely conversant in the > language the library is written in. In this case, Python. So it's not > clear to me if submitting the code would actually result in meaningful > code improvements. > > Also, there has been no effort to sustain a community around this > code. There is no bug tracking, no separate mailing list, no regular > releases. Debian is running ancient, broken code because nothing's > been tagged in over two years. > > I would love to have you maintain and improve this code, but if you > aren't then you can't just say "I'm the upstream bow to me!". We're > shipping this code in Fedora and it needs active maintenance. Now that > we're all on an even licensing footing, code can flow both ways, and > even into your commercial version. Yes. Apart from reaching out the developers directly, I am not aware of other communication channels. There is a git and wiki though. Nick, if you guys have the same license now, nothing should stop you to pull the changes from the targetcli-fb repo. You can start afresh now and comply with what the community guidelines are. Licensing is just one part of it. And regarding this whole licensing dispute in the open, the kernel maintainers should have resolved this long ago, when it was decided to have LIO as the in-kernel target for Linux. You might say that the target component complied by the kernel's licensing requirements. But for a subsystem inclusion, you don't look just at the code, but also the maintainer. Their support tools. And their long term plans. These rules have been applied, in the past, for other features in the kernel. -- Ritesh Raj Sarraf | http://people.debian.org/~rrs Debian - The Universal Operating System -- 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: [RFC/RFT PATCH 0/5] mm: ARM nobootmem and few dma_mask fixes
On Friday 26 July 2013 11:10 AM, Russell King - ARM Linux wrote: > On Fri, Jul 12, 2013 at 05:48:09PM -0400, Santosh Shilimkar wrote: >> The series is an attempt to move ARM port to NO_BOOTMEM. As discussed >> on list NO_BOOTMEM move needed updates to max*pfn meaning to be maximum >> PFNs but that breaks the dma_mask for few block layer drivers since >> ARM start of physical memory is not PFN0 unlike most of the architectures. >> Some more read on it is here: >> http://lwn.net/Articles/543408/ >> http://lwn.net/Articles/543424/ >> >> To address this issue, we introduce generic dma_max_pfn() helper which >> can be overridden from the architectures. >> >> Another intention behind move to nobootmem is also to convert ARM to >> switch to memblock and getting rid of bootmem allocator dependency which >> don't work for LPAE machines which has physical memory starting beyond >> 4 GB boundary. It needs changes to core kernel and also a new memblock >> API. More on this can be found here: >> https://lkml.org/lkml/2013/6/29/77 >> >> I have been trying to cook up these patches with kind help from Russell >> and we know series don't solve all the dma_mask bad assumptions. But at >> least I am hoping that it can get the ball rolling. >> >> Comments/testing help is welcome !! > > As this is related to some of the cleanup of dma_mask which I've been > doing, I think it may make sense to roll this into one tree. Any > objection to that? > > Can we get any acks on this stuff from Jens and Jejb etc - especially > for the bits which touch block/ and for the scsi bits as these are > touching other subsystems. (oddly, linux-scsi wasn't on the original > mail for this series summary.) > Sorry I missed the scsi lists on the summary patch. While browsing the code I found another spot in mmc layer which needs fixing. The patch is at the end of the email with Chris and linux-mmc cc'ed here. Regards, Santosh >From 06a27a784a1fd86bf22adf1b247ac82a7c21d46b Mon Sep 17 00:00:00 2001 From: Santosh Shilimkar Date: Fri, 19 Jul 2013 21:36:46 -0400 Subject: [PATCH] mmc: Use dma_max_pfn(dev) helper for bounce_limit calculations DMA bounce limit is the maximum direct DMA'able memory beyond which bounce buffers has to be used to perform dma operations. MMC queue layer relies on dma_mask but its calculation is based on max_*pfn which don't have uniform meaning across architectures. So make use of dma_max_pfn() which is expected to return the DMAable maximum pfn value across architectures. Cc: Russell King Cc: Chris Ball Signed-off-by: Santosh Shilimkar --- drivers/mmc/card/queue.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c index fa9632e..357bbc5 100644 --- a/drivers/mmc/card/queue.c +++ b/drivers/mmc/card/queue.c @@ -15,6 +15,7 @@ #include #include #include +#include #include #include @@ -196,7 +197,7 @@ int mmc_init_queue(struct mmc_queue *mq, struct mmc_card *card, struct mmc_queue_req *mqrq_prev = &mq->mqrq[1]; if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask) - limit = *mmc_dev(host)->dma_mask; + limit = dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT; mq->card = card; mq->queue = blk_init_queue(mmc_request_fn, lock); -- 1.7.9.5 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] pm80xx: fix Adaptec 71605H hang
The IO command size is 128 bytes for these new controllers as opposed to 64
for the old 8001 controller.
The Adaptec out-of-tree driver did this correctly. After comparing the two
this turned out to be the crucial difference.
So don't hardcode the IO command size, instead use pm8001_ha->iomb_size as
that is the correct value for both old and new controllers.
Signed-off-by: Hans Verkuil
Cc: sta...@vger.kernel.org # for v3.10 and up
---
drivers/scsi/pm8001/pm8001_hwi.c | 4 ++--
drivers/scsi/pm8001/pm80xx_hwi.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 69dd49c..ce3f129d 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -221,7 +221,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_interrupt = 0x01;
for (i = 0; i < PM8001_MAX_INB_NUM; i++) {
pm8001_ha->inbnd_q_tbl[i].element_pri_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x00<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16) |
(0x00<<30);
pm8001_ha->inbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[IB + i].phys_addr_hi;
pm8001_ha->inbnd_q_tbl[i].lower_base_addr =
@@ -247,7 +247,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
}
for (i = 0; i < PM8001_MAX_OUTB_NUM; i++) {
pm8001_ha->outbnd_q_tbl[i].element_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x01<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16) |
(0x01<<30);
pm8001_ha->outbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[OB + i].phys_addr_hi;
pm8001_ha->outbnd_q_tbl[i].lower_base_addr =
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 302514d..e1c4896 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -275,7 +275,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
for (i = 0; i < PM8001_MAX_SPCV_INB_NUM; i++) {
pm8001_ha->inbnd_q_tbl[i].element_pri_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x00<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16) |
(0x00<<30);
pm8001_ha->inbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[IB + i].phys_addr_hi;
pm8001_ha->inbnd_q_tbl[i].lower_base_addr =
@@ -301,7 +301,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
}
for (i = 0; i < PM8001_MAX_SPCV_OUTB_NUM; i++) {
pm8001_ha->outbnd_q_tbl[i].element_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x01<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16) |
(0x01<<30);
pm8001_ha->outbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[OB + i].phys_addr_hi;
pm8001_ha->outbnd_q_tbl[i].lower_base_addr =
--
1.8.3.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
Ejected Nook (usb mass storage) prevents suspend
This is kernel 3.9.9-302.fc19.x86_64. I plugged in a BN Nook (a usb mass storage device), used it, and ejected it. This makes suspend fail: [50135.265514] PM: Entering freeze sleep [50135.265517] Suspending console(s) (use no_console_suspend to debug) [50135.287724] sd 7:0:0:0: [sdb] Synchronizing SCSI cache [50135.290413] sd 7:0:0:0: [sdb] [50135.290415] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE [50135.290418] sd 7:0:0:0: [sdb] [50135.290422] Sense Key : Not Ready [current] [50135.290424] sd 7:0:0:0: [sdb] [50135.290429] Add. Sense: Medium not present [50135.290448] dpm_run_callback(): scsi_bus_suspend+0x0/0x40 returns -5 [50135.290454] PM: Device 7:0:0:0 failed to suspend async: error -5 [50138.486917] PM: Some devices failed to suspend [50138.525007] PM: resume of devices complete after 38.132 msecs [50138.525315] pci_pm_runtime_suspend(): hcd_pci_runtime_suspend+0x0/0x50 returns -16 [50138.536357] PM: Finishing wakeup. Experimenting a bit, here's what happens. - Plug in device (so it's working). Suspend works. - eject /dev/sdb. Suspend fails. - Physically unplug the device. Suspend works again. This is actually a real issue -- my laptop can continue to power its usb port while suspended, and the Nook is functional (and charges) while plugged in but ejected, but I can't suspend my laptop so it can charge my Nook. Curiously, this issue seems to be Nook-specific. I can't reproduce it with a Corsair Flash Voyager. --Andy -- 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/4] scsi_debug: fix endianness bug in sdebug_build_parts()
2013/7/26 Martin Peschke : > On Mon, 2013-07-15 at 20:52 +0900, Akinobu Mita wrote: >> With module parameter num_parts > 0, partition table is built on the >> ramdisk storage when loading the driver. Unfortunately, there is an >> endianness bug in sdebug_build_parts(). So the partition table is not >> correctly initialized on big-endian systems. >> >> Signed-off-by: Akinobu Mita >> Cc: "James E.J. Bottomley" >> Cc: Douglas Gilbert >> Cc: linux-scsi@vger.kernel.org >> --- >> drivers/scsi/scsi_debug.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c >> index cb4fefa..2f39b13 100644 >> --- a/drivers/scsi/scsi_debug.c >> +++ b/drivers/scsi/scsi_debug.c >> @@ -2659,8 +2659,8 @@ static void __init sdebug_build_parts(unsigned char >> *ramp, >> / sdebug_sectors_per; >> pp->end_sector = (end_sec % sdebug_sectors_per) + 1; >> >> - pp->start_sect = start_sec; >> - pp->nr_sects = end_sec - start_sec + 1; >> + pp->start_sect = cpu_to_le32(start_sec); >> + pp->nr_sects = cpu_to_le32(end_sec - start_sec + 1); >> pp->sys_ind = 0x83; /* plain Linux partition */ >> } >> } > > > I have posted the same fix several times, e.g. > http://marc.info/?l=linux-scsi&m=137051617907423&w=2 > Good luck! > > Acked-by: Martin Peschke Thanks. I found this problem from sparse warning noticed by 0-DAY kernel build testing. -- 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] fnic: BUG: sleeping function called from invalid context during probe
On 7/23/13 1:04 PM, "Chris Leech" wrote:
>I hit this during driver probe with the latest fnic updates (this trace
>is from a backport into a distro kernel, but the issue is the same).
>
>> BUG: sleeping function called from invalid context at mm/slab.c:3113
>> in_atomic(): 0, irqs_disabled(): 1, pid: 610, name: work_for_cpu
>> INFO: lockdep is turned off.
>> irq event stamp: 0
>> hardirqs last enabled at (0): [<(null)>] (null)
>> hardirqs last disabled at (0): []
>> copy_process+0x5e5/0x1670
>> softirqs last enabled at (0): []
>> copy_process+0x5e5/0x1670
>> softirqs last disabled at (0): [<(null)>] (null)
>> Pid: 610, comm: work_for_cpu Not tainted
>> Call Trace:
>> [] ? print_irqtrace_events+0xd0/0xe0
>> [] ? __might_sleep+0xf7/0x130
>> [] ? kmem_cache_alloc_trace+0x20b/0x2d0
>> [] ? __create_workqueue_key+0x3e/0x1d0
>> [] ? __create_workqueue_key+0x3e/0x1d0
>> [] ? fnic_probe+0x977/0x11aa [fnic]
>> [] ? fnic_probe+0x9a3/0x11aa [fnic]
>> [] ? do_work_for_cpu+0x0/0x30
>> [] ? local_pci_probe+0x17/0x20
>> [] ? do_work_for_cpu+0x18/0x30
>> [] ? kthread+0x96/0xa0
>> [] ? child_rip+0xa/0x20
>> [] ? _spin_unlock_irq+0x30/0x40
>> [] ? restore_args+0x0/0x30
>> [] ? kthread+0x0/0xa0
>> [] ? child_rip+0x0/0x20
>
>The problem is in this hunk of "FIP VLAN Discovery Feature Support"
>(d3c995f1dcf938f1084388d92b8fb97bec366566)
>
>create_singlethreaded_workqueue cannot be called with irqs disabled
>
>@@ -620,7 +634,29 @@ static int __devinit fnic_probe(struct pci_dev
>*pdev,
>vnic_dev_packet_filter(fnic->vdev, 1, 1, 0, 0, 0);
>vnic_dev_add_addr(fnic->vdev, FIP_ALL_ENODE_MACS);
>vnic_dev_add_addr(fnic->vdev, fnic->ctlr.ctl_src_addr);
>+ fnic->set_vlan = fnic_set_vlan;
>fcoe_ctlr_init(&fnic->ctlr, FIP_MODE_AUTO);
>+ setup_timer(&fnic->fip_timer, fnic_fip_notify_timer,
>+ (unsigned long)fnic);
>+ spin_lock_init(&fnic->vlans_lock);
>+ INIT_WORK(&fnic->fip_frame_work, fnic_handle_fip_frame);
>+ INIT_WORK(&fnic->event_work, fnic_handle_event);
>+ skb_queue_head_init(&fnic->fip_frame_queue);
>+ spin_lock_irqsave(&fnic_list_lock, flags);
>+ if (!fnic_fip_queue) {
>+ fnic_fip_queue =
>+ create_singlethread_workqueue("fnic_fip_q");
>+ if (!fnic_fip_queue) {
>+ spin_unlock_irqrestore(&fnic_list_lock, flags);
>+ printk(KERN_ERR PFX "fnic FIP work queue "
>+"create failed\n");
>+ err = -ENOMEM;
>+ goto err_out_free_max_pool;
>+ }
>+ }
>+ spin_unlock_irqrestore(&fnic_list_lock, flags);
>+ INIT_LIST_HEAD(&fnic->evlist);
>+ INIT_LIST_HEAD(&fnic->vlans);
>} else {
>shost_printk(KERN_INFO, fnic->lport->host,
> "firmware uses non-FIP mode\n");
>
>The attempts to make fnic_fip_queue a single instance for the driver
>while it's being created in probe look awkward anyway, why is this not
>created in fnic_init_module like the event workqueue?
>
>Built and loaded, tested all-right through probe without triggering the
>same issue. Not tested by me for functionality, as the fnic hardware I
>had access to isn't actually attached to an FCoE SAN.
>
>Signed-off-by: Chris Leech
>---
>
>Patch Changelog
>
>v2: At Hiral's request I've bumped the driver version
>
> drivers/scsi/fnic/fnic.h | 2 +-
> drivers/scsi/fnic/fnic_main.c | 22 +-
> 2 files changed, 10 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
>index b6d1f92..c18c681 100644
>--- a/drivers/scsi/fnic/fnic.h
>+++ b/drivers/scsi/fnic/fnic.h
>@@ -38,7 +38,7 @@
>
> #define DRV_NAME "fnic"
> #define DRV_DESCRIPTION "Cisco FCoE HBA Driver"
>-#define DRV_VERSION "1.5.0.22"
>+#define DRV_VERSION "1.5.0.23"
> #define PFX DRV_NAME ": "
> #define DFX DRV_NAME "%d: "
>
>diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
>index 5f09d18..42e15ee 100644
>--- a/drivers/scsi/fnic/fnic_main.c
>+++ b/drivers/scsi/fnic/fnic_main.c
>@@ -642,19 +642,6 @@ static int fnic_probe(struct pci_dev *pdev, const
>struct pci_device_id *ent)
> INIT_WORK(&fnic->fip_frame_work, fnic_handle_fip_frame);
> INIT_WORK(&fnic->event_work, fnic_handle_event);
> skb_queue_head_init(&fnic->fip_frame_queue);
>- spin_lock_irqsave(&fnic_list_lock, flags);
>- if (!fnic_fip_queue) {
>- fnic_fip_queue =
>- create_singlethread_workqueue("fnic_fip_q");
>- if (!fnic_fip_queue) {
>- spin_unlock_irqrestore(&fnic_list_lock, flags);
>- printk(KERN_ERR PFX "fnic FIP work queue "
>- "create failed\n");
>-
Re: Ejected Nook (usb mass storage) prevents suspend
On Fri, 26 Jul 2013, Andy Lutomirski wrote: > This is kernel 3.9.9-302.fc19.x86_64. > > I plugged in a BN Nook (a usb mass storage device), used it, and > ejected it. This makes suspend fail: > > [50135.265514] PM: Entering freeze sleep > [50135.265517] Suspending console(s) (use no_console_suspend to debug) > [50135.287724] sd 7:0:0:0: [sdb] Synchronizing SCSI cache > [50135.290413] sd 7:0:0:0: [sdb] > [50135.290415] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE > [50135.290418] sd 7:0:0:0: [sdb] > [50135.290422] Sense Key : Not Ready [current] > [50135.290424] sd 7:0:0:0: [sdb] > [50135.290429] Add. Sense: Medium not present > [50135.290448] dpm_run_callback(): scsi_bus_suspend+0x0/0x40 returns -5 > [50135.290454] PM: Device 7:0:0:0 failed to suspend async: error -5 > [50138.486917] PM: Some devices failed to suspend > [50138.525007] PM: resume of devices complete after 38.132 msecs > [50138.525315] pci_pm_runtime_suspend(): > hcd_pci_runtime_suspend+0x0/0x50 returns -16 > [50138.536357] PM: Finishing wakeup. It looks like sd_sync_cache() should return immediately if no media is present or the media has been changed. In addition, scsi_start_stop_device() should return immediately if no media is present. Or at least, it shouldn't treat "medium not present" responses as errors. Alan Stern -- 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: Questions about your scsi-mq prototype
On Fri, 2013-07-26 at 08:29 -0500, scame...@beardog.cce.hp.com wrote: > On Thu, Jul 25, 2013 at 07:16:39PM -0700, Nicholas A. Bellinger wrote: > > On Tue, 2013-07-16 at 10:39 -0700, Nicholas A. Bellinger wrote: > > Hi Stephen & Robert, > > > > Just curious if you've been able to make any progress here..? > > Only just started to try to make the scsi variant of the sop driver > work again yesterday and start bringing it up to date since it was > last touched in December, and so I haven't done anything with the > scsi-mq code yet. Too many other things demanding my attention at > work lately, but at least I was able to spend nearly all day yesterday > working on the scsi SOP driver. > Great, thanks for the update. Looking forward to seeing the drivers synced. > > > > Also as per our earlier off-list discussion, you should be able to > > safely run existing scsi_request_fn() based LLDs along side new > > SHT->scsi_mq=true enabled code for prototyping purposes. > > Thanks, good to know. > > > > > Btw, any chance to get an SOP sample to hack on..? I'd like to connect > > one of these to IvyBridge-EP silicon for various scsi-mq and target mode > > performance testing. ;) > > You mean hardware? Yes! > Not for me to answer, but I'd say it's not very likely at the moment. If you can point me to a person off-list to chat with about such an arrangement, it would be appreciated. I'm happy to sign an NDA and whatever legal bits are necessary to make a scsi-mq enabled SOP driver a reality. --nab -- 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: Ejected Nook (usb mass storage) prevents suspend
On Fri, 26 Jul 2013, Andy Lutomirski wrote:
> This is kernel 3.9.9-302.fc19.x86_64.
>
> I plugged in a BN Nook (a usb mass storage device), used it, and
> ejected it. This makes suspend fail:
>
> [50135.265514] PM: Entering freeze sleep
> [50135.265517] Suspending console(s) (use no_console_suspend to debug)
> [50135.287724] sd 7:0:0:0: [sdb] Synchronizing SCSI cache
> [50135.290413] sd 7:0:0:0: [sdb]
> [50135.290415] Result: hostbyte=DID_OK driverbyte=DRIVER_SENSE
> [50135.290418] sd 7:0:0:0: [sdb]
> [50135.290422] Sense Key : Not Ready [current]
> [50135.290424] sd 7:0:0:0: [sdb]
> [50135.290429] Add. Sense: Medium not present
> [50135.290448] dpm_run_callback(): scsi_bus_suspend+0x0/0x40 returns -5
> [50135.290454] PM: Device 7:0:0:0 failed to suspend async: error -5
> [50138.486917] PM: Some devices failed to suspend
> [50138.525007] PM: resume of devices complete after 38.132 msecs
> [50138.525315] pci_pm_runtime_suspend():
> hcd_pci_runtime_suspend+0x0/0x50 returns -16
> [50138.536357] PM: Finishing wakeup.
In addition to my earlier comment, the patch below should be applied.
It will fix your immediate problem, although not in the best way.
Alan Stern
Index: usb-3.10/drivers/scsi/scsi_pm.c
===
--- usb-3.10.orig/drivers/scsi/scsi_pm.c
+++ usb-3.10/drivers/scsi/scsi_pm.c
@@ -48,8 +48,6 @@ static int scsi_dev_type_resume(struct d
static int
scsi_bus_suspend_common(struct device *dev, int (*cb)(struct device *))
{
- int err = 0;
-
if (scsi_is_sdev_device(dev)) {
/*
* All the high-level SCSI drivers that implement runtime
@@ -59,10 +57,10 @@ scsi_bus_suspend_common(struct device *d
if (pm_runtime_suspended(dev))
return 0;
- err = scsi_dev_type_suspend(dev, cb);
+ scsi_dev_type_suspend(dev, cb);
}
- return err;
+ return 0; /* System suspend should never fail */
}
static int
--
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 RESEND 0/1] AHCI: Optimize interrupt processing
On Thu, 2013-07-25 at 20:09 -0600, Jens Axboe wrote: > On Thu, Jul 25 2013, Nicholas A. Bellinger wrote: > > On Thu, 2013-07-25 at 12:16 +0200, Alexander Gordeev wrote: > > > On Mon, Jul 22, 2013 at 02:10:36PM -0700, Nicholas A. Bellinger wrote: > > > > Np. FYI, you'll want to use the latest commit e7827b351 HEAD from > > > > target-pending/scsi-mq, which now has functioning scsi-generic support. > > > > > > Survives a boot, a kernel build and the build's result :) > > > > Great. Thanks for the feedback Alexander! > > > > So the next step on my end is to enable -mq for ahci, and verify initial > > correctness using QEMU/KVM hardware emulation. > > > > Btw, I've been looking at enabling the SHT->cmd_size for struct > > ata_queued_cmd descriptor pre-allocation, but AFAICT these descriptors > > are already all pre-allocated by libata and obtained via ata_qc_new() -> > > __ata_qc_from_tag() during ata_scsi_queuecmd(). > > Might still not be a bad idea to do it: > > - Cleans up a driver, getting rid of the need to alloc, maintain, and > free those structures. > > - Should be some cache locality benefits to having it all sequential. > Looking at this some more, there are a number of locations outside of the main blk_mq_ops->queue_rq() -> SHT->queuecommand_mq() dispatch that use *ata_qc_from_tag() to obtain *ata_queued_cmd, and a few without a associated struct scsi_cmnd like libata-core.c:ata_exec_internal_sg() for example.. So I don't think (completely) getting rid of ata_port->qcmds[] will be possible, and just converting the ata_scsi_queuecmd() path to use the extra SHT->cmd_size pre-allocation for *ata_queued_cmd might end up being more trouble that it's worth. Still undecided on that part.. Tejun, do you have any thoughts + input here..? --nab -- 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 RESEND 0/1] AHCI: Optimize interrupt processing
On Fri, 2013-07-26 at 14:14 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2013-07-25 at 20:09 -0600, Jens Axboe wrote:
> > On Thu, Jul 25 2013, Nicholas A. Bellinger wrote:
> > > On Thu, 2013-07-25 at 12:16 +0200, Alexander Gordeev wrote:
> > > > On Mon, Jul 22, 2013 at 02:10:36PM -0700, Nicholas A. Bellinger wrote:
> > > > > Np. FYI, you'll want to use the latest commit e7827b351 HEAD from
> > > > > target-pending/scsi-mq, which now has functioning scsi-generic
> > > > > support.
> > > >
> > > > Survives a boot, a kernel build and the build's result :)
> > >
> > > Great. Thanks for the feedback Alexander!
> > >
> > > So the next step on my end is to enable -mq for ahci, and verify initial
> > > correctness using QEMU/KVM hardware emulation.
> > >
> > > Btw, I've been looking at enabling the SHT->cmd_size for struct
> > > ata_queued_cmd descriptor pre-allocation, but AFAICT these descriptors
> > > are already all pre-allocated by libata and obtained via ata_qc_new() ->
> > > __ata_qc_from_tag() during ata_scsi_queuecmd().
> >
> > Might still not be a bad idea to do it:
> >
> > - Cleans up a driver, getting rid of the need to alloc, maintain, and
> > free those structures.
> >
> > - Should be some cache locality benefits to having it all sequential.
> >
>
> Looking at this some more, there are a number of locations outside of
> the main blk_mq_ops->queue_rq() -> SHT->queuecommand_mq() dispatch that
> use *ata_qc_from_tag() to obtain *ata_queued_cmd, and a few without a
> associated struct scsi_cmnd like libata-core.c:ata_exec_internal_sg()
> for example..
>
> So I don't think (completely) getting rid of ata_port->qcmds[] will be
> possible, and just converting the ata_scsi_queuecmd() path to use the
> extra SHT->cmd_size pre-allocation for *ata_queued_cmd might end up
> being more trouble that it's worth. Still undecided on that part..
>
> Tejun, do you have any thoughts + input here..?
>
OK, so I decided to give this a shot anyways.. Here is a quick
conversion for libata + AHCI to use blk-mq -> scsi-mq pre-allocation for
ata_queued_cmd descriptors:
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 2b50dfd..61b3db8 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -92,6 +92,9 @@ static int ahci_pci_device_resume(struct pci_dev *pdev);
static struct scsi_host_template ahci_sht = {
AHCI_SHT("ahci"),
+ .scsi_mq = true,
+ .cmd_size = sizeof(struct ata_queued_cmd),
+ .queuecommand_mq = ata_scsi_queuecmd,
};
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index f218427..e21814d 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -4725,29 +4725,25 @@ void swap_buf_le16(u16 *buf, unsigned int buf_words)
/**
* ata_qc_new - Request an available ATA command, for queueing
* @ap: target port
+ * @sc: incoming scsi_cmnd descriptor
*
* LOCKING:
* None.
*/
-static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap)
+static struct ata_queued_cmd *ata_qc_new(struct ata_port *ap,
+struct scsi_cmnd *sc)
{
struct ata_queued_cmd *qc = NULL;
- unsigned int i;
+ struct request *rq = sc->request;
/* no command while frozen */
if (unlikely(ap->pflags & ATA_PFLAG_FROZEN))
return NULL;
- /* the last tag is reserved for internal command. */
- for (i = 0; i < ATA_MAX_QUEUE - 1; i++)
- if (!test_and_set_bit(i, &ap->qc_allocated)) {
- qc = __ata_qc_from_tag(ap, i);
- break;
- }
-
- if (qc)
- qc->tag = i;
+ qc = (struct ata_queued_cmd *)sc->SCp.ptr;
+ qc->scsicmd = sc;
+ qc->tag = rq->tag;
return qc;
}
@@ -4755,19 +4751,20 @@ static struct ata_queued_cmd *ata_qc_new(struct
ata_port *ap)
/**
* ata_qc_new_init - Request an available ATA command, and initialize it
* @dev: Device from whom we request an available command structure
+ * @sc: incoming scsi_cmnd descriptor
*
* LOCKING:
* None.
*/
-struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev)
+struct ata_queued_cmd *ata_qc_new_init(struct ata_device *dev,
+ struct scsi_cmnd *sc)
{
struct ata_port *ap = dev->link->ap;
struct ata_queued_cmd *qc;
- qc = ata_qc_new(ap);
+ qc = ata_qc_new(ap, sc);
if (qc) {
- qc->scsicmd = NULL;
qc->ap = ap;
qc->dev = dev;
@@ -4797,10 +4794,9 @@ void ata_qc_free(struct ata_queued_cmd *qc)
qc->flags = 0;
tag = qc->tag;
- if (likely(ata_tag_valid(tag))) {
+
+ if (likely(ata_tag_valid(tag)))
qc->tag = ATA_TAG_POISON;
- clear_bit(tag, &ap->qc_allocated);
- }
}
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 0101af5..e5ab880 100644
---
RE: [PATCHv2] pm80xx: fix Adaptec 71605H hang
Hi Hans,
Thanks for addressing the comment.
Acked-by: anandkumar.santha...@pmcs.com
Regards
Anand
-Original Message-
From: Hans Verkuil [mailto:hverk...@xs4all.nl]
Sent: Friday, July 26, 2013 10:14 PM
To: linux-scsi@vger.kernel.org
Cc: Anand Kumar Santhanam; lindar_...@usish.com; Sangeetha Gnanasekaran;
xjtu...@gmail.com
Subject: [PATCHv2] pm80xx: fix Adaptec 71605H hang
The IO command size is 128 bytes for these new controllers as opposed to
64 for the old 8001 controller.
The Adaptec out-of-tree driver did this correctly. After comparing the
two this turned out to be the crucial difference.
So don't hardcode the IO command size, instead use pm8001_ha->iomb_size
as that is the correct value for both old and new controllers.
Signed-off-by: Hans Verkuil
Cc: sta...@vger.kernel.org # for v3.10 and up
---
drivers/scsi/pm8001/pm8001_hwi.c | 4 ++--
drivers/scsi/pm8001/pm80xx_hwi.c | 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c
b/drivers/scsi/pm8001/pm8001_hwi.c
index 69dd49c..ce3f129d 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -221,7 +221,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
pm8001_ha->main_cfg_tbl.pm8001_tbl.fatal_err_interrupt
= 0x01;
for (i = 0; i < PM8001_MAX_INB_NUM; i++) {
pm8001_ha->inbnd_q_tbl[i].element_pri_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x00<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16)
| (0x00<<30);
pm8001_ha->inbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[IB +
i].phys_addr_hi;
pm8001_ha->inbnd_q_tbl[i].lower_base_addr =
@@ -247,7 +247,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
}
for (i = 0; i < PM8001_MAX_OUTB_NUM; i++) {
pm8001_ha->outbnd_q_tbl[i].element_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x01<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16)
| (0x01<<30);
pm8001_ha->outbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[OB +
i].phys_addr_hi;
pm8001_ha->outbnd_q_tbl[i].lower_base_addr =
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c
b/drivers/scsi/pm8001/pm80xx_hwi.c
index 302514d..e1c4896 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -275,7 +275,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
for (i = 0; i < PM8001_MAX_SPCV_INB_NUM; i++) {
pm8001_ha->inbnd_q_tbl[i].element_pri_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x00<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16)
| (0x00<<30);
pm8001_ha->inbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[IB +
i].phys_addr_hi;
pm8001_ha->inbnd_q_tbl[i].lower_base_addr =
@@ -301,7 +301,7 @@ static void init_default_table_values(struct
pm8001_hba_info *pm8001_ha)
}
for (i = 0; i < PM8001_MAX_SPCV_OUTB_NUM; i++) {
pm8001_ha->outbnd_q_tbl[i].element_size_cnt =
- PM8001_MPI_QUEUE | (64 << 16) | (0x01<<30);
+ PM8001_MPI_QUEUE | (pm8001_ha->iomb_size << 16)
| (0x01<<30);
pm8001_ha->outbnd_q_tbl[i].upper_base_addr =
pm8001_ha->memoryMap.region[OB +
i].phys_addr_hi;
pm8001_ha->outbnd_q_tbl[i].lower_base_addr =
--
1.8.3.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: [trivial PATCH] treewide: Add __GFP_NOWARN to k.alloc calls with v.alloc fallbacks
On Wed, 2013-06-19 at 12:15 -0700, Joe Perches wrote: > Don't emit OOM warnings when k.alloc calls fail when > there there is a v.alloc immediately afterwards. > > Converted a kmalloc/vmalloc with memset to kzalloc/vzalloc. Hey Jiri. What's your schedule for accepting or rejecting these sorts of patches? -- 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: [RFC PATCH 2/2] ata: acpi: rework the ata acpi bind support
On Thu, Jul 25, 2013 at 10:52:29AM -0400, Tejun Heo wrote: > > I like it but am wondering why we weren't doing this before. Was the > acpi support added before we made ata objects proper devices? Yes. -- Matthew Garrett | mj...@srcf.ucam.org -- 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
