| Message ID | 20220310161650.289387-4-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | RZN1 UART DMA support | expand |
On Thu, Mar 10, 2022 at 05:16:46PM +0100, Miquel Raynal wrote: > From: Phil Edworthy <phil.edworthy@renesas.com> > > This UART controller can be synthesized without the CPR register. > In that case, let's use the platform information to provide a CPR value. ... > +#include <linux/of_device.h> > + const struct dw8250_platform_data *plat = of_device_get_match_data(up->port.dev); No. Please use device property APIs.
Hi Andy, andriy.shevchenko@linux.intel.com wrote on Thu, 10 Mar 2022 20:02:41 +0200: > On Thu, Mar 10, 2022 at 05:16:46PM +0100, Miquel Raynal wrote: > > From: Phil Edworthy <phil.edworthy@renesas.com> > > > > This UART controller can be synthesized without the CPR register. > > In that case, let's use the platform information to provide a CPR value. > > ... > > > +#include <linux/of_device.h> > > > + const struct dw8250_platform_data *plat = of_device_get_match_data(up->port.dev); > > No. Please use device property APIs. Are you suggesting the use of CPR DT property? If yes, what is the point if there is one CPR per SoC? I would argue that DT description is already quite complex and supporting one or another register is up to the OS as long as we can map CPR registers to SoCs? TBH Phil initially introduced a DT property which I turned into a pdata entry because when I can avoid playing with the bindings, I generally do so. Thanks, Miquèl
On Thu, Mar 10, 2022 at 08:01:01PM +0100, Miquel Raynal wrote: > andriy.shevchenko@linux.intel.com wrote on Thu, 10 Mar 2022 20:02:41 > +0200: > > On Thu, Mar 10, 2022 at 05:16:46PM +0100, Miquel Raynal wrote: > > > From: Phil Edworthy <phil.edworthy@renesas.com> ... > > > +#include <linux/of_device.h> > > > > > + const struct dw8250_platform_data *plat = of_device_get_match_data(up->port.dev); > > > > No. Please use device property APIs. > > Are you suggesting the use of CPR DT property? If yes, what is the > point if there is one CPR per SoC? I would argue that DT description is > already quite complex and supporting one or another register is up to > the OS as long as we can map CPR registers to SoCs? I'm suggesting to use device property APIs, I'm not talking about ABI. In this case instead of above you may simply do #include <linux/property.h> const struct dw8250_platform_data *plat = device_get_match_data(up->port.dev);
diff --git a/drivers/tty/serial/8250/8250_dwlib.c b/drivers/tty/serial/8250/8250_dwlib.c index 622d3b0d89e7..5cf298c5a0f9 100644 --- a/drivers/tty/serial/8250/8250_dwlib.c +++ b/drivers/tty/serial/8250/8250_dwlib.c @@ -3,6 +3,7 @@ #include <linux/bitops.h> #include <linux/device.h> +#include <linux/of_device.h> #include <linux/io.h> #include <linux/kernel.h> #include <linux/serial_8250.h> @@ -90,6 +91,7 @@ EXPORT_SYMBOL_GPL(dw8250_do_set_termios); 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); u32 reg; /* @@ -116,8 +118,12 @@ void dw8250_setup_port(struct uart_port *p) } reg = dw8250_readl_ext(p, DW_UART_CPR); - if (!reg) - return; + if (!reg) { + if (!plat) + return; + + reg = plat->cpr; + } /* Select the type based on FIFO */ if (reg & DW_UART_CPR_FIFO_MODE) { diff --git a/drivers/tty/serial/8250/8250_dwlib.h b/drivers/tty/serial/8250/8250_dwlib.h index ef63eaf7e598..ffce2744a28e 100644 --- a/drivers/tty/serial/8250/8250_dwlib.h +++ b/drivers/tty/serial/8250/8250_dwlib.h @@ -16,6 +16,10 @@ struct dw8250_port_data { u8 dlf_size; }; +struct dw8250_platform_data { + u32 cpr; +}; + struct dw8250_data { struct dw8250_port_data data;