- 28 Feb, 2022 4 commits
-
-
Finn Thain authored
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.orgReviewed-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>
-
Khazhismel Kumykov authored
Most importantly: eh_timed_out() is not limited by scmd->allowed, and can reset timer forever. Fixes: c829c394 ("[SCSI] FC transport : Avoid device offline cases by stalling aborts until device unblocked") Link: https://lore.kernel.org/r/20220219001601.3534043-1-khazhy@google.comReviewed-by:
Bart Van Assche <bvanassche@acm.org> Signed-off-by:
Khazhismel Kumykov <khazhy@google.com> Signed-off-by:
-
Christophe JAILLET authored
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.frSigned-off-by:Christophe JAILLET <christophe.jaillet@wanadoo.fr> Signed-off-by:
-
Gustavo A. R. Silva authored
Use flexible-array members in struct fc_fdmi_attr_entry and fs_fdmi_attrs instead of one-element arrays, and refactor the code accordingly. Also, this helps with the ongoing efforts to globally enable -Warray-bounds and get us closer to being able to tighten the FORTIFY_SOURCE routines on memcpy(). https://github.com/KSPP/linux/issues/79 https://github.com/ClangBuiltLinux/linux/issues/1590 Link: https://lore.kernel.org/r/20220214223903.GA859464@embeddedorSigned-off-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Signed-off-by:
-
- 23 Feb, 2022 36 commits
-
-
Damien Le Moal authored
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.comReviewed-by:
Jack Wang <jinpu.wang@ionos.com> Signed-off-by:
Damien Le Moal <damien.lemoal@opensource.wdc.com> Signed-off-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:John Garry <john.garry@huawei.com> Reviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comSigned-off-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
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.comReviewed-by:
-
Damien Le Moal authored
In pm8001_send_abort_all(), make sure to free the allocated sas task if pm8001_tag_alloc() or pm8001_mpi_build_cmd() fail. Link: https://lore.kernel.org/r/20220220031810.738362-21-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
The function pm8001_tag_alloc() determines free tags using the function find_first_zero_bit() which can return 0 when the first bit of the bitmap being inspected is 0. As such, tag 0 is a valid tag value that should not be dismissed as invalid. Fix the functions pm8001_work_fn(), mpi_sata_completion(), pm8001_mpi_task_abort_resp() and pm8001_open_reject_retry() to not dismiss 0 tags as invalid. The value 0xffffffff is used for invalid tags for unused ccb information structures. Add the macro definition PM8001_INVALID_TAG to define this value. Link: https://lore.kernel.org/r/20220220031810.738362-20-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
The call to pm8001_ccb_task_free() at the end of pm8001_mpi_task_abort_resp() already frees the ccb tag. So when the device NCQ_ABORT_ALL_FLAG is set, the tag should not be freed again. Also change the hardcoded 0xBFFFFFFF value to ~NCQ_ABORT_ALL_FLAG as it ought to be. Link: https://lore.kernel.org/r/20220220031810.738362-19-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
In mpi_set_phy_profile_req() and in pm8001_set_phy_profile_single(), if pm8001_tag_alloc() fails to allocate a new tag, a warning message is issued but the uninitialized tag variable is still used to build a command. Avoid this by returning early in case of tag allocation failure. Also make sure to always return the error code returned by pm8001_tag_alloc() when this function fails instead of an arbitrary value. Link: https://lore.kernel.org/r/20220220031810.738362-18-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
In pm80xx_send_abort_all(), the n_elem field of the ccb used is not initialized to 0. This missing initialization sometimes lead to the task completion path seeing the ccb with a non-zero n_elem resulting in the execution of invalid dma_unmap_sg() calls in pm8001_ccb_task_free(), causing a crash such as: [ 197.676341] RIP: 0010:iommu_dma_unmap_sg+0x6d/0x280 [ 197.700204] RSP: 0018:ffff889bbcf89c88 EFLAGS: 00010012 [ 197.705485] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff83d0bda0 [ 197.712687] RDX: 0000000000000002 RSI: 0000000000000000 RDI: ffff88810dffc0d0 [ 197.719887] RBP: 0000000000000000 R08: 0000000000000000 R09: ffff8881c790098b [ 197.727089] R10: ffffed1038f20131 R11: 0000000000000001 R12: 0000000000000000 [ 197.734296] R13: ffff88810dffc0d0 R14: 0000000000000010 R15: 0000000000000000 [ 197.741493] FS: 0000000000000000(0000) GS:ffff889bbcf80000(0000) knlGS:0000000000000000 [ 197.749659] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 197.755459] CR2: 00007f16c1b42734 CR3: 0000000004814000 CR4: 0000000000350ee0 [ 197.762656] Call Trace: [ 197.765127] <IRQ> [ 197.767162] pm8001_ccb_task_free+0x5f1/0x820 [pm80xx] [ 197.772364] ? do_raw_spin_unlock+0x54/0x220 [ 197.776680] pm8001_mpi_task_abort_resp+0x2ce/0x4f0 [pm80xx] [ 197.782406] process_oq+0xe85/0x7890 [pm80xx] [ 197.786817] ? lock_acquire+0x194/0x490 [ 197.790697] ? handle_irq_event+0x10e/0x1b0 [ 197.794920] ? mpi_sata_completion+0x2d70/0x2d70 [pm80xx] [ 197.800378] ? __wake_up_bit+0x100/0x100 [ 197.804340] ? lock_is_held_type+0x98/0x110 [ 197.808565] pm80xx_chip_isr+0x94/0x130 [pm80xx] [ 197.813243] tasklet_action_common.constprop.0+0x24b/0x2f0 [ 197.818785] __do_softirq+0x1b5/0x82d [ 197.822485] ? do_raw_spin_unlock+0x54/0x220 [ 197.826799] __irq_exit_rcu+0x17e/0x1e0 [ 197.830678] irq_exit_rcu+0xa/0x20 [ 197.834114] common_interrupt+0x78/0x90 [ 197.840051] </IRQ> [ 197.844236] <TASK> [ 197.848397] asm_common_interrupt+0x1e/0x40 Avoid this issue by always initializing the ccb n_elem field to 0 in pm8001_send_abort_all(), pm8001_send_read_log() and pm80xx_send_abort_all(). Link: https://lore.kernel.org/r/20220220031810.738362-17-damien.lemoal@opensource.wdc.com Fixes: c6b9ef57 ("[SCSI] pm80xx: NCQ error handling changes") Reviewed-by:
-
Damien Le Moal authored
NCQ NON DATA is an NCQ command with the DMA_NONE DMA direction and so a register-device-to-host-FIS response is expected for it. However, for an IO_SUCCESS case, mpi_sata_completion() expects a set-device-bits-FIS for any ata task with an use_ncq field true, which includes NCQ NON DATA commands. Fix this to correctly treat NCQ NON DATA commands as non-data by also testing for the DMA_NONE DMA direction. Link: https://lore.kernel.org/r/20220220031810.738362-16-damien.lemoal@opensource.wdc.com Fixes: dbf9bfe6 ("[SCSI] pm8001: add SAS/SATA HBA driver") Reviewed-by:
-
Damien Le Moal authored
In the pm8001_chip_sata_req() and pm80xx_chip_sata_req() functions, all tasks with a DMA direction of DMA_NONE (no data transfer) are initialized using the ATAP value 0x04. However, NCQ NON DATA commands, while being DMA_NONE commands are NCQ commands and need to be initialized using the value 0x07 for ATAP, similarly to other NCQ commands. Make sure that NCQ NON DATA command tasks are initialized similarly to other NCQ commands by also testing the task "use_ncq" field in addition to the DMA direction. While at it, reorganize the code into a chain of if - else if - else to avoid useless affectations and debug messages. Link: https://lore.kernel.org/r/20220220031810.738362-15-damien.lemoal@opensource.wdc.com Fixes: dbf9bfe6 ("[SCSI] pm8001: add SAS/SATA HBA driver") Reviewed-by:
-
Damien Le Moal authored
In pm8001_pci_resume(), the use of the u32 type for the local variable device_state causes a sparse warning: warning: incorrect type in assignment (different base types) expected unsigned int [usertype] device_state got restricted pci_power_t [usertype] current_state Since this variable is used only once in the function, remove it and use pdev->current_state directly. While at it, also add a blank line after the last local variable declaration. Link: https://lore.kernel.org/r/20220220031810.738362-14-damien.lemoal@opensource.wdc.comReviewed-by: -
Damien Le Moal authored
In mpi_set_phy_profile_req() and pm8001_set_phy_profile_single(), use cpu_to_le32() to initialize the ppc_phyid field of struct set_phy_profile_req. Furthermore, fix the definition of the reserved field of this structure to be an array of __le32, to match the use of cpu_to_le32() when assigning values. These changes remove several sparse type warnings. Link: https://lore.kernel.org/r/20220220031810.738362-13-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
Make sure that the __le32 fields of struct sata_cmd are manipulated after applying the correct endian conversion. That is, use cpu_to_le32() for assigning values and le32_to_cpu() for consulting a field value. In particular, make sure that the calculations for the 4G boundary check are done using CPU endianness and *not* little endian values. With these fixes, many sparse warnings are removed. While at it, fix some code identation and add blank lines after variable declarations and in some other places to make this code more readable. Link: https://lore.kernel.org/r/20220220031810.738362-12-damien.lemoal@opensource.wdc.com Fixes: 0ecdf00b ("[SCSI] pm80xx: 4G boundary fix.") Reviewed-by:
-
Damien Le Moal authored
Make sure that the __le32 fields of struct ssp_ini_io_start_req are manipulated after applying the correct endian conversion. That is, use cpu_to_le32() for assigning values and le32_to_cpu() for consulting a field value. In particular, make sure that the calculations for the 4G boundary check are done using CPU endianness and *not* little endian values. With these fixes, many sparse warnings are removed. While at it, add blank lines after variable declarations and in some other places to make this code more readable. Link: https://lore.kernel.org/r/20220220031810.738362-11-damien.lemoal@opensource.wdc.com Fixes: 0ecdf00b ("[SCSI] pm80xx: 4G boundary fix.") Reviewed-by:
-
Damien Le Moal authored
All fields of the kek_mgmt_req structure have the type __le32. So make sure to use cpu_to_le32() to initialize them. This suppresses the sparse warning: warning: incorrect type in assignment (different base types) expected restricted __le32 [addressable] [assigned] [usertype] new_curidx_ksop got int Link: https://lore.kernel.org/r/20220220031810.738362-10-damien.lemoal@opensource.wdc.com Fixes: f5860992 ("[SCSI] pm80xx: Added SPCv/ve specific hardware functionalities and relevant changes in common files") Reviewed-by:
-
Damien Le Moal authored
All fields of the SASProtocolTimerConfig structure have the __le32 type. As such, use cpu_to_le32() to initialize them. This change suppresses many sparse warnings: warning: incorrect type in assignment (different base types) expected restricted __le32 [addressable] [usertype] pageCode got int Note that the check to limit the value of the STP_IDLE_TMO field is removed as this field is initialized using the fixed (and small) value defined by the STP_IDLE_TIME macro. The pm8001_dbg() calls printing the values of the SASProtocolTimerConfig structure fileds are changed to use le32_to_cpu() to present the values in human readable form. Link: https://lore.kernel.org/r/20220220031810.738362-9-damien.lemoal@opensource.wdc.com Fixes: a6cb3d01 ("[SCSI] pm80xx: thermal, sas controller config and error handling update") Reviewed-by:
-
Damien Le Moal authored
The fields of the set_ctrl_cfg_req structure have the __le32 type, so use cpu_to_le32() to assign them. This removes the sparse warnings: warning: incorrect type in assignment (different base types) expected restricted __le32 got unsigned int Link: https://lore.kernel.org/r/20220220031810.738362-8-damien.lemoal@opensource.wdc.com Fixes: 842784e0 ("pm80xx: Update For Thermal Page Code") Fixes: f5860992 ("[SCSI] pm80xx: Added SPCv/ve specific hardware functionalities and relevant changes in common files") Reviewed-by: -
Damien Le Moal authored
The ds_ads_m field of struct ssp_ini_tm_start_req has the type __le32. Assigning a value to it should thus use cpu_to_le32(). This fixes the sparse warning: warning: incorrect type in assignment (different base types) expected restricted __le32 [addressable] [assigned] [usertype] ds_ads_m got int Link: https://lore.kernel.org/r/20220220031810.738362-7-damien.lemoal@opensource.wdc.com Fixes: dbf9bfe6 ("[SCSI] pm8001: add SAS/SATA HBA driver") Reviewed-by:
-
Damien Le Moal authored
The declaration of the local variable destination1 in pm80xx_pci_mem_copy() as a pointer to a u32 results in the sparse warning: warning: incorrect type in assignment (different base types) expected unsigned int [usertype] got restricted __le32 [usertype] Furthermore, the destination" argument of pm80xx_pci_mem_copy() is wrongly declared with the const attribute. Fix both problems by changing the type of the "destination" argument to "__le32 *" and use this argument directly inside the pm80xx_pci_mem_copy() function, thus removing the need for the destination1 local variable. Link: https://lore.kernel.org/r/20220220031810.738362-6-damien.lemoal@opensource.wdc.comReviewed-by: -
Damien Le Moal authored
Since the sata_cmd struct is zeroed out before its fields are initialized, there is no need for using "|=" to initialize the ncqtag_atap_dir_m field. Using a standard assignment removes the sparse warning: warning: invalid assignment: |= Also, since the ncqtag_atap_dir_m field has type __le32, use cpu_to_le32() to generate the assigned value. Link: https://lore.kernel.org/r/20220220031810.738362-5-damien.lemoal@opensource.wdc.com Fixes: c6b9ef57 ("[SCSI] pm80xx: NCQ error handling changes") Reviewed-by:
-
Damien Le Moal authored
Change the type of partitionSizeTmp from u32 to __be32 to suppress the sparse warning: warning: cast to restricted __be32 Link: https://lore.kernel.org/r/20220220031810.738362-4-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
Avoid the sparse warning "warning: cast removes address space '__iomem' of expression" by declaring the qp pointer as "u32 __iomem *". Accordingly, change the accesses to the qp array to use readl(). Link: https://lore.kernel.org/r/20220220031810.738362-3-damien.lemoal@opensource.wdc.comReviewed-by:
-
Damien Le Moal authored
To detect for the DMA_NONE (no data transfer) DMA direction, sas_ata_qc_issue() tests if the command protocol is ATA_PROT_NODATA. This test does not include the ATA_CMD_NCQ_NON_DATA command as this command protocol is defined as ATA_PROT_NCQ_NODATA (equal to ATA_PROT_FLAG_NCQ) and not as ATA_PROT_NODATA. To include both NCQ and non-NCQ commands when testing for the DMA_NONE DMA direction, use "!ata_is_data()". Link: https://lore.kernel.org/r/20220220031810.738362-2-damien.lemoal@opensource.wdc.com Fixes: 176ddd89 ("scsi: libsas: Reset num_scatter if libata marks qc as NODATA") Cc: stable@vger.kernel.org Reviewed-by:
-
Yang Li authored
The print function dev_err() is redundant because platform_get_irq() already prints an error. Eliminate the follow coccicheck warnings: ./drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:1661:3-10: line 1661 is redundant because platform_get_irq() already prints an error ./drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:1642:4-11: line 1642 is redundant because platform_get_irq() already prints an error ./drivers/scsi/hisi_sas/hisi_sas_v1_hw.c:1679:3-10: line 1679 is redundant because platform_get_irq() already prints an error Link: https://lore.kernel.org/r/20220215020524.44268-1-yang.lee@linux.alibaba.comReported-by:
Abaci Robot <abaci@linux.alibaba.com> Acked-by:
Yang Li <yang.lee@linux.alibaba.com> Signed-off-by:
-
Christoph Hellwig authored
No more users of REQ_OP_WRITE_SAME or drivers implementing it are left, so remove the infrastructure. [mkp: fold in and tweak sysfs reporting fix] Link: https://lore.kernel.org/r/20220209082828.2629273-8-hch@lst.deReviewed-by:
Chaitanya Kulkarni <kch@nvidia.com> Signed-off-by:
-
Christoph Hellwig authored
There are no more end-users of REQ_OP_WRITE_SAME left, so we can start deleting it. Link: https://lore.kernel.org/r/20220209082828.2629273-7-hch@lst.deReviewed-by:
Mike Snitzer <snitzer@redhat.com> Signed-off-by:
-
Christoph Hellwig authored
There are no more end-users of REQ_OP_WRITE_SAME left, so we can start deleting it. Link: https://lore.kernel.org/r/20220209082828.2629273-6-hch@lst.deSigned-off-by:
-
Christoph Hellwig authored
There are no more end-users of REQ_OP_WRITE_SAME left, so we can start deleting it. Link: https://lore.kernel.org/r/20220209082828.2629273-5-hch@lst.deReviewed-by:
-
