lkml.org 
[lkml]   [2019]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [patch v4 1/5] AST2500 DMA UART driver
On Fri, Jul 26, 2019 at 06:57:16PM +0530, sudheer.v wrote:
> From: sudheer veliseti <sudheer.open@gmail.com>
>
> UART driver for Aspeed's bmc chip AST2500
>
> Design approch:
> AST2500 has dedicated Uart DMA controller which has 12 sets of Tx and RX channels
> connected to UART controller directly.
> Since the DMA controller have dedicated buffers and registers,
> there would be little benifit in adding DMA framework overhead.
> So the software for DMA controller is included within the UART driver itself.
>
> implementation details:
> 'struct ast_uart_port' is populated and registered with uart_core.
> code is organised into two layers UART-layer and DMA-Layer,both of them are
> in the same file.UART-layer requests Rx and Tx dma channels
> and registers callbacks with DMA controller software Layer
> Interrupt service routine for DMA controller is the crucial one for Handling all
> the tx and rx data. ISRs installed for individual uarts are just dummy,and are helpful
> only to report any spurious interrupts in hardware.
>
>
> Signed-off-by: sudheer veliseti <sudheer.open@gmail.com>
> ---
>
> Changes from v3->v4:
> - per port uart structures are registerd directly with uart core
> Instead of registering through 8250 Frame work,
> ast_uart_port is registered using uart_add_one_port
> -SDMA_RX_FIX macro replaced with CONFIG_AST_UART_DMA_RX_INTERRUPT
> -ast_uart_sdma_isr : DMA interrupt handler code is improvised
> -replaced pr_debug with ftrace wherever appropriate
> -dev_err is used in all error return cases
> -uart driver structure ast25xx_uart_reg is modified
> -driver name changed to ast2500-uart-dma-drv
> -rx_timer initialisation and callback fn modified
>
> Changes from v2->v3:
> -custom debug replaced by in kerenl dynamic debug: pr_debug
> -change-logs added
>
>
> .../tty/serial/8250/8250_ast2500_uart_dma.c | 1901 +++++++++++++++++
> 1 file changed, 1901 insertions(+)
> create mode 100644 drivers/tty/serial/8250/8250_ast2500_uart_dma.c
>
> diff --git a/drivers/tty/serial/8250/8250_ast2500_uart_dma.c b/drivers/tty/serial/8250/8250_ast2500_uart_dma.c
> new file mode 100644
> index 000000000000..bc830d605372
> --- /dev/null
> +++ b/drivers/tty/serial/8250/8250_ast2500_uart_dma.c
> @@ -0,0 +1,1901 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMA UART Driver for ASPEED BMC chip: AST2500
> + *
> + * Copyright (C) 2019 sudheer Kumar veliseti, Aspeed technology Inc.
> + * <open.sudheer@gmail.com>

What was the copyright on the file you copied? Please properly
attribute that here.


> + *
> + */
> +#include <linux/clk.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/tty.h>
> +#include <linux/tty_flip.h>
> +#include "8250.h"
> +
> +#define SERIAL8250_CONSOLE NULL
> +#define TTY_AST_MAJOR 204
> +#define TTY_AST_MINOR 68

Where did you get this minor number from?

> +
> +#define DMA_BUFF_SIZE 0x1000
> +#define SDMA_RX_BUFF_SIZE 0x10000
> +#define PASS_LIMIT 256
> +#define UART_DMA_NR CONFIG_AST_NR_DMA_UARTS
> +#define AST_UART_SDMA_CH 12
> +
> +/* enum ast_uart_chan_op
> + * operation codes passed to the DMA code by the user, and also used
> + * to inform the current channel owner of any changes to the system state
> + */
> +enum ast_uart_chan_op {
> + AST_UART_DMAOP_TRIGGER,
> + AST_UART_DMAOP_STOP,
> + AST_UART_DMAOP_PAUSE,
> +};
> +
> +/* ast_uart_dma_cbfn: buffer callback routinei type */
> +typedef void (*ast_uart_dma_cbfn)(void *dev_id, u16 len);
> +
> +struct ast_sdma_info {
> + u8 ch_no;
> + u8 direction;
> + u8 enable;
> + void *priv;
> + char *sdma_virt_addr;
> + dma_addr_t dma_phy_addr;
> + /* cdriver callbacks */
> + ast_uart_dma_cbfn callback_fn; /* buffer done callback */
> +};
> +
> +struct ast_sdma_ch {
> + struct ast_sdma_info tx_dma_info[AST_UART_SDMA_CH];
> + struct ast_sdma_info rx_dma_info[AST_UART_SDMA_CH];
> +};
> +
> +struct ast_sdma {
> + void __iomem *reg_base;
> + int dma_irq;
> + struct ast_sdma_ch *dma_ch;
> + struct regmap *map;
> +};
> +
> +#define UART_TX_SDMA_EN 0x00
> +#define UART_RX_SDMA_EN 0x04
> +#define UART_SDMA_CONF 0x08 /* Misc, Buffer size */
> +#define UART_SDMA_TIMER 0x0C
> +#define UART_TX_SDMA_REST 0x20
> +#define UART_RX_SDMA_REST 0x24
> +#define UART_TX_SDMA_IER 0x30
> +#define UART_TX_SDMA_ISR 0x34
> +#define UART_RX_SDMA_IER 0x38
> +#define UART_RX_SDMA_ISR 0x3C
> +#define UART_TX_R_POINT(x) (0x40 + ((x) * 0x20))
> +#define UART_TX_W_POINT(x) (0x44 + ((x) * 0x20))
> +#define UART_TX_SDMA_ADDR(x) (0x48 + ((x) * 0x20))
> +#define UART_RX_R_POINT(x) (0x50 + ((x) * 0x20))
> +#define UART_RX_W_POINT(x) (0x54 + ((x) * 0x20))
> +#define UART_RX_SDMA_ADDR(x) (0x58 + ((x) * 0x20))
> +#define SDMA_CH_EN(x) BIT(x)
> +
> +#define SDMA_TX_BUFF_SIZE_MASK (0x3)
> +#define SDMA_SET_TX_BUFF_SIZE(x)(x)
> +#define SDMA_BUFF_SIZE_1KB (0x0)
> +#define SDMA_BUFF_SIZE_4KB (0x1)
> +#define SDMA_BUFF_SIZE_16KB (0x2)
> +#define SDMA_BUFF_SIZE_64KB (0x3)
> +#define SDMA_RX_BUFF_SIZE_MASK (0x3 << 2)
> +#define SDMA_SET_RX_BUFF_SIZE(x)((x) << 2)
> +#define SDMA_TIMEOUT_DIS BIT(4)
> +
> +#define UART_SDMA11_INT BIT(11)
> +#define UART_SDMA10_INT BIT(10)
> +#define UART_SDMA9_INT BIT(9)
> +#define UART_SDMA8_INT BIT(8)
> +#define UART_SDMA7_INT BIT(7)
> +#define UART_SDMA6_INT BIT(6)
> +#define UART_SDMA5_INT BIT(5)
> +#define UART_SDMA4_INT BIT(4)
> +#define UART_SDMA3_INT BIT(3)
> +#define UART_SDMA2_INT BIT(2)
> +#define UART_SDMA1_INT BIT(1)
> +#define UART_SDMA0_INT BIT(0)
> +
> +/*
> + * Configuration:
> + * share_irqs - whether we pass IRQF_SHARED to request_irq().
> + * This option is unsafe when used on edge-triggered interrupts.
> + */
> +static unsigned int share_irqs = SERIAL8250_SHARE_IRQS;
> +
> +static unsigned int nr_uarts = CONFIG_AST_RUNTIME_DMA_UARTS;
> +
> +struct ast_uart_port {
> + struct uart_port port;
> + unsigned short capabilities; /* port capabilities */
> + unsigned short bugs; /* port bugs */
> + unsigned int tx_loadsz; /* transmit fifo load size */
> + unsigned char acr;
> + unsigned char ier;
> + unsigned char lcr;
> + unsigned char mcr;
> + unsigned char mcr_mask; /* mask of user bits */
> + unsigned char mcr_force; /* mask of forced bits */
> + struct circ_buf rx_dma_buf;
> + struct circ_buf tx_dma_buf;
> + unsigned char dma_channel;
> + dma_addr_t dma_rx_addr; /* Mapped ADMA descr. table */
> + dma_addr_t dma_tx_addr; /* Mapped ADMA descr. table */
> +#ifdef CONFIG_AST_UART_DMA_RX_INTERRUPT
> + struct tasklet_struct rx_tasklet;
> +#else
> + struct timer_list rx_timer;
> + unsigned int workaround;
> +#endif
> + struct tasklet_struct tx_tasklet;
> + spinlock_t lock;
> + int tx_done;
> + int tx_count;
> + struct platform_device *ast_uart_pdev;
> +/*
> + * Some bits in registers are cleared on a read, so they must
> + * be saved whenever the register is read but the bits will not
> + * be immediately processed.
> + */
> +#define LSR_SAVE_FLAGS UART_LSR_BRK_ERROR_BITS
> + unsigned char lsr_saved_flags;
> +#define MSR_SAVE_FLAGS UART_MSR_ANY_DELTA
> + unsigned char msr_saved_flags;
> +
> + /*
> + * We provide a per-port pm hook.
> + */
> + void (*pm)(struct uart_port *port, unsigned int state,
> + unsigned int old);
> +};
> +
> +static struct ast_uart_port ast_uart_ports[UART_DMA_NR];
> +
> +#define GET_DEV(ast_uart_port_priv_ptr)\
> + (ast_uart_port_priv_ptr->ast_uart_pdev->dev)
> +
> +static inline struct ast_uart_port *
> +to_ast_dma_uart_port(struct uart_port *uart) {
> + return container_of(uart, struct ast_uart_port, port);
> +}
> +
> +struct irq_info {
> + spinlock_t lock;
> + struct ast_uart_port *up;
> +};
> +
> +static struct irq_info ast_uart_irq[1];
> +static DEFINE_MUTEX(ast_uart_mutex);
> +
> +/*
> + * Here we define the default xmit fifo size used for each type of UART.
> + */
> +static const struct serial8250_config uart_config[] = {
> + [PORT_UNKNOWN] = {
> + .name = "unknown",
> + .fifo_size = 1,
> + .tx_loadsz = 1,
> + },
> + [PORT_8250] = {
> + .name = "8250",
> + .fifo_size = 1,
> + .tx_loadsz = 1,
> + },
> + [PORT_16450] = {
> + .name = "16450",
> + .fifo_size = 1,
> + .tx_loadsz = 1,
> + },
> + [PORT_16550] = {
> + .name = "16550",
> + .fifo_size = 1,
> + .tx_loadsz = 1,
> + },
> + [PORT_16550A] = {
> + .name = "16550A",
> + .fifo_size = 16,
> + .tx_loadsz = 16,
> + .fcr = UART_FCR_ENABLE_FIFO | UART_FCR_R_TRIG_10
> + | UART_FCR_DMA_SELECT,
> + .flags = UART_CAP_FIFO,
> + },
> +};

I doubt you need all of these port types, right? You only have one type
of device, please strip out _ALL_ of the unneeded code in here. You did
a wholesale copy of the old driver, to get away with that you then need
to customize it to work properly with your hardware _AND_ take away all
code that is not needed for your hardware.

Lots of this file can be removed, please do so.

thanks,

greg k-h

\
 
 \ /
  Last update: 2019-07-30 17:49    [W:0.077 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site