Kernel messages produced during runtime PM can cause a never-ending cycle
because user space utilities (e.g. journald or rsyslog) write the messages
back to storage, causing runtime resume, more messages, and so on.
Messages that tell of things that are expected to happen are arguably
unnecessary, so add a flag to suppress them. This flag is used by the UFS
driver.
Link: https://lore.kernel.org/r/20220228113652.970857-2-adrian.hunter@intel.com
Cc: stable@vger.kernel.org
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
We only need the rport structure for lpfc_chk_tgt_mapped().
Link: https://lore.kernel.org/r/20220301143718.40913-6-hare@suse.de
Cc: James Smart <james.smart@broadcom.com>
Reviewed-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Instead of passing in a scsi_cmnd we should be using the rport; we already
have the target and LUN ID as parameters, so there's no need to pass the
scsi_cmnd too.
Link: https://lore.kernel.org/r/20220301143718.40913-5-hare@suse.de
Cc: James Smart <james.smart@broadcom.com>
Reviewed-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Use fc_block_rport() instead of fc_block_scsi_eh() as the SCSI command will
be removed as argument for SCSI EH functions.
Link: https://lore.kernel.org/r/20220301143718.40913-4-hare@suse.de
Cc: James Smart <james.smart@broadcom.com>
Reviewed-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The default SCSI EH action for a non-existing EH callback is to return
FAILED, so having a callback just returning FAILED is pointless.
Link: https://lore.kernel.org/r/20220301143718.40913-3-hare@suse.de
Cc: James Smart <james.smart@broadcom.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
lpfc_bus_reset_handler() is really just a loop calling
lpfc_target_reset_handler() over all targets, which is what the error
handler will be doing anyway.
Link: https://lore.kernel.org/r/20220301143718.40913-2-hare@suse.de
Cc: James Smart <james.smart@broadcom.com>
Reviewed-by: James Smart <jsmart2021@gmail.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
During the process of driver probing, the probe function should return < 0
for failure, otherwise the kernel will treat value >= 0 as success.
Set 'err' to the error value returned by dma_set_mask() in case of failure.
Link: https://lore.kernel.org/r/1646060055-11361-1-git-send-email-zheyuma97@gmail.com
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
When the workqueue code was created it didn't allow variable args so we
have been using a temp buffer. Drop that.
Link: https://lore.kernel.org/r/20220226230435.38733-7-michael.christie@oracle.com
Reviewed-by: Chris Leech <cleech@redhat.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Use the session workqueue for recovery and unbinding. If there are delays
during device blocking/cleanup then it will no longer affect other
sessions.
Link: https://lore.kernel.org/r/20220226230435.38733-6-michael.christie@oracle.com
Reviewed-by: Chris Leech <cleech@redhat.com>
Reviewed-by: Lee Duncan <lduncan@suse.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
We currently allocate a workqueue per host and only use it for removing the
target. For the session per host case we could be using this workqueue to
be able to do recoveries (block, unblock, timeout handling) in parallel. To
also allow offload drivers to do their session recoveries in parallel, this
drops the per host workqueue and replaces it with a per session one.
Link: https://lore.kernel.org/r/20220226230435.38733-5-michael.christie@oracle.com
Reviewed-by: Lee Duncan <lduncan@suse.com>
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
qla4xxx does not use iscsi_scan_finished() anymore so remove it.
Link: https://lore.kernel.org/r/20220226230435.38733-4-michael.christie@oracle.com
Reviewed-by: Lee Duncan <lduncan@suse.com>
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
When the iSCSI class was added upstream, blocking a queue was fast because
it just set some flag bits and didn't handle I/O that was in the process of
being sent to the driver. That's no longer the case so blocking a queue is
expensive and we can end up with a backlog of blocks by the time we have
relogged in and are trying to start the queues.
For the session unblock case, this has try to cancel the block and recovery
work in case they are still queued so we can avoid unneeded queue
manipulations. For removal, we also now try to cancel all the recovery
related works since a couple lines down we will set the session and device
state so running those functions are not necessary.
Link: https://lore.kernel.org/r/20220226230435.38733-3-michael.christie@oracle.com
Reviewed-by: Lee Duncan <lduncan@suse.com>
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If the user sets the iscsi_eh_timer_workq/iscsi_eh workqueue's max_active
to greater than 1, the recovery_work could be running when
__iscsi_unblock_session() runs. The cancel_delayed_work() will then not
wait for the running work and we can race where we end up with the wrong
session state and scsi_device state set.
This replaces the cancel_delayed_work() with the sync version.
Link: https://lore.kernel.org/r/20220226230435.38733-2-michael.christie@oracle.com
Reviewed-by: Lee Duncan <lduncan@suse.com>
Reviewed-by: Chris Leech <cleech@redhat.com>
Signed-off-by: Mike Christie <michael.christie@oracle.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In the original FPIN commit, stats were incremented by the event_count.
Event_count is the minimum # of events that must occur before an FPIN is
sent. Thus, its not the actual number of events, and could be significantly
off (too low) as it doesn't reflect anything not reported. Rather than
attempt to count events, have the statistic count how many FPINS cross the
threshold and were reported.
Link: https://lore.kernel.org/r/20220301175536.60250-1-jsmart2021@gmail.com
Fixes: 3dcfe0de5a ("scsi: fc: Parse FPIN packets and update statistics")
Cc: <stable@vger.kernel.org> # v5.11+
Cc: Shyam Sundar <ssundar@marvell.com>
Cc: Nilesh Javali <njavali@marvell.com>
Reviewed-by: Himanshu Madhani <himanshu.madhani@oracle.com>
Signed-off-by: James Smart <jsmart2021@gmail.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Sparse throws a warning about context imbalance ("different lock contexts
for basic block") in sas_form_port() as it gets confused with the fact that
a port is locked within one of the two search loops and unlocked afterward
outside of the search loops once the phy is added to the port. Since this
code is not easy to follow, improve it by factoring out the code adding the
phy to the port once the port is locked into the helper function
sas_form_port_add_phy(). This helper can then be called directly within the
port search loops, avoiding confusion and clearing the sparse warning.
Link: https://lore.kernel.org/r/20220228094857.557329-1-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This header is empty now except for an include of <linux/blk-mq.h>, so
remove it.
Link: https://lore.kernel.org/r/20220224175552.988286-9-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Let submitters initialize the scmd->allowed field directly instead of
indirecting through struct scsi_request and remove the now superfluous
structure.
Link: https://lore.kernel.org/r/20220224175552.988286-8-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Prepare for removing the scsi_request structure by moving the result field
to struct scsi_cmnd.
Link: https://lore.kernel.org/r/20220224175552.988286-7-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Prepare for removing the scsi_request structure by moving the resid_len
field to struct scsi_cmnd.
Link: https://lore.kernel.org/r/20220224175552.988286-6-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Just use the sense_buffer field in struct scsi_cmnd for the sense data and
move the sense_len field over to struct scsi_cmnd.
Link: https://lore.kernel.org/r/20220224175552.988286-5-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Now that each scsi_request is backed by a scsi_cmnd, there is no need to
indirect the CDB storage. Change all submitters of SCSI passthrough
requests to store the CDB information directly in the scsi_cmnd, and while
doing so allocate the full 32 bytes that cover all Linux supported SCSI
hosts instead of requiring dynamic allocation for > 16 byte CDBs. On
64-bit systems this does not change the size of the scsi_cmnd at all, while
on 32-bit systems it slightly increases it for now, but that increase will
be made up by the removal of the remaining scsi_request fields.
Link: https://lore.kernel.org/r/20220224175552.988286-4-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Replace the big fat memset that requires saving and restoring various
fields with just initializing those fields that need initialization.
All the clearing to 0 is moved to scsi_prepare_cmd() as scsi_ioctl_reset()
alreadly uses kzalloc() to allocate a pre-zeroed command.
This is still conservative and can probably be optimized further.
Link: https://lore.kernel.org/r/20220224175552.988286-3-hch@lst.de
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Copy directly from the se_cmd CDB to the one in the scsi_request. This
temporarily limits the pscsi backend to supporting only up to 16 byte CDBs,
but this restriction will be lifted later in this series.
Link: https://lore.kernel.org/r/20220224175552.988286-2-hch@lst.de
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Function queue_work() returns a bool, so use a bool to hold this value for
the return code from callers, which should make the code a tiny bit more
clear.
Also take this opportunity to condense the code of the those callers, such
as sas_queue_work(), as suggested by Damien.
Link: https://lore.kernel.org/r/1645786656-221630-3-git-send-email-john.garry@huawei.com
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Nobody checks the return codes, so make them return void. Indeed, if the
LLDD cannot send an event, nothing much can be done in the LLDD about it.
Also remove prototype for sas_notify_phy_event() in sas_internal.h, which
should not be there.
Link: https://lore.kernel.org/r/1645786656-221630-2-git-send-email-john.garry@huawei.com
Reviewed-by: Xiang Chen <chenxiang66@hisilicon.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In case of SSP underflow allow the response frame IU to be examined for
setting the response stat value rather than always setting
SAS_DATA_UNDERRUN.
This will mean that we call sas_ssp_task_response() in those scenarios and
may send sense data to upper layer.
Such a condition would be for bad blocks were we just reporting an
underflow error to upper layer, but now the sense data will tell
immediately that the media is faulty.
Link: https://lore.kernel.org/r/1645703489-87194-7-git-send-email-john.garry@huawei.com
Signed-off-by: Xingui Yang <yangxingui@huawei.com>
Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Add a file operation for "cnt" file under bist directory, so users can only
read "cnt" or clear "cnt" to zero, but cannot randomly modify.
Link: https://lore.kernel.org/r/1645703489-87194-6-git-send-email-john.garry@huawei.com
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
If the driver probe fails to request the channel IRQ or fatal IRQ, the
driver will free the IRQ vectors before freeing the IRQs in free_irq(),
and this will cause a kernel BUG like this:
------------[ cut here ]------------
kernel BUG at drivers/pci/msi.c:369!
Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
Call trace:
free_msi_irqs+0x118/0x13c
pci_disable_msi+0xfc/0x120
pci_free_irq_vectors+0x24/0x3c
hisi_sas_v3_probe+0x360/0x9d0 [hisi_sas_v3_hw]
local_pci_probe+0x44/0xb0
work_for_cpu_fn+0x20/0x34
process_one_work+0x1d0/0x340
worker_thread+0x2e0/0x460
kthread+0x180/0x190
ret_from_fork+0x10/0x20
---[ end trace b88990335b610c11 ]---
So we use devm_add_action() to control the order in which we free the
vectors.
Link: https://lore.kernel.org/r/1645703489-87194-4-git-send-email-john.garry@huawei.com
Signed-off-by: Qi Liu <liuqi115@huawei.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The time of phyup not only depends on the controller but also the type of
disk connected. As an example, from experience, for some SATA disks the
amount of time from reset/power-on to receive the D2H FIS for phyup can
take upto and more than 10s sometimes. According to the specification of
some SATA disks such as ST14000NM0018, the max time from power-on to ready
is 30s.
Based on this the current timeout of phyup at 2s which is not enough. So
set the value as HISI_SAS_WAIT_PHYUP_TIMEOUT (30s) in
hisi_sas_control_phy().
For v3 hw there is a pre-existing workaround for a HW bug, being that we
issue a link reset when the OOB occurs but the phyup does not. The current
phyup timeout is HISI_SAS_WAIT_PHYUP_TIMEOUT. So if this does occur from
when issuing a phy enable or similar via hisi_sas_control_phy(), the
subsequent HW workaround linkreset processing calls hisi_sas_control_phy(),
but this will pend the original phy reset timing out, so it is safe.
Link: https://lore.kernel.org/r/1645703489-87194-3-git-send-email-john.garry@huawei.com
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Currently the permission of parameter prot_mask is 0x0, which means that
the member does not appear in sysfs. Change it as other module parameters
to 0444 for world-readable.
[mkp: s/v3/v2/]
Link: https://lore.kernel.org/r/1645703489-87194-2-git-send-email-john.garry@huawei.com
Fixes: d6a9000b81 ("scsi: hisi_sas: Add support for DIF feature for v2 hw")
Reported-by: Yihang Li <liyihang6@hisilicon.com>
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
__setup() handlers should return 1 if the command line option is handled
and 0 if not (or maybe never return 0; doing so just pollutes init's
environment with strings that are not init arguments/parameters).
Return 1 from aha152x_setup() to indicate that the boot option has been
handled.
Link: lore.kernel.org/r/64644a2f-4a20-bab3-1e15-3b2cdd0defe3@omprussia.ru
Link: https://lore.kernel.org/r/20220223000623.5920-1-rdunlap@infradead.org
Cc: "Juergen E. Fischer" <fischer@norbit.de>
Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
Reported-by: Igor Zhbanov <i.zhbanov@omprussia.ru>
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Firmware expects host driver to clear scratchpad rsvd 0 register after
non-fatal error is found.
This is done when firmware raises fatal error interrupt and indicates
non-fatal error. At this point firmware updates scratchpad rsvd 0 register
with non-fatal error value. Here host has to clear the register after
reading it during non-fatal errors.
Rename:
- MSGU_HOST_SCRATCH_PAD_6 to MSGU_SCRATCH_PAD_RSVD_0
- MSGU_HOST_SCRATCH_PAD_7 to MSGU_SCRATCH_PAD_RSVD_1
Link: https://lore.kernel.org/r/20220222092618.108198-1-Ajish.Koshy@microchip.com
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Acked-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Ajish Koshy <Ajish.Koshy@microchip.com>
Signed-off-by: Viswas G <Viswas.G@microchip.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This driver doesn't use SCp.ptr to save a SCSI command data pointer which
means "scsi pointer" is a complete misnomer here. Only a few members of
struct scsi_pointer are used and the rest waste memory. Avoid the "struct
foo { struct bar; };" silliness.
Link: https://lore.kernel.org/r/3529a59873a7de8455a27af2528341afe5069adc.1645484982.git.fthain@linux-m68k.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
This driver doesn't use SCp.ptr to save a SCSI command data pointer which
means "scsi pointer" is a complete misnomer here. Only a few members of
struct scsi_pointer are used and the rest waste memory. Avoid the "struct
foo { struct bar; };" silliness.
Link: https://lore.kernel.org/r/fbf930e64af5b15ca028dfe25b00fe933951f19b.1645484982.git.fthain@linux-m68k.org
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Finn Thain <fthain@linux-m68k.org>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Most importantly: eh_timed_out() is not limited by scmd->allowed, and can
reset timer forever.
Fixes: c829c39416 ("[SCSI] FC transport : Avoid device offline cases by stalling aborts until device unblocked")
Link: https://lore.kernel.org/r/20220219001601.3534043-1-khazhy@google.com
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Khazhismel Kumykov <khazhy@google.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Just a few lines below this kzalloc() we have a mutex_lock() which can
sleep.
Moreover, the only way to call this function is when a delayed work is
schedule. And delayed work can sleep:
INIT_DELAYED_WORK(&fw_event->work, mptsas_firmware_event_work);
--> mptsas_firmware_event_work()
--> mptsas_send_link_status_event()
--> mptsas_expander_add()
So there is really no good reason to use GFP_ATOMIC here. Change it to
GFP_KERNEL to give more opportunities to the kernel.
Link: https://lore.kernel.org/r/eccb2179ce800529851ed4fabc9d3f95fbbf7d7f.1644906731.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Make the driver messages more readable by adding a space after the message
prefix ":" and removing the extra space between function name and line
number.
Link: https://lore.kernel.org/r/20220220031810.738362-32-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Both pm8001_send_abort_all() and pm80xx_send_abort_all() are called only
for a non null device with the NCQ_READ_LOG_FLAG set, so remove the device
check on entry of these functions. Furthermore, setting the
NCQ_ABORT_ALL_FLAG device id flag and clearing the NCQ_READ_LOG_FLAG is
always done before calling these functions. Move these operations inside
the functions.
Link: https://lore.kernel.org/r/20220220031810.738362-31-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The task argument of the pm8001_ccb_task_free() function can be inferred
from the ccb argument ccb_task field. So there is no need to have this
argument. Likewise, the ccb_index argument is always equal to the ccb tag
field and is not needed either. Remove both arguments and update all call
sites. The pm8001_ccb_task_free_done() helper is also modified to match
this change.
Link: https://lore.kernel.org/r/20220220031810.738362-30-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
The main part of the pm8001_task_exec() function uses a do {} while(0) loop
that is useless and only makes the code harder to read. Remove this
loop. The unnecessary local variable t is also removed.
Additionally, avoid repeatedly declaring "struct task_status_struct *ts" to
handle error cases by declaring this variable for the entire function
scope. This allows simplifying the error cases, and together with the
addition of blank lines make the code more readable.
Finally, handling of the running_req counter is fixed to avoid decrementing
it without a corresponding incrementation in the case of an invalid task
protocol.
Link: https://lore.kernel.org/r/20220220031810.738362-29-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
There is no need to pass a pointer to a struct inbound_queue_table to
pm8001_mpi_build_cmd(). Passing the start index in the inbound queue table
of the adapter is enough. This change allows avoiding the declaration of a
struct inbound_queue_table pointer (circularQ variables) in many functions,
simplifying the code.
While at it, blank lines are added i(e.g. after local variable
declarations) to make the code more readable.
Link: https://lore.kernel.org/r/20220220031810.738362-28-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Introduce the pm8001_ccb_alloc() and pm8001_ccb_free() helpers to replace
the typical code patterns:
res = pm8001_tag_alloc(pm8001_ha, &ccb_tag);
if (res)
...
ccb = &pm8001_ha->ccb_info[ccb_tag];
ccb->device = pm8001_ha_dev;
ccb->ccb_tag = ccb_tag;
ccb->task = task;
ccb->n_elem = 0;
and
ccb->task = NULL;
ccb->ccb_tag = PM8001_INVALID_TAG;
pm8001_tag_free(pm8001_ha, tag);
With the simpler function calls:
ccb = pm8001_ccb_alloc(pm8001_ha, pm8001_ha_dev, task);
if (!ccb)
...
and
pm8001_ccb_free(pm8001_ha, ccb);
The pm8001_ccb_alloc() helper ensures that all fields of the ccb info
structure for the newly allocated tag are all initialized, except the
buf_prd field. The pm8001_ccb_free() helper clears the initialized fields
and the ccb tag to ensure that iteration over the adapter ccb_info array
detects ccbs that are in use.
All call site of the pm8001_tag_alloc() function that use a ccb info
associated with an allocated tag are converted to use the new helpers.
Link: https://lore.kernel.org/r/20220220031810.738362-27-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
To detect if a command is NCQ, there is no need to test all possible NCQ
command codes. Instead, use ata_is_ncq() to test the command protocol.
Link: https://lore.kernel.org/r/20220220031810.738362-26-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Replace the goto statement in the for loop with "break" and remove the
ex_err label. Also fix long lines, identation and blank lines to make the
code more readable.
Link: https://lore.kernel.org/r/20220220031810.738362-25-damien.lemoal@opensource.wdc.com
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
To detect if a command is NCQ, there is no need to test all possible NCQ
command codes. Instead, use ata_is_ncq() to test the command protocol.
Link: https://lore.kernel.org/r/20220220031810.738362-24-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In pm8001_chip_fw_flash_update_build(), if
pm8001_chip_fw_flash_update_build() fails, the struct fw_control_ex
allocated must be freed.
Link: https://lore.kernel.org/r/20220220031810.738362-23-damien.lemoal@opensource.wdc.com
Reviewed-by: Jack Wang <jinpu.wang@ionos.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
In pm8001_chip_set_dev_state_req(), pm8001_chip_fw_flash_update_req(),
pm80xx_chip_phy_ctl_req() and pm8001_chip_reg_dev_req() add missing calls
to pm8001_tag_free() to free the allocated tag when pm8001_mpi_build_cmd()
fails.
Similarly, in pm8001_exec_internal_task_abort(), if the chip ->task_abort
method fails, the tag allocated for the abort request task must be
freed. Add the missing call to pm8001_tag_free().
Link: https://lore.kernel.org/r/20220220031810.738362-22-damien.lemoal@opensource.wdc.com
Reviewed-by: John Garry <john.garry@huawei.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Adrian Hunter