From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kefeng Wang Subject: Re: [PATCH v4] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() Date: Tue, 19 Apr 2016 16:29:22 +0800 Message-ID: <5715EC62.5010204@huawei.com> References: <1459827166-13861-1-git-send-email-wangkefeng.wang@huawei.com> <1459835585-25751-1-git-send-email-wangkefeng.wang@huawei.com> <1459853407.12843.7.camel@linux.intel.com> <57061B5E.7040707@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <57061B5E.7040707@huawei.com> Sender: linux-kernel-owner@vger.kernel.org To: Andy Shevchenko , Noam Camus , Greg Kroah-Hartman Cc: Heikki Krogerus , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, guohanjun@huawei.com, xuwei5@hisilicon.com List-Id: linux-serial@vger.kernel.org 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 >>> --- >>> >>> 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. > > > > > . > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752924AbcDSIdL (ORCPT ); Tue, 19 Apr 2016 04:33:11 -0400 Received: from szxga02-in.huawei.com ([119.145.14.65]:16524 "EHLO szxga02-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751745AbcDSIdI (ORCPT ); Tue, 19 Apr 2016 04:33:08 -0400 Subject: Re: [PATCH v4] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() To: Andy Shevchenko , Noam Camus , Greg Kroah-Hartman References: <1459827166-13861-1-git-send-email-wangkefeng.wang@huawei.com> <1459835585-25751-1-git-send-email-wangkefeng.wang@huawei.com> <1459853407.12843.7.camel@linux.intel.com> <57061B5E.7040707@huawei.com> CC: Heikki Krogerus , , , , From: Kefeng Wang Message-ID: <5715EC62.5010204@huawei.com> Date: Tue, 19 Apr 2016 16:29:22 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <57061B5E.7040707@huawei.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.19.180] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020204.5715EC70.016F,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-06-18 04:22:30, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 13419d3c548e41815751f9e43a52078a Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> --- >>> >>> 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. > > > > > . >