Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752110AbdHGQVM (ORCPT ); Mon, 7 Aug 2017 12:21:12 -0400 Received: from hs01.dk-develop.de ([213.136.71.231]:46980 "EHLO hs01.dk-develop.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752090AbdHGQVK (ORCPT ); Mon, 7 Aug 2017 12:21:10 -0400 Date: Mon, 7 Aug 2017 18:21:07 +0200 From: Danilo Krummrich To: Linus Walleij Cc: "linux-kernel@vger.kernel.org" , Linux Input , Dmitry Torokhov , "devicetree@vger.kernel.org" Subject: Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus Message-Id: <20170807182107.6b25804213443850c157c123@dk-develop.de> In-Reply-To: References: <20170731222452.22887-1-danilokrummrich@dk-develop.de> Organization: DKdevelop X-Mailer: Sylpheed 3.6.0 (GTK+ 2.24.30; x86_64-pc-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1390 Lines: 51 Hi Linus, thanks for reviewing. Commented the ones which still holds. On Mon, 7 Aug 2017 11:03:53 +0200 Linus Walleij wrote: > On Tue, Aug 1, 2017 at 12:24 AM, Danilo Krummrich > wrote: > > > When you add DT bindings you have to CC devicetree@vger.kernel.org > I will do prospectively. > > +#include > > Use only: > > #include > Done in v6. > > +#include > > Should not be needed. > > Removed in v6. > > +static int ps2_gpio_write(struct serio *serio, unsigned char val) > > +{ > > + struct ps2_gpio_data *drvdata = serio->port_data; > > + > > + drvdata->mode = PS2_MODE_TX; > > + drvdata->tx_byte = val; > > + /* Make sure ISR running on other CPU notice changes. */ > > + barrier(); > > This seems overengineered, is this really needed? > > If we have races like this, the error is likely elsewhere, and should be > fixed in the GPIO driver MMIO access or so. > Yes, seems it can be removed. I didn't saw any explicit barriers in the GPIO driver (I'm testing on bcm2835), but it seems MMIO operations on SMP archs does contain barriers. Not sure if all do. If some do not this barrier might be needed to ensure ISR on other CPU notice the correct mode and byte to send. -- Danilo Krummrich