[PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Ed Blake]

Add a set_ldisc function to enable/disable IrDA SIR mode according to
the new line discipline.

Signed-off-by: Ed Blake <ed.blake@imgtec.com>
---
 drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 459d726..7dd5369 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -53,6 +53,8 @@
 /* Helper for fifo size calculation */
 #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
 
+/* DesignWare specific register fields */
+#define DW_UART_MCR_SIRE		BIT(6)
 
 struct dw8250_data {
 	u8			usr_reg;
@@ -254,6 +256,20 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
 	serial8250_do_set_termios(p, termios, old);
 }
 
+static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
+{
+	unsigned int mcr = p->serial_in(p, UART_MCR);
+
+	if (termios->c_line == N_IRDA)
+		mcr |= DW_UART_MCR_SIRE;
+	else
+		mcr &= ~DW_UART_MCR_SIRE;
+
+	p->serial_out(p, UART_MCR, mcr);
+
+	serial8250_do_set_ldisc(p, termios);
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -392,6 +408,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->iotype	= UPIO_MEM;
 	p->serial_in	= dw8250_serial_in;
 	p->serial_out	= dw8250_serial_out;
+	p->set_ldisc	= dw8250_set_ldisc;
 
 	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
 	if (!p->membase)
-- 
1.9.1

[PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Ed Blake]

Add a set_ldisc function to enable/disable IrDA SIR mode according to
the new line discipline.

Signed-off-by: Ed Blake <ed.blake@imgtec.com>
---
 drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index 459d726..7dd5369 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -53,6 +53,8 @@
 /* Helper for fifo size calculation */
 #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
 
+/* DesignWare specific register fields */
+#define DW_UART_MCR_SIRE		BIT(6)
 
 struct dw8250_data {
 	u8			usr_reg;
@@ -254,6 +256,20 @@ static void dw8250_set_termios(struct uart_port *p, struct ktermios *termios,
 	serial8250_do_set_termios(p, termios, old);
 }
 
+static void dw8250_set_ldisc(struct uart_port *p, struct ktermios *termios)
+{
+	unsigned int mcr = p->serial_in(p, UART_MCR);
+
+	if (termios->c_line == N_IRDA)
+		mcr |= DW_UART_MCR_SIRE;
+	else
+		mcr &= ~DW_UART_MCR_SIRE;
+
+	p->serial_out(p, UART_MCR, mcr);
+
+	serial8250_do_set_ldisc(p, termios);
+}
+
 /*
  * dw8250_fallback_dma_filter will prevent the UART from getting just any free
  * channel on platforms that have DMA engines, but don't have any channels
@@ -392,6 +408,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->iotype	= UPIO_MEM;
 	p->serial_in	= dw8250_serial_in;
 	p->serial_out	= dw8250_serial_out;
+	p->set_ldisc	= dw8250_set_ldisc;
 
 	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
 	if (!p->membase)
-- 
1.9.1

Re: [PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Andy Shevchenko]

On Thu, 2016-10-27 at 17:38 +0100, Ed Blake wrote:
> Add a set_ldisc function to enable/disable IrDA SIR mode according to
> the new line discipline.

Thanks for the patch.

Consider that not all implementations based on 8250_dw support IRDA
feature. I suppose you need to read capability register bits 6 and 7,
set flag and then based on the value accept or reject IRDA mode.

P.S. Use -n when prepare patches to link them if you consider them
dependent.

> 
> Signed-off-by: Ed Blake <ed.blake@imgtec.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c
> b/drivers/tty/serial/8250/8250_dw.c
> index 459d726..7dd5369 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -53,6 +53,8 @@
>  /* Helper for fifo size calculation */
>  #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
>  
> +/* DesignWare specific register fields */
> +#define DW_UART_MCR_SIRE		BIT(6)
>  
>  struct dw8250_data {
>  	u8			usr_reg;
> @@ -254,6 +256,20 @@ static void dw8250_set_termios(struct uart_port
> *p, struct ktermios *termios,
>  	serial8250_do_set_termios(p, termios, old);
>  }
>  
> +static void dw8250_set_ldisc(struct uart_port *p, struct ktermios
> *termios)
> +{
> +	unsigned int mcr = p->serial_in(p, UART_MCR);
> +
> +	if (termios->c_line == N_IRDA)
> +		mcr |= DW_UART_MCR_SIRE;
> +	else
> +		mcr &= ~DW_UART_MCR_SIRE;
> +
> +	p->serial_out(p, UART_MCR, mcr);
> +
> +	serial8250_do_set_ldisc(p, termios);
> +}
> +
>  /*
>   * dw8250_fallback_dma_filter will prevent the UART from getting just
> any free
>   * channel on platforms that have DMA engines, but don't have any
> channels
> @@ -392,6 +408,7 @@ static int dw8250_probe(struct platform_device
> *pdev)
>  	p->iotype	= UPIO_MEM;
>  	p->serial_in	= dw8250_serial_in;
>  	p->serial_out	= dw8250_serial_out;
> +	p->set_ldisc	= dw8250_set_ldisc;
>  
>  	p->membase = devm_ioremap(dev, regs->start,
> resource_size(regs));
>  	if (!p->membase)

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Ed Blake]

On 10/27/16 17:56, Andy Shevchenko wrote:
> On Thu, 2016-10-27 at 17:38 +0100, Ed Blake wrote:
>> Add a set_ldisc function to enable/disable IrDA SIR mode according to
>> the new line discipline.
> Thanks for the patch.
>
> Consider that not all implementations based on 8250_dw support IRDA
> feature. I suppose you need to read capability register bits 6 and 7,
> set flag and then based on the value accept or reject IRDA mode.

Thanks for the review, I'll add this for the second version.

Ed.

>
> P.S. Use -n when prepare patches to link them if you consider them
> dependent.
>
>> Signed-off-by: Ed Blake <ed.blake@imgtec.com>
>> ---
>>   drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c
>> b/drivers/tty/serial/8250/8250_dw.c
>> index 459d726..7dd5369 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -53,6 +53,8 @@
>>   /* Helper for fifo size calculation */
>>   #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
>>   
>> +/* DesignWare specific register fields */
>> +#define DW_UART_MCR_SIRE		BIT(6)
>>   
>>   struct dw8250_data {
>>   	u8			usr_reg;
>> @@ -254,6 +256,20 @@ static void dw8250_set_termios(struct uart_port
>> *p, struct ktermios *termios,
>>   	serial8250_do_set_termios(p, termios, old);
>>   }
>>   
>> +static void dw8250_set_ldisc(struct uart_port *p, struct ktermios
>> *termios)
>> +{
>> +	unsigned int mcr = p->serial_in(p, UART_MCR);
>> +
>> +	if (termios->c_line == N_IRDA)
>> +		mcr |= DW_UART_MCR_SIRE;
>> +	else
>> +		mcr &= ~DW_UART_MCR_SIRE;
>> +
>> +	p->serial_out(p, UART_MCR, mcr);
>> +
>> +	serial8250_do_set_ldisc(p, termios);
>> +}
>> +
>>   /*
>>    * dw8250_fallback_dma_filter will prevent the UART from getting just
>> any free
>>    * channel on platforms that have DMA engines, but don't have any
>> channels
>> @@ -392,6 +408,7 @@ static int dw8250_probe(struct platform_device
>> *pdev)
>>   	p->iotype	= UPIO_MEM;
>>   	p->serial_in	= dw8250_serial_in;
>>   	p->serial_out	= dw8250_serial_out;
>> +	p->set_ldisc	= dw8250_set_ldisc;
>>   
>>   	p->membase = devm_ioremap(dev, regs->start,
>> resource_size(regs));
>>   	if (!p->membase)

Re: [PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Ed Blake]

On 10/27/16 17:56, Andy Shevchenko wrote:
> On Thu, 2016-10-27 at 17:38 +0100, Ed Blake wrote:
>> Add a set_ldisc function to enable/disable IrDA SIR mode according to
>> the new line discipline.
> Thanks for the patch.
>
> Consider that not all implementations based on 8250_dw support IRDA
> feature. I suppose you need to read capability register bits 6 and 7,
> set flag and then based on the value accept or reject IRDA mode.

Thanks for the review, I'll add this for the second version.

Ed.

>
> P.S. Use -n when prepare patches to link them if you consider them
> dependent.
>
>> Signed-off-by: Ed Blake <ed.blake@imgtec.com>
>> ---
>>   drivers/tty/serial/8250/8250_dw.c | 17 +++++++++++++++++
>>   1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/tty/serial/8250/8250_dw.c
>> b/drivers/tty/serial/8250/8250_dw.c
>> index 459d726..7dd5369 100644
>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -53,6 +53,8 @@
>>   /* Helper for fifo size calculation */
>>   #define DW_UART_CPR_FIFO_SIZE(a)	(((a >> 16) & 0xff) * 16)
>>   
>> +/* DesignWare specific register fields */
>> +#define DW_UART_MCR_SIRE		BIT(6)
>>   
>>   struct dw8250_data {
>>   	u8			usr_reg;
>> @@ -254,6 +256,20 @@ static void dw8250_set_termios(struct uart_port
>> *p, struct ktermios *termios,
>>   	serial8250_do_set_termios(p, termios, old);
>>   }
>>   
>> +static void dw8250_set_ldisc(struct uart_port *p, struct ktermios
>> *termios)
>> +{
>> +	unsigned int mcr = p->serial_in(p, UART_MCR);
>> +
>> +	if (termios->c_line == N_IRDA)
>> +		mcr |= DW_UART_MCR_SIRE;
>> +	else
>> +		mcr &= ~DW_UART_MCR_SIRE;
>> +
>> +	p->serial_out(p, UART_MCR, mcr);
>> +
>> +	serial8250_do_set_ldisc(p, termios);
>> +}
>> +
>>   /*
>>    * dw8250_fallback_dma_filter will prevent the UART from getting just
>> any free
>>    * channel on platforms that have DMA engines, but don't have any
>> channels
>> @@ -392,6 +408,7 @@ static int dw8250_probe(struct platform_device
>> *pdev)
>>   	p->iotype	= UPIO_MEM;
>>   	p->serial_in	= dw8250_serial_in;
>>   	p->serial_out	= dw8250_serial_out;
>> +	p->set_ldisc	= dw8250_set_ldisc;
>>   
>>   	p->membase = devm_ioremap(dev, regs->start,
>> resource_size(regs));
>>   	if (!p->membase)

Re: [PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Heikki Krogerus]

Hi Andy,

On Thu, Oct 27, 2016 at 07:56:10PM +0300, Andy Shevchenko wrote:
> On Thu, 2016-10-27 at 17:38 +0100, Ed Blake wrote:
> > Add a set_ldisc function to enable/disable IrDA SIR mode according to
> > the new line discipline.
> 
> Thanks for the patch.
> 
> Consider that not all implementations based on 8250_dw support IRDA
> feature. I suppose you need to read capability register bits 6 and 7,
> set flag and then based on the value accept or reject IRDA mode.
> 
> P.S. Use -n when prepare patches to link them if you consider them
> dependent.

These are numbered? Or do you mean that please prefer sending
series of patches as threads?

        % git send-email --thread...

Or something like that? I guess coverletter would be nice too.


Thanks,

-- 
heikki

Re: [PATCH 2/2] serial: 8250_dw: Add support for IrDA SIR mode

[Andy Shevchenko]

On Fri, 2016-10-28 at 11:20 +0300, Heikki Krogerus wrote:
> Hi Andy,
> 
> On Thu, Oct 27, 2016 at 07:56:10PM +0300, Andy Shevchenko wrote:
> > P.S. Use -n when prepare patches to link them if you consider them
> > dependent.
> 
> These are numbered? Or do you mean that please prefer sending
> series of patches as threads?
> 
>         % git send-email --thread...
> 
> Or something like that? I guess coverletter would be nice too.

Yes, you are right. Sorry for confusion.


-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy