| Message ID | 20180727210916.66642-3-chris.brandt@renesas.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | serial: sh-sci: Clean up previous RZ/A2 support | expand |
Hi Chris, On Fri, Jul 27, 2018 at 11:09 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Some devices with SCIx_SH4_SCIF_REGTYPE have no space between registers. > Use the register area size to determine the spacing between register. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > drivers/tty/serial/sh-sci.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > index 6ff6f2bf3b9b..138296ec9a7d 100644 > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -348,15 +348,15 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = { > [SCIx_SH4_SCIF_REGTYPE] = { > .regs = { > [SCSMR] = { 0x00, 16 }, > - [SCBRR] = { 0x04, 8 }, > - [SCSCR] = { 0x08, 16 }, > - [SCxTDR] = { 0x0c, 8 }, > - [SCxSR] = { 0x10, 16 }, > - [SCxRDR] = { 0x14, 8 }, > - [SCFCR] = { 0x18, 16 }, > - [SCFDR] = { 0x1c, 16 }, > - [SCSPTR] = { 0x20, 16 }, > - [SCLSR] = { 0x24, 16 }, > + [SCBRR] = { 0x02, 8 }, > + [SCSCR] = { 0x04, 16 }, > + [SCxTDR] = { 0x06, 8 }, > + [SCxSR] = { 0x08, 16 }, > + [SCxRDR] = { 0x0a, 8 }, > + [SCFCR] = { 0x0c, 16 }, > + [SCFDR] = { 0x0e, 16 }, > + [SCSPTR] = { 0x10, 16 }, > + [SCLSR] = { 0x12, 16 }, > }, > .fifosize = 16, > .overrun_reg = SCLSR, > @@ -2939,6 +2939,10 @@ static int sci_init_single(struct platform_device *dev, > port->regshift = 1; > } > > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > + if (sci_port->reg_size >= 0x20) > + port->regshift = 1; I've accidentally discovered this breaks SCIF on SH7751R2D (QEMU), as the board code doesn't fill in regtype, so it is 0 = SCIx_PROBE_REGTYPE. The proper (default) regtype of SCIx_SH4_SCIF_REGTYPE is derived by sci_probe_regmap(). However, that value is never fed back to sci_init_single(), as plat_sci_port is const. So regshift ends up being wrong. I made it work by changing the check to: - if (p->regtype == SCIx_SH4_SCIF_REGTYPE) + if (p->regtype == SCIx_SH4_SCIF_REGTYPE || + (p->regtype == SCIx_PROBE_REGTYPE && port->type == PORT_SCIF)) Perhaps there's a better way? Gr{oetje,eeting}s, Geert
Hi Chris, On Sun, Jul 29, 2018 at 1:11 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Fri, Jul 27, 2018 at 11:09 PM Chris Brandt <chris.brandt@renesas.com> wrote: > > Some devices with SCIx_SH4_SCIF_REGTYPE have no space between registers. > > Use the register area size to determine the spacing between register. > > > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > > --- > > drivers/tty/serial/sh-sci.c | 22 +++++++++++++--------- > > 1 file changed, 13 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c > > index 6ff6f2bf3b9b..138296ec9a7d 100644 > > --- a/drivers/tty/serial/sh-sci.c > > +++ b/drivers/tty/serial/sh-sci.c > > @@ -348,15 +348,15 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = { > > [SCIx_SH4_SCIF_REGTYPE] = { > > .regs = { > > [SCSMR] = { 0x00, 16 }, > > - [SCBRR] = { 0x04, 8 }, > > - [SCSCR] = { 0x08, 16 }, > > - [SCxTDR] = { 0x0c, 8 }, > > - [SCxSR] = { 0x10, 16 }, > > - [SCxRDR] = { 0x14, 8 }, > > - [SCFCR] = { 0x18, 16 }, > > - [SCFDR] = { 0x1c, 16 }, > > - [SCSPTR] = { 0x20, 16 }, > > - [SCLSR] = { 0x24, 16 }, > > + [SCBRR] = { 0x02, 8 }, > > + [SCSCR] = { 0x04, 16 }, > > + [SCxTDR] = { 0x06, 8 }, > > + [SCxSR] = { 0x08, 16 }, > > + [SCxRDR] = { 0x0a, 8 }, > > + [SCFCR] = { 0x0c, 16 }, > > + [SCFDR] = { 0x0e, 16 }, > > + [SCSPTR] = { 0x10, 16 }, > > + [SCLSR] = { 0x12, 16 }, > > }, > > .fifosize = 16, > > .overrun_reg = SCLSR, > > @@ -2939,6 +2939,10 @@ static int sci_init_single(struct platform_device *dev, > > port->regshift = 1; > > } > > > > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > > + if (sci_port->reg_size >= 0x20) > > + port->regshift = 1; > > I've accidentally discovered this breaks SCIF on SH7751R2D (QEMU), as the > board code doesn't fill in regtype, so it is 0 = SCIx_PROBE_REGTYPE. > The proper (default) regtype of SCIx_SH4_SCIF_REGTYPE is derived by > sci_probe_regmap(). However, that value is never fed back to sci_init_single(), > as plat_sci_port is const. So regshift ends up being wrong. > > I made it work by changing the check to: > > - if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE || > + (p->regtype == SCIx_PROBE_REGTYPE && port->type == PORT_SCIF)) > > Perhaps there's a better way? Like (whitespace-damaged-gmail): --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -3340,7 +3340,7 @@ static int sci_init_single(struct platform_device *dev, { struct uart_port *port = &sci_port->port; const struct resource *res; - unsigned int i; + unsigned int i, regtype; int ret; sci_port->cfg = p; @@ -3381,6 +3381,7 @@ static int sci_init_single(struct platform_device *dev, if (unlikely(sci_port->params == NULL)) return -EINVAL; + regtype = sci_port->params - sci_port_params; switch (p->type) { case PORT_SCIFB: sci_port->rx_trigger = 48; @@ -3435,7 +3436,7 @@ static int sci_init_single(struct platform_device *dev, port->regshift = 1; } - if (p->regtype == SCIx_SH4_SCIF_REGTYPE) + if (regtype == SCIx_SH4_SCIF_REGTYPE) if (sci_port->reg_size >= 0x20) port->regshift = 1; Gr{oetje,eeting}s, Geert
Hi Geert, On Sunday, July 29, 2018, Geert Uytterhoeven wrote: > I've accidentally discovered this breaks SCIF on SH7751R2D (QEMU), as the > board code doesn't fill in regtype, so it is 0 = SCIx_PROBE_REGTYPE. Oh that pesky SH7751!! On Monday, July 30, 2018, Geert Uytterhoeven wrote: > > I made it work by changing the check to: > > > > - if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE || > > + (p->regtype == SCIx_PROBE_REGTYPE && port->type == > PORT_SCIF)) > > > > Perhaps there's a better way? > > Like (whitespace-damaged-gmail): > > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -3340,7 +3340,7 @@ static int sci_init_single(struct platform_device > *dev, > { > struct uart_port *port = &sci_port->port; > const struct resource *res; > - unsigned int i; > + unsigned int i, regtype; > int ret; > > sci_port->cfg = p; > @@ -3381,6 +3381,7 @@ static int sci_init_single(struct platform_device > *dev, > if (unlikely(sci_port->params == NULL)) > return -EINVAL; > > + regtype = sci_port->params - sci_port_params; > switch (p->type) { > case PORT_SCIFB: > sci_port->rx_trigger = 48; > @@ -3435,7 +3436,7 @@ static int sci_init_single(struct platform_device > *dev, > port->regshift = 1; > } > > - if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > + if (regtype == SCIx_SH4_SCIF_REGTYPE) > if (sci_port->reg_size >= 0x20) > port->regshift = 1; That's clever. If it worked for SH7751, I'll go test it on RZ/A2 as well and resubmit the patch. Chris
diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c index 6ff6f2bf3b9b..138296ec9a7d 100644 --- a/drivers/tty/serial/sh-sci.c +++ b/drivers/tty/serial/sh-sci.c @@ -348,15 +348,15 @@ static const struct sci_port_params sci_port_params[SCIx_NR_REGTYPES] = { [SCIx_SH4_SCIF_REGTYPE] = { .regs = { [SCSMR] = { 0x00, 16 }, - [SCBRR] = { 0x04, 8 }, - [SCSCR] = { 0x08, 16 }, - [SCxTDR] = { 0x0c, 8 }, - [SCxSR] = { 0x10, 16 }, - [SCxRDR] = { 0x14, 8 }, - [SCFCR] = { 0x18, 16 }, - [SCFDR] = { 0x1c, 16 }, - [SCSPTR] = { 0x20, 16 }, - [SCLSR] = { 0x24, 16 }, + [SCBRR] = { 0x02, 8 }, + [SCSCR] = { 0x04, 16 }, + [SCxTDR] = { 0x06, 8 }, + [SCxSR] = { 0x08, 16 }, + [SCxRDR] = { 0x0a, 8 }, + [SCFCR] = { 0x0c, 16 }, + [SCFDR] = { 0x0e, 16 }, + [SCSPTR] = { 0x10, 16 }, + [SCLSR] = { 0x12, 16 }, }, .fifosize = 16, .overrun_reg = SCLSR, @@ -2939,6 +2939,10 @@ static int sci_init_single(struct platform_device *dev, port->regshift = 1; } + if (p->regtype == SCIx_SH4_SCIF_REGTYPE) + if (sci_port->reg_size >= 0x20) + port->regshift = 1; + /* * The UART port needs an IRQ value, so we peg this to the RX IRQ * for the multi-IRQ ports, which is where we are primarily
Some devices with SCIx_SH4_SCIF_REGTYPE have no space between registers. Use the register area size to determine the spacing between register. Signed-off-by: Chris Brandt <chris.brandt@renesas.com> --- drivers/tty/serial/sh-sci.c | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-)