From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754420AbcDAJ5N (ORCPT ); Fri, 1 Apr 2016 05:57:13 -0400 Received: from mga01.intel.com ([192.55.52.88]:63957 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753135AbcDAJ5K (ORCPT ); Fri, 1 Apr 2016 05:57:10 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,427,1455004800"; d="scan'208";a="945948444" Message-ID: <1459504683.5907.151.camel@linux.intel.com> Subject: Re: [PATCH] serial: 8250_dw: fix wrong logic in dw8250_check_lcr() From: Andy Shevchenko To: Kefeng Wang , Noam Camus , Greg Kroah-Hartman Cc: Heikki Krogerus , linux-serial@vger.kernel.org, linux-kernel@vger.kernel.org, guohanjun@huawei.com, xuwei5@hisilicon.com Date: Fri, 01 Apr 2016 12:58:03 +0300 In-Reply-To: <1459501618-21021-1-git-send-email-wangkefeng.wang@huawei.com> References: <1459501618-21021-1-git-send-email-wangkefeng.wang@huawei.com> Organization: Intel Finland Oy Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.1-1 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 2016-04-01 at 17:06 +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. > > But for !PORT_OCTEON, we better to use coincident write func. > > Signed-off-by: Kefeng Wang > --- > > We met following calltrace twice, if the change is not necessary, > correct me, > and any advice will be appreciated, thanks. So, does this fix you issue? If it does than perhaps you need to Cc stable@ and put Fixes tag as well. > --- 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 You may also get rid of this one, like if (IS_ENABLED(CONFIG_64BIT) && p->type == PORT_OCTEON)  ... else if ... >   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 -- Andy Shevchenko Intel Finland Oy