Re: armv7/arm64: enable fifo in com_fdt for "snps,dw-apb-uart"

[Artturi Alm]

On Mon, Nov 13, 2017 at 10:02:35PM +0100, Mark Kettenis wrote:
> > Date: Fri, 10 Nov 2017 00:16:26 +0200
> > From: Artturi Alm 
> > 
> > Hi,
> > 
> > does also remove ti,omapX-uart compatibles from arm64 com_fdt,
> > as they're never to be found there.
> > iirc. the lenght is based on minimal lenght i found from any datasheet
> > for SoCs w/"snps,dw-apb-uart", and conclusions drawn from gpl-sources.
> 
> As far as I can tell the FIFO functionality is optional for the
> Synopys "IP".  It's just safer to not bother with the FIFO.
> 

Well, we're setting up the busy interrupt handling w/o consulting DT too.
i would be more worried of running into that, than finding this UART w/o
fifo enabled, when even the cheapest cheapshit SoCs got this fifo(=sunxi).
See "snps,uart-16550-compatible" in snps-dw-apb-uart.txt if you care, i don't.

diff below has safer version for enabling the fifo i have ran w/before com_fdt.
-Artturi


diff --git a/sys/arch/arm64/dev/com_fdt.c b/sys/arch/arm64/dev/com_fdt.c
index 9f2a8eb760a..30234188c82 100644
--- a/sys/arch/arm64/dev/com_fdt.c
+++ b/sys/arch/arm64/dev/com_fdt.c
@@ -35,10 +35,15 @@
 #include 
 #include 
 
-#define com_usr 31 /* Synopsys DesignWare UART */
+/* Synopsys DesignWare UART specific registers */
+#definecom_usr 31  /* UART Status Register */
+#definecom_cpr 244 /* UART Component Paramter Register */
+#defineCPR_FIFOSZ(x)   (((x) & 0x00ff) >> 12)
+#definecom_cvr 248 /* UART Component Version Register */
 
 intcom_fdt_match(struct device *, void *, void *);
 void   com_fdt_attach(struct device *, struct device *, void *);
+intcom_fdt_fifo_designware(struct com_softc *);
 intcom_fdt_intr_designware(void *);
 
 struct com_fdt_softc {
@@ -155,6 +160,12 @@ com_fdt_attach(struct device *parent, struct device *self, 
void *aux)
return;
}
 
+   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
+   sc->sc.sc_fifolen = com_fdt_fifo_designware(>sc);
+   if (sc->sc.sc_fifolen > 0)
+   sc->sc.sc_uarttype = COM_UART_16550A;
+   }
+
pinctrl_byname(faa->fa_node, "default");
 
com_attach_subr(>sc);
@@ -163,6 +174,15 @@ com_fdt_attach(struct device *parent, struct device *self, 
void *aux)
sc, sc->sc.sc_dev.dv_xname);
 }
 
+int
+com_fdt_fifo_designware(struct com_softc *sc)
+{
+   if (bus_space_read_4(sc->sc_iot, sc->sc_ioh, com_cvr) == 0)
+   return 0;
+
+   return CPR_FIFOSZ(bus_space_read_4(sc->sc_iot, sc->sc_ioh, com_cpr));
+}
+
 int
 com_fdt_intr_designware(void *cookie)
 {
diff --git a/sys/arch/armv7/dev/com_fdt.c b/sys/arch/armv7/dev/com_fdt.c
index 00504801850..7fe0f2bebdc 100644
--- a/sys/arch/armv7/dev/com_fdt.c
+++ b/sys/arch/armv7/dev/com_fdt.c
@@ -36,10 +36,15 @@
 #include 
 #include 
 
-#define com_usr 31 /* Synopsys DesignWare UART */
+/* Synopsys DesignWare UART specific registers */
+#definecom_usr 31  /* UART Status Register */
+#definecom_cpr 244 /* UART Component Paramter Register */
+#defineCPR_FIFOSZ(x)   (((x) & 0x00ff) >> 12)
+#definecom_cvr 248 /* UART Component Version Register */
 
 intcom_fdt_match(struct device *, void *, void *);
 void   com_fdt_attach(struct device *, struct device *, void *);
+intcom_fdt_fifo_designware(struct com_softc *);
 intcom_fdt_intr_designware(void *);
 
 extern int comcnspeed;
@@ -159,6 +164,12 @@ com_fdt_attach(struct device *parent, struct device *self, 
void *aux)
return;
}
 
