| Age | Commit message (Collapse) | Author | Files | Lines |
|---|
|
There are few place in the code which check whether pending_sg_req list is
empty despite of the check already being done. Let's remove the duplicated
checks to keep code clean.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The runtime PM is always available on all Tegra SoCs since the commit
40b2bb1b132a ("ARM: tegra: enforce PM requirement"), so there is no
need to handle the case of unavailable RPM in the code anymore.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
There is no need to re-initialize the already initialized variables.
The tdc->config_init=false after driver's probe and after channel's
freeing, so there is no need to re-initialize it on the channel's
allocation.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch fixes few dozens of coding style problems reported by
checkpatch and prettifies code where makes sense.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Use resource-managed variant of request_irq for brevity.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Use devm_platform_ioremap_resource to keep code cleaner a tad.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
There is no need to kill tasklet when driver's probe fails because tasklet
can't be scheduled at this time. It is also cleaner to kill tasklet on
channel's freeing rather than to kill it on driver's removal, otherwise
tasklet could perform a dummy execution after channel's releasing, which
isn't very nice.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
It's incorrect to check the channel's "busy" state without taking a lock.
That shouldn't cause any real troubles, nevertheless it's always better
not to have any race conditions in the code.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The ISR tasklet could be kept scheduled after DMA transfer termination,
let's add synchronization hook which blocks until tasklet is finished.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The interrupt handler puts a half-completed DMA descriptor on a free list
and then schedules tasklet to process bottom half of the descriptor that
executes client's callback, this creates possibility to pick up the busy
descriptor from the free list. Thus, let's disallow descriptor's re-use
until it is fully processed.
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
I was doing some experiments with I2C and noticed that Tegra APB DMA
driver crashes sometime after I2C DMA transfer termination. The crash
happens because tegra_dma_terminate_all() bails out immediately if pending
list is empty, and thus, it doesn't release the half-completed descriptors
which are getting re-used before ISR tasklet kicks-in.
tegra-i2c 7000c400.i2c: DMA transfer timeout
elants_i2c 0-0010: elants_i2c_irq: failed to read data: -110
------------[ cut here ]------------
WARNING: CPU: 0 PID: 142 at lib/list_debug.c:45 __list_del_entry_valid+0x45/0xac
list_del corruption, ddbaac44->next is LIST_POISON1 (00000100)
Modules linked in:
CPU: 0 PID: 142 Comm: kworker/0:2 Not tainted 5.5.0-rc2-next-20191220-00175-gc3605715758d-dirty #538
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: events_freezable_power_ thermal_zone_device_check
[<c010e5c5>] (unwind_backtrace) from [<c010a1c5>] (show_stack+0x11/0x14)
[<c010a1c5>] (show_stack) from [<c0973925>] (dump_stack+0x85/0x94)
[<c0973925>] (dump_stack) from [<c011f529>] (__warn+0xc1/0xc4)
[<c011f529>] (__warn) from [<c011f7e9>] (warn_slowpath_fmt+0x61/0x78)
[<c011f7e9>] (warn_slowpath_fmt) from [<c042497d>] (__list_del_entry_valid+0x45/0xac)
[<c042497d>] (__list_del_entry_valid) from [<c047a87f>] (tegra_dma_tasklet+0x5b/0x154)
[<c047a87f>] (tegra_dma_tasklet) from [<c0124799>] (tasklet_action_common.constprop.0+0x41/0x7c)
[<c0124799>] (tasklet_action_common.constprop.0) from [<c01022ab>] (__do_softirq+0xd3/0x2a8)
[<c01022ab>] (__do_softirq) from [<c0124683>] (irq_exit+0x7b/0x98)
[<c0124683>] (irq_exit) from [<c0168c19>] (__handle_domain_irq+0x45/0x80)
[<c0168c19>] (__handle_domain_irq) from [<c043e429>] (gic_handle_irq+0x45/0x7c)
[<c043e429>] (gic_handle_irq) from [<c0101aa5>] (__irq_svc+0x65/0x94)
Exception stack(0xde2ebb90 to 0xde2ebbd8)
Signed-off-by: Dmitry Osipenko <[email protected]>
Acked-by: Jon Hunter <[email protected]>
Cc: <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Add basic interleaved support via EDMA.
Signed-off-by: Peter Ujfalusi <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Tasklets run with all the interrupts enabled. This means that we should
replace all the (already present) spin_lock_irqsave() uses in the tasklet
with spin_lock_irq() to protect being interrupted by a IRQ which tries
to get the same lock (via calls to device_prep_dma_* for example).
spin_lock and spin_lock_bh in tasklets are not enough to protect from IRQs,
update these to spin_lock_irq().
at_xdmac_advance_work() can be called with all the interrupts enabled (when
called from tasklet), or with interrupts disabled (when called from
at_xdmac_issue_pending). Move the locking in the callers to be able to use
spin_lock_irq() and spin_lock_irqsave() for these cases.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
device_alloc_chan_resources can sleep, use GFP_KERNEL flag.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
There is no need for locking in device_alloc_chan_resources(),
the DMA core takes care of it by using a dma_list_mutex around
the DMA devices.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The code in cause is already in the else case of
'if (at_xdmac_chan_is_cyclic(atchan))', drop the redundant check.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Fix the following deadlocks:
1/ atc_handle_cyclic() and atc_chain_complete() called
dmaengine_desc_get_callback_invoke() while wrongly holding the
atchan->lock. Clients can set the callback to dmaengine_terminate_sync()
which will end up trying to get the same lock, thus a deadlock occurred.
2/ dma_run_dependencies() was called with the atchan->lock held, but the
method calls device_issue_pending() which tries to get the same lock,
and so a deadlock occurred.
The driver must not hold the lock when invoking the callback or when
running dependencies. Releasing the spinlock within a called function
before calling the callback is not a nice thing to do -> called functions
become non-atomic when called within an atomic region. Thus the lock is
now taken in the child routines whereever is needed.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Avoids sleeping without depleting the emergency pool.
The rationale being that in most cases a dma device is either
offloading an operation that will automatically fallback to
software when the descriptor allocation fails, or we can simply poll
and wait for the dma device to release some in use descriptors.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Probably a leftover, drop it.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Having a list of descriptors allocated for the channel at
device_alloc_chan_resources() time is a sign for bad free usage.
Return err and add a debug message in case the channel is not
free from a previous use.
atchan->descs_allocated becomes useless, get rid of it. More,
drop the error message in atc_desc_get() because now it would
introduce an extra if statement. The callers of atc_desc_get()
already print error messages in case the callee fails, no one
is hurt.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
There is no need for locking in device_alloc_chan_resources(),
the DMA core takes care of it by using a dma_list_mutex around
the DMA devices.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
All members of the structure are initialized below in the function,
there is no need to use kzalloc.
Signed-off-by: Tudor Ambarus <[email protected]>
Acked-by: Ludovic Desroches <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
In overlay application we noticed that dma channel node probe order is
inverted i.e s2mm channel is probed first followed by mm2s channel. The
reason for this inversion is fdtoverlay utility which uses a function
called fdt_add_subnode(*). It stores the subnodes after the properties,
this has the effect of inserting the new subnode before any others and
the end result is a reversal.
Because of this inverted channel probe order, the node probed first is
assigned a '0' index instead of Channel ID should be '0' for tx and '1'
for rx and dmatest client using the DT convention fails in dma transfer
as channel are swapped.
To fix above behavior and make channel assignment index independent
of probe order, always assign mm2s channel at '0' index and the s2mm
channel at IP specific fixed offset derived from the max_channels
count.
Signed-off-by: Radhey Shyam Pandey <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Extend dma_config structure to store the max channel count. This input is
used to populate dma device channel nodes at the fixed offset. It serves
as a preparatory patch for removing dma channel DT node order dependency,
added in the subsequent commit.
Signed-off-by: Radhey Shyam Pandey <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
To avoid race with vchan_complete, use the race free way to terminate
running transfer.
Move vdesc->node list_del in stm32_dma_start_transfer instead of in
stm32_mdma_chan_complete to avoid another race in vchan_dma_desc_free_list.
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch fixes BUG: sleeping function called from invalid context in
stm32_dma_disable_chan function.
The goal of this function is to force channel disable if it has not been
disabled by hardware. This consists in clearing STM32_DMA_SCR_EN bit and
read it as 0 to ensure the channel is well disabled and the last transfer
is over.
In previous implementation, the waiting loop was based on a do...while (1)
with a call to cond_resched to give the scheduler a chance to run a higher
priority process.
But in some conditions, stm32_dma_disable_chan can be called while
preemption is disabled, on a stm32_dma_stop call for example. So
cond_resched must not be used.
To avoid this, use readl_relaxed_poll_timeout_atomic to poll
STM32_DMA_SCR_EN bit cleared.
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch adds copy_align property in accordance with hardware
restriction.
Signed-off-by: Ludovic Barre <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch adds dma_set_max_seg_size to define sg dma constraint.
This constraint may be taken into account by client to scatter/gather
its buffer.
Signed-off-by: Ludovic Barre <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Enable client to resubmit already processed descriptors
in order to save descriptor creation time.
Signed-off-by: Pierre-Yves MORDRET <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Change STM32 DMA driver to defer its probe operation when reset
controller is expected but has not been probed yet when DMA
device is probed.
Changes error traces when failing to get a system resource so that
it is not printed on failure with deferred probing.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Remove reset controller reference from device instance since it is
used only at probe time.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Add suspend/resume power management relying on PM Runtime engine.
Signed-off-by: Pierre-Yves MORDRET <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
There is a DMA problem with the serial ports on i.MX6.
When the following sequence is performed:
1) Open a port
2) Write some data
3) Close the port
4) Open a *different* port
5) Write some data
6) Close the port
The second write sends nothing and the second close hangs.
If the first close() is omitted it works.
Adding logs to the the UART driver shows that the DMA is being setup but
the callback is never invoked for the second write.
This used to work in 4.19.
Git bisect leads to:
ad0d92d: "dmaengine: imx-sdma: refine to load context only once"
This commit adds a "context_loaded" flag used to avoid unnecessary context
setups.
However the flag is only reset in sdma_channel_terminate_work(),
which is only invoked in a worker triggered by sdma_terminate_all() IF
there is an active descriptor.
So, if no active descriptor remains when the channel is terminated, the
flag is not reset and, when the channel is later reused the old context
is used.
Fix the problem by always resetting the flag in sdma_free_chan_resources().
Cc: [email protected]
Signed-off-by: Martin Fuzzey <[email protected]>
Fixes: ad0d92d7ba6a ("dmaengine: imx-sdma: refine to load context only once")
Reviewed-by: Fabio Estevam <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Reset DMA channel after stop to ensure that pending transfers and FIFOs
in the datapath are flushed or completed. It also cleanup the terminate
path and removes stop for the cyclic mode as after the reset stop is not
required. This fixes intermittent data verification failure when xilinx
dma test the client is stressed and loaded/unloaded multiple times.
Signed-off-by: Radhey Shyam Pandey <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Changes STM32 DMAMUX driver to defer its probe operation when
reset controller is expected but has not been probed yet.
Changes error traces when failing to get a system resource so that
it is not printed on failure with deferred probing.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Remove reset controller reference from device instance since it is
used only at probe time.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This change ensures the DMAMUX device is reset only once it is clocked
and that clock is released in a safe state when probe operation fails.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Add suspend/resume power management relying on PM Runtime engine.
Signed-off-by: Pierre-Yves MORDRET <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
To avoid race with vchan_complete, use the race free way to terminate
running transfer.
Move vdesc->node list_del in stm32_mdma_start_transfer instead of in
stm32_mdma_xfer_end to avoid another race in vchan_dma_desc_free_list.
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Enable descriptor reuse to allow client to resubmit already processed
descriptors in order to save descriptor creation time.
Signed-off-by: Pierre-Yves MORDRET <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch changes error log when failing to get the clock so that it is
not printed on failure with probe deferring.
It also defers probe when reset controller is expected but has not been
probed yet when MDMA device is probed.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
This patch disables the clock in case of error during probe. The unneeded
err_unregister label is renamed err_clk instead.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Remove reset controller reference from device instance since it is
used only at probe time.
Signed-off-by: Etienne Carriere <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
Add suspend/resume power management relying on PM Runtime engine.
Signed-off-by: Pierre-Yves MORDRET <[email protected]>
Signed-off-by: Amelie Delaunay <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The current size_store() function for idxd sysfs does not check the total
wq size. This allows configuration of all wqs with total wq size. Add check
to make sure the wq sysfs attribute rejects storing of size over the total
wq size.
Fixes: c52ca478233c ("dmaengine: idxd: add configuration component of driver")
Reported-by: Jerry Chen <[email protected]>
Signed-off-by: Dave Jiang <[email protected]>
Link: https://lore.kernel.org/r/158213309629.2509.3583411832507185041.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Vinod Koul <[email protected]>
|
|
Currently when inputing an unrecognized wq type, we set the wq type to
"none". It really should return error and not change the existing wq type
that's in the kernel.
Fixes: c52ca478233c ("dmaengine: idxd: add configuration component of driver")
Reported-by: Yixin Zhang <[email protected]>
Signed-off-by: Dave Jiang <[email protected]>
Link: https://lore.kernel.org/r/158213304803.2290.13336343633425868211.stgit@djiang5-desk3.ch.intel.com
Signed-off-by: Vinod Koul <[email protected]>
|
|
The caller is already holding the lock so this will deadlock.
Fixes: 0b58828c923e ("DMAENGINE: COH 901 318 remove irq counting")
Signed-off-by: Dan Carpenter <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Vinod Koul <[email protected]>
|
|
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Link: https://lore.kernel.org/r/20200214171302.GA20586@embeddedor
Signed-off-by: Vinod Koul <[email protected]>
|
|
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Link: https://lore.kernel.org/r/20200214171657.GA25663@embeddedor
Signed-off-by: Vinod Koul <[email protected]>
|
|
The current codebase makes use of the zero-length array language
extension to the C90 standard, but the preferred mechanism to declare
variable-length types such as these ones is a flexible array member[1][2],
introduced in C99:
struct foo {
int stuff;
struct boo array[];
};
By making use of the mechanism above, we will get a compiler warning
in case the flexible array does not occur last in the structure, which
will help us prevent some kind of undefined behavior bugs from being
inadvertently introduced[3] to the codebase from now on.
Also, notice that, dynamic memory allocations won't be affected by
this change:
"Flexible array members have incomplete type, and so the sizeof operator
may not be applied. As a quirk of the original implementation of
zero-length arrays, sizeof evaluates to zero."[1]
This issue was found with the help of Coccinelle.
[1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
[2] https://github.com/KSPP/linux/issues/21
[3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
Signed-off-by: Gustavo A. R. Silva <[email protected]>
Reviewed-by: Baolin Wang <[email protected]>
Link: https://lore.kernel.org/r/20200214171536.GA24077@embeddedor
Signed-off-by: Vinod Koul <[email protected]>
|