From: Arnd Bergmann <arnd@arndb.de>
To: linux-arm-kernel@lists.infradead.org
Cc: Yuan Yao <yao.yuan@freescale.com>,
gregkh@linuxfoundation.org, linux-serial@vger.kernel.org
Subject: Re: [PATCH] the eDMA support for the LPUART send driver
Date: Sun, 5 Jan 2014 15:44:49 +0100 [thread overview]
Message-ID: <201401051544.50036.arnd@arndb.de> (raw)
In-Reply-To: <1388143449-28640-1-git-send-email-yao.yuan@freescale.com>
On Friday 27 December 2013, Yuan Yao wrote:
> This patch add eDMA support for LPUART send function.
>
> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> ---
> arch/arm/boot/dts/vf610.dtsi | 12 +++
> drivers/tty/serial/fsl_lpuart.c | 187 ++++++++++++++++++++++++++++++++--------
> 2 files changed, 163 insertions(+), 36 deletions(-)
Not sure if this got applied already, but you are missing the respective
change to Documentation/devicetree/bindings/serial/fsl-lpuart.txt.
Please document the dma-names you use here. While you are at it, please
also add the "ipg" clock-name.
Arnd
> +static int fsl_request_dma(struct uart_port *port)
> +{
> + struct lpuart_port *sport = container_of(port,
> + struct lpuart_port, port);
> + struct dma_chan *tx_chan;
> + struct dma_slave_config dma_tx_sconfig;
> + dma_addr_t dma_phys;
> + unsigned char *dma_buf;
> + int ret;
> +
> + tx_chan = dma_request_slave_channel(sport->port.dev, "lpuart-tx");
> +
> + if (!tx_chan) {
> + dev_err(sport->port.dev, "Dma TX channel request failed!\n");
> + return -ENODEV;
> + }
> +
> + dma_phys = dma_map_single(sport->port.dev,
> + sport->port.state->xmit.buf,
> + UART_XMIT_SIZE, DMA_TO_DEVICE);
This is wrong: Since the dma is performed by the dma engine rather
than the uart, the first argument here needs to be the dma device
pointer. In fact, dma_map_single is normally supposed to fail on
the uart device as the dma_mask value should be zero. Not sure
why this worked.
> + if (!dma_phys) {
> + dev_err(sport->port.dev, "Dma_phys single failed\n");
> + return -ENOMEM;
> + }
Please also change all references to "phys" -- it's not a phys
address but a bus address. These are often the same, but that's
not for the driver to know.
Arnd
WARNING: multiple messages have this Message-ID (diff)
From: arnd@arndb.de (Arnd Bergmann)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] the eDMA support for the LPUART send driver
Date: Sun, 5 Jan 2014 15:44:49 +0100 [thread overview]
Message-ID: <201401051544.50036.arnd@arndb.de> (raw)
In-Reply-To: <1388143449-28640-1-git-send-email-yao.yuan@freescale.com>
On Friday 27 December 2013, Yuan Yao wrote:
> This patch add eDMA support for LPUART send function.
>
> Signed-off-by: Yuan Yao <yao.yuan@freescale.com>
> ---
> arch/arm/boot/dts/vf610.dtsi | 12 +++
> drivers/tty/serial/fsl_lpuart.c | 187 ++++++++++++++++++++++++++++++++--------
> 2 files changed, 163 insertions(+), 36 deletions(-)
Not sure if this got applied already, but you are missing the respective
change to Documentation/devicetree/bindings/serial/fsl-lpuart.txt.
Please document the dma-names you use here. While you are at it, please
also add the "ipg" clock-name.
Arnd
> +static int fsl_request_dma(struct uart_port *port)
> +{
> + struct lpuart_port *sport = container_of(port,
> + struct lpuart_port, port);
> + struct dma_chan *tx_chan;
> + struct dma_slave_config dma_tx_sconfig;
> + dma_addr_t dma_phys;
> + unsigned char *dma_buf;
> + int ret;
> +
> + tx_chan = dma_request_slave_channel(sport->port.dev, "lpuart-tx");
> +
> + if (!tx_chan) {
> + dev_err(sport->port.dev, "Dma TX channel request failed!\n");
> + return -ENODEV;
> + }
> +
> + dma_phys = dma_map_single(sport->port.dev,
> + sport->port.state->xmit.buf,
> + UART_XMIT_SIZE, DMA_TO_DEVICE);
This is wrong: Since the dma is performed by the dma engine rather
than the uart, the first argument here needs to be the dma device
pointer. In fact, dma_map_single is normally supposed to fail on
the uart device as the dma_mask value should be zero. Not sure
why this worked.
> + if (!dma_phys) {
> + dev_err(sport->port.dev, "Dma_phys single failed\n");
> + return -ENOMEM;
> + }
Please also change all references to "phys" -- it's not a phys
address but a bus address. These are often the same, but that's
not for the driver to know.
Arnd
next prev parent reply other threads:[~2014-01-05 14:45 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-27 11:24 [PATCH] the eDMA support for the LPUART send driver Yuan Yao
2013-12-27 11:24 ` Yuan Yao
2014-01-05 14:44 ` Arnd Bergmann [this message]
2014-01-05 14:44 ` Arnd Bergmann
2014-01-05 14:50 ` Russell King - ARM Linux
2014-01-05 14:50 ` Russell King - ARM Linux
2014-01-07 6:32 ` Yao Yuan
2014-01-07 6:32 ` Yao Yuan
2014-01-07 8:53 ` Arnd Bergmann
2014-01-07 8:53 ` Arnd Bergmann
2014-01-07 9:49 ` Yao Yuan
2014-01-07 9:49 ` Yao Yuan
2014-01-06 5:37 ` Shawn Guo
2014-01-06 5:37 ` Shawn Guo
2014-01-07 9:44 ` Yao Yuan
2014-01-07 9:44 ` Yao Yuan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=201401051544.50036.arnd@arndb.de \
--to=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-serial@vger.kernel.org \
--cc=yao.yuan@freescale.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.