+   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
+   sc->sc.sc_fifolen = com_fdt_fifo_designware(>sc);
+   if (sc->sc.sc_fifolen > 0)
+   sc->sc.sc_uarttype = COM_UART_16550A;
+   }
+
pinctrl_byname(faa->fa_node, "default");
 
com_attach_subr(>sc);
@@ -167,6 +178,15 @@ com_fdt_attach(struct device *parent, struct device *self, 
void *aux)
sc, sc->sc.sc_dev.dv_xname);
 }
 
+int
+com_fdt_fifo_designware(struct com_softc *sc)
+{
+   if (bus_space_read_4(sc->sc_iot, sc->sc_ioh, com_cvr) == 0)
+   return 0;
+
+   return CPR_FIFOSZ(bus_space_read_4(sc->sc_iot, sc->sc_ioh, com_cpr));
+}
+
 int
 com_fdt_intr_designware(void *cookie)
 {

Re: armv7/arm64: enable fifo in com_fdt for "snps,dw-apb-uart"

[Mark Kettenis]

> Date: Fri, 10 Nov 2017 00:16:26 +0200
> From: Artturi Alm 
> 
> Hi,
> 
> does also remove ti,omapX-uart compatibles from arm64 com_fdt,
> as they're never to be found there.
> iirc. the lenght is based on minimal lenght i found from any datasheet
> for SoCs w/"snps,dw-apb-uart", and conclusions drawn from gpl-sources.

As far as I can tell the FIFO functionality is optional for the
Synopys "IP".  It's just safer to not bother with the FIFO.

> diff --git a/sys/arch/arm64/dev/com_fdt.c b/sys/arch/arm64/dev/com_fdt.c
> index 9f2a8eb760a..1dd67c7c3e0 100644
> --- a/sys/arch/arm64/dev/com_fdt.c
> +++ b/sys/arch/arm64/dev/com_fdt.c
> @@ -66,9 +66,7 @@ com_fdt_init_cons(void)
>   void *node;
>  
>   if ((node = fdt_find_cons("brcm,bcm2835-aux-uart")) == NULL &&
> - (node = fdt_find_cons("snps,dw-apb-uart")) == NULL &&
> - (node = fdt_find_cons("ti,omap3-uart")) == NULL &&
> - (node = fdt_find_cons("ti,omap4-uart")) == NULL)
> + (node = fdt_find_cons("snps,dw-apb-uart")) == NULL)
>   return;
>   if (fdt_get_reg(node, 0, ))
>   return;
> @@ -95,9 +93,7 @@ com_fdt_match(struct device *parent, void *match, void *aux)
>   struct fdt_attach_args *faa = aux;
>  
>   return (OF_is_compatible(faa->fa_node, "brcm,bcm2835-aux-uart") ||
> - OF_is_compatible(faa->fa_node, "snps,dw-apb-uart") ||
> - OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
> - OF_is_compatible(faa->fa_node, "ti,omap4-uart"));
> + OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"));
>  }
>  
>  void
> @@ -136,12 +132,11 @@ com_fdt_attach(struct device *parent, struct device 
> *self, void *aux)
>   sc->sc.sc_uarttype = COM_UART_16550;
>   sc->sc.sc_frequency = freq ? freq : COM_FREQ;
>  
> - if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
> + if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
> + sc->sc.sc_uarttype = COM_UART_16550A;
> + sc->sc.sc_fifolen = 64;
>   intr = com_fdt_intr_designware;
> -
> - if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
> - OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
> - sc->sc.sc_uarttype = COM_UART_TI16750;
> + }
>  
>   if (stdout_node == faa->fa_node) {
>   SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> diff --git a/sys/arch/armv7/dev/com_fdt.c b/sys/arch/armv7/dev/com_fdt.c
> index 00504801850..97c535a5a05 100644
> --- a/sys/arch/armv7/dev/com_fdt.c
> +++ b/sys/arch/armv7/dev/com_fdt.c
> @@ -140,8 +140,11 @@ com_fdt_attach(struct device *parent, struct device 
> *self, void *aux)
>   sc->sc.sc_uarttype = COM_UART_16550;
>   sc->sc.sc_frequency = freq ? freq : COM_FREQ;
>  
> - if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
> + if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
> + sc->sc.sc_uarttype = COM_UART_16550A;
> + sc->sc.sc_fifolen = 64;
>   intr = com_fdt_intr_designware;
> + }
>  
>   if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
>   OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
> 
> 

armv7/arm64: enable fifo in com_fdt for "snps,dw-apb-uart"

[Artturi Alm]

Hi,

does also remove ti,omapX-uart compatibles from arm64 com_fdt,
as they're never to be found there.
iirc. the lenght is based on minimal lenght i found from any datasheet
for SoCs w/"snps,dw-apb-uart", and conclusions drawn from gpl-sources.

-Artturi


diff --git a/sys/arch/arm64/dev/com_fdt.c b/sys/arch/arm64/dev/com_fdt.c
index 9f2a8eb760a..1dd67c7c3e0 100644
--- a/sys/arch/arm64/dev/com_fdt.c
+++ b/sys/arch/arm64/dev/com_fdt.c
@@ -66,9 +66,7 @@ com_fdt_init_cons(void)
void *node;
 
if ((node = fdt_find_cons("brcm,bcm2835-aux-uart")) == NULL &&
-   (node = fdt_find_cons("snps,dw-apb-uart")) == NULL &&
-   (node = fdt_find_cons("ti,omap3-uart")) == NULL &&
-   (node = fdt_find_cons("ti,omap4-uart")) == NULL)
+   (node = fdt_find_cons("snps,dw-apb-uart")) == NULL)
return;
if (fdt_get_reg(node, 0, ))
return;
@@ -95,9 +93,7 @@ com_fdt_match(struct device *parent, void *match, void *aux)
struct fdt_attach_args *faa = aux;
 
return (OF_is_compatible(faa->fa_node, "brcm,bcm2835-aux-uart") ||
-   OF_is_compatible(faa->fa_node, "snps,dw-apb-uart") ||
-   OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
-   OF_is_compatible(faa->fa_node, "ti,omap4-uart"));
+   OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"));
 }
 
 void
@@ -136,12 +132,11 @@ com_fdt_attach(struct device *parent, struct device 
*self, void *aux)
sc->sc.sc_uarttype = COM_UART_16550;
sc->sc.sc_frequency = freq ? freq : COM_FREQ;
 
-   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
+   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
+   sc->sc.sc_uarttype = COM_UART_16550A;
+   sc->sc.sc_fifolen = 64;
intr = com_fdt_intr_designware;
-
-   if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
-   OF_is_compatible(faa->fa_node, "ti,omap4-uart"))
-   sc->sc.sc_uarttype = COM_UART_TI16750;
+   }
 
if (stdout_node == faa->fa_node) {
SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
diff --git a/sys/arch/armv7/dev/com_fdt.c b/sys/arch/armv7/dev/com_fdt.c
index 00504801850..97c535a5a05 100644
--- a/sys/arch/armv7/dev/com_fdt.c
+++ b/sys/arch/armv7/dev/com_fdt.c
@@ -140,8 +140,11 @@ com_fdt_attach(struct device *parent, struct device *self, 
void *aux)
sc->sc.sc_uarttype = COM_UART_16550;
sc->sc.sc_frequency = freq ? freq : COM_FREQ;
 
-   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart"))
+   if (OF_is_compatible(faa->fa_node, "snps,dw-apb-uart")) {
+   sc->sc.sc_uarttype = COM_UART_16550A;
+   sc->sc.sc_fifolen = 64;
intr = com_fdt_intr_designware;
+   }
 
if (OF_is_compatible(faa->fa_node, "ti,omap3-uart") ||
OF_is_compatible(faa->fa_node, "ti,omap4-uart"))