Re: [PATCH v7 0/5] scsi: ufs: add ufs driver code for Hisilicon Hi3660 SoC

2018-01-07 Thread zhangfei

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()

2017-01-03 Thread zhangfei



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

2017-01-03 Thread zhangfei



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

2017-01-03 Thread zhangfei



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

2016-12-06 Thread zhangfei



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

2016-11-23 Thread Zhangfei Gao
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

2016-11-23 Thread Zhangfei Gao
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

2016-11-23 Thread Zhangfei Gao
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

2016-11-23 Thread Zhangfei Gao
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

2016-11-23 Thread Zhangfei Gao
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()

2016-11-15 Thread Zhangfei Gao
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

2016-11-15 Thread Zhangfei Gao
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

2016-11-15 Thread Zhangfei Gao
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()

2016-11-15 Thread Zhangfei Gao
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

2016-11-15 Thread Zhangfei Gao
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()

2016-11-15 Thread Zhangfei Gao
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

2016-11-15 Thread Zhangfei Gao
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()

2016-11-15 Thread Zhangfei Gao
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()

2016-11-15 Thread Zhangfei Gao
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

2016-11-15 Thread Zhangfei Gao
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

2016-08-20 Thread zhangfei



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

2016-07-07 Thread Zhangfei Gao
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

2016-07-04 Thread Zhangfei Gao
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

2016-06-05 Thread zhangfei



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

2016-06-05 Thread zhangfei



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

2016-04-13 Thread Zhangfei Gao
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

2015-10-27 Thread zhangfei



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

2015-10-19 Thread zhangfei



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

2015-10-15 Thread zhangfei



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

2015-10-14 Thread zhangfei



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

2015-10-13 Thread zhangfei



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

2015-10-13 Thread zhangfei



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

2015-10-13 Thread zhangfei

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