Re: [PATCH v7 0/5] scsi: ufs: add ufs driver code for Hisilicon Hi3660 SoC
Hi, Wei On 2018年01月06日 17:51, Li Wei wrote: This patchset adds driver support for UFS for Hi3660 SoC. It is verified on HiKey960 board. Usually here should list the change compared with the last change set, to make it easier to reviewer, who may pay more attention to the differences. For example v7:xxx //change since v6 v6:xxx // change since v5 Li Wei (5): scsi: ufs: add Hisilicon ufs driver code dt-bindings: scsi: ufs: add document for hisi-ufs arm64: dts: add ufs dts node arm64: defconfig: enable configs for Hisilicon ufs arm64: defconfig: enable f2fs and squashfs Documentation/devicetree/bindings/ufs/ufs-hisi.txt | 43 ++ arch/arm64/boot/dts/hisilicon/hi3660.dtsi | 20 + arch/arm64/configs/defconfig | 11 + drivers/scsi/ufs/Kconfig | 9 + drivers/scsi/ufs/Makefile | 1 + drivers/scsi/ufs/ufs-hisi.c| 621 + drivers/scsi/ufs/ufs-hisi.h| 116 7 files changed, 821 insertions(+) create mode 100644 Documentation/devicetree/bindings/ufs/ufs-hisi.txt create mode 100644 drivers/scsi/ufs/ufs-hisi.c create mode 100644 drivers/scsi/ufs/ufs-hisi.h
Re: [PATCH 3/3] scsi: hisi_sas: lock sensitive region in hisi_sas_slot_abort()
On 2017年01月03日 20:24, John Garry wrote: When we call hisi_sas_slot_task_free() we should grab the hisi_hba.lock, as hisi_sas_slot_task_free() accesses common hisi_hba elements. Function hisi_sas_slot_abort() is missing this, so add it. 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 2/3] scsi: hisi_sas: lock sensitive regions when servicing CQ interrupt
On 2017年01月03日 20:24, John Garry wrote: There is a bug in the current driver in that certain hisi_hba and port structure elements which we access when servicing the CQ interrupt do not use thread-safe accesses; these include hisi_sas_port linked-list of active slots (hisi_sas_port.entry), bitmap of currently allocated IPTT (in hisi_hba.slot_index_tags), and completion queue read pointer. As a solution, lock these elements with the hisi_hba.lock. 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 1/3] scsi: hisi_sas: service v2 hw CQ ISR with tasklet
On 2017年01月03日 20:24, John Garry wrote: Currently the all the slot processing for the completion queue is done in ISR context. It is judged that the slot processing can take a long time, especially when a SATA NCQ completes (upto 32 slots). So, as a solution, defer the bulk of the ISR processing to tasklet context. Each CQ will have its down tasklet. 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] scsi: hisi_sas: support deferred probe for v2 hw
On 2016年12月06日 20:44, John Garry wrote: In the hip06 and hip07 SoCs, the interrupt lines from the SAS controllers are connected to mbigen hw module [1]. The mbigen module is probed with module_init, and, as such, is not guaranteed to probe before the SAS driver. So we need to support deferred probe. We check for probe deferral in the hw layer probe, so we not probe into the main layer and allocate shost, memories, etc., to later learn that we need to defer the probe. [1] ./Documentation/devicetree/bindings/interrupt-controller/hisilicon,mbigen-v2.txt Signed-off-by: John Garry Reviewed-by: Zhangfei Gao 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: UFS_MASK macro definition
On Thu, Nov 24, 2016 at 12:21 PM, Subhash Jadavani wrote: > On 2016-11-23 19:23, Zhangfei Gao wrote: >> >> Hi, Vinayak >> >> Checked 4.9-rc6, and not find UFS_MASK macro definition. >> Have find this patch from google, >> >> http://www.spinics.net/lists/linux-scsi/msg58634.html >> [PATCH 2/2] [SCSI] ufs: Add missing UFS_MASK macro definition >> >> Is this patch has been merged? > > > Good catch. ufshci.h history shows that this was never merged. > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/drivers/scsi/ufs/ufshci.h?id=refs/tags/v4.9-rc6 > > Zhangfei, > do you want to take above patch and repost it to linux-scsi mailing list? > You may keep the original author & signed-off intact and may add your > signed-off. Got it, will send the patch. 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
[PATCH] ufs: Add missing UFS_MASK macro definition
From: Santosh Y Reported-by: Venkatraman S Reviewed-by: Vinayak Holikatti Signed-off-by: Santosh Y Signed-off-by: Zhangfei Gao --- Find the original patch from http://www.spinics.net/lists/linux-scsi/msg58634.html drivers/scsi/ufs/ufshci.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/ufs/ufshci.h b/drivers/scsi/ufs/ufshci.h index 9599741..bbd8df9 100644 --- a/drivers/scsi/ufs/ufshci.h +++ b/drivers/scsi/ufs/ufshci.h @@ -83,6 +83,8 @@ enum { MASK_UIC_DME_TEST_MODE_SUPPORT = 0x0400, }; +#define UFS_MASK(mask, offset) ((mask) << (offset)) + /* UFS Version 08h */ #define MINOR_VERSION_NUM_MASK UFS_MASK(0x, 0) #define MAJOR_VERSION_NUM_MASK UFS_MASK(0x, 16) -- 2.7.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
UFS_MASK macro definition
Hi, Vinayak Checked 4.9-rc6, and not find UFS_MASK macro definition. Have find this patch from google, http://www.spinics.net/lists/linux-scsi/msg58634.html [PATCH 2/2] [SCSI] ufs: Add missing UFS_MASK macro definition Is this patch has been merged? There will be build error if some definition defined by UFS_MASK is used. Otherwise it may keep silent. 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 01/11] hisi_sas: add v2 hw support for ECC and AXI bus fatal error
On Wed, Nov 23, 2016 at 4:59 PM, John Garry wrote: > On 16/11/2016 01:47, Zhangfei Gao wrote: >> >> On Mon, Nov 7, 2016 at 8:48 PM, John Garry wrote: >>> >>> From: Xiang Chen Reviewed-by: Zhangfei Gao >>> >>> 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 >> >> > > > Hi Zhangfei, > > We are actually now working on supporting controller reset for certain > AXI/ECC errors, so that we will not need to panic. Got it, thanks for the info. 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 05/11] hisi_sas: replace WARN_ON() with dev_warn() for internal abort
On Mon, Nov 7, 2016 at 8:48 PM, John Garry wrote: > From: Xiang Chen > > Replace WARN_ON() with dev_warn() print when internal abort fails. > > Signed-off-by: Xiang Chen > Signed-off-by: John Garry Reviewed-by: Zhangfei Gao Sorry, miss this one. -- 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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 Garry wrote: > 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
Re: [PATCH 1/8] hisi_sas: add internal abort core code
On 2016年08月10日 21:19, John Garry wrote: Add core code for internal abort functionality. The internal abort features allows the host controller to abort commands which are still active in the controller but have not yet been sent to the slave device. Typically a command only spends a relatively short time in the controller when compared to the amount of the time after it is sent to the slave device. Two modes of internal abort are supported: - device - individual command For device, when the internal abort is issued all commands in the host for that device are aborted. For a single command, only that command is aborted if it is still in the host. In HW the internal abort command is executed similar to any other sort of command, like SSP. Signed-off-by: John Garry --- drivers/scsi/hisi_sas/hisi_sas.h | 3 + drivers/scsi/hisi_sas/hisi_sas_main.c | 154 ++ 2 files changed, 157 insertions(+) diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h index 4731d32..4ae864d 100644 --- a/drivers/scsi/hisi_sas/hisi_sas.h +++ b/drivers/scsi/hisi_sas/hisi_sas.h @@ -146,6 +146,9 @@ struct hisi_sas_hw { struct hisi_sas_slot *slot); int (*prep_stp)(struct hisi_hba *hisi_hba, struct hisi_sas_slot *slot); + int (*prep_abort)(struct hisi_hba *hisi_hba, + struct hisi_sas_slot *slot, + int device_id, int abort_flag, int tag_to_abort); How about add comments to abort_flag and tag_to_abort. As a result, not sure why differently calling in hisi_sas_abort_task hisi_sas_internal_task_abort(hisi_hba, device, 1, tag); hisi_sas_internal_task_abort(hisi_hba, device, 0, tag); 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: how to test pscsi with vhost
Hi, Lingshan On Mon, Jul 4, 2016 at 10:02 PM, Zhu Lingshan wrote: > Hi Zhangfei, > > I am also interested in pscsi, you can try kvm, seems you can create a > virtualized pscsi device in kvm / virt-manager. I haven't tried that yet, > hope this can help. > Somehow I failed to setup virt-manager error like: Failed to connect to Mir: Failed to connect to server socket: No such file or directory Unable to init server: Could not connect: Connection refused (virt-manager:6661): Gtk-CRITICAL **: gtk_settings_get_for_screen: assertion 'GDK_IS_SCREEN (screen)' failed Still in check with qemu. The issue is scsi_probe_lun can not get correct lun. Have hacked with scsi_execute_req INQUIRY sereral times but same result. sc->result=0x800 sc->sense_buffer[0]=0x0 after targetcli setup, we can get: [ 104.238740] PSCSI[0]: Referencing SCSI Host ID: 1 [ 104.243471] PSCSI[0]: Referencing SCSI Channel ID: 0 [ 104.248463] PSCSI[0]: Referencing SCSI Target ID: 1 [ 104.253368] PSCSI[0]: Referencing SCSI LUN ID: 0 pscsi_show_configfs_dev_params SCSI Device Bus Location: Channel ID: 0 Target ID: 1 LUN: 0 Host ID: 1 Vendor: HGST Model: HUSMM1640ASS204 Rev: C2D0 Suppose drivers/scsi/virtio_scsi.c & drivers/target/target_core_pscsi.c have been verified. Suspect sas driver itself need do something to support pscsi. Still not find the root cause. 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
how to test pscsi with vhost
I am testing with pscsi, but fail to find any block device under /dev to mount. The scsi_probe_lun seems can not return correctly, scsi scan: INQUIRY failes. Is this usage correct? /backstores/pscsi> create name=pscsi_backend dev=/dev/sdb Note: block backstore recommended for SCSI block devices Created pscsi storage object pscsi_backend using /dev/sdb /backstores/pscsi> cd /vhost /vhost> create wwn=naa.60014052cc816bf4 /vhost> cd naa.60014052cc816bf4/tpg1/luns /vhost/naa.60...bf4/tpg1/luns> create /backstores/pscsi/pscsi_backend Selected LUN 0. Created LUN 0. /vhost/naa.60...bf4/tpg1/luns> cd / /> ls o- / . [...] o- backstores .. [...] | o- fileio ... [0 Storage Object] | o- iblock ... [0 Storage Object] | o- pscsi [1 Storage Object] | | o- pscsi_backend [/dev/sdb activated] | o- rd_mcp ... [0 Storage Object] o- iscsi . [0 Targets] o- loopback .. [0 Targets] o- vhost .. [1 Target] o- naa.60014052cc816bf4 [1 TPG] o- tpg1 ... [naa.6001405030badd8a] o- luns [1 LUN] o- lun0 . [pscsi/pscsi_backend (/dev/sdb)] /> /> saveconfig qemu.git/aarch64-softmmu/qemu-system-aarch64 \ -enable-kvm -nographic -kernel Image \ -device vhost-scsi-pci,wwpn=naa.60014052cc816bf4 \ -m 512 -M virt -cpu host \ -append "earlyprintk console=ttyAMA0 mem=512M" root@(none)$ ls /dev autofs ptyp8 tty30 tty62 console ptyp9 tty31 tty63 cpu_dma_latency ptypa tty32 tty7 cuseptypb tty33 tty8 fullptypc tty34 tty9 fuseptypd tty35 ttyAMA0 input ptype tty36 ttyS0 kmemptypf tty37 ttyS1 kmsgrandom tty38 ttyS2 loop-controlsnd tty39 ttyS3 loop0 tty tty4ttyp0 loop1 tty0tty40 ttyp1 loop2 tty1tty41 ttyp2 loop3 tty10 tty42 ttyp3 loop4 tty11 tty43 ttyp4 loop5 tty12 tty44 ttyp5 loop6 tty13 tty45 ttyp6 loop7 tty14 tty46 ttyp7 mem tty15 tty47 ttyp8 memory_bandwidthtty16 tty48 ttyp9 net tty17 tty49 ttypa network_latency tty18 tty5ttypb network_throughput tty19 tty50 ttypc nulltty2tty51 ttypd porttty20 tty52 ttype psaux tty21 tty53 ttypf ptmxtty22 tty54 urandom ptyp0 tty23 tty55 vcs ptyp1 tty24 tty56 vcs1 ptyp2 tty25 tty57 vcsa ptyp3 tty26 tty58 vcsa1 ptyp4 tty27 tty59 vga_arbiter ptyp5 tty28 tty6vhost-scsi ptyp6 tty29 tty60 zero ptyp7 tty3tty61 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 1/3] hisi_sas: add v2 hw ACPI support
On 05/31/2016 08:38 PM, John Garry wrote: Add support in v2 hw driver for ACPI. A check on whether an ACPI handle is available for the device is used to decide on whether to use ACPI reset handler or syscon for hw reset. Signed-off-by: John Garry Signed-off-by: Wei Xu Reviewed-by: Zhangfei Gao 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 2/3] hisi_sas: fix the inconsistent lock issue reported by CONFIG_PROVE_LOCKING
On 05/31/2016 08:38 PM, John Garry wrote: It is not necessary to surround call to notify_port_event(, PORTE_BROADCAST_RCVD) by spin_lock_irqsave(), so remove. This was causing a warn, as below: = [ INFO: inconsistent lock state ] 4.4.8+ #12 Not tainted - inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage. kworker/u64:1/168 [HC0[0]:SC0[0]:HE1:SE1] takes: (&(&hisi_hba->lock)->rlock){?.}, at: [] alloc_dev_quirk_v2_hw+0x48/0xec {IN-HARDIRQ-W} state was registered at: [] mark_lock+0x19c/0x6a0 [] __lock_acquire+0xa2c/0x1d00 [] lock_acquire+0x58/0x7c [] _raw_spin_lock_irqsave+0x54/0x6c [] int_chnl_int_v2_hw+0x1c4/0x248 [] handle_irq_event_percpu+0x9c/0x144 [] handle_irq_event+0x44/0x74 [] handle_fasteoi_irq+0xb4/0x188 [] generic_handle_irq+0x24/0x38 [] __handle_domain_irq+0x60/0xac [] gic_handle_irq+0xcc/0x168 [] el1_irq+0x6c/0xe0 [] default_idle_call+0x1c/0x34 [] cpu_startup_entry+0x1d4/0x228 [] rest_init+0x150/0x160 [] start_kernel+0x3a4/0x3b8 [<008bb000>] 0x8bb000 irq event stamp: 32661 hardirqs last enabled at (32661): [] __mutex_unlock_slowpath+0x108/0x18c hardirqs last disabled at (32660): [] __mutex_unlock_slowpath+0x44/0x18c softirqs last enabled at (25114): [] __do_softirq+0x210/0x27c softirqs last disabled at (25095): [] irq_exit+0x9c/0xe8 other info that might help us debug this: Possible unsafe locking scenario: CPU0 lock(&(&hisi_hba->lock)->rlock); lock(&(&hisi_hba->lock)->rlock); *** DEADLOCK *** 2 locks held by kworker/u64:1/168: #0: ("%s"shost->work_q_name){.+}, at: [] process_one_work+0x134/0x3cc #1: ((&sw->work)#2){+.+.+.}, at: [] process_one_work+0x134/0x3cc stack backtrace: CPU: 4 PID: 168 Comm: kworker/u64:1 Not tainted 4.4.8+ #12 Hardware name: Huawei Technologies Co., Ltd. D03/D03, BIOS 1.12 01/01/1900 Workqueue: scsi_wq_1 sas_discover_domain Call trace: [] dump_backtrace+0x0/0x114 [] show_stack+0x14/0x1c [] dump_stack+0xb4/0xf0 [] print_usage_bug+0x210/0x2b4 [] mark_lock+0x5fc/0x6a0 [] __lock_acquire+0x800/0x1d00 [] lock_acquire+0x58/0x7c [] _raw_spin_lock+0x44/0x58 [] alloc_dev_quirk_v2_hw+0x48/0xec [] hisi_sas_dev_found+0x48/0x1b8 [] sas_notify_lldd_dev_found+0x34/0xe0 [] sas_discover_root_expander+0x58/0x128 [] sas_discover_domain+0x4bc/0x564 [] process_one_work+0x1a0/0x3cc [] worker_thread+0x138/0x438 [] kthread+0xdc/0xf0 [] ret_from_fork+0x10/0x40 Signed-off-by: Wei Xu 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 0/5] hisi_sas: v2 hw SATA fixes
On Fri, Apr 8, 2016 at 5:23 PM, John Garry wrote: > This patchset introduces SATA support fixes for > the HiSilicon v2 hw SAS controller. > > Fixes include: > - attach issue for SATA disk attached through expander > - intermittent issue for directly attaching multiple > SATA disks > - add support for directly attaching SATA disk to phy > index 4+ > - ITCT config issue > > John Garry (5): > hisi_sas: use device linkrate in MCR for v2 hw > hisi_sas: fix v2 hw multiple SATA disk issue > hisi_sas: add v2 hw support for >4 SATA phys > hisi_sas: for v2 hw only set ITCT qw2 for SAS device > hisi_sas: update driver version to 1.4 For the series, Reviewed-by: Zhangfei Gao 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 v2 02/32] devicetree: bindings: scsi: HiSi SAS
On 10/27/2015 10:39 PM, Mark Rutland wrote: On Tue, Oct 27, 2015 at 01:09:15PM +, John Garry wrote: On 26/10/2015 14:45, Mark Rutland wrote: On Mon, Oct 26, 2015 at 10:14:33PM +0800, John Garry wrote: Add devicetree bindings for HiSilicon SAS driver. Signed-off-by: John Garry --- .../devicetree/bindings/scsi/hisilicon-sas.txt | 70 ++ 1 file changed, 70 insertions(+) create mode 100644 Documentation/devicetree/bindings/scsi/hisilicon-sas.txt diff --git a/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt new file mode 100644 index 000..d1e7b2a --- /dev/null +++ b/Documentation/devicetree/bindings/scsi/hisilicon-sas.txt @@ -0,0 +1,70 @@ +* HiSilicon SAS controller + +The HiSilicon SAS controller supports SAS/SATA. + +Main node required properties: + - compatible : value should be as follows: + (a) "hisilicon,sas-controller-v1" for v1 of HiSilicon SAS controller IP + - reg : Address and length of the SAS register + - hisilicon,sas-syscon: phandle of syscon used for sas control + - ctrl-reg : offset to the following SAS control registers (in order): + - reset assert + - clock disable + - reset status + - reset de-assert + - clock enable This needs a better name, and it should probably be split up into several properties. However, it sounds like the syscon is actually a clock+reset controller, and should be modelled as such. It's not actually a part of the SAS controller as such. The syscon block is a general subsystem control block, and it is not specifically only for controlling reset and enabling clocks (other functions include serdes control, for example). It is also shared with other peripherals. So we can remove the ctrl-reg property (since it is not part of the SAS controller), and add the relevant syscon register offsets to the "hisilicon,sas-syscon" property, like this: hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; Ok? It would be better to have each offset in a separate property. These register are not used for different purpose. Instead, they are all used for one purpose, reset the sas controller; Though a bit complicated, the silicon has special requirement here. So still prefer using the original method, ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; Since we can simply use of_property_read_u32_array. 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 02/25] devicetree: bindings: scsi: HiSi SAS
On 10/19/2015 06:48 PM, John Garry wrote: On 16/10/2015 14:47, Rob Herring wrote: + - reg : Address and length of the register sets for the device + - SAS controller registers + - SAS controller control registers + + - reset-reg : offset to reset, status, and clock registers in control registers Within the above register range? If so and if this varies, then that implies there is more than 1 version of IP. In that case you should have a more specific compatible string. The registers in the second region are for syscon register offsets. See last note, below. How long is this property I count 3 cells here, but the example has 5. Define what each cell corresponds to specifically. We will add all the cells to the decription, which are: Reset assert, clock disable, reset status, reset de-assert, and clock enable. + We have switch to using syscon, The dts has been changed to sas_ctrl0: sas_ctrl@c000 { compatible = "hisilicon,sas-ctrl", "syscon"; reg = <0x0 0xc000 0x0 0x1>; }; sas0: sas@c100 { compatible = "hisilicon,sas-controller-v1"; reg = <0x0 0xc100 0x0 0x1>; hisilicon,sas-syscon = <&sas_ctrl0>; ctrl-reg = <0xa60 0x33c 0x5a30 0xa64 0x338>; ctrl-reg contains several regs in sas-ctrl, which need to be accessed since some complicated requirement of the silicon. Have considered using hisilicon,sas-syscon = <&sas_ctrl0 0xa60 0x33c 0x5a30 0xa64 0x338>; But of_property_read_u32_array cat not get array from index 1. Then we have to use of_property_read_u32_index one by one. So instead we add additional ctrl-reg, and get the array one time via of_property_read_u32_array. 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 03/25] scsi: hisi_sas: add initial bare driver
On 10/15/2015 05:23 PM, John Garry wrote: On 15/10/2015 09:49, Xinwei Kong wrote: +++ b/drivers/scsi/hisi_sas/hisi_sas.h @@ -0,0 +1,24 @@ +/* + * Copyright (c) 2015 Linaro Ltd. + * Copyright (c) 2015 Hisilicon Limited. + * + * 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. +MODULE_VERSION(DRV_VERSION); +MODULE_LICENSE("GPL"); V2 Can add. We do say in the header that it is v2. No, should use MODULE_LICENSE("GPL"); include/linux/module.h /* * The following license idents are currently accepted as indicating free * software modules * * "GPL" [GNU Public License v2 or later] * "GPL v2"[GNU Public License v2] 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 09/25] scsi: hisi_sas: add phy SAS ADDR initialization
On 10/14/2015 11:18 PM, Arnd Bergmann wrote: On Wednesday 14 October 2015 16:05:21 John Garry wrote: OK, we can look at adding the ability to read the SAS HBA address from a FW image or EFI variables. The easiest way is usually to have a DT property that gets updated by the firmware. Yes In net subsystem, there is mac-address. In dts, we set default mac-address, which will be modified by boot-loader, if all 0 random address will be used. mac-address = [00 00 00 00 00 00]; In driver, mac-address can be get via of_get_mac_address. Can we use the similar method here? 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 07/25] scsi: hisi_sas: add ioremap for device HW
On 10/13/2015 08:20 PM, Arnd Bergmann wrote: On Tuesday 13 October 2015 17:47:02 zhangfei wrote: On 10/12/2015 11:21 PM, Arnd Bergmann wrote: On Monday 12 October 2015 23:20:19 John Garry wrote: +int hisi_sas_ioremap(struct hisi_hba *hisi_hba) +{ + struct platform_device *pdev = hisi_hba->pdev; + struct device *dev = &pdev->dev; + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + hisi_hba->regs = devm_ioremap(dev, + res->start, + resource_size(res)); + if (!hisi_hba->regs) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + hisi_hba->ctrl_regs = devm_ioremap(dev, + res->start, + resource_size(res)); + if (!hisi_hba->ctrl_regs) + return -ENOMEM; + + return 0; +} static const struct of_device_id sas_of_match[] = { Better use devm_ioremap_resource() here, which registers the resource so they are checked for conflicts and listed in /proc/iomem. Yes, hisi_hba->regs can use devm_ioremap_resource. However ctrl_regs have to use devm_ioremap, since the address are sharing among different nodes, unfortunately, and devm_ioremap_resource will fail. This sounds like it should be fixed in the DT binding then, to ensure that the ranges don't overlap. Mapping the same register region multiple times is generally considered a bad idea because the drivers that map them often don't have global locks that serialize the access, so it's better to have code in place that ensures that they are distinct. What is the purpose of the ctrl_regs region, and why is it shared across multiple devices? Are all users of these registers in the same driver? We are considering using syscon for ctrl_regs. Thanks Arnd. -- 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/25] scsi: hisi_sas: add ioremap for device HW
On 10/12/2015 11:21 PM, Arnd Bergmann wrote: On Monday 12 October 2015 23:20:19 John Garry wrote: +int hisi_sas_ioremap(struct hisi_hba *hisi_hba) +{ + struct platform_device *pdev = hisi_hba->pdev; + struct device *dev = &pdev->dev; + struct resource *res; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); + hisi_hba->regs = devm_ioremap(dev, + res->start, + resource_size(res)); + if (!hisi_hba->regs) + return -ENOMEM; + + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); + hisi_hba->ctrl_regs = devm_ioremap(dev, + res->start, + resource_size(res)); + if (!hisi_hba->ctrl_regs) + return -ENOMEM; + + return 0; +} static const struct of_device_id sas_of_match[] = { Better use devm_ioremap_resource() here, which registers the resource so they are checked for conflicts and listed in /proc/iomem. Yes, hisi_hba->regs can use devm_ioremap_resource. However ctrl_regs have to use devm_ioremap, since the address are sharing among different nodes, unfortunately, and devm_ioremap_resource will fail. 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 05/25] scsi: hisi_sas: allocate memories and create pools
Hi, Arnd On 10/12/2015 11:15 PM, Arnd Bergmann wrote: On Monday 12 October 2015 23:20:17 John Garry wrote: + interrupt_count = of_property_count_u32_elems(np, "interrupts"); + if (interrupt_count < 0) + goto err_out; + + if (of_property_read_u32(np, "#interrupt-cells", &interrupt_cells)) + goto err_out; + + hisi_hba->int_names = devm_kcalloc(&pdev->dev, + interrupt_count / interrupt_cells, + HISI_SAS_NAME_LEN, + GFP_KERNEL); This computation looks wrong: the "interrupts" property refers to interrupts that are referenced by this node and provided by an interrupt-controller, while the "#interrupt-cells" property refers to interrupts provided by this node. They don't need to have any relation. We will use of_irq_count instead. 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