Linux Serial subsystem development
 help / color / mirror / Atom feed
* [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr()
@ 2016-04-05  2:49 Kefeng Wang
  2016-04-05  2:53 ` Kefeng Wang
  2016-04-05  2:59 ` kbuild test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Kefeng Wang @ 2016-04-05  2:49 UTC (permalink / raw)
  To: Noam Camus, Greg Kroah-Hartman
  Cc: Andy Shevchenko, Heikki Krogerus, linux-serial, linux-kernel,
	guohanjun, xuwei5, Kefeng Wang

Commit cdcea058e510("serial: 8250_dw: Avoid serial_outx code duplicate
with new dw8250_check_lcr()") introduce a wrong logic when write val to
LCR reg. When CONFIG_64BIT enabled, __raw_writeq is used unconditionally.

The __raw_readq/__raw_writeq is introduced by commit bca2092d7897("serial:
8250_dw: Use 64-bit access for OCTEON") for OCTEON, so for !PORT_OCTEON,
we better to use coincident write func.

Fixes: cdcea058e510("serial: 8250_dw: Avoid serial_outx code duplicate with new dw8250_check_lcr()")
Signe:-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
---
 drivers/tty/serial/8250/8250_dw.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index a3fb95d..003ce60 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -103,16 +103,14 @@ static void dw8250_check_lcr(struct uart_port *p, int value)
 
 		dw8250_force_idle(p);
 
-#ifdef CONFIG_64BIT
-		__raw_writeq(value & 0xff, offset);
-#else
-		if (p->iotype == UPIO_MEM32)
+		if (IS_ENABLED(CONFIG_64BIT) && p->type == PORT_OCTEON)
+			__raw_writeq(value & 0xff, offset);
+		else if (p->iotype == UPIO_MEM32)
 			writel(value, offset);
 		else if (p->iotype == UPIO_MEM32BE)
 			iowrite32be(value, offset);
 		else
 			writeb(value, offset);
-#endif
 	}
 	/*
 	 * FIXME: this deadlocks if port->lock is already held
-- 
2.6.0.GIT

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr()
  2016-04-05  2:49 [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() Kefeng Wang
@ 2016-04-05  2:53 ` Kefeng Wang
  2016-04-05  2:59 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Kefeng Wang @ 2016-04-05  2:53 UTC (permalink / raw)
  To: Noam Camus, Greg Kroah-Hartman
  Cc: Andy Shevchenko, Heikki Krogerus, linux-serial, linux-kernel,
	guohanjun, xuwei5

Please ignore, sorry for typos, and will resend.

On 2016/4/5 10:49, Kefeng Wang wrote:
> Commit cdcea058e510("serial: 8250_dw: Avoid serial_outx code duplicate
> with new dw8250_check_lcr()") introduce a wrong logic when write val to
> LCR reg. When CONFIG_64BIT enabled, __raw_writeq is used unconditionally.
> 
> The __raw_readq/__raw_writeq is introduced by commit bca2092d7897("serial:
> 8250_dw: Use 64-bit access for OCTEON") for OCTEON, so for !PORT_OCTEON,
> we better to use coincident write func.
> 
> Fixes: cdcea058e510("serial: 8250_dw: Avoid serial_outx code duplicate with new dw8250_check_lcr()")
> Signe:-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  drivers/tty/serial/8250/8250_dw.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
> index a3fb95d..003ce60 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -103,16 +103,14 @@ static void dw8250_check_lcr(struct uart_port *p, int value)
>  
>  		dw8250_force_idle(p);
>  
> -#ifdef CONFIG_64BIT
> -		__raw_writeq(value & 0xff, offset);
> -#else
> -		if (p->iotype == UPIO_MEM32)
> +		if (IS_ENABLED(CONFIG_64BIT) && p->type == PORT_OCTEON)
> +			__raw_writeq(value & 0xff, offset);
> +		else if (p->iotype == UPIO_MEM32)
>  			writel(value, offset);
>  		else if (p->iotype == UPIO_MEM32BE)
>  			iowrite32be(value, offset);
>  		else
>  			writeb(value, offset);
> -#endif
>  	}
>  	/*
>  	 * FIXME: this deadlocks if port->lock is already held
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr()
  2016-04-05  2:49 [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() Kefeng Wang
  2016-04-05  2:53 ` Kefeng Wang
@ 2016-04-05  2:59 ` kbuild test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kbuild test robot @ 2016-04-05  2:59 UTC (permalink / raw)
  Cc: kbuild-all, Noam Camus, Greg Kroah-Hartman, Andy Shevchenko,
	Heikki Krogerus, linux-serial, linux-kernel, guohanjun, xuwei5,
	Kefeng Wang

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Hi Kefeng,

[auto build test ERROR on tty/tty-testing]
[also build test ERROR on v4.6-rc2 next-20160404]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Kefeng-Wang/serial-8250_dw-fix-wrong-logic-in-dw8250_check_lcr/20160405-104859
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing
config: i386-randconfig-x009-201614 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/tty/serial/8250/8250_dw.c: In function 'dw8250_check_lcr':
>> drivers/tty/serial/8250/8250_dw.c:107:4: error: implicit declaration of function '__raw_writeq' [-Werror=implicit-function-declaration]
       __raw_writeq(value & 0xff, offset);
       ^
   cc1: some warnings being treated as errors

vim +/__raw_writeq +107 drivers/tty/serial/8250/8250_dw.c

6979f8d28 James Hogan 2013-12-10  101  		if ((value & ~UART_LCR_SPAR) == (lcr & ~UART_LCR_SPAR))
c49436b65 Tim Kryger  2013-10-01  102  			return;
cdcea058e Noam Camus  2015-12-12  103  
c49436b65 Tim Kryger  2013-10-01  104  		dw8250_force_idle(p);
cdcea058e Noam Camus  2015-12-12  105  
8158f08d3 Kefeng Wang 2016-04-05  106  		if (IS_ENABLED(CONFIG_64BIT) && p->type == PORT_OCTEON)
cdcea058e Noam Camus  2015-12-12 @107  			__raw_writeq(value & 0xff, offset);
8158f08d3 Kefeng Wang 2016-04-05  108  		else if (p->iotype == UPIO_MEM32)
cdcea058e Noam Camus  2015-12-12  109  			writel(value, offset);
5a43140cc Noam Camus  2015-12-12  110  		else if (p->iotype == UPIO_MEM32BE)

:::::: The code at line 107 was first introduced by commit
:::::: cdcea058e51008479545f29201b4fa577c59733c serial: 8250_dw: Avoid serial_outx code duplicate with new dw8250_check_lcr()

:::::: TO: Noam Camus <noamc@ezchip.com>
:::::: CC: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 22816 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2016-04-05  2:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-04-05  2:49 [PATCH v2] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() Kefeng Wang
2016-04-05  2:53 ` Kefeng Wang
2016-04-05  2:59 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox