| Message ID | 20180725143850.32985-1-chris.brandt@renesas.com |
|---|---|
| Headers | show |
| Series | serial: sh-sci: Add support for RZ/A2 | expand |
Hi Chris, On Wed, Jul 25, 2018 at 4:39 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> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -2869,6 +2869,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; > + So you have to be careful not to round up the reg size in DT to the next power of two (0x20), like you did for RZ/A1 (64 is used there). Note that while no SH board file uses SCIx_SH4_SCIF_REGTYPE, it is the default reg type for PORT_SCIF, so some board files may be affected. However, they all set reg_size to 0x100, so this change should be OK. Gr{oetje,eeting}s, Geert
Hi Chris, On Wed, Jul 25, 2018 at 4:39 PM Chris Brandt <chris.brandt@renesas.com> wrote: > Some SCIF versions mux error and break interrupts together and then provide > a separate interrupt ID for just TEI/DRI. > > Allow all 6 types of interrupts to be specified via platform data (or DT) > and for any signals that are muxed together (have the same interrupt > number) simply register one handler. > > Signed-off-by: Chris Brandt <chris.brandt@renesas.com> > --- > v2: > * Move compressed SCIF reg address space to a separate commit > * Handle all 6 possible interrupt types Thanks for your patch! > --- a/drivers/tty/serial/sh-sci.c > +++ b/drivers/tty/serial/sh-sci.c > @@ -1683,11 +1685,26 @@ static irqreturn_t sci_tx_interrupt(int irq, void *ptr) > return IRQ_HANDLED; > } > > +static irqreturn_t sci_br_interrupt(int irq, void *ptr); You can avoid the forward declaration by moving the whole function here. > + > static irqreturn_t sci_er_interrupt(int irq, void *ptr) > { > struct uart_port *port = ptr; > struct sci_port *s = to_sci_port(port); > > + if (s->irqs[SCIx_ERI_IRQ] == s->irqs[SCIx_BRI_IRQ]) { > + /* Break and Error interrupts are muxed */ > + unsigned short ssr_status = serial_port_in(port, SCxSR); > + > + /* Break Interrupt */ > + if (ssr_status & SCxSR_BRK(port)) > + sci_br_interrupt(irq, ptr); > + > + /* Break only? */ > + if (!(ssr_status & SCxSR_ERRORS(port))) > + return IRQ_HANDLED; > + } > + > /* Handle errors */ > if (port->type == PORT_SCI) { > if (sci_handle_errors(port)) { > @@ -2809,6 +2845,8 @@ static int sci_init_single(struct platform_device *dev, > sci_port->irqs[1] = sci_port->irqs[0]; > sci_port->irqs[2] = sci_port->irqs[0]; > sci_port->irqs[3] = sci_port->irqs[0]; > + sci_port->irqs[4] = sci_port->irqs[0]; > + sci_port->irqs[5] = sci_port->irqs[0]; You may want to start using a loop from 1 to ARRAY_SIZE(sci_port->irqs) - 1 instead. Gr{oetje,eeting}s, Geert
Hi Chris, On Wed, Jul 25, 2018 at 4:39 PM Chris Brandt <chris.brandt@renesas.com> wrote: > The RZ/A2 uses a modified SCIF that until recently was only used in > Renesas MCU devices (not MPU devices). > So, while it functions mostly the same as a normal SCIF, some things > needed to be shifted around. > > In the end, a standard compatible = "renesas,scif" is all that is really > needed (not a SoC specific "renesas,scif-r7s9210"). > Chris Brandt (3): > serial: sh-sci: Allow for compressed SCIF address space > serial: sh-sci: Add support for separate TEI+DRI interrupts > serial: sh-sci: Document r7s9210 bindings > > .../bindings/serial/renesas,sci-serial.txt | 17 +++++- > drivers/tty/serial/sh-sci.c | 66 ++++++++++++++++++---- > 2 files changed, 70 insertions(+), 13 deletions(-) Thanks for your series! Unfortunately Greg has already applied your v1 (and my fix for a use-after-free), so either these have to be reverted first, or you have to rebase against tty-next. Gr{oetje,eeting}s, Geert
Hi Geert, On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > Hi Chris, > > On Wed, Jul 25, 2018 at 4:39 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> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Thank you. > > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > > + if (sci_port->reg_size >= 0x20) > > + port->regshift = 1; > > + > > So you have to be careful not to round up the reg size in DT to the next > power of two (0x20), like you did for RZ/A1 (64 is used there). Me???? It was Wolfram that committed the RZ/A1 DT scif code back in 2014 ;) Chris
Hi Chris, On Thu, Jul 26, 2018 at 2:14 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > > On Wed, Jul 25, 2018 at 4:39 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> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Thank you. > > > > + if (p->regtype == SCIx_SH4_SCIF_REGTYPE) > > > + if (sci_port->reg_size >= 0x20) > > > + port->regshift = 1; > > > + > > > > So you have to be careful not to round up the reg size in DT to the next > > power of two (0x20), like you did for RZ/A1 (64 is used there). > > Me???? > It was Wolfram that committed the RZ/A1 DT scif code back in 2014 ;) I stand corrected. (and RZLSP didn't use resources yet, but the old mapbase, so there are no sizes ;-) Gr{oetje,eeting}s, Geert
Hi Chris, On Thu, Jul 26, 2018 at 2:25 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > > Thanks for your series! > > > > Unfortunately Greg has already applied your v1 (and my fix for a > > use-after-free), so either these have to be reverted first, or you have to > > rebase against tty-next. > > I assume you would prefer the newer implementation I did. > > In your opinion, which one would be better (revert or rebase)? [looking at the incremental differences] I think the easiest for Greg is to rebase, and send 3 patches: 1. Document interrupt order in bindings, 2. Unify SCIx_RZ_SCIFA_REGTYPE/SCIx_SH4_SCIF_REGTYPE regtypes, 3. Simplify interrupts. Thanks! Gr{oetje,eeting}s, Geert
Hi Geert, On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > > In your opinion, which one would be better (revert or rebase)? > > [looking at the incremental differences] > > I think the easiest for Greg is to rebase, and send 3 patches: > 1. Document interrupt order in bindings, > 2. Unify SCIx_RZ_SCIFA_REGTYPE/SCIx_SH4_SCIF_REGTYPE regtypes, > 3. Simplify interrupts. Thank you! Chris
On Thu, Jul 26, 2018 at 03:01:11PM +0200, Geert Uytterhoeven wrote: > Hi Chris, > > On Thu, Jul 26, 2018 at 2:25 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > > > Thanks for your series! > > > > > > Unfortunately Greg has already applied your v1 (and my fix for a > > > use-after-free), so either these have to be reverted first, or you have to > > > rebase against tty-next. > > > > I assume you would prefer the newer implementation I did. > > > > In your opinion, which one would be better (revert or rebase)? > > [looking at the incremental differences] > > I think the easiest for Greg is to rebase, and send 3 patches: Greg does not rebase his public trees. Nor should anyone else :) I can revert patches if you want me to, just let me know what ones. Or send incremental patches on top of my tree please. But no reverts are going to be happening. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday, July 28, 2018 1, Greg KH wrote: > > > In your opinion, which one would be better (revert or rebase)? > > > > [looking at the incremental differences] > > > > I think the easiest for Greg is to rebase, and send 3 patches: > > Greg does not rebase his public trees. Nor should anyone else :) > > I can revert patches if you want me to, just let me know what ones. Or > send incremental patches on top of my tree please. But no reverts are > going to be happening. Yesterday I sent a new patch series that will apply on top of the current tty-next and basically reverts what I did and replaces it with a new version that incorporates Geert's suggestions. So no reverts are needed. I assume now I'm just waiting to see if Geert has any feedback on the new series. Chris -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Greg, On Sat, Jul 28, 2018 at 4:55 PM Greg KH <gregkh@linuxfoundation.org> wrote: > On Thu, Jul 26, 2018 at 03:01:11PM +0200, Geert Uytterhoeven wrote: > > On Thu, Jul 26, 2018 at 2:25 PM Chris Brandt <Chris.Brandt@renesas.com> wrote: > > > On Thursday, July 26, 2018, Geert Uytterhoeven wrote: > > > > Thanks for your series! > > > > > > > > Unfortunately Greg has already applied your v1 (and my fix for a > > > > use-after-free), so either these have to be reverted first, or you have to > > > > rebase against tty-next. > > > > > > I assume you would prefer the newer implementation I did. > > > > > > In your opinion, which one would be better (revert or rebase)? > > > > [looking at the incremental differences] > > > > I think the easiest for Greg is to rebase, and send 3 patches: > > Greg does not rebase his public trees. Nor should anyone else :) I know ;-) FTR, the sentence above was addressed to Chris: "I think the easiest for Greg(,) is (for you) to rebase (your tree), and send 3 patches". Which is what Chris did in the mean time. Apparently I didn't formulate it well. Will try to do better in the future. Gr{oetje,eeting}s, Geert
On Sat, Jul 28, 2018 at 03:55:14PM +0000, Chris Brandt wrote: > On Saturday, July 28, 2018 1, Greg KH wrote: > > > > In your opinion, which one would be better (revert or rebase)? > > > > > > [looking at the incremental differences] > > > > > > I think the easiest for Greg is to rebase, and send 3 patches: > > > > Greg does not rebase his public trees. Nor should anyone else :) > > > > I can revert patches if you want me to, just let me know what ones. Or > > send incremental patches on top of my tree please. But no reverts are > > going to be happening. s/no reverts/no rebases/ ? > > Yesterday I sent a new patch series that will apply on top of the > current tty-next and basically reverts what I did and replaces it with a > new version that incorporates Geert's suggestions. So no reverts are > needed. > > I assume now I'm just waiting to see if Geert has any feedback on the > new series. > > Chris > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
The RZ/A2 uses a modified SCIF that until recently was only used in Renesas MCU devices (not MPU devices). So, while it functions mostly the same as a normal SCIF, some things needed to be shifted around. In the end, a standard compatible = "renesas,scif" is all that is really needed (not a SoC specific "renesas,scif-r7s9210"). Becase there is no device tree yet, here is sample of what it would look like: scif0: serial@e8007000 { compatible = "renesas,scif-r7s9210", "renesas,scif"; reg = <0xe8007000 18>; interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 266 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 267 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>, <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>; clocks = <&mstp4_clks R7S9210_CLK_SCIF0>; clock-names = "fck"; power-domains = <&cpg_clocks>; status = "disabled"; }; Chris Brandt (3): serial: sh-sci: Allow for compressed SCIF address space serial: sh-sci: Add support for separate TEI+DRI interrupts serial: sh-sci: Document r7s9210 bindings .../bindings/serial/renesas,sci-serial.txt | 17 +++++- drivers/tty/serial/sh-sci.c | 66 ++++++++++++++++++---- 2 files changed, 70 insertions(+), 13 deletions(-)