* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Sergey Senozhatsky @ 2018-06-19 0:53 UTC (permalink / raw)
To: Alan Cox
Cc: Sergey Senozhatsky, Petr Mladek, Steven Rostedt,
Greg Kroah-Hartman, Jiri Slaby, Linus Torvalds, Peter Zijlstra,
Andrew Morton, Dmitry Vyukov, linux-kernel, linux-serial,
Sergey Senozhatsky
In-Reply-To: <20180618143818.50b2f2f9@alans-desktop>
Thanks for taking a look!
On (06/18/18 14:38), Alan Cox wrote:
> > It doesn't come as a surprise that recursive printk() calls are not the
> > only way for us to deadlock in printk() and we still have a whole bunch
> > of other printk() deadlock scenarios. For instance, those that involve
> > TTY port->lock spin_lock and UART port->lock spin_lock.
>
> The tty layer code there is not re-entrant. Nor is it supposed to be
Could be.
But at least we have circular locking dependency in tty,
see [1] for more details:
tty_port->lock => uart_port->lock
CPU0
tty
spin_lock(&tty_port->lock)
printk()
call_console_drivers()
foo_console_write()
spin_lock(&uart_port->lock)
Whereas we normally have
uart_port->lock => tty_port->lock
CPU1
IRQ
foo_console_handle_IRQ()
spin_lock(&uart_port->lock)
tty
spin_lock(&tty_port->lock)
If we switch to printk_safe when we take tty_port->lock then we
remove the printk->uart_port chain from the picture.
> > So the idea of this patch set is to take tty_port->lock and
> > uart_port->lock from printk_safe context and to eliminate some
> > of non-recursive printk() deadlocks - the ones that don't start
> > in printk(), but involve console related locks and thus eventually
> > deadlock us in printk(). For this purpose the patch set introduces
> > several helper macros:
>
> I don't see how this helps - if you recurse into the uart code you are
> still hitting the paths that are unsafe when re-entered. All you've done
> is messed up a pile of locking code on critical performance paths.
>
> As it stands I think it's a bad idea.
The only new thing is that we inc/dec per-CPU printk context
variable when we lock/unlock tty/uart port lock:
printk_safe_enter() -> this_cpu_inc(printk_context);
printk_safe_exit() -> this_cpu_dec(printk_context);
How does this help? Suppose we have the following
IRQ
foo_console_handle_IRQ()
spin_lock(&uart_port->lock)
uart_write_wakeup()
tty_port_tty_wakeup()
tty_port_default_wakeup()
printk()
call_console_drivers()
foo_console_write()
spin_lock(&uart_port->lock) << deadlock
If we take uart_port lock from printk_safe context, we remove the
printk->call_console_drivers->foo_console_write->spin_lock
chain. Because printk() output will endup in a per-CPU buffer,
which will be flushed later from irq_work. So the whole thing
becomes:
IRQ
foo_console_handle_IRQ()
printk_safe_enter()
spin_lock(&uart_port->lock)
uart_write_wakeup()
tty_port_tty_wakeup()
tty_port_default_wakeup()
printk() << we don't re-enter foo_console_driver
<< from printk() anymore
printk_safe_log_store()
irq_work_queue
spin_unlock(&uart_port->lock)
printk_safe_exit()
iret
#flush per-CPU buffer
IRQ
printk_safe_flush_buffer()
vprintk_deferred()
> > Of course, TTY and UART port spin_locks are not the only locks that
> > we can deadlock on. So this patch set does not address all deadlock
> > scenarios, it just makes a small step forward.
> >
> > Any opinions?
>
> The cure is worse than the disease.
Because of this_cpu_inc(printk_context) / this_cpu_dec(printk_context)?
May be. That's why I put RFC :)
> The only case that's worth looking at is the direct polled console code
> paths. The moment you touch the other layers you add essentially never
> needed code to hot paths.
>
> Given printk nowdays is already somewhat unreliable with all the perf
> related changes, and we have other good debug tools I think it would be
> far cleaner to have some kind of
>
>
> if (spin_trylock(...)) {
> console_defer(buffer);
> return;
> }
>
> helper layer in the printk/console logic, at least for the non panic/oops
> cases.
spin_trylock() in every ->foo_console_write() callback?
This still will not address the reported deadlock [1].
[1] lkml.kernel.org/r/000000000000d557e7056e1c7a01@google.com
-ss
^ permalink raw reply
* [PATCH] serial: mps2-uart: Initialize early console
From: Guenter Roeck @ 2018-06-19 4:54 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Jiri Slaby, Vladimir Murzin, linux-serial, linux-arm-kernel,
linux-kernel, Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi,
Guenter Roeck
The early console code for mps2-uart assumes that the serial hardware is
enabled for transmit when the system boots. However, this is not the case
after reset. This results in a hang in mps2_early_putchar() if the serial
transmitter is not enabled by a boot loader or ROM monitor.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/tty/serial/mps2-uart.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
index 9f8f63719126..0743a0551ce1 100644
--- a/drivers/tty/serial/mps2-uart.c
+++ b/drivers/tty/serial/mps2-uart.c
@@ -448,6 +448,14 @@ static struct console mps2_uart_console = {
#define MPS2_SERIAL_CONSOLE (&mps2_uart_console)
+static void mps2_early_init(struct uart_port *port)
+{
+ u8 control = readb(port->membase + UARTn_CTRL);
+
+ control |= UARTn_CTRL_TX_ENABLE;
+ writeb(control, port->membase + UARTn_CTRL);
+}
+
static void mps2_early_putchar(struct uart_port *port, int ch)
{
while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL)
@@ -469,6 +477,7 @@ static int __init mps2_early_console_setup(struct earlycon_device *device,
if (!device->port.membase)
return -ENODEV;
+ mps2_early_init(&device->port);
device->con->write = mps2_early_write;
return 0;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/7] MIPS: dts: Add aliases node for lantiq danube serial
From: Wu, Songjun @ 2018-06-19 6:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: hua.ma, yixin.zhu, chuanhua.lei,
open list:RALINK MIPS ARCHITECTURE, qi-ming.wu, linux-clk,
linux-serial, DTML, James Hogan, Linux Kernel Mailing List,
Thomas Gleixner, Philippe Ombredanne, Rob Herring, Kate Stewart,
Greg Kroah-Hartman, Mark Rutland, Ralf Baechle
In-Reply-To: <CAK8P3a0WVxHU98zjY6d8jN0bDtwabFkrbcxr3a7xRkxSjD2ZLg@mail.gmail.com>
On 6/18/2018 6:59 PM, Arnd Bergmann wrote:
> On Mon, Jun 18, 2018 at 11:42 AM, Wu, Songjun
> <songjun.wu@linux.intel.com> wrote:
>> On 6/14/2018 6:03 PM, Arnd Bergmann wrote:
>>> On Tue, Jun 12, 2018 at 7:40 AM, Songjun Wu <songjun.wu@linux.intel.com>
>>> wrote:
>>>> Previous implementation uses a hard-coded register value to check if
>>>> the current serial entity is the console entity.
>>>> Now the lantiq serial driver uses the aliases for the index of the
>>>> serial port.
>>>> The lantiq danube serial dts are updated with aliases to support this.
>>>>
>>>> Signed-off-by: Songjun Wu <songjun.wu@linux.intel.com>
>>>> ---
>>>>
>>>> arch/mips/boot/dts/lantiq/danube.dtsi | 6 +++++-
>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/mips/boot/dts/lantiq/danube.dtsi
>>>> b/arch/mips/boot/dts/lantiq/danube.dtsi
>>>> index 2dd950181f8a..7a9e15da6bd0 100644
>>>> --- a/arch/mips/boot/dts/lantiq/danube.dtsi
>>>> +++ b/arch/mips/boot/dts/lantiq/danube.dtsi
>>>> @@ -4,6 +4,10 @@
>>>> #size-cells = <1>;
>>>> compatible = "lantiq,xway", "lantiq,danube";
>>>>
>>>> + aliases {
>>>> + serial0 = &asc1;
>>>> + };
>>>> +
>>> You generally want the aliases to be part of the board specific file,
>>> not every board numbers their serial ports in the same way.
>>>
>> In this chip only asc1 can be used as console, so serial0 is defined in
>> chip specific file.
> This was a more general comment about 'aliases' being board specific
> in principle (though we've had exceptions in the past). Even if there
> is only one uart on the chip, I'd recommend following the same
> conventions as the other chips that have more than one uart.
>
> Arnd
Accept, 'aliases' will be move to the board specific file.
^ permalink raw reply
* Re: possible deadlock in console_unlock
From: Petr Mladek @ 2018-06-19 8:04 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Sergey Senozhatsky, syzbot, linux-kernel, rostedt, syzkaller-bugs,
Greg Kroah-Hartman, Jiri Slaby, linux-serial, Andrew Morton
In-Reply-To: <20180615083804.GB8964@jagdpanzerIV>
On Fri 2018-06-15 17:38:04, Sergey Senozhatsky wrote:
> On (06/08/18 10:18), Petr Mladek wrote:
> [..]
> > > Could be.
> > > The good thing about printk_safe is that printk_safe sections can nest.
> > > I suspect there might be locks/printk_safe sections nesting at some
> > > point. In any case, switching to a new flavor of printk_safe will be
> > > pretty easy - just replace printk_safe_enter() with printk_foo_enter()
> > > and the same for printk_save_exit().
> >
> > We could allow nesting. It is just a matter of how many bits we
> > reserve for it in printk_context variable.
> [..]
> > In each case, I would like to keep the printk_safe context usage
> > at minimum. It has its own problems caused by limited per-cpu buffers
> > and the need to flush them.
>
> May be. Every new printk_safe flavour comes with increasing memory
> usage.
This must be a misunderstanding. My intention was to introduce
printk_deferred() context. Where any printk() called in this
context would behave like printk_deferred(). It does not need
any extra buffers.
IMHO, this problem is similar to the problems that we solve
in scheduler and timer code. The cure might be the same.
I just suggest to introduce a context to make our life easier.
> > It is basically needed only to prevent deadlocks related to logbuf_lock.
>
> I wouldn't say that we need printk_safe for logbuf_lock only.
> printk_safe helps us to avoid deadlocks on:
>
> - logbuf_lock spin_lock
logbuf_lock is already guarded by printk_safe context everywhere.
> - console_sem ->lock spin_lock
> - console_owner spin_lock
> - scheduler ->pi_lock spin_lock
> - and probably something else.
printk_deferred should be enough for others. Or do I miss anything?
Best Regards,
Petr
^ permalink raw reply
* Re: possible deadlock in console_unlock
From: Sergey Senozhatsky @ 2018-06-19 8:08 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Sergey Senozhatsky, syzbot, linux-kernel,
rostedt, syzkaller-bugs, Greg Kroah-Hartman, Jiri Slaby,
linux-serial, Andrew Morton
In-Reply-To: <20180619080412.zkdanx6bmq5qm3dz@pathway.suse.cz>
On (06/19/18 10:04), Petr Mladek wrote:
> > >
> > > We could allow nesting. It is just a matter of how many bits we
> > > reserve for it in printk_context variable.
> > [..]
> > > In each case, I would like to keep the printk_safe context usage
> > > at minimum. It has its own problems caused by limited per-cpu buffers
> > > and the need to flush them.
> >
> > May be. Every new printk_safe flavour comes with increasing memory
> > usage.
>
> This must be a misunderstanding. My intention was to introduce
> printk_deferred() context. Where any printk() called in this
> context would behave like printk_deferred(). It does not need
> any extra buffers.
Ah, got it. Yes, this can work.
-ss
^ permalink raw reply
* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Peter Zijlstra @ 2018-06-19 8:08 UTC (permalink / raw)
To: Alan Cox
Cc: Sergey Senozhatsky, Petr Mladek, Steven Rostedt,
Greg Kroah-Hartman, Jiri Slaby, Linus Torvalds, Andrew Morton,
Dmitry Vyukov, linux-kernel, linux-serial, Sergey Senozhatsky
In-Reply-To: <20180618143818.50b2f2f9@alans-desktop>
On Mon, Jun 18, 2018 at 02:38:18PM +0100, Alan Cox wrote:
> Given printk nowdays is already somewhat unreliable with all the perf
> related changes,
printk is a steaming pile of @#$#@, unreliable doesn't even begin to
cover it.
> and we have other good debug tools
What other good debug tools do you have? The only thing I have that
actually works is earlyser/8250_early -- and I route everything printk
into that.
^ permalink raw reply
* Re: [RFC PATCH v2 1/6] serial: uartps: Do not initialize field to zero again
From: Michal Simek @ 2018-06-19 8:09 UTC (permalink / raw)
To: Michal Simek, linux-kernel, gnomes, Alexander Graf, shubhraj,
robh, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-arm-kernel
In-Reply-To: <e8b0169887855a5ef2c1a0456842f650601b451c.1528288895.git.michal.simek@xilinx.com>
On 6.6.2018 14:41, Michal Simek wrote:
> Writing zero and NULLs to already initialized fields is not needed.
> Remove this additional writes.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
> Changes in v2:
> - new patch - it can be sent separately too
>
> drivers/tty/serial/xilinx_uartps.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
> index 8a3e34234e98..5f116f3ecd4a 100644
> --- a/drivers/tty/serial/xilinx_uartps.c
> +++ b/drivers/tty/serial/xilinx_uartps.c
> @@ -1510,15 +1510,12 @@ static int cdns_uart_probe(struct platform_device *pdev)
>
> /* At this point, we've got an empty uart_port struct, initialize it */
> spin_lock_init(&port->lock);
> - port->membase = NULL;
> - port->irq = 0;
> port->type = PORT_UNKNOWN;
> port->iotype = UPIO_MEM32;
> port->flags = UPF_BOOT_AUTOCONF;
> port->ops = &cdns_uart_ops;
> port->fifosize = CDNS_UART_FIFO_SIZE;
> port->line = id;
> - port->dev = NULL;
>
> /*
> * Register the port.
>
Alan, Rob, Greg: Any comment about this RFC?
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs
^ permalink raw reply
* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Petr Mladek @ 2018-06-19 8:30 UTC (permalink / raw)
To: Sergey Senozhatsky
Cc: Alan Cox, Steven Rostedt, Greg Kroah-Hartman, Jiri Slaby,
Linus Torvalds, Peter Zijlstra, Andrew Morton, Dmitry Vyukov,
linux-kernel, linux-serial, Sergey Senozhatsky
In-Reply-To: <20180619005308.GA405@jagdpanzerIV>
On Tue 2018-06-19 09:53:08, Sergey Senozhatsky wrote:
> Thanks for taking a look!
>
> On (06/18/18 14:38), Alan Cox wrote:
> > > It doesn't come as a surprise that recursive printk() calls are not the
> > > only way for us to deadlock in printk() and we still have a whole bunch
> > > of other printk() deadlock scenarios. For instance, those that involve
> > > TTY port->lock spin_lock and UART port->lock spin_lock.
> >
> > The tty layer code there is not re-entrant. Nor is it supposed to be
It is re-entrant via printk(). I mean that any printk() called inside
the locked sections might cause recursion if the same lock is needed
also by con->write() callbacks.
> Could be.
>
> But at least we have circular locking dependency in tty,
> see [1] for more details:
>
> tty_port->lock => uart_port->lock
>
> CPU0
> tty
> spin_lock(&tty_port->lock)
> printk()
> call_console_drivers()
> foo_console_write()
> spin_lock(&uart_port->lock)
>
> Whereas we normally have
>
> uart_port->lock => tty_port->lock
>
> CPU1
> IRQ
> foo_console_handle_IRQ()
> spin_lock(&uart_port->lock)
> tty
> spin_lock(&tty_port->lock)
This is even more complicated situation because printk() allowed
an ABBA lock scenario.
> If we switch to printk_safe when we take tty_port->lock then we
> remove the printk->uart_port chain from the picture.
>
> > > So the idea of this patch set is to take tty_port->lock and
> > > uart_port->lock from printk_safe context and to eliminate some
> > > of non-recursive printk() deadlocks - the ones that don't start
> > > in printk(), but involve console related locks and thus eventually
> > > deadlock us in printk(). For this purpose the patch set introduces
> > > several helper macros:
> >
> > I don't see how this helps - if you recurse into the uart code you are
> > still hitting the paths that are unsafe when re-entered. All you've done
> > is messed up a pile of locking code on critical performance paths.
> >
> > As it stands I think it's a bad idea.
>
> The only new thing is that we inc/dec per-CPU printk context
> variable when we lock/unlock tty/uart port lock:
>
> printk_safe_enter() -> this_cpu_inc(printk_context);
> printk_safe_exit() -> this_cpu_dec(printk_context);
To make it clear. This patch set adds yet another spin_lock API.
It behaves exactly as spin_lock_irqsafe()/spin_unlock_irqrestore()
but in addition it sets printk_context.
Where printk_context defines what printk implementation is safe. We
basically have four possibilities:
1. normal (store in logbuf, try to handle consoles)
2. deferred (store in logbuf, deffer consoles)
3. safe (store in per-CPU buffer, deffer everything)
4. safe_nmi (store in another per-CPU buffer, deffer everything)
This patchset forces safe context around TTY and UART locks.
In fact, the deferred context would be enough to prevent
all the mentioned deadlocks.
IMHO, the only question is if people might get familiar with
yet another spin_lock API.
Best Regards,
Petr
^ permalink raw reply
* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Sergey Senozhatsky @ 2018-06-19 8:59 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, Alan Cox, Steven Rostedt, Greg Kroah-Hartman,
Jiri Slaby, Linus Torvalds, Peter Zijlstra, Andrew Morton,
Dmitry Vyukov, linux-kernel, linux-serial, Sergey Senozhatsky
In-Reply-To: <20180619083021.4avsgvcqjrpkat6s@pathway.suse.cz>
On (06/19/18 10:30), Petr Mladek wrote:
> It is re-entrant via printk(). I mean that any printk() called inside
> the locked sections might cause recursion if the same lock is needed
> also by con->write() callbacks.
Perhaps Alan meant that we cannot return back to tty once we passed
the control from tty to printk -> uart serial console. So tty is
probably (but I didn't check) not re-entrant, but uart definitely is
re-entrant: IRQ -> uart console -> tty -> printk -> uart console.
The patch set attempts to address several issues, and re-entrant uart
is just one of them.
[..]
> This patchset forces safe context around TTY and UART locks.
Right.
> In fact, the deferred context would be enough to prevent
> all the mentioned deadlocks.
Agree.
But we can leave it as a printk_safe implementation detail and
change it later, outside of this patch, to avoid further confusion.
> IMHO, the only question is if people might get familiar with
> yet another spin_lock API.
Right. That's why I thought about renaming uart_port and tty_port
->lock to ->____lock. So the naming will suggest [hopefully] that
those locks are not meant to be used directly [anymore] and instead
people should use uart_port_lock/tty_port_lock API.
-ss
^ permalink raw reply
* Re: [PATCH] serial: mps2-uart: Initialize early console
From: Vladimir Murzin @ 2018-06-19 9:07 UTC (permalink / raw)
To: Guenter Roeck, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-arm-kernel, linux-kernel,
Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi
In-Reply-To: <1529384097-1631-1-git-send-email-linux@roeck-us.net>
Hi Guenter,
On 19/06/18 05:54, Guenter Roeck wrote:
> The early console code for mps2-uart assumes that the serial hardware is
> enabled for transmit when the system boots. However, this is not the case
> after reset. This results in a hang in mps2_early_putchar() if the serial
> transmitter is not enabled by a boot loader or ROM monitor.
I was under impression that for earlycon there is an assumption/requirement
that the serial port must already be setup and configured. For instance, I
see such requirement for pl011. So it looks like boot code's fault not to
enable serial (for mps2 it needs to setup BAUDDIV as well).
I'm not against the patch per se, but I'd like to hear if my understanding of
earlycon requirements is correct or not.
Cheers
Vladimir
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/tty/serial/mps2-uart.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
> index 9f8f63719126..0743a0551ce1 100644
> --- a/drivers/tty/serial/mps2-uart.c
> +++ b/drivers/tty/serial/mps2-uart.c
> @@ -448,6 +448,14 @@ static struct console mps2_uart_console = {
>
> #define MPS2_SERIAL_CONSOLE (&mps2_uart_console)
>
> +static void mps2_early_init(struct uart_port *port)
> +{
> + u8 control = readb(port->membase + UARTn_CTRL);
> +
> + control |= UARTn_CTRL_TX_ENABLE;
> + writeb(control, port->membase + UARTn_CTRL);
> +}
> +
> static void mps2_early_putchar(struct uart_port *port, int ch)
> {
> while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL)
> @@ -469,6 +477,7 @@ static int __init mps2_early_console_setup(struct earlycon_device *device,
> if (!device->port.membase)
> return -ENODEV;
>
> + mps2_early_init(&device->port);
> device->con->write = mps2_early_write;
>
> return 0;
>
^ permalink raw reply
* Re: [PATCH 1/8] dt-bindings: tegra186-hsp: Add shared interrupts
From: Mikko Perttunen @ 2018-06-19 12:41 UTC (permalink / raw)
To: Jon Hunter, Mikko Perttunen, robh+dt, mark.rutland,
jassisinghbrar, gregkh, thierry.reding
Cc: araza, devicetree, linux-serial, linux-tegra, linux-arm-kernel,
linux-kernel
In-Reply-To: <2bed23e6-19d5-66f8-774e-d613a91b1607@nvidia.com>
On 22.05.2018 18:15, Jon Hunter wrote:
>
> On 08/05/18 12:43, Mikko Perttunen wrote:
>> Non-doorbell interrupts are routed through "shared interrupts". These
>> interrupts can be mapped to various internal interrupt lines. Add
>> interrupt properties for shared interrupts to the tegra186-hsp device
>> tree bindings.
>
> Reading the Tegra documentation, although the doorbells have dedicated
> interrupts, it appears that the doorbell interrupts can also be routed
> via these shared interrupts.
Thanks, I changed the text slightly to account for this.
>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>> Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt |
>> 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>> b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>> index b99d25fc2f26..9edcdf82d719 100644
>> --- a/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>> +++ b/Documentation/devicetree/bindings/mailbox/nvidia,tegra186-hsp.txt
>> @@ -21,6 +21,8 @@ Required properties:
>> Contains a list of names for the interrupts described by the
>> interrupt
>> property. May contain the following entries, in any order:
>> - "doorbell"
>> + - "sharedN", where 'N' is a number from zero up to the number of
>> + external interrupts supported by the HSP instance minus one.
>> Users of this binding MUST look up entries in the interrupt
>> property
>> by name, using this interrupt-names property to do so.
>> - interrupts
>
> How is the mapping of shared-mailboxes interrupts to the actual
> 'sharedN' interrupt managed?
Currently the driver always uses shared0 for mailbox-full interrupt, and
nothing else, which is what downstream does as well. It's difficult to
do anything else as we can trample on some other driver's configuration..
>
> Cheers
> Jon
>
Thanks,
Mikko
^ permalink raw reply
* Re: [PATCH 4/8] mailbox: tegra-hsp: Refactor in preparation of mailboxes
From: Mikko Perttunen @ 2018-06-19 12:52 UTC (permalink / raw)
To: Jon Hunter, Mikko Perttunen, robh+dt, mark.rutland,
jassisinghbrar, gregkh, thierry.reding
Cc: araza, devicetree, linux-serial, linux-tegra, linux-arm-kernel,
linux-kernel
In-Reply-To: <8306b033-e7f5-748c-6e6a-131dfd6a26b8@nvidia.com>
On 22.05.2018 18:36, Jon Hunter wrote:
>
> On 08/05/18 12:43, Mikko Perttunen wrote:
>> The HSP driver is currently in many places written with the assumption
>> of only supporting doorbells. Prepare for the addition of shared
>> mailbox support by removing these assumptions and cleaning up the code.
>>
>> Signed-off-by: Mikko Perttunen <mperttunen@nvidia.com>
>> ---
>> drivers/mailbox/tegra-hsp.c | 124
>> +++++++++++++++++++++++++++++---------------
>> 1 file changed, 82 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/mailbox/tegra-hsp.c b/drivers/mailbox/tegra-hsp.c
>> index 0cde356c11ab..16eb970f2c9f 100644
>> --- a/drivers/mailbox/tegra-hsp.c
>> +++ b/drivers/mailbox/tegra-hsp.c
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
>> + * Copyright (c) 2016-2018, NVIDIA CORPORATION. All rights reserved.
>> *
>> * This program is free software; you can redistribute it and/or
>> modify it
>> * under the terms and conditions of the GNU General Public License,
>> @@ -42,6 +42,7 @@ struct tegra_hsp_channel;
>> struct tegra_hsp;
>> struct tegra_hsp_channel {
>> + unsigned int type;
>> struct tegra_hsp *hsp;
>> struct mbox_chan *chan;
>> void __iomem *regs;
>> @@ -55,6 +56,12 @@ struct tegra_hsp_doorbell {
>> unsigned int index;
>> };
>> +static inline struct tegra_hsp_doorbell *
>> +channel_to_doorbell(struct tegra_hsp_channel *channel)
>> +{
>> + return container_of(channel, struct tegra_hsp_doorbell, channel);
>> +}
>> +
>> struct tegra_hsp_db_map {
>> const char *name;
>> unsigned int master;
>> @@ -69,7 +76,7 @@ struct tegra_hsp {
>> const struct tegra_hsp_soc *soc;
>> struct mbox_controller mbox;
>> void __iomem *regs;
>> - unsigned int irq;
>> + unsigned int doorbell_irq;
>> unsigned int num_sm;
>> unsigned int num_as;
>> unsigned int num_ss;
>> @@ -194,7 +201,7 @@ tegra_hsp_doorbell_create(struct tegra_hsp *hsp,
>> const char *name,
>> if (!db)
>> return ERR_PTR(-ENOMEM);
>> - offset = (1 + (hsp->num_sm / 2) + hsp->num_ss + hsp->num_as) << 16;
>> + offset = (1 + (hsp->num_sm / 2) + hsp->num_ss + hsp->num_as) *
>> SZ_64K;
>> offset += index * 0x100;
>> db->channel.regs = hsp->regs + offset;
>> @@ -218,18 +225,8 @@ static void __tegra_hsp_doorbell_destroy(struct
>> tegra_hsp_doorbell *db)
>> kfree(db);
>> }
>> -static int tegra_hsp_doorbell_send_data(struct mbox_chan *chan, void
>> *data)
>> -{
>> - struct tegra_hsp_doorbell *db = chan->con_priv;
>> -
>> - tegra_hsp_channel_writel(&db->channel, 1, HSP_DB_TRIGGER);
>> -
>> - return 0;
>> -}
>> -
>> -static int tegra_hsp_doorbell_startup(struct mbox_chan *chan)
>> +static int tegra_hsp_doorbell_startup(struct tegra_hsp_doorbell *db)
>> {
>> - struct tegra_hsp_doorbell *db = chan->con_priv;
>> struct tegra_hsp *hsp = db->channel.hsp;
>> struct tegra_hsp_doorbell *ccplex;
>> unsigned long flags;
>> @@ -260,9 +257,8 @@ static int tegra_hsp_doorbell_startup(struct
>> mbox_chan *chan)
>> return 0;
>> }
>> -static void tegra_hsp_doorbell_shutdown(struct mbox_chan *chan)
>> +static void tegra_hsp_doorbell_shutdown(struct tegra_hsp_doorbell *db)
>> {
>> - struct tegra_hsp_doorbell *db = chan->con_priv;
>> struct tegra_hsp *hsp = db->channel.hsp;
>> struct tegra_hsp_doorbell *ccplex;
>> unsigned long flags;
>> @@ -281,35 +277,61 @@ static void tegra_hsp_doorbell_shutdown(struct
>> mbox_chan *chan)
>> spin_unlock_irqrestore(&hsp->lock, flags);
>> }
>> -static const struct mbox_chan_ops tegra_hsp_doorbell_ops = {
>> - .send_data = tegra_hsp_doorbell_send_data,
>> - .startup = tegra_hsp_doorbell_startup,
>> - .shutdown = tegra_hsp_doorbell_shutdown,
>> +static int tegra_hsp_send_data(struct mbox_chan *chan, void *data)
>> +{
>> + struct tegra_hsp_channel *channel = chan->con_priv;
>> + struct tegra_hsp_doorbell *db;
>> +
>> + switch (channel->type) {
>> + case TEGRA_HSP_MBOX_TYPE_DB:
>> + db = channel_to_doorbell(channel);
>> + tegra_hsp_channel_writel(&db->channel, 1, HSP_DB_TRIGGER);
>
> The above appears to be redundant. We go from channel to db and then end
> up passing channels. Do we really need the 'db' struct above?
Fixed.
>
>> + }
>> +
>> + return -EINVAL;
>
> Does this function always return -EINVAL?
Fixed.
>
>> +}
>> +
>> +static int tegra_hsp_startup(struct mbox_chan *chan)
>> +{
>> + struct tegra_hsp_channel *channel = chan->con_priv;
>> +
>> + switch (channel->type) {
>> + case TEGRA_HSP_MBOX_TYPE_DB:
>> + return tegra_hsp_doorbell_startup(channel_to_doorbell(channel));
>> + }
>> +
>> + return -EINVAL;
>> +}
>> +
>> +static void tegra_hsp_shutdown(struct mbox_chan *chan)
>> +{
>> + struct tegra_hsp_channel *channel = chan->con_priv;
>> +
>> + switch (channel->type) {
>> + case TEGRA_HSP_MBOX_TYPE_DB:
>> + tegra_hsp_doorbell_shutdown(channel_to_doorbell(channel));
>> + break;
>> + }
>> +}
>> +
>> +static const struct mbox_chan_ops tegra_hsp_ops = {
>> + .send_data = tegra_hsp_send_data,
>> + .startup = tegra_hsp_startup,
>> + .shutdown = tegra_hsp_shutdown,
>> };
>> -static struct mbox_chan *of_tegra_hsp_xlate(struct mbox_controller
>> *mbox,
>> - const struct of_phandle_args *args)
>> +static struct mbox_chan *tegra_hsp_doorbell_xlate(struct tegra_hsp *hsp,
>> + unsigned int master)
>> {
>> struct tegra_hsp_channel *channel = ERR_PTR(-ENODEV);
>> - struct tegra_hsp *hsp = to_tegra_hsp(mbox);
>> - unsigned int type = args->args[0];
>> - unsigned int master = args->args[1];
>> struct tegra_hsp_doorbell *db;
>> struct mbox_chan *chan;
>> unsigned long flags;
>> unsigned int i;
>> - switch (type) {
>> - case TEGRA_HSP_MBOX_TYPE_DB:
>> - db = tegra_hsp_doorbell_get(hsp, master);
>> - if (db)
>> - channel = &db->channel;
>> -
>> - break;
>> -
>> - default:
>> - break;
>> - }
>> + db = tegra_hsp_doorbell_get(hsp, master);
>> + if (db)
>> + channel = &db->channel;
>> if (IS_ERR(channel))
>> return ERR_CAST(channel);
>> @@ -321,6 +343,7 @@ static struct mbox_chan *of_tegra_hsp_xlate(struct
>> mbox_controller *mbox,
>> if (!chan->con_priv) {
>> chan->con_priv = channel;
>> channel->chan = chan;
>> + channel->type = TEGRA_HSP_MBOX_TYPE_DB;
>> break;
>
> I see that you are making the above only used for doorbells, but don't
> we still need to set the chan->con_priv for shared mailboxes as well?
That's done elsewhere in the next patch that actually adds the shared
mailbox support.
>
> Cheers
> Jon
>
Thanks,
Mikko
^ permalink raw reply
* Re: [PATCH] serial: mps2-uart: Initialize early console
From: Guenter Roeck @ 2018-06-19 13:01 UTC (permalink / raw)
To: Vladimir Murzin, Greg Kroah-Hartman
Cc: Jiri Slaby, linux-serial, linux-arm-kernel, linux-kernel,
Liviu Dudau, Sudeep Holla, Lorenzo Pieralisi
In-Reply-To: <173551dd-3632-08a6-ec42-2ee416ccb9ad@arm.com>
On 06/19/2018 02:07 AM, Vladimir Murzin wrote:
> Hi Guenter,
>
> On 19/06/18 05:54, Guenter Roeck wrote:
>> The early console code for mps2-uart assumes that the serial hardware is
>> enabled for transmit when the system boots. However, this is not the case
>> after reset. This results in a hang in mps2_early_putchar() if the serial
>> transmitter is not enabled by a boot loader or ROM monitor.
>
> I was under impression that for earlycon there is an assumption/requirement
> that the serial port must already be setup and configured. For instance, I
> see such requirement for pl011. So it looks like boot code's fault not to
> enable serial (for mps2 it needs to setup BAUDDIV as well).
>
Good to know. Fine with me as well; I wasn't aware that such a requirement
existed.
Guenter
> I'm not against the patch per se, but I'd like to hear if my understanding of
> earlycon requirements is correct or not.
>
> Cheers
> Vladimir
>
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/tty/serial/mps2-uart.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/tty/serial/mps2-uart.c b/drivers/tty/serial/mps2-uart.c
>> index 9f8f63719126..0743a0551ce1 100644
>> --- a/drivers/tty/serial/mps2-uart.c
>> +++ b/drivers/tty/serial/mps2-uart.c
>> @@ -448,6 +448,14 @@ static struct console mps2_uart_console = {
>>
>> #define MPS2_SERIAL_CONSOLE (&mps2_uart_console)
>>
>> +static void mps2_early_init(struct uart_port *port)
>> +{
>> + u8 control = readb(port->membase + UARTn_CTRL);
>> +
>> + control |= UARTn_CTRL_TX_ENABLE;
>> + writeb(control, port->membase + UARTn_CTRL);
>> +}
>> +
>> static void mps2_early_putchar(struct uart_port *port, int ch)
>> {
>> while (readb(port->membase + UARTn_STATE) & UARTn_STATE_TX_FULL)
>> @@ -469,6 +477,7 @@ static int __init mps2_early_console_setup(struct earlycon_device *device,
>> if (!device->port.membase)
>> return -ENODEV;
>>
>> + mps2_early_init(&device->port);
>> device->con->write = mps2_early_write;
>>
>> return 0;
>>
>
>
^ permalink raw reply
* [PATCH v5 0/7] add virt-dma support for imx-sdma
From: Robin Gong @ 2018-06-19 16:56 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
The legacy sdma driver has below limitations or drawbacks:
1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
one page size for one channel regardless of only few BDs needed
most time. But in few cases, the max PAGE_SIZE maybe not enough.
2. One SDMA channel can't stop immediatley once channel disabled which
means SDMA interrupt may come in after this channel terminated.There
are some patches for this corner case such as commit "2746e2c389f9",
but not cover non-cyclic.
The common virt-dma overcomes the above limitations. It can alloc bd
dynamically and free bd once this tx transfer done. No memory wasted or
maximum limititation here, only depends on how many memory can be requested
from kernel. For No.2, such issue can be workaround by checking if there
is available descript("sdmac->desc") now once the unwanted interrupt
coming. At last the common virt-dma is easier for sdma driver maintain.
Change from v4:
1. identify lockdep issue which caused by allocate memory with
'GFP_KERNEL', change to 'GFP_NOWAIT' instead so that lockdep
ignore check. That also make sense since Audio/uart driver may
call dma function after spin_lock_irqsave()...
2. use dma pool instead for bd description allocated,since audio
driver may call dma_terminate_all in irq. Please refer to 7/7.
3. remove 7/7 serial patch in v4, since lockdep issued fixed by No.1
Change from v3:
1. add two uart patches which impacted by this patchset.
2. unlock 'vc.lock' before cyclic dma callback and lock again after
it because some driver such as uart will call dmaengine_tx_status
which will acquire 'vc.lock' again and dead lock comes out.
3. remove 'Revert commit' stuff since that patch is not wrong and
combine two patch into one patch as Sascha's comment.
Change from v2:
1. include Sascha's patch to make the main patch easier to review.
Thanks Sacha.
2. remove useless 'desc'/'chan' in struct sdma_channe.
Change from v1:
1. split v1 patch into 5 patches.
2. remove some unnecessary condition check.
3. remove unnecessary 'pending' list.
Robin Gong (6):
tty: serial: imx: correct dma cookie status
dmaengine: imx-sdma: add virt-dma support
dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct
sdma_channel'
dmaengine: imx-sdma: remove the maximum limitation for bd numbers
dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
dmaengine: imx-sdma: alloclate bd memory from dma pool
Sascha Hauer (1):
dmaengine: imx-sdma: factor out a struct sdma_desc from struct
sdma_channel
drivers/dma/Kconfig | 1 +
drivers/dma/imx-sdma.c | 400 +++++++++++++++++++++++++++--------------------
drivers/tty/serial/imx.c | 2 +-
3 files changed, 235 insertions(+), 168 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH v5 1/7] tty: serial: imx: correct dma cookie status
From: Robin Gong @ 2018-06-19 16:56 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: dmaengine, linux-imx, linux-kernel, linux-serial,
linux-arm-kernel
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
Correct to check the right rx dma cookie status in spit of it
works because only one cookie is running in the current sdma.
But it will not once sdma driver support multi cookies
running based on virt-dma.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/tty/serial/imx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 4e85357..2879407 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1051,7 +1051,7 @@ static void imx_uart_dma_rx_callback(void *data)
unsigned int r_bytes;
unsigned int bd_size;
- status = dmaengine_tx_status(chan, (dma_cookie_t)0, &state);
+ status = dmaengine_tx_status(chan, sport->rx_cookie, &state);
if (status == DMA_ERROR) {
imx_uart_clear_rx_errors(sport);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 2/7] dmaengine: imx-sdma: factor out a struct sdma_desc from struct sdma_channel
From: Robin Gong @ 2018-06-19 16:56 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
From: Sascha Hauer <s.hauer@pengutronix.de>
This is a preparation step to make the adding of virt-dma easier.
We create a struct sdma_desc, move some fields from struct sdma_channel
there and add a pointer from the former to the latter. For now we
allocate the data statically in struct sdma_channel, but with
virt-dma support it will be dynamically allocated.
Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 137 ++++++++++++++++++++++++++++++-------------------
1 file changed, 83 insertions(+), 54 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f0779926..19c351f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -289,6 +289,30 @@ struct sdma_context_data {
struct sdma_engine;
/**
+ * struct sdma_desc - descriptor structor for one transfer
+ * @vd descriptor for virt dma
+ * @num_bd max NUM_BD. number of descriptors currently handling
+ * @buf_tail ID of the buffer that was processed
+ * @buf_ptail ID of the previous buffer that was processed
+ * @period_len period length, used in cyclic.
+ * @chn_real_count the real count updated from bd->mode.count
+ * @chn_count the transfer count setuped
+ * @sdmac sdma_channel pointer
+ * @bd pointer of alloced bd
+ */
+struct sdma_desc {
+ unsigned int num_bd;
+ dma_addr_t bd_phys;
+ unsigned int buf_tail;
+ unsigned int buf_ptail;
+ unsigned int period_len;
+ unsigned int chn_real_count;
+ unsigned int chn_count;
+ struct sdma_channel *sdmac;
+ struct sdma_buffer_descriptor *bd;
+};
+
+/**
* struct sdma_channel - housekeeping for a SDMA channel
*
* @sdma pointer to the SDMA engine for this channel
@@ -298,11 +322,10 @@ struct sdma_engine;
* @event_id0 aka dma request line
* @event_id1 for channels that use 2 events
* @word_size peripheral access size
- * @buf_tail ID of the buffer that was processed
- * @buf_ptail ID of the previous buffer that was processed
- * @num_bd max NUM_BD. number of descriptors currently handling
*/
struct sdma_channel {
+ struct sdma_desc *desc;
+ struct sdma_desc _desc;
struct sdma_engine *sdma;
unsigned int channel;
enum dma_transfer_direction direction;
@@ -310,12 +333,6 @@ struct sdma_channel {
unsigned int event_id0;
unsigned int event_id1;
enum dma_slave_buswidth word_size;
- unsigned int buf_tail;
- unsigned int buf_ptail;
- unsigned int num_bd;
- unsigned int period_len;
- struct sdma_buffer_descriptor *bd;
- dma_addr_t bd_phys;
unsigned int pc_from_device, pc_to_device;
unsigned int device_to_device;
unsigned long flags;
@@ -325,10 +342,8 @@ struct sdma_channel {
u32 shp_addr, per_addr;
struct dma_chan chan;
spinlock_t lock;
- struct dma_async_tx_descriptor desc;
+ struct dma_async_tx_descriptor txdesc;
enum dma_status status;
- unsigned int chn_count;
- unsigned int chn_real_count;
struct tasklet_struct tasklet;
struct imx_dma_data data;
bool enabled;
@@ -391,6 +406,8 @@ struct sdma_engine {
u32 spba_start_addr;
u32 spba_end_addr;
unsigned int irq;
+ dma_addr_t bd0_phys;
+ struct sdma_buffer_descriptor *bd0;
};
static struct sdma_driver_data sdma_imx31 = {
@@ -625,7 +642,7 @@ static int sdma_run_channel0(struct sdma_engine *sdma)
static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
u32 address)
{
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
void *buf_virt;
dma_addr_t buf_phys;
int ret;
@@ -700,7 +717,9 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* call callback function.
*/
while (1) {
- bd = &sdmac->bd[sdmac->buf_tail];
+ struct sdma_desc *desc = sdmac->desc;
+
+ bd = &desc->bd[desc->buf_tail];
if (bd->mode.status & BD_DONE)
break;
@@ -716,11 +735,11 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* the number of bytes present in the current buffer descriptor.
*/
- sdmac->chn_real_count = bd->mode.count;
+ desc->chn_real_count = bd->mode.count;
bd->mode.status |= BD_DONE;
- bd->mode.count = sdmac->period_len;
- sdmac->buf_ptail = sdmac->buf_tail;
- sdmac->buf_tail = (sdmac->buf_tail + 1) % sdmac->num_bd;
+ bd->mode.count = desc->period_len;
+ desc->buf_ptail = desc->buf_tail;
+ desc->buf_tail = (desc->buf_tail + 1) % desc->num_bd;
/*
* The callback is called from the interrupt context in order
@@ -729,7 +748,7 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* executed.
*/
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
+ dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
if (error)
sdmac->status = old_status;
@@ -742,17 +761,17 @@ static void mxc_sdma_handle_channel_normal(unsigned long data)
struct sdma_buffer_descriptor *bd;
int i, error = 0;
- sdmac->chn_real_count = 0;
+ sdmac->desc->chn_real_count = 0;
/*
* non loop mode. Iterate over all descriptors, collect
* errors and call callback function
*/
- for (i = 0; i < sdmac->num_bd; i++) {
- bd = &sdmac->bd[i];
+ for (i = 0; i < sdmac->desc->num_bd; i++) {
+ bd = &sdmac->desc->bd[i];
if (bd->mode.status & (BD_DONE | BD_RROR))
error = -EIO;
- sdmac->chn_real_count += bd->mode.count;
+ sdmac->desc->chn_real_count += bd->mode.count;
}
if (error)
@@ -760,9 +779,9 @@ static void mxc_sdma_handle_channel_normal(unsigned long data)
else
sdmac->status = DMA_COMPLETE;
- dma_cookie_complete(&sdmac->desc);
+ dma_cookie_complete(&sdmac->txdesc);
- dmaengine_desc_get_callback_invoke(&sdmac->desc, NULL);
+ dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
}
static irqreturn_t sdma_int_handler(int irq, void *dev_id)
@@ -890,7 +909,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
int channel = sdmac->channel;
int load_address;
struct sdma_context_data *context = sdma->context;
- struct sdma_buffer_descriptor *bd0 = sdma->channel[0].bd;
+ struct sdma_buffer_descriptor *bd0 = sdma->bd0;
int ret;
unsigned long flags;
@@ -1093,18 +1112,22 @@ static int sdma_set_channel_priority(struct sdma_channel *sdmac,
static int sdma_request_channel(struct sdma_channel *sdmac)
{
struct sdma_engine *sdma = sdmac->sdma;
+ struct sdma_desc *desc;
int channel = sdmac->channel;
int ret = -EBUSY;
- sdmac->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &sdmac->bd_phys,
+ sdmac->desc = &sdmac->_desc;
+ desc = sdmac->desc;
+
+ desc->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &desc->bd_phys,
GFP_KERNEL);
- if (!sdmac->bd) {
+ if (!desc->bd) {
ret = -ENOMEM;
goto out;
}
- sdma->channel_control[channel].base_bd_ptr = sdmac->bd_phys;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
sdma_set_channel_priority(sdmac, MXC_SDMA_DEFAULT_PRIORITY);
return 0;
@@ -1169,10 +1192,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ahb;
- dma_async_tx_descriptor_init(&sdmac->desc, chan);
- sdmac->desc.tx_submit = sdma_tx_submit;
+ dma_async_tx_descriptor_init(&sdmac->txdesc, chan);
+ sdmac->txdesc.tx_submit = sdma_tx_submit;
/* txd.flags will be overwritten in prep funcs */
- sdmac->desc.flags = DMA_CTRL_ACK;
+ sdmac->txdesc.flags = DMA_CTRL_ACK;
return 0;
@@ -1187,6 +1210,7 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
+ struct sdma_desc *desc = sdmac->desc;
sdma_disable_channel(chan);
@@ -1200,7 +1224,7 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
sdma_set_channel_priority(sdmac, 0);
- dma_free_coherent(NULL, PAGE_SIZE, sdmac->bd, sdmac->bd_phys);
+ dma_free_coherent(NULL, PAGE_SIZE, desc->bd, desc->bd_phys);
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
@@ -1216,6 +1240,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
int ret, i, count;
int channel = sdmac->channel;
struct scatterlist *sg;
+ struct sdma_desc *desc = sdmac->desc;
if (sdmac->status == DMA_IN_PROGRESS)
return NULL;
@@ -1223,9 +1248,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->chn_real_count = 0;
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
@@ -1242,9 +1267,9 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
goto err_out;
}
- sdmac->chn_count = 0;
+ desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = sg->dma_address;
@@ -1259,7 +1284,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
}
bd->mode.count = count;
- sdmac->chn_count += count;
+ desc->chn_count += count;
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES) {
ret = -EINVAL;
@@ -1300,10 +1325,10 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
bd->mode.status = param;
}
- sdmac->num_bd = sg_len;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ desc->num_bd = sg_len;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
- return &sdmac->desc;
+ return &sdmac->txdesc;
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1319,6 +1344,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
int ret, i = 0, buf = 0;
+ struct sdma_desc *desc = sdmac->desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1327,10 +1353,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
- sdmac->buf_tail = 0;
- sdmac->buf_ptail = 0;
- sdmac->chn_real_count = 0;
- sdmac->period_len = period_len;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->chn_real_count = 0;
+ desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
sdmac->direction = direction;
@@ -1351,7 +1377,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
}
while (buf < buf_len) {
- struct sdma_buffer_descriptor *bd = &sdmac->bd[i];
+ struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
bd->buffer_addr = dma_addr;
@@ -1382,10 +1408,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
i++;
}
- sdmac->num_bd = num_periods;
- sdma->channel_control[channel].current_bd_ptr = sdmac->bd_phys;
+ desc->num_bd = num_periods;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
- return &sdmac->desc;
+ return &sdmac->txdesc;
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1424,13 +1450,14 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
struct dma_tx_state *txstate)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
+ struct sdma_desc *desc = sdmac->desc;
u32 residue;
if (sdmac->flags & IMX_DMA_SG_LOOP)
- residue = (sdmac->num_bd - sdmac->buf_ptail) *
- sdmac->period_len - sdmac->chn_real_count;
+ residue = (desc->num_bd - desc->buf_ptail) *
+ desc->period_len - desc->chn_real_count;
else
- residue = sdmac->chn_count - sdmac->chn_real_count;
+ residue = desc->chn_count - desc->chn_real_count;
dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
residue);
@@ -1654,6 +1681,8 @@ static int sdma_init(struct sdma_engine *sdma)
if (ret)
goto err_dma_alloc;
+ sdma->bd0 = sdma->channel[0].desc->bd;
+
sdma_config_ownership(&sdma->channel[0], false, true, false);
/* Set Command Channel (Channel Zero) */
--
2.7.4
^ permalink raw reply related
* [PATCH v5 3/7] dmaengine: imx-sdma: add virt-dma support
From: Robin Gong @ 2018-06-19 16:57 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
The legacy sdma driver has below limitations or drawbacks:
1. Hardcode the max BDs number as "PAGE_SIZE / sizeof(*)", and alloc
one page size for one channel regardless of only few BDs needed
most time. But in few cases, the max PAGE_SIZE maybe not enough.
2. One SDMA channel can't stop immediatley once channel disabled which
means SDMA interrupt may come in after this channel terminated.There
are some patches for this corner case such as commit "2746e2c389f9",
but not cover non-cyclic.
The common virt-dma overcomes the above limitations. It can alloc bd
dynamically and free bd once this tx transfer done. No memory wasted or
maximum limititation here, only depends on how many memory can be requested
from kernel. For No.2, such issue can be workaround by checking if there
is available descript("sdmac->desc") now once the unwanted interrupt
coming. At last the common virt-dma is easier for sdma driver maintain.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/Kconfig | 1 +
drivers/dma/imx-sdma.c | 263 ++++++++++++++++++++++++++++++++-----------------
2 files changed, 171 insertions(+), 93 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index ca1680a..d4a4230 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -250,6 +250,7 @@ config IMX_SDMA
tristate "i.MX SDMA support"
depends on ARCH_MXC
select DMA_ENGINE
+ select DMA_VIRTUAL_CHANNELS
help
Support the i.MX SDMA engine. This engine is integrated into
Freescale i.MX25/31/35/51/53/6 chips.
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 19c351f..86fa799 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -41,6 +41,7 @@
#include <linux/mfd/syscon/imx6q-iomuxc-gpr.h>
#include "dmaengine.h"
+#include "virt-dma.h"
/* SDMA registers */
#define SDMA_H_C0PTR 0x000
@@ -301,6 +302,7 @@ struct sdma_engine;
* @bd pointer of alloced bd
*/
struct sdma_desc {
+ struct virt_dma_desc vd;
unsigned int num_bd;
dma_addr_t bd_phys;
unsigned int buf_tail;
@@ -324,8 +326,8 @@ struct sdma_desc {
* @word_size peripheral access size
*/
struct sdma_channel {
+ struct virt_dma_chan vc;
struct sdma_desc *desc;
- struct sdma_desc _desc;
struct sdma_engine *sdma;
unsigned int channel;
enum dma_transfer_direction direction;
@@ -340,11 +342,8 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- struct dma_chan chan;
spinlock_t lock;
- struct dma_async_tx_descriptor txdesc;
enum dma_status status;
- struct tasklet_struct tasklet;
struct imx_dma_data data;
bool enabled;
};
@@ -698,6 +697,35 @@ static void sdma_event_disable(struct sdma_channel *sdmac, unsigned int event)
writel_relaxed(val, sdma->regs + chnenbl);
}
+static struct sdma_desc *to_sdma_desc(struct dma_async_tx_descriptor *t)
+{
+ return container_of(t, struct sdma_desc, vd.tx);
+}
+
+static void sdma_start_desc(struct sdma_channel *sdmac)
+{
+ struct virt_dma_desc *vd = vchan_next_desc(&sdmac->vc);
+ struct sdma_desc *desc;
+ struct sdma_engine *sdma = sdmac->sdma;
+ int channel = sdmac->channel;
+
+ if (!vd) {
+ sdmac->desc = NULL;
+ return;
+ }
+ sdmac->desc = desc = to_sdma_desc(&vd->tx);
+ /*
+ * Do not delete the node in desc_issued list in cyclic mode, otherwise
+ * the desc alloced will never be freed in vchan_dma_desc_free_list
+ */
+ if (!(sdmac->flags & IMX_DMA_SG_LOOP))
+ list_del(&vd->node);
+
+ sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
+ sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
+ sdma_enable_channel(sdma, sdmac->channel);
+}
+
static void sdma_update_channel_loop(struct sdma_channel *sdmac)
{
struct sdma_buffer_descriptor *bd;
@@ -716,7 +744,7 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* loop mode. Iterate over descriptors, re-setup them and
* call callback function.
*/
- while (1) {
+ while (sdmac->desc) {
struct sdma_desc *desc = sdmac->desc;
bd = &desc->bd[desc->buf_tail];
@@ -747,15 +775,16 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
* SDMA transaction status by the time the client tasklet is
* executed.
*/
-
- dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
+ spin_unlock(&sdmac->vc.lock);
+ dmaengine_desc_get_callback_invoke(&desc->vd.tx, NULL);
+ spin_lock(&sdmac->vc.lock);
if (error)
sdmac->status = old_status;
}
}
-static void mxc_sdma_handle_channel_normal(unsigned long data)
+static void mxc_sdma_handle_channel_normal(struct sdma_channel *data)
{
struct sdma_channel *sdmac = (struct sdma_channel *) data;
struct sdma_buffer_descriptor *bd;
@@ -778,10 +807,6 @@ static void mxc_sdma_handle_channel_normal(unsigned long data)
sdmac->status = DMA_ERROR;
else
sdmac->status = DMA_COMPLETE;
-
- dma_cookie_complete(&sdmac->txdesc);
-
- dmaengine_desc_get_callback_invoke(&sdmac->txdesc, NULL);
}
static irqreturn_t sdma_int_handler(int irq, void *dev_id)
@@ -797,12 +822,21 @@ static irqreturn_t sdma_int_handler(int irq, void *dev_id)
while (stat) {
int channel = fls(stat) - 1;
struct sdma_channel *sdmac = &sdma->channel[channel];
+ struct sdma_desc *desc;
+
+ spin_lock(&sdmac->vc.lock);
+ desc = sdmac->desc;
+ if (desc) {
+ if (sdmac->flags & IMX_DMA_SG_LOOP) {
+ sdma_update_channel_loop(sdmac);
+ } else {
+ mxc_sdma_handle_channel_normal(sdmac);
+ vchan_cookie_complete(&desc->vd);
+ sdma_start_desc(sdmac);
+ }
+ }
- if (sdmac->flags & IMX_DMA_SG_LOOP)
- sdma_update_channel_loop(sdmac);
- else
- tasklet_schedule(&sdmac->tasklet);
-
+ spin_unlock(&sdmac->vc.lock);
__clear_bit(channel, &stat);
}
@@ -958,7 +992,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
static struct sdma_channel *to_sdma_chan(struct dma_chan *chan)
{
- return container_of(chan, struct sdma_channel, chan);
+ return container_of(chan, struct sdma_channel, vc.chan);
}
static int sdma_disable_channel(struct dma_chan *chan)
@@ -980,7 +1014,16 @@ static int sdma_disable_channel(struct dma_chan *chan)
static int sdma_disable_channel_with_delay(struct dma_chan *chan)
{
+ struct sdma_channel *sdmac = to_sdma_chan(chan);
+ unsigned long flags;
+ LIST_HEAD(head);
+
sdma_disable_channel(chan);
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ vchan_get_all_descriptors(&sdmac->vc, &head);
+ sdmac->desc = NULL;
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
+ vchan_dma_desc_free_list(&sdmac->vc, &head);
/*
* According to NXP R&D team a delay of one BD SDMA cost time
@@ -1109,46 +1152,56 @@ static int sdma_set_channel_priority(struct sdma_channel *sdmac,
return 0;
}
-static int sdma_request_channel(struct sdma_channel *sdmac)
+static int sdma_request_channel0(struct sdma_engine *sdma)
{
- struct sdma_engine *sdma = sdmac->sdma;
- struct sdma_desc *desc;
- int channel = sdmac->channel;
int ret = -EBUSY;
- sdmac->desc = &sdmac->_desc;
- desc = sdmac->desc;
-
- desc->bd = dma_zalloc_coherent(NULL, PAGE_SIZE, &desc->bd_phys,
- GFP_KERNEL);
- if (!desc->bd) {
+ sdma->bd0 = dma_zalloc_coherent(NULL, PAGE_SIZE, &sdma->bd0_phys,
+ GFP_NOWAIT);
+ if (!sdma->bd0) {
ret = -ENOMEM;
goto out;
}
- sdma->channel_control[channel].base_bd_ptr = desc->bd_phys;
- sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
+ sdma->channel_control[0].base_bd_ptr = sdma->bd0_phys;
+ sdma->channel_control[0].current_bd_ptr = sdma->bd0_phys;
- sdma_set_channel_priority(sdmac, MXC_SDMA_DEFAULT_PRIORITY);
+ sdma_set_channel_priority(&sdma->channel[0], MXC_SDMA_DEFAULT_PRIORITY);
return 0;
out:
return ret;
}
-static dma_cookie_t sdma_tx_submit(struct dma_async_tx_descriptor *tx)
+
+static int sdma_alloc_bd(struct sdma_desc *desc)
{
- unsigned long flags;
- struct sdma_channel *sdmac = to_sdma_chan(tx->chan);
- dma_cookie_t cookie;
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
+ int ret = 0;
- spin_lock_irqsave(&sdmac->lock, flags);
+ desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
+ GFP_ATOMIC);
+ if (!desc->bd) {
+ ret = -ENOMEM;
+ goto out;
+ }
+out:
+ return ret;
+}
- cookie = dma_cookie_assign(tx);
+static void sdma_free_bd(struct sdma_desc *desc)
+{
+ u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
- spin_unlock_irqrestore(&sdmac->lock, flags);
+ dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+}
- return cookie;
+static void sdma_desc_free(struct virt_dma_desc *vd)
+{
+ struct sdma_desc *desc = container_of(vd, struct sdma_desc, vd);
+
+ sdma_free_bd(desc);
+ kfree(desc);
}
static int sdma_alloc_chan_resources(struct dma_chan *chan)
@@ -1184,19 +1237,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ipg;
- ret = sdma_request_channel(sdmac);
- if (ret)
- goto disable_clk_ahb;
-
ret = sdma_set_channel_priority(sdmac, prio);
if (ret)
goto disable_clk_ahb;
- dma_async_tx_descriptor_init(&sdmac->txdesc, chan);
- sdmac->txdesc.tx_submit = sdma_tx_submit;
- /* txd.flags will be overwritten in prep funcs */
- sdmac->txdesc.flags = DMA_CTRL_ACK;
-
return 0;
disable_clk_ahb:
@@ -1210,9 +1254,8 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
- struct sdma_desc *desc = sdmac->desc;
- sdma_disable_channel(chan);
+ sdma_disable_channel_with_delay(chan);
if (sdmac->event_id0)
sdma_event_disable(sdmac, sdmac->event_id0);
@@ -1224,8 +1267,6 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
sdma_set_channel_priority(sdmac, 0);
- dma_free_coherent(NULL, PAGE_SIZE, desc->bd, desc->bd_phys);
-
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
}
@@ -1240,7 +1281,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
int ret, i, count;
int channel = sdmac->channel;
struct scatterlist *sg;
- struct sdma_desc *desc = sdmac->desc;
+ struct sdma_desc *desc;
if (sdmac->status == DMA_IN_PROGRESS)
return NULL;
@@ -1248,23 +1289,34 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
sdmac->flags = 0;
+ desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
+ if (!desc)
+ goto err_out;
+
desc->buf_tail = 0;
desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = sg_len;
desc->chn_real_count = 0;
+ if (sdma_alloc_bd(desc)) {
+ kfree(desc);
+ goto err_out;
+ }
+
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
sdmac->direction = direction;
ret = sdma_load_context(sdmac);
if (ret)
- goto err_out;
+ goto err_bd_out;
if (sg_len > NUM_BD) {
dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
channel, sg_len, NUM_BD);
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
desc->chn_count = 0;
@@ -1280,7 +1332,7 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
dev_err(sdma->dev, "SDMA channel %d: maximum bytes for sg entry exceeded: %d > %d\n",
channel, count, 0xffff);
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
bd->mode.count = count;
@@ -1288,25 +1340,25 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES) {
ret = -EINVAL;
- goto err_out;
+ goto err_bd_out;
}
switch (sdmac->word_size) {
case DMA_SLAVE_BUSWIDTH_4_BYTES:
bd->mode.command = 0;
if (count & 3 || sg->dma_address & 3)
- return NULL;
+ goto err_bd_out;
break;
case DMA_SLAVE_BUSWIDTH_2_BYTES:
bd->mode.command = 2;
if (count & 1 || sg->dma_address & 1)
- return NULL;
+ goto err_bd_out;
break;
case DMA_SLAVE_BUSWIDTH_1_BYTE:
bd->mode.command = 1;
break;
default:
- return NULL;
+ goto err_bd_out;
}
param = BD_DONE | BD_EXTD | BD_CONT;
@@ -1325,10 +1377,10 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
bd->mode.status = param;
}
- desc->num_bd = sg_len;
- sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
-
- return &sdmac->txdesc;
+ return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
+err_bd_out:
+ sdma_free_bd(desc);
+ kfree(desc);
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1344,7 +1396,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
int ret, i = 0, buf = 0;
- struct sdma_desc *desc = sdmac->desc;
+ struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
@@ -1353,27 +1405,39 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
sdmac->status = DMA_IN_PROGRESS;
+ desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
+ if (!desc)
+ goto err_out;
+
desc->buf_tail = 0;
desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = num_periods;
desc->chn_real_count = 0;
desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
sdmac->direction = direction;
+
+ if (sdma_alloc_bd(desc)) {
+ kfree(desc);
+ goto err_bd_out;
+ }
+
ret = sdma_load_context(sdmac);
if (ret)
- goto err_out;
+ goto err_bd_out;
if (num_periods > NUM_BD) {
dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
channel, num_periods, NUM_BD);
- goto err_out;
+ goto err_bd_out;
}
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
- goto err_out;
+ goto err_bd_out;
}
while (buf < buf_len) {
@@ -1385,7 +1449,7 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
bd->mode.count = period_len;
if (sdmac->word_size > DMA_SLAVE_BUSWIDTH_4_BYTES)
- goto err_out;
+ goto err_bd_out;
if (sdmac->word_size == DMA_SLAVE_BUSWIDTH_4_BYTES)
bd->mode.command = 0;
else
@@ -1408,10 +1472,10 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
i++;
}
- desc->num_bd = num_periods;
- sdma->channel_control[channel].current_bd_ptr = desc->bd_phys;
-
- return &sdmac->txdesc;
+ return vchan_tx_prep(&sdmac->vc, &desc->vd, flags);
+err_bd_out:
+ sdma_free_bd(desc);
+ kfree(desc);
err_out:
sdmac->status = DMA_ERROR;
return NULL;
@@ -1450,14 +1514,31 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
struct dma_tx_state *txstate)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
- struct sdma_desc *desc = sdmac->desc;
+ struct sdma_desc *desc;
u32 residue;
+ struct virt_dma_desc *vd;
+ enum dma_status ret;
+ unsigned long flags;
- if (sdmac->flags & IMX_DMA_SG_LOOP)
- residue = (desc->num_bd - desc->buf_ptail) *
- desc->period_len - desc->chn_real_count;
- else
- residue = desc->chn_count - desc->chn_real_count;
+ ret = dma_cookie_status(chan, cookie, txstate);
+ if (ret == DMA_COMPLETE || !txstate)
+ return ret;
+
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ vd = vchan_find_desc(&sdmac->vc, cookie);
+ if (vd) {
+ desc = to_sdma_desc(&vd->tx);
+ if (sdmac->flags & IMX_DMA_SG_LOOP)
+ residue = (desc->num_bd - desc->buf_ptail) *
+ desc->period_len - desc->chn_real_count;
+ else
+ residue = desc->chn_count - desc->chn_real_count;
+ } else if (sdmac->desc && sdmac->desc->vd.tx.cookie == cookie) {
+ residue = sdmac->desc->chn_count - sdmac->desc->chn_real_count;
+ } else {
+ residue = 0;
+ }
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
dma_set_tx_state(txstate, chan->completed_cookie, chan->cookie,
residue);
@@ -1468,10 +1549,12 @@ static enum dma_status sdma_tx_status(struct dma_chan *chan,
static void sdma_issue_pending(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
- struct sdma_engine *sdma = sdmac->sdma;
+ unsigned long flags;
- if (sdmac->status == DMA_IN_PROGRESS)
- sdma_enable_channel(sdma, sdmac->channel);
+ spin_lock_irqsave(&sdmac->vc.lock, flags);
+ if (vchan_issue_pending(&sdmac->vc) && !sdmac->desc)
+ sdma_start_desc(sdmac);
+ spin_unlock_irqrestore(&sdmac->vc.lock, flags);
}
#define SDMA_SCRIPT_ADDRS_ARRAY_SIZE_V1 34
@@ -1677,12 +1760,10 @@ static int sdma_init(struct sdma_engine *sdma)
for (i = 0; i < MAX_DMA_CHANNELS; i++)
writel_relaxed(0, sdma->regs + SDMA_CHNPRI_0 + i * 4);
- ret = sdma_request_channel(&sdma->channel[0]);
+ ret = sdma_request_channel0(sdma);
if (ret)
goto err_dma_alloc;
- sdma->bd0 = sdma->channel[0].desc->bd;
-
sdma_config_ownership(&sdma->channel[0], false, true, false);
/* Set Command Channel (Channel Zero) */
@@ -1843,20 +1924,15 @@ static int sdma_probe(struct platform_device *pdev)
sdmac->sdma = sdma;
spin_lock_init(&sdmac->lock);
- sdmac->chan.device = &sdma->dma_device;
- dma_cookie_init(&sdmac->chan);
sdmac->channel = i;
-
- tasklet_init(&sdmac->tasklet, mxc_sdma_handle_channel_normal,
- (unsigned long) sdmac);
+ sdmac->vc.desc_free = sdma_desc_free;
/*
* Add the channel to the DMAC list. Do not add channel 0 though
* because we need it internally in the SDMA driver. This also means
* that channel 0 in dmaengine counting matches sdma channel 1.
*/
if (i)
- list_add_tail(&sdmac->chan.device_node,
- &sdma->dma_device.channels);
+ vchan_init(&sdmac->vc, &sdma->dma_device);
}
ret = sdma_init(sdma);
@@ -1961,7 +2037,8 @@ static int sdma_remove(struct platform_device *pdev)
for (i = 0; i < MAX_DMA_CHANNELS; i++) {
struct sdma_channel *sdmac = &sdma->channel[i];
- tasklet_kill(&sdmac->tasklet);
+ tasklet_kill(&sdmac->vc.task);
+ sdma_free_chan_resources(&sdmac->vc.chan);
}
platform_set_drvdata(pdev, NULL);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 4/7] dmaengine: imx-sdma: remove useless 'lock' and 'enabled' in 'struct sdma_channel'
From: Robin Gong @ 2018-06-19 16:57 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
Since 'sdmac->vc.lock' and 'sdmac->desc' can be used as 'lock' and
'enabled' in 'struct sdma_channel sdmac', remove them.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 23 -----------------------
1 file changed, 23 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 86fa799..d1d3494 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -342,10 +342,8 @@ struct sdma_channel {
unsigned long event_mask[2];
unsigned long watermark_level;
u32 shp_addr, per_addr;
- spinlock_t lock;
enum dma_status status;
struct imx_dma_data data;
- bool enabled;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -606,14 +604,7 @@ static int sdma_config_ownership(struct sdma_channel *sdmac,
static void sdma_enable_channel(struct sdma_engine *sdma, int channel)
{
- unsigned long flags;
- struct sdma_channel *sdmac = &sdma->channel[channel];
-
writel(BIT(channel), sdma->regs + SDMA_H_START);
-
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = true;
- spin_unlock_irqrestore(&sdmac->lock, flags);
}
/*
@@ -731,14 +722,6 @@ static void sdma_update_channel_loop(struct sdma_channel *sdmac)
struct sdma_buffer_descriptor *bd;
int error = 0;
enum dma_status old_status = sdmac->status;
- unsigned long flags;
-
- spin_lock_irqsave(&sdmac->lock, flags);
- if (!sdmac->enabled) {
- spin_unlock_irqrestore(&sdmac->lock, flags);
- return;
- }
- spin_unlock_irqrestore(&sdmac->lock, flags);
/*
* loop mode. Iterate over descriptors, re-setup them and
@@ -1000,15 +983,10 @@ static int sdma_disable_channel(struct dma_chan *chan)
struct sdma_channel *sdmac = to_sdma_chan(chan);
struct sdma_engine *sdma = sdmac->sdma;
int channel = sdmac->channel;
- unsigned long flags;
writel_relaxed(BIT(channel), sdma->regs + SDMA_H_STATSTOP);
sdmac->status = DMA_ERROR;
- spin_lock_irqsave(&sdmac->lock, flags);
- sdmac->enabled = false;
- spin_unlock_irqrestore(&sdmac->lock, flags);
-
return 0;
}
@@ -1922,7 +1900,6 @@ static int sdma_probe(struct platform_device *pdev)
struct sdma_channel *sdmac = &sdma->channel[i];
sdmac->sdma = sdma;
- spin_lock_init(&sdmac->lock);
sdmac->channel = i;
sdmac->vc.desc_free = sdma_desc_free;
--
2.7.4
^ permalink raw reply related
* [PATCH v5 5/7] dmaengine: imx-sdma: remove the maximum limitation for bd numbers
From: Robin Gong @ 2018-06-19 16:57 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
No this limitation now after virtual dma used since bd is allocated
dynamically instead of static.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index d1d3494..9f1a462 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -285,7 +285,6 @@ struct sdma_context_data {
u32 scratch7;
} __attribute__ ((packed));
-#define NUM_BD (int)(PAGE_SIZE / sizeof(struct sdma_buffer_descriptor))
struct sdma_engine;
@@ -1290,13 +1289,6 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
if (ret)
goto err_bd_out;
- if (sg_len > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, sg_len, NUM_BD);
- ret = -EINVAL;
- goto err_bd_out;
- }
-
desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
@@ -1406,12 +1398,6 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
if (ret)
goto err_bd_out;
- if (num_periods > NUM_BD) {
- dev_err(sdma->dev, "SDMA channel %d: maximum number of sg exceeded: %d > %d\n",
- channel, num_periods, NUM_BD);
- goto err_bd_out;
- }
-
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
channel, period_len, 0xffff);
--
2.7.4
^ permalink raw reply related
* [PATCH v5 6/7] dmaengine: imx-sdma: add sdma_transfer_init to decrease code overlap
From: Robin Gong @ 2018-06-19 16:57 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
There are lot of codes overlap between prep_sg and prep_cyclic function.
Add sdma_transfer_init() function to elimated the code overlap.
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 83 ++++++++++++++++++++++----------------------------
1 file changed, 37 insertions(+), 46 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 9f1a462..f8becaf 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1248,6 +1248,40 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ahb);
}
+static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
+ enum dma_transfer_direction direction, u32 bds)
+{
+ struct sdma_desc *desc;
+
+ desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
+ if (!desc)
+ goto err_out;
+
+ sdmac->status = DMA_IN_PROGRESS;
+ sdmac->direction = direction;
+ sdmac->flags = 0;
+
+ desc->chn_count = 0;
+ desc->chn_real_count = 0;
+ desc->buf_tail = 0;
+ desc->buf_ptail = 0;
+ desc->sdmac = sdmac;
+ desc->num_bd = bds;
+
+ if (sdma_alloc_bd(desc))
+ goto err_desc_out;
+
+ if (sdma_load_context(sdmac))
+ goto err_desc_out;
+
+ return desc;
+
+err_desc_out:
+ kfree(desc);
+err_out:
+ return NULL;
+}
+
static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct dma_chan *chan, struct scatterlist *sgl,
unsigned int sg_len, enum dma_transfer_direction direction,
@@ -1260,36 +1294,13 @@ static struct dma_async_tx_descriptor *sdma_prep_slave_sg(
struct scatterlist *sg;
struct sdma_desc *desc;
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
- sdmac->status = DMA_IN_PROGRESS;
-
- sdmac->flags = 0;
-
- desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
+ desc = sdma_transfer_init(sdmac, direction, sg_len);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = sg_len;
- desc->chn_real_count = 0;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_out;
- }
-
dev_dbg(sdma->dev, "setting up %d entries for channel %d.\n",
sg_len, channel);
- sdmac->direction = direction;
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
-
- desc->chn_count = 0;
for_each_sg(sgl, sg, sg_len, i) {
struct sdma_buffer_descriptor *bd = &desc->bd[i];
int param;
@@ -1365,38 +1376,18 @@ static struct dma_async_tx_descriptor *sdma_prep_dma_cyclic(
struct sdma_engine *sdma = sdmac->sdma;
int num_periods = buf_len / period_len;
int channel = sdmac->channel;
- int ret, i = 0, buf = 0;
+ int i = 0, buf = 0;
struct sdma_desc *desc;
dev_dbg(sdma->dev, "%s channel: %d\n", __func__, channel);
- if (sdmac->status == DMA_IN_PROGRESS)
- return NULL;
-
- sdmac->status = DMA_IN_PROGRESS;
-
- desc = kzalloc((sizeof(*desc)), GFP_NOWAIT);
+ desc = sdma_transfer_init(sdmac, direction, num_periods);
if (!desc)
goto err_out;
- desc->buf_tail = 0;
- desc->buf_ptail = 0;
- desc->sdmac = sdmac;
- desc->num_bd = num_periods;
- desc->chn_real_count = 0;
desc->period_len = period_len;
sdmac->flags |= IMX_DMA_SG_LOOP;
- sdmac->direction = direction;
-
- if (sdma_alloc_bd(desc)) {
- kfree(desc);
- goto err_bd_out;
- }
-
- ret = sdma_load_context(sdmac);
- if (ret)
- goto err_bd_out;
if (period_len > 0xffff) {
dev_err(sdma->dev, "SDMA channel %d: maximum period size exceeded: %zu > %d\n",
--
2.7.4
^ permalink raw reply related
* [PATCH v5 7/7] dmaengine: imx-sdma: alloclate bd memory from dma pool
From: Robin Gong @ 2018-06-19 16:57 UTC (permalink / raw)
To: vkoul, s.hauer, l.stach, dan.j.williams, gregkh, jslaby
Cc: linux-serial, dmaengine, linux-kernel, linux-arm-kernel,
linux-imx
In-Reply-To: <1529427424-12321-1-git-send-email-yibin.gong@nxp.com>
dma_terminate_all maybe called in interrupt context which means
WARN_ON() will be triggered as below when bd memory freed. Allocat
bd memory from dma pool instead.
[ 29.161079] WARNING: CPU: 1 PID: 533 at ./include/linux/dma-mapping.h:541 sdma_free_bd+0xa4/0xb4
[ 29.169883] Modules linked in:
[ 29.172990] CPU: 1 PID: 533 Comm: mpegaudioparse0 Not tainted 4.18.0-rc1-next-20180618-00009-gf79f22c #20
[ 29.182597] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
[ 29.189163] Backtrace:
[ 29.191685] [<c010d1e0>] (dump_backtrace) from [<c010d4a0>] (show_stack+0x18/0x1c)
[ 29.199306] r7:00000000 r6:600f0093 r5:00000000 r4:c107db7c
[ 29.205029] [<c010d488>] (show_stack) from [<c0a5bba0>] (dump_stack+0xb4/0xe8)
[ 29.212312] [<c0a5baec>] (dump_stack) from [<c012703c>] (__warn+0x104/0x130)
[ 29.219411] r9:ec3e817c r8:0000021d r7:00000009 r6:c0d1d440 r5:00000000 r4:00000000
[ 29.227204] [<c0126f38>] (__warn) from [<c0127180>] (warn_slowpath_null+0x44/0x50)
[ 29.234821] r8:ed129dc4 r7:c0b01978 r6:c04d4e90 r5:0000021d r4:c0d1d440
[ 29.241574] [<c012713c>] (warn_slowpath_null) from [<c04d4e90>] (sdma_free_bd+0xa4/0xb4)
[ 29.249706] r6:4c001000 r5:f082e000 r4:00000024
[ 29.254376] [<c04d4dec>] (sdma_free_bd) from [<c04d4eb4>] (sdma_desc_free+0x14/0x20)
[ 29.262163] r7:ec3e8110 r6:00000100 r5:00000200 r4:ecf89a00
[ 29.267873] [<c04d4ea0>] (sdma_desc_free) from [<c04d229c>] (vchan_dma_desc_free_list+0xa4/0xac)
[ 29.276697] r5:00000200 r4:ed129d9c
[ 29.280326] [<c04d21f8>] (vchan_dma_desc_free_list) from [<c04d482c>] (sdma_disable_channel_with_delay+0x14c/0x188)
[ 29.290808] r9:ecae560c r8:ec3e815c r7:00000000 r6:c1008908 r5:ed129dc4 r4:ec3e8110
[ 29.298605] [<c04d46e0>] (sdma_disable_channel_with_delay) from [<c07c5c84>] (snd_dmaengine_pcm_trigger+0x90/0x1b0)
[ 29.309087] r8:ecae5000 r7:ec940800 r6:ed31bd80 r5:ecadb200 r4:ec26a700
[ 29.315855] [<c07c5bf4>] (snd_dmaengine_pcm_trigger) from [<c07dd800>] (soc_pcm_trigger+0xb4/0x130)
[ 29.324953] r8:ecae5000 r7:ec940800 r6:00000000 r5:ecadb200 r4:ec26a700
[ 29.331716] [<c07dd74c>] (soc_pcm_trigger) from [<c07bc008>] (snd_pcm_do_stop+0x58/0x5c)
[ 29.339859] r9:ecaed5a8 r8:ed31bdc0 r7:00000000 r6:00000001 r5:ecadb200 r4:c0b9c4d0
[ 29.347652] [<c07bbfb0>] (snd_pcm_do_stop) from [<c07bbde8>] (snd_pcm_action_single+0x40/0x80)
[ 29.356315] [<c07bbda8>] (snd_pcm_action_single) from [<c07bbf1c>] (snd_pcm_action+0xf4/0xfc)
[ 29.364883] r7:00000001 r6:c0b9c4d0 r5:ecadb2d4 r4:ecadb200
[ 29.370593] [<c07bbe28>] (snd_pcm_action) from [<c07bc8dc>] (snd_pcm_drop+0x58/0x9c)
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
---
drivers/dma/imx-sdma.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index f8becaf..7dab7e9 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -24,6 +24,7 @@
#include <linux/spinlock.h>
#include <linux/device.h>
#include <linux/dma-mapping.h>
+#include <linux/dmapool.h>
#include <linux/firmware.h>
#include <linux/slab.h>
#include <linux/platform_device.h>
@@ -343,6 +344,7 @@ struct sdma_channel {
u32 shp_addr, per_addr;
enum dma_status status;
struct imx_dma_data data;
+ struct dma_pool *bd_pool;
};
#define IMX_DMA_SG_LOOP BIT(0)
@@ -1153,11 +1155,10 @@ static int sdma_request_channel0(struct sdma_engine *sdma)
static int sdma_alloc_bd(struct sdma_desc *desc)
{
- u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
int ret = 0;
- desc->bd = dma_zalloc_coherent(NULL, bd_size, &desc->bd_phys,
- GFP_ATOMIC);
+ desc->bd = dma_pool_alloc(desc->sdmac->bd_pool, GFP_ATOMIC,
+ &desc->bd_phys);
if (!desc->bd) {
ret = -ENOMEM;
goto out;
@@ -1168,9 +1169,7 @@ static int sdma_alloc_bd(struct sdma_desc *desc)
static void sdma_free_bd(struct sdma_desc *desc)
{
- u32 bd_size = desc->num_bd * sizeof(struct sdma_buffer_descriptor);
-
- dma_free_coherent(NULL, bd_size, desc->bd, desc->bd_phys);
+ dma_pool_free(desc->sdmac->bd_pool, desc->bd, desc->bd_phys);
}
static void sdma_desc_free(struct virt_dma_desc *vd)
@@ -1218,6 +1217,10 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
if (ret)
goto disable_clk_ahb;
+ sdmac->bd_pool = dma_pool_create("bd_pool", chan->device->dev,
+ sizeof(struct sdma_buffer_descriptor),
+ 32, 0);
+
return 0;
disable_clk_ahb:
@@ -1246,6 +1249,9 @@ static void sdma_free_chan_resources(struct dma_chan *chan)
clk_disable(sdma->clk_ipg);
clk_disable(sdma->clk_ahb);
+
+ dma_pool_destroy(sdmac->bd_pool);
+ sdmac->bd_pool = NULL;
}
static struct sdma_desc *sdma_transfer_init(struct sdma_channel *sdmac,
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] serial: 8250_omap: Add compatible for AM654 UART
From: Nishanth Menon @ 2018-06-19 19:44 UTC (permalink / raw)
To: Mark Rutland, Rob Herring, Greg Kroah-Hartman
Cc: linux-kernel, devicetree, linux-serial, Nishanth Menon,
Tony Lindgren, Tero Kristo, Vignesh R, Jiri Slaby, Sekhar Nori
Hi,
This series was previously send out as part of a larger AM654 UART
support series, but was determined to create a bit of merge conflicts
due to interdependence.
Changes since RFC are indicated in corresponding patches in the series
Instead this series is split out. The series itself is based off
v4.18-rc1 and available here:
https://github.com/nmenon/linux-2.6-playground/commits/upstream/v4.18-rc1/k3-1-am6-uart
Consolidated all patches (including all series) are available here:
https://github.com/nmenon/linux-2.6-playground/commits/upstream/v4.18-rc1/k3-am6-integ
Full Boot log (integrated of all series) is available here:
https://pastebin.ubuntu.com/p/bBFmnzYtCd/
Nishanth Menon (2):
dt-bindings: serial: 8250_omap: Add compatible for AM654 UART
controller
serial: 8250_omap: Add support for AM654 UART controller
Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
drivers/tty/serial/8250/8250_omap.c | 1 +
2 files changed, 2 insertions(+)
--
2.15.1
^ permalink raw reply
* [PATCH 1/2] dt-bindings: serial: 8250_omap: Add compatible for AM654 UART controller
From: Nishanth Menon @ 2018-06-19 19:44 UTC (permalink / raw)
To: Mark Rutland, Rob Herring, Greg Kroah-Hartman
Cc: linux-kernel, devicetree, linux-serial, Nishanth Menon,
Tony Lindgren, Tero Kristo, Vignesh R, Jiri Slaby, Sekhar Nori
In-Reply-To: <20180619194450.6353-1-nm@ti.com>
AM654 uses a UART controller that is only partially compatible with
existing 8250 UART. UART DMA integration is substantially different
and even a match against standard 8250 or omap4 would result in
non-working UART once DMA is enabled by default.
Introduce a specific compatible to help build up the differences in
follow on patches.
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Vignesh R <vigneshr@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Changes since RFC:
* DT binding has been split out as it's own patch and commit message elaborated
RFC: https://patchwork.kernel.org/patch/10447641/
Documentation/devicetree/bindings/serial/omap_serial.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/serial/omap_serial.txt b/Documentation/devicetree/bindings/serial/omap_serial.txt
index 4b0f05adb228..c35d5ece1156 100644
--- a/Documentation/devicetree/bindings/serial/omap_serial.txt
+++ b/Documentation/devicetree/bindings/serial/omap_serial.txt
@@ -1,6 +1,7 @@
OMAP UART controller
Required properties:
+- compatible : should be "ti,am654-uart" for AM654 controllers
- compatible : should be "ti,omap2-uart" for OMAP2 controllers
- compatible : should be "ti,omap3-uart" for OMAP3 controllers
- compatible : should be "ti,omap4-uart" for OMAP4 controllers
--
2.15.1
^ permalink raw reply related
* [PATCH 2/2] serial: 8250_omap: Add support for AM654 UART controller
From: Nishanth Menon @ 2018-06-19 19:44 UTC (permalink / raw)
To: Mark Rutland, Rob Herring, Greg Kroah-Hartman
Cc: linux-kernel, devicetree, linux-serial, Nishanth Menon,
Tony Lindgren, Tero Kristo, Vignesh R, Jiri Slaby, Sekhar Nori
In-Reply-To: <20180619194450.6353-1-nm@ti.com>
AM654 uses a UART controller that is compatible (partially) with
existing 8250 UART, however, has a few differences with respect to DMA
support and control paths. Introduce a base definition that allows us
to build up the differences in follow on patches.
Cc: Sekhar Nori <nsekhar@ti.com>
Cc: Vignesh R <vigneshr@ti.com>
Signed-off-by: Nishanth Menon <nm@ti.com>
---
Changes since RFC: Driver change has been split out with corresponding description.
RFC: https://patchwork.kernel.org/patch/10447641/
drivers/tty/serial/8250/8250_omap.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/8250/8250_omap.c b/drivers/tty/serial/8250/8250_omap.c
index 1b337fee07ed..a019286f8bb6 100644
--- a/drivers/tty/serial/8250/8250_omap.c
+++ b/drivers/tty/serial/8250/8250_omap.c
@@ -1115,6 +1115,7 @@ static const u8 am3352_habit = OMAP_DMA_TX_KICK | UART_ERRATA_CLOCK_DISABLE;
static const u8 dra742_habit = UART_ERRATA_CLOCK_DISABLE;
static const struct of_device_id omap8250_dt_ids[] = {
+ { .compatible = "ti,am654-uart" },
{ .compatible = "ti,omap2-uart" },
{ .compatible = "ti,omap3-uart" },
{ .compatible = "ti,omap4-uart", .data = &omap4_habit, },
--
2.15.1
^ permalink raw reply related
* Re: [RFC][PATCH 0/6] Use printk_safe context for TTY and UART port locks
From: Linus Torvalds @ 2018-06-20 2:01 UTC (permalink / raw)
To: Petr Mladek
Cc: Sergey Senozhatsky, One Thousand Gnomes, Steven Rostedt,
Greg Kroah-Hartman, Jiri Slaby, Peter Zijlstra, Andrew Morton,
Dmitry Vyukov, Linux Kernel Mailing List, linux-serial,
SergeySenozhatsky
In-Reply-To: <20180619083021.4avsgvcqjrpkat6s@pathway.suse.cz>
On Tue, Jun 19, 2018 at 5:30 PM Petr Mladek <pmladek@suse.com> wrote:
>
> To make it clear. This patch set adds yet another spin_lock API.
> It behaves exactly as spin_lock_irqsafe()/spin_unlock_irqrestore()
> but in addition it sets printk_context.
>
> This patchset forces safe context around TTY and UART locks.
> In fact, the deferred context would be enough to prevent
> all the mentioned deadlocks.
Please stop this garbage.
The rule is simple: DO NOT DO THAT THEN.
Don't make recursive locks. Don't make random complexity. Just stop
doing the thing that hurts.
There is no valid reason why an UART driver should do a printk() of
any sort inside the critical region where the console is locked.
Just remove those printk's, don't add new crazy locking.
If you had a spinlock that deadlocked because it was inside an already
spinlocked region, you'd say "that's buggy".
This is the exact same issue. We don't work around buggy garbage. We
fix the bug - by removing the problematic printk.
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox