| Message ID | 20220310161650.289387-6-miquel.raynal@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | RZN1 UART DMA support | expand |
On Thu, Mar 10, 2022 at 05:16:48PM +0100, Miquel Raynal wrote: > From: Phil Edworthy <phil.edworthy@renesas.com> > > The CPR register can give the information whether the IP is DMA capable > or not. Let's extract this information and use it to discriminate when > the DMA can be hooked up or not. ... > + /* If we have a valid fifosize and DMA support, try hooking up DMA */ > + if (p->fifosize && data->dma_capable) { > + if (reg & DW_UART_CPR_DMA_EXTRA) > + data->dma_capable = 1; How many designs will be broken by this change? ... > + unsigned int dma_capable:1; Note, we use up->dma == NULL for no-DMA, no additional flag is needed. Just make sure that for your platform you enable DMA by filling that.
On Thu, Mar 10, 2022 at 08:13:51PM +0100, Miquel Raynal wrote: > andriy.shevchenko@linux.intel.com wrote on Thu, 10 Mar 2022 20:06:25 > +0200: > > On Thu, Mar 10, 2022 at 05:16:48PM +0100, Miquel Raynal wrote: > > > From: Phil Edworthy <phil.edworthy@renesas.com> ... > > > + /* If we have a valid fifosize and DMA support, try hooking up DMA */ > > > + if (p->fifosize && data->dma_capable) { > > > > > + if (reg & DW_UART_CPR_DMA_EXTRA) > > > + data->dma_capable = 1; > > > > How many designs will be broken by this change? > > My understanding was that CPR registers where always synthesized until > now even though it was not mandatory and that the RZN1 SoC was the > first one to not embed it. My hope was that people using this driver > would have brought "external" CPR support earlier if they needed it, > but I understand this assumption might be wrong. > > Anyway, I also hesitated to do something more custom for the RZN1 I'll > try something else. My point is that you have put this requirement to the above conditionals. Have you checked all supported platforms that announce CPR that that bit is set when DMA is indeed in use? AFAIK on Intel SoCs the native UART DMA signalling is not used, the integration between DMA and UART is done differently because it requires more signals to connect. It might be that I misread the documentation and this is not the case and we indeed set that bit as well. Also, what to do with the platforms that have no CPR but support DMA currently? ... > > > + unsigned int dma_capable:1; > > > > Note, we use up->dma == NULL for no-DMA, no additional flag is needed. > > Just make sure that for your platform you enable DMA by filling that. > > dma_capable is just a capability the SoC has. It was discovered at > probe time and should be saved to know, later, if DMA can be hooked up > or not. At the time we look at the CPR register we don't yet have DMA > fields populated so its too early to set up->dma to NULL. 'up->dma == NULL' check will tell you that, no?
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index 1f7a423d6ef2..c0f54284bc70 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -556,8 +556,8 @@ static int dw8250_probe(struct platform_device *pdev) if (!data->skip_autocfg) dw8250_setup_port(p); - /* If we have a valid fifosize, try hooking up DMA */ - if (p->fifosize) { + /* If we have a valid fifosize and DMA support, try hooking up DMA */ + if (p->fifosize && data->dma_capable) { data->data.dma.rxconf.src_maxburst = p->fifosize / 4; data->data.dma.txconf.dst_maxburst = p->fifosize / 4; up->dma = &data->data.dma; diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c index 5cf298c5a0f9..5ec7c12ed117 100644 --- a/drivers/tty/serial/8250/8250_dwlib.c +++ b/drivers/tty/serial/8250/8250_dwlib.c @@ -92,6 +92,8 @@ void dw8250_setup_port(struct uart_port *p) { struct uart_8250_port *up = up_to_u8250p(p); const struct dw8250_platform_data *plat = of_device_get_match_data(up->port.dev); + struct dw8250_port_data *d = p->private_data; + struct dw8250_data *data = container_of(d, struct dw8250_data, data); u32 reg; /* @@ -110,8 +112,6 @@ void dw8250_setup_port(struct uart_port *p) dw8250_writel_ext(p, DW_UART_DLF, 0); if (reg) { - struct dw8250_port_data *d = p->private_data; - d->dlf_size = fls(reg); p->get_divisor = dw8250_get_divisor; p->set_divisor = dw8250_set_divisor; @@ -138,5 +138,8 @@ void dw8250_setup_port(struct uart_port *p) if (reg & DW_UART_CPR_SIR_MODE) up->capabilities |= UART_CAP_IRDA; + + if (reg & DW_UART_CPR_DMA_EXTRA) + data->dma_capable = 1; } EXPORT_SYMBOL_GPL(dw8250_setup_port); diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h index ffce2744a28e..900b2321aff0 100644 --- a/drivers/tty/serial/8250/8250_dwlib.h +++ b/drivers/tty/serial/8250/8250_dwlib.h @@ -34,6 +34,7 @@ struct dw8250_data { unsigned int skip_autocfg:1; unsigned int uart_16550_compatible:1; + unsigned int dma_capable:1; }; void dw8250_do_set_termios(struct uart_port *p, struct ktermios *termios, struct ktermios *old);