| Message ID | 1459835585-25751-1-git-send-email-wangkefeng.wang@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
On 2016/4/5 18:50, Andy Shevchenko wrote: > On Tue, 2016-04-05 at 13:53 +0800, 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()") >> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >> --- >> >> Changes since v3: >> - Add patch change log, suggested by Greg Kroah-Hartman. >> Changes since v2: >> - Add #ifdef CONFIG_64BIT back, ensure it can be built under > > Oh, true. Since it's a native IO we can't use writeq() helper from io- > 64-nonatomic-*. > >> configuration lacking readq/writeq. >> Changes since v1: >> - Repace '#ifdef CONFIG_64BIT' with IS_ENABLED(CONFIG_64BIT). >> - Enrich patch log, and add Fixes tag. [...] > > So, this changes logic to write the value on any 64 platform, using > different (non-64-bit) accessors, so, the case to fix is > actually "64BIT && !PORT_OCTEON". Perhaps commit message should be > amended to point that clearly. Yes, it's more clear. thanks for review and point it out. To Greg, should I resend it or can you help me to change the patch log when you merge it. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Greg, ping... On 2016/4/7 16:33, Kefeng Wang wrote: > > > On 2016/4/5 18:50, Andy Shevchenko wrote: >> On Tue, 2016-04-05 at 13:53 +0800, 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()") >>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> >>> --- >>> >>> Changes since v3: >>> - Add patch change log, suggested by Greg Kroah-Hartman. >>> Changes since v2: >>> - Add #ifdef CONFIG_64BIT back, ensure it can be built under >> >> Oh, true. Since it's a native IO we can't use writeq() helper from io- >> 64-nonatomic-*. >> >>> configuration lacking readq/writeq. >>> Changes since v1: >>> - Repace '#ifdef CONFIG_64BIT' with IS_ENABLED(CONFIG_64BIT). >>> - Enrich patch log, and add Fixes tag. > [...] >> >> So, this changes logic to write the value on any 64 platform, using >> different (non-64-bit) accessors, so, the case to fix is >> actually "64BIT && !PORT_OCTEON". Perhaps commit message should be >> amended to point that clearly. > > Yes, it's more clear. thanks for review and point it out. > > To Greg, should I resend it or can you help me to change the patch log when you merge it. Thanks. > > > > > . > -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c index a3fb95d..47d1f3e 100644 --- a/drivers/tty/serial/8250/8250_dw.c +++ b/drivers/tty/serial/8250/8250_dw.c @@ -104,15 +104,16 @@ 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->type == PORT_OCTEON) + __raw_writeq(value & 0xff, offset); + else +#endif 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
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()") Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- Changes since v3: - Add patch change log, suggested by Greg Kroah-Hartman. Changes since v2: - Add #ifdef CONFIG_64BIT back, ensure it can be built under configuration lacking readq/writeq. Changes since v1: - Repace '#ifdef CONFIG_64BIT' with IS_ENABLED(CONFIG_64BIT). - Enrich patch log, and add Fixes tag. drivers/tty/serial/8250/8250_dw.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.6.0.GIT -- To unsubscribe from this list: send the line "unsubscribe linux-serial" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html