[PATCH v2] serial: sh-sci: modify sci_break_ctl()

[PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Shimoda, Yoshihiro]

SCIF modules which have SCSPTR can output the break signal.
This patch supports it.

This patch tested on sh7757lcr.

Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
---
 about v2:
  - add serial_sci.h
  - change sci_in/sci_out to serial_port_in/serial_port_out

 drivers/tty/serial/sh-sci.c |   30 ++++++++++++++++++++++++++----
 include/linux/serial_sci.h  |    2 ++
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
index 3158e17..3e471fc 100644
--- a/drivers/tty/serial/sh-sci.c
+++ b/drivers/tty/serial/sh-sci.c
@@ -1564,10 +1564,32 @@ static void sci_enable_ms(struct uart_port *port)

 static void sci_break_ctl(struct uart_port *port, int break_state)
 {
-	/*
-	 * Not supported by hardware. Most parts couple break and rx
-	 * interrupts together, with break detection always enabled.
-	 */
+	struct sci_port *s = to_sci_port(port);
+	unsigned short scscr, scsptr;
+
+	switch (s->cfg->regtype) {
+	case SCIx_SH4_SCIF_REGTYPE:
+		scsptr = serial_port_in(port, SCSPTR);
+		scscr = serial_port_in(port, SCSCR);
+
+		if (break_state = -1) {
+			scsptr = (scsptr | SCSPTR_SPB2IO) & ~SCSPTR_SPB2DT;
+			scscr &= ~SCSCR_TE;
+		} else {
+			scsptr = (scsptr | SCSPTR_SPB2DT) & ~SCSPTR_SPB2IO;
+			scscr |= SCSCR_TE;
+		}
+
+		serial_port_out(port, SCSPTR, scsptr);
+		serial_port_out(port, SCSCR, scscr);
+		break;
+	default:
+		/*
+		 * Not supported by hardware. Most parts couple break and rx
+		 * interrupts together, with break detection always enabled.
+		 */
+		break;
+	}
 }

 #ifdef CONFIG_SERIAL_SH_SCI_DMA
diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
index 7877907..eb763ad 100644
--- a/include/linux/serial_sci.h
+++ b/include/linux/serial_sci.h
@@ -52,6 +52,8 @@ enum {
 /* SCSPTR, optional */
 #define SCSPTR_RTSIO	(1 << 7)
 #define SCSPTR_CTSIO	(1 << 5)
+#define SCSPTR_SPB2IO	(1 << 1)
+#define SCSPTR_SPB2DT	(1 << 0)

 /* Offsets into the sci_port->irqs array */
 enum {
-- 
1.7.1

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Simon Horman]

On Fri, Apr 06, 2012 at 09:59:14AM +0900, Shimoda, Yoshihiro wrote:
> SCIF modules which have SCSPTR can output the break signal.
> This patch supports it.
> 
> This patch tested on sh7757lcr.

Hi Shimoda-san,

I have checked that this change boots on the sh7757lcr
and that the serial console seems to work. I am curious to
know if there is anything else that can be tested.

> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  about v2:
>   - add serial_sci.h
>   - change sci_in/sci_out to serial_port_in/serial_port_out
> 
>  drivers/tty/serial/sh-sci.c |   30 ++++++++++++++++++++++++++----
>  include/linux/serial_sci.h  |    2 ++
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/tty/serial/sh-sci.c b/drivers/tty/serial/sh-sci.c
> index 3158e17..3e471fc 100644
> --- a/drivers/tty/serial/sh-sci.c
> +++ b/drivers/tty/serial/sh-sci.c
> @@ -1564,10 +1564,32 @@ static void sci_enable_ms(struct uart_port *port)
> 
>  static void sci_break_ctl(struct uart_port *port, int break_state)
>  {
> -	/*
> -	 * Not supported by hardware. Most parts couple break and rx
> -	 * interrupts together, with break detection always enabled.
> -	 */
> +	struct sci_port *s = to_sci_port(port);
> +	unsigned short scscr, scsptr;
> +
> +	switch (s->cfg->regtype) {
> +	case SCIx_SH4_SCIF_REGTYPE:
> +		scsptr = serial_port_in(port, SCSPTR);
> +		scscr = serial_port_in(port, SCSCR);
> +
> +		if (break_state = -1) {
> +			scsptr = (scsptr | SCSPTR_SPB2IO) & ~SCSPTR_SPB2DT;
> +			scscr &= ~SCSCR_TE;
> +		} else {
> +			scsptr = (scsptr | SCSPTR_SPB2DT) & ~SCSPTR_SPB2IO;
> +			scscr |= SCSCR_TE;
> +		}
> +
> +		serial_port_out(port, SCSPTR, scsptr);
> +		serial_port_out(port, SCSCR, scscr);
> +		break;
> +	default:
> +		/*
> +		 * Not supported by hardware. Most parts couple break and rx
> +		 * interrupts together, with break detection always enabled.
> +		 */
> +		break;
> +	}
>  }
> 
>  #ifdef CONFIG_SERIAL_SH_SCI_DMA
> diff --git a/include/linux/serial_sci.h b/include/linux/serial_sci.h
> index 7877907..eb763ad 100644
> --- a/include/linux/serial_sci.h
> +++ b/include/linux/serial_sci.h
> @@ -52,6 +52,8 @@ enum {
>  /* SCSPTR, optional */
>  #define SCSPTR_RTSIO	(1 << 7)
>  #define SCSPTR_CTSIO	(1 << 5)
> +#define SCSPTR_SPB2IO	(1 << 1)
> +#define SCSPTR_SPB2DT	(1 << 0)
> 
>  /* Offsets into the sci_port->irqs array */
>  enum {
> -- 
> 1.7.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Shimoda, Yoshihiro]

Hi Simon-san,

2012/04/06 10:57, Simon Horman wrote:
> On Fri, Apr 06, 2012 at 09:59:14AM +0900, Shimoda, Yoshihiro wrote:
>> SCIF modules which have SCSPTR can output the break signal.
>> This patch supports it.
>>
>> This patch tested on sh7757lcr.
> 
> Hi Shimoda-san,
> 
> I have checked that this change boots on the sh7757lcr
> and that the serial console seems to work. I am curious to
> know if there is anything else that can be tested.

Thank you for the check.
setup-xxxx.c which have "PORT_SCIF" can be tested:
 - SH7724, SH7785, R8A7779, etc.

Best regards,
Yoshihiro Shimoda

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Simon Horman]

On Fri, Apr 06, 2012 at 11:13:24AM +0900, Shimoda, Yoshihiro wrote:
> Hi Simon-san,
> 
> 2012/04/06 10:57, Simon Horman wrote:
> > On Fri, Apr 06, 2012 at 09:59:14AM +0900, Shimoda, Yoshihiro wrote:
> >> SCIF modules which have SCSPTR can output the break signal.
> >> This patch supports it.
> >>
> >> This patch tested on sh7757lcr.
> > 
> > Hi Shimoda-san,
> > 
> > I have checked that this change boots on the sh7757lcr
> > and that the serial console seems to work. I am curious to
> > know if there is anything else that can be tested.
> 
> Thank you for the check.
> setup-xxxx.c which have "PORT_SCIF" can be tested:
>  - SH7724, SH7785, R8A7779, etc.

Thanks, I will try and test on ecovec which I believe has SH7724.
Do you have any information on how I should test the code,
other than booting it

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Shimoda, Yoshihiro]

2012/04/06 11:44, Simon Horman wrote:
> On Fri, Apr 06, 2012 at 11:13:24AM +0900, Shimoda, Yoshihiro wrote:
>>
>> Thank you for the check.
>> setup-xxxx.c which have "PORT_SCIF" can be tested:
>>  - SH7724, SH7785, R8A7779, etc.
> 
> Thanks, I will try and test on ecovec which I believe has SH7724.

Yes, ecovec has SH7724.

> Do you have any information on how I should test the code,
> other than booting it

If userland calls ioctl with TIOCSBRK/TIOCCBRK, the sci_break_ctl
is called. After TIOCSBRK we cannot send data from SCIF.
After TIOCCBRK, we can send data from SCIF.

Best regards,
Yoshihiro Shimoda

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Simon Horman]

On Fri, Apr 06, 2012 at 12:50:00PM +0900, Shimoda, Yoshihiro wrote:
> 2012/04/06 11:44, Simon Horman wrote:
> > On Fri, Apr 06, 2012 at 11:13:24AM +0900, Shimoda, Yoshihiro wrote:
> >>
> >> Thank you for the check.
> >> setup-xxxx.c which have "PORT_SCIF" can be tested:
> >>  - SH7724, SH7785, R8A7779, etc.
> > 
> > Thanks, I will try and test on ecovec which I believe has SH7724.
> 
> Yes, ecovec has SH7724.
> 
> > Do you have any information on how I should test the code,
> > other than booting it
> 
> If userland calls ioctl with TIOCSBRK/TIOCCBRK, the sci_break_ctl
> is called. After TIOCSBRK we cannot send data from SCIF.
> After TIOCCBRK, we can send data from SCIF.

Thanks, that make a lot of sense.

Unfortunately the ecovec doesn't exercise the new code
as the regtype of its scif is SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
but your code only operates on SCIx_SH4_SCIF_REGTYPE.

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Shimoda, Yoshihiro]

2012/04/06 14:11, Simon Horman wrote:
> On Fri, Apr 06, 2012 at 12:50:00PM +0900, Shimoda, Yoshihiro wrote:
>>
>> If userland calls ioctl with TIOCSBRK/TIOCCBRK, the sci_break_ctl
>> is called. After TIOCSBRK we cannot send data from SCIF.
>> After TIOCCBRK, we can send data from SCIF.
> 
> Thanks, that make a lot of sense.
> 
> Unfortunately the ecovec doesn't exercise the new code
> as the regtype of its scif is SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
> but your code only operates on SCIx_SH4_SCIF_REGTYPE.

Sorry I forgot that the SH7724 has SCIF and SCIFA.
The ecovec uses SCIF, and it doesn't have SCSPTR.
I checked the ecovec schematics, but it cannot use SCIFA because
other functions use the multiplex pins.

Best regards,
Yoshihiro Shimoda

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Simon Horman]

On Fri, Apr 06, 2012 at 02:39:00PM +0900, Shimoda, Yoshihiro wrote:
> 2012/04/06 14:11, Simon Horman wrote:
> > On Fri, Apr 06, 2012 at 12:50:00PM +0900, Shimoda, Yoshihiro wrote:
> >>
> >> If userland calls ioctl with TIOCSBRK/TIOCCBRK, the sci_break_ctl
> >> is called. After TIOCSBRK we cannot send data from SCIF.
> >> After TIOCCBRK, we can send data from SCIF.
> > 
> > Thanks, that make a lot of sense.
> > 
> > Unfortunately the ecovec doesn't exercise the new code
> > as the regtype of its scif is SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
> > but your code only operates on SCIx_SH4_SCIF_REGTYPE.
> 
> Sorry I forgot that the SH7724 has SCIF and SCIFA.
> The ecovec uses SCIF, and it doesn't have SCSPTR.
> I checked the ecovec schematics, but it cannot use SCIFA because
> other functions use the multiplex pins.

No problem.

In any case, Reviewed-by: Simon Horman <horms@verge.net.au>

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Paul Mundt]

On Fri, Apr 06, 2012 at 09:59:14AM +0900, Shimoda, Yoshihiro wrote:
> SCIF modules which have SCSPTR can output the break signal.
> This patch supports it.
> 
> This patch tested on sh7757lcr.
> 
> Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
> ---
>  about v2:
>   - add serial_sci.h
>   - change sci_in/sci_out to serial_port_in/serial_port_out
> 
Looks good to me, I'll queue it up, thanks.

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Paul Mundt]

On Fri, Apr 06, 2012 at 02:39:00PM +0900, Shimoda, Yoshihiro wrote:
> 2012/04/06 14:11, Simon Horman wrote:
> > On Fri, Apr 06, 2012 at 12:50:00PM +0900, Shimoda, Yoshihiro wrote:
> >>
> >> If userland calls ioctl with TIOCSBRK/TIOCCBRK, the sci_break_ctl
> >> is called. After TIOCSBRK we cannot send data from SCIF.
> >> After TIOCCBRK, we can send data from SCIF.
> > 
> > Thanks, that make a lot of sense.
> > 
> > Unfortunately the ecovec doesn't exercise the new code
> > as the regtype of its scif is SCIx_SH4_SCIF_NO_SCSPTR_REGTYPE
> > but your code only operates on SCIx_SH4_SCIF_REGTYPE.
> 
> Sorry I forgot that the SH7724 has SCIF and SCIFA.
> The ecovec uses SCIF, and it doesn't have SCSPTR.

Well, one thing that you can do is test for the SCSPTR existence and
simply not care about the port type. This is roughly what the generic
sci_init_pins() does for example.

You would have to ensure that the bits you are twiddling also exist for
the SCIx_SH2_SCIF_FIFODATA_REGTYPE, SCIx_SH2_SCIF_FIFODATA_REGTYPE, and
SCIx_SH4_SCIF_FIFODATA_REGTYPE, though.

> I checked the ecovec schematics, but it cannot use SCIFA because
> other functions use the multiplex pins.
> 
You should be able to plug them in for the port and let the sh-sci driver
try to grab the port. The port will simply be skipped if pin demux fails.
Take a look at 50f0959ad4f9ac1c5ee208bb820de299a1b3730b for an idea of
how to wire it up.

Re: [PATCH v2] serial: sh-sci: modify sci_break_ctl()

[Shimoda, Yoshihiro]

2012/04/11 13:28, Paul Mundt wrote:
> On Fri, Apr 06, 2012 at 02:39:00PM +0900, Shimoda, Yoshihiro wrote:
>>
>> Sorry I forgot that the SH7724 has SCIF and SCIFA.
>> The ecovec uses SCIF, and it doesn't have SCSPTR.
> 
> Well, one thing that you can do is test for the SCSPTR existence and
> simply not care about the port type. This is roughly what the generic
> sci_init_pins() does for example.
> 
> You would have to ensure that the bits you are twiddling also exist for
> the SCIx_SH2_SCIF_FIFODATA_REGTYPE, SCIx_SH2_SCIF_FIFODATA_REGTYPE, and
> SCIx_SH4_SCIF_FIFODATA_REGTYPE, though.

Thank you for the suggestion. I will modify the code.

>> I checked the ecovec schematics, but it cannot use SCIFA because
>> other functions use the multiplex pins.
>>
> You should be able to plug them in for the port and let the sh-sci driver
> try to grab the port. The port will simply be skipped if pin demux fails.
> Take a look at 50f0959ad4f9ac1c5ee208bb820de299a1b3730b for an idea of
> how to wire it up.
> 

I'm sorry, I don't understand this comment.
I think that if the sh-sci driver grab the port, other driver cannot
work correctly when other driver is working.

Best regards,
Yoshihiro Shimoda