NO_IRQ is a relic from the old days. It is not used anymore in core
functions. By the way, function irq_of_parse_and_map() returns value 0
on error.
In some drivers, NO_IRQ is erroneously used to check the return of
irq_of_parse_and_map().
It is not a real bug today because the only architectures using the
drivers being fixed by this patch define NO_IRQ as 0, but there are
architectures which define NO_IRQ as -1. If one day those
architectures start using the non fixed drivers, there will be a
problem.
Long time ago Linus advocated for not using NO_IRQ, see
https://lkml.org/lkml/2005/11/21/221 . He re-iterated the same view
recently in https://lkml.org/lkml/2022/10/12/622
So test !irq instead of tesing irq == NO_IRQ.
And remove the fallback definition of NO_IRQ at the top of the file.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Variable num_processed is just being incremented and it's never used
anywhere else. The variable and the increment are redundant so
remove it.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
the driver uses libata's "tag" values from in various arrays.
Since the mentioned patch bumped the ATA_TAG_INTERNAL to 32,
the value of the SATA_DWC_QCMD_MAX needs to account for that.
Otherwise ATA_TAG_INTERNAL usage cause similar crashes like
this as reported by Tice Rex on the OpenWrt Forum and
reproduced (with symbols) here:
| BUG: Kernel NULL pointer dereference at 0x00000000
| Faulting instruction address: 0xc03ed4b8
| Oops: Kernel access of bad area, sig: 11 [#1]
| BE PAGE_SIZE=4K PowerPC 44x Platform
| CPU: 0 PID: 362 Comm: scsi_eh_1 Not tainted 5.4.163 #0
| NIP: c03ed4b8 LR: c03d27e8 CTR: c03ed36c
| REGS: cfa59950 TRAP: 0300 Not tainted (5.4.163)
| MSR: 00021000 <CE,ME> CR: 42000222 XER: 00000000
| DEAR: 00000000 ESR: 00000000
| GPR00: c03d27e8 cfa59a08 cfa55fe0 00000000 0fa46bc0 [...]
| [..]
| NIP [c03ed4b8] sata_dwc_qc_issue+0x14c/0x254
| LR [c03d27e8] ata_qc_issue+0x1c8/0x2dc
| Call Trace:
| [cfa59a08] [c003f4e0] __cancel_work_timer+0x124/0x194 (unreliable)
| [cfa59a78] [c03d27e8] ata_qc_issue+0x1c8/0x2dc
| [cfa59a98] [c03d2b3c] ata_exec_internal_sg+0x240/0x524
| [cfa59b08] [c03d2e98] ata_exec_internal+0x78/0xe0
| [cfa59b58] [c03d30fc] ata_read_log_page.part.38+0x1dc/0x204
| [cfa59bc8] [c03d324c] ata_identify_page_supported+0x68/0x130
| [...]
This is because sata_dwc_dma_xfer_complete() NULLs the
dma_pending's next neighbour "chan" (a *dma_chan struct) in
this '32' case right here (line ~735):
> hsdevp->dma_pending[tag] = SATA_DWC_DMA_PENDING_NONE;
Then the next time, a dma gets issued; dma_dwc_xfer_setup() passes
the NULL'd hsdevp->chan to the dmaengine_slave_config() which then
causes the crash.
With this patch, SATA_DWC_QCMD_MAX is now set to ATA_MAX_QUEUE + 1.
This avoids the OOB. But please note, there was a worthwhile discussion
on what ATA_TAG_INTERNAL and ATA_MAX_QUEUE is. And why there should not
be a "fake" 33 command-long queue size.
Ideally, the dw driver should account for the ATA_TAG_INTERNAL.
In Damien Le Moal's words: "... having looked at the driver, it
is a bigger change than just faking a 33rd "tag" that is in fact
not a command tag at all."
Fixes: 28361c4036 ("libata: add extra internal command")
Cc: stable@kernel.org # 4.18+
BugLink: https://github.com/openwrt/openwrt/issues/9505
Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Driver has been converted to dynamic debugging, so the compile-time
options don't have any functionality left.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Remove the 'check_status' argument from sata_dwc_qc_complete() and
sata_dwc_dma_xfer_complete() as it has no functionality.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Obsolete, and has been converted to tracepoints.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Add tracepoints for bus-master DMA and taskfile related functions.
That allows us to drop the relevant DPRINTK() calls.
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
sata_dwc_port_stop() is not used before being defined,
remove redundant forward declaration.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Use temporary variable for struct device to make code neater.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Use devm_platform_get_and_ioremap_resource() and
devm_platform_ioremap_resource() APIs instead of their
open coded analogues.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Last change to device managed APIs cleaned up error path to simple phy_exit()
call, which in some cases has been executed with NULL parameter. This per se
is not a problem, but rather logical misconception: no need to free resource
when it's for sure has not been allocated yet. Fix the driver accordingly.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Link: https://lore.kernel.org/r/20210727125130.19977-1-andriy.shevchenko@linux.intel.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
The return value of ata_qc_from_tag() is checked in the whole
kernel except for two calls in sata_dwc_isr(), which may lead
to null-pointer-dereference. Add return value checks to avoid
such case.
Signed-off-by: Dinghao Liu <dinghao.liu@zju.edu.cn>
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Based on 1 normalized pattern(s):
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
extracted by the scancode license scanner the SPDX license identifier
GPL-2.0-or-later
has been chosen to replace the boilerplate/reference in 3029 file(s).
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Allison Randal <allison@lohutok.net>
Cc: linux-spdx@vger.kernel.org
Link: https://lkml.kernel.org/r/20190527070032.746973796@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Anything that goes to the hardware should use ->hw_tag, anything
related to internal lookup should be using ->tag.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
Signed-off-by: Tejun Heo <tj@kernel.org>
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Propagate platform device ID to DMA driver to distinguish relationship
between DMA and SATA instances.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
ata_host_alloc_pinfo() assigns the host pointer to the
struct device * drvdata, do not assign it a second time.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Tejun Heo <tj@kernel.org>
There is a error message within devm_ioremap_resource
already, so remove the dev_err call to avoid redundant
error message.
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Signed-off-by: Tejun Heo <tj@kernel.org>
This avoid the need to always translate between the two in ata_prot_flags
and generally cleans up the taskfile protocol usage.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tejun Heo <tj@kernel.org>
Add a new taskfile protocol ATA_PROT_NCQ_NODATA to handle
ATA NCQ NO-DATA commands correctly.
And fixup ata_scsi_zbc_out_xlat() to use it.
Signed-off-by: Hannes Reinecke <hare@suse.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Tejun Heo <tj@kernel.org>
There is a duplication in the debug messages when accessing SCR registers.
Remove duplication to make the messages neat.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
DMA operates with physical addresses which is not exactly the same as ioremap()
returns.
Introduce variable to keep physical address of the SATA FIFO register and
supply it when prepare DMA channel.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This simplifies error handling and cleanup by using devm to manage
IO mappings.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This consolidates the reads from each of the if/else branches
to one place making the code a lot nicer to look at.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Rename the register access macros and use standard _relaxed()
ops instead of __raw variants with explicit byte swapping.
The original driver used the ppc-specific in/out_le32(). When it
was adapted to other systems, these were added to the driver
under ifdefs. However, those names are not defined as macros on
ppc, so it ended up replacing them there as well with altered
semantics. This patch restores the original semantics on ppc and
makes the accesses no less strict on other systems.
Also fixes too many sparse warnings to count.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Convert dmaengine_terminate_all() calls to synchronous and asynchronous
versions where appropriate.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The pointer to the mmio register base is missing the __iomem
annotation. Fix this.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The (void *__iomem) cast is wrong. Change the target type of the
"base" pointer to void __iomem instead and drop the cast.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Casting a pointer to unsigned long only to immediately cast it back
to a pointer makes no sense. Fix this.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The sata_dwc_qc_prep() does nothing. Use the default ata_noop_qc_prep
instead.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Here we refactor HOSTDEV{P}_FROM_*() macros to fit one line and fix the
definition of HSDEV_FROM_HSDEVP() where wrong name of the parameter waas used.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This moves all global data into the driver private struct, thus
permitting multiple devices of this type to be used.
The core_scr_read/write() functions are replaced with equivalent
calls to the existing sata_dwc_scr_read/write().
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This adds support for powering on an optional PHY when activating the
device.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Currently this driver only works with a DesignWare DMA engine which it
registers manually using the second "reg" address range and interrupt
number from the DT node.
This patch makes the driver instead use the "dmas" property if present,
otherwise optionally falling back on the old way so existing device
trees can continue to work.
With this change, there is no longer any reason to depend on the 460EX
machine type so drop that from Kconfig.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
ata_sff_qc_issue() can't handle DMA commands and thus we have to avoid it for
them. Do call ata_bmdma_qc_issue() instead for this case. Note that the former
one distinguishes PIO and DMA mode and behaves accordingly.
Suggested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Calling dmaengine_prep_slave_sg() for non-dma ATA commands is
unnecessary at best and could be harmful if the dma driver reacts
badly to this. It also causes this driver to print a bogus error
message in these cases.
This patch changes sata_dwc_qc_issue() to only do the dma setup
for dma commands and also reports an error to libata if if fails.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Mans Rullgard <mans@mansr.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
In the original code the DMA is always a flow controller. Set this accordingly
in updated code.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The burst size as defined by DMAengine API is in items of address width. Derive
burst size from AHB_DMA_BRST_DFLT (64 bytes) by dividing it to
DMA_SLAVE_BUSWIDTH_4_BYTES (4 bytes) that gives us 16 items.
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The original code states:
Make sure a LLI block is not created that will span 8K max FIS
boundary. If the block spans such a FIS boundary, there is a chance
that a DMA burst will cross that boundary -- this results in an error
in the host controller.
Since we have switched to generic DMAengine API we satisfy above by setting
dma_boundary value to 0x1fff.
Suggested-by: Mans Rullgard <mans@mansr.com>
Tested-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This patch fixes Machine Check "Data Write PLB Error" which happens
when libata-sff's ata_sff_dev_select is trying to write into the
device_addr in order to select a drive. However, SATA has no master
or slave devices like the old ATA Bus, therefore selecting a
different drive is kind of pointless.
Data Write PLB Error
Oops: Machine check, sig: 7 [#1]
PowerPC 44x Platform
Modules linked in:
CPU: 0 PID: 508 Comm: scsi_eh_0 Not tainted 4.6.0-rc3-next-20160412+ #10
[...]
NIP [c027e820] ata_sff_dev_select+0x3c/0x44
LR [c027e810] ata_sff_dev_select+0x2c/0x44
Call Trace:
[cec31cd0] [c027da00] ata_sff_postreset+0x40/0xb4 (unreliable)
[cec31ce0] [c027a03c] ata_eh_reset+0x5cc/0x928
[cec31d60] [c027a840] ata_eh_recover+0x330/0x10bc
[cec31df0] [c027bae0] ata_do_eh+0x4c/0xa4
[...]
Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
We pass struct dw_dma_chip to dw_dma_probe() anyway, thus we may use it to
pass a platform data as well.
While here, constify the source of the platform data.
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
The source and destination masters are reflecting buses or their layers to
where the different devices can be connected. The patch changes the master
names to reflect which one is related to which independently on the transfer
direction.
The outcome of the change is that the memory data width is now always limited
by a data width of the master which is dedicated to communicate to memory.
The patch will not break anything since all current users have the same data
width for all masters. Though it would be nice to revisit avr32 platforms to
check what is the actual hardware topology in use there. It seems that it has
one bus and two masters on it as stated by Table 8-2, that's why everything
works independently on the master in use. The purpose of the sequential patch
is to fix the driver for configuration of more than one bus.
The change is done in the assumption that src_master and dst_master are
reflecting a connection to the memory and peripheral correspondently on avr32
and otherwise on the rest.
Acked-by: Hans-Christian Egtvedt <egtvedt@samfundet.no>
Acked-by: Mark Brown <broonie@kernel.org>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
We shuffled some code around in 8b3444852a ('sata_dwc_460ex: move to
generic DMA driver') an accidentally deleted a tab character here. It
causes a Smatch warning "if statement not indented".
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This patch re-uses hsdev->dev which is allocated on heap. Therefore, the
private structure, which is global variable, is reduced by one field.
In one case ap->dev is used and there it seems to be right decision.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The SATA implementation based on two actually different devices, i.e. SATA and
DMA controllers.
For Synopsys DesignWare DMA we have already a generic implementation of the
driver. Thus, the patch converts the code to use DMAEngine framework and
dw_dmac driver.
In future it will be better to split the devices inside DTS as well like it's
done on other platforms.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
It it better to have full message on one line. It simplifies to search for line
in the code by message when debugging.
Note that the lines which will be removed by sequential patch are not fixed
here.
There is no functional change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
The patch converts ->probe() to use devm_kzalloc that simplifies error path.
Note that ata_host_alloc_pinfo() has been using device resources already.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
There is no need to print a message about failure of memory allocation. The
caller will get an error code and may print the same.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
This patch fixes the style of usage of a node pointer. There is no functional
change.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
