| Message ID | 1531312174-17489-3-git-send-email-phil.edworthy@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
Hi Phil, On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy <phil.edworthy@renesas.com> wrote: > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has additional > registers for DMA. This patch does not address the changes required for DMA > support, it simply adds the compatible string. > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> Thanks for your patch! What happens if someone would boot a kernel that has only this patch applied and a DTB that already has the to-be-supported dmas properties? > --- a/drivers/tty/serial/8250/8250_dw.c > +++ b/drivers/tty/serial/8250/8250_dw.c > @@ -693,6 +693,7 @@ static const struct of_device_id dw8250_of_match[] = { > { .compatible = "snps,dw-apb-uart" }, > { .compatible = "cavium,octeon-3860-uart" }, > { .compatible = "marvell,armada-38x-uart" }, > + { .compatible = "renesas,uart-rzn1" }, renesas,rzn1-uart > { /* Sentinel */ } Gr{oetje,eeting}s, Geert
Hi Geert, On 11 July 2018 13:42, Geert Uytterhoeven wrote: > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has > > additional registers for DMA. This patch does not address the changes > > required for DMA support, it simply adds the compatible string. > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > Thanks for your patch! > > What happens if someone would boot a kernel that has only this patch > applied and a DTB that already has the to-be-supported dmas properties? The driver only sets up dma if the fifo depth is >0, and this is read from CPR register. This is an optional (as in RTL configuration) register for the Synopsys UART block. On rzn1 devices, this register returns 0, so the driver will not set up dma => so the uart still works. Additionally, the uart normally used for the console (used because the BootROM uses it) does not have any dma capabilities. Thanks Phil > > --- a/drivers/tty/serial/8250/8250_dw.c > > +++ b/drivers/tty/serial/8250/8250_dw.c > > @@ -693,6 +693,7 @@ static const struct of_device_id dw8250_of_match[] > = { > > { .compatible = "snps,dw-apb-uart" }, > > { .compatible = "cavium,octeon-3860-uart" }, > > { .compatible = "marvell,armada-38x-uart" }, > > + { .compatible = "renesas,uart-rzn1" }, > > renesas,rzn1-uart > > > { /* Sentinel */ } > > Gr{oetje,eeting}s, > > Geert > > -- > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux- > m68k.org > > In personal conversations with technical people, I call myself a hacker. But > when I'm talking to journalists I just say "programmer" or something like that. > -- Linus Torvalds
Hi Geert, On 11 July 2018 16:02, Phil Edworthy wrote: > On 11 July 2018 13:42, Geert Uytterhoeven wrote: > > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has > > > additional registers for DMA. This patch does not address the > > > changes required for DMA support, it simply adds the compatible string. > > > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > > > Thanks for your patch! > > [...] > > > --- a/drivers/tty/serial/8250/8250_dw.c > > > +++ b/drivers/tty/serial/8250/8250_dw.c > > > @@ -693,6 +693,7 @@ static const struct of_device_id > > > dw8250_of_match[] > > = { > > > { .compatible = "snps,dw-apb-uart" }, > > > { .compatible = "cavium,octeon-3860-uart" }, > > > { .compatible = "marvell,armada-38x-uart" }, > > > + { .compatible = "renesas,uart-rzn1" }, > > > > renesas,rzn1-uart I missed this comment. I followed the renesas convention of renesas,peripheral-device that is used for most R-Car drivers, but happy to change. Thanks Phil
Hi Phil, On Thu, Jul 12, 2018 at 2:03 PM Phil Edworthy <phil.edworthy@renesas.com> wrote: > On 11 July 2018 16:02, Phil Edworthy wrote: > > On 11 July 2018 13:42, Geert Uytterhoeven wrote: > > > On Wed, Jul 11, 2018 at 2:30 PM Phil Edworthy wrote: > > > > The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has > > > > additional registers for DMA. This patch does not address the > > > > changes required for DMA support, it simply adds the compatible string. > > > > > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> > > > > > > Thanks for your patch! > > > > [...] > > > > --- a/drivers/tty/serial/8250/8250_dw.c > > > > +++ b/drivers/tty/serial/8250/8250_dw.c > > > > @@ -693,6 +693,7 @@ static const struct of_device_id > > > > dw8250_of_match[] > > > = { > > > > { .compatible = "snps,dw-apb-uart" }, > > > > { .compatible = "cavium,octeon-3860-uart" }, > > > > { .compatible = "marvell,armada-38x-uart" }, > > > > + { .compatible = "renesas,uart-rzn1" }, > > > > > > renesas,rzn1-uart > I missed this comment. I followed the renesas convention of renesas,peripheral-device > that is used for most R-Car drivers, but happy to change. "renesas,<peripheral>-<family-or-device>" is the old scheme, which we cannot change for existing devices due to backwards compatibility. "renesas,<family-or-device>-<peripheral>" is the "new" scheme, consistent with other vendors. Gr{oetje,eeting}s, Geert
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index aff04f1..c92f5ad 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -693,6 +693,7 @@ static const struct of_device_id dw8250_of_match[] = { { .compatible = "snps,dw-apb-uart" }, { .compatible = "cavium,octeon-3860-uart" }, { .compatible = "marvell,armada-38x-uart" }, + { .compatible = "renesas,uart-rzn1" }, { /* Sentinel */ } }; MODULE_DEVICE_TABLE(of, dw8250_of_match);
The Renesas RZ/N1 UART is based on the Synopsys DW UART, but has additional registers for DMA. This patch does not address the changes required for DMA support, it simply adds the compatible string. Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com> --- drivers/tty/serial/8250/8250_dw.c | 1 + 1 file changed, 1 insertion(+)