From: Danilo Krummrich <danilokrummrich@dk-develop.de>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Linux Input <linux-input@vger.kernel.org>,
Dmitry Torokhov <dmitry.torokhov@gmail.com>,
"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>
Subject: Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus
Date: Mon, 7 Aug 2017 18:21:07 +0200 [thread overview]
Message-ID: <20170807182107.6b25804213443850c157c123@dk-develop.de> (raw)
In-Reply-To: <CACRpkdYz_+soss4Rb9zMiP4eZZtXqjW_Xy7hH=wm1x2m_R9R3A@mail.gmail.com>
Hi Linus,
thanks for reviewing. Commented the ones which still holds.
On Mon, 7 Aug 2017 11:03:53 +0200
Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Aug 1, 2017 at 12:24 AM, Danilo Krummrich
> <danilokrummrich@dk-develop.de> wrote:
>
>
> When you add DT bindings you have to CC devicetree@vger.kernel.org
>
I will do prospectively.
> > +#include <linux/gpio.h>
>
> Use only:
>
> #include <linux/gpio/consumer.h>
>
Done in v6.
> > +#include <linux/of_gpio.h>
>
> 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 <danilokrummrich@dk-develop.de>
next prev parent reply other threads:[~2017-08-07 16:21 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-31 22:24 [PATCH] serio: PS2 gpio bit banging driver for the serio bus Danilo Krummrich
2017-08-07 9:03 ` Linus Walleij
2017-08-07 16:21 ` Danilo Krummrich [this message]
2017-08-08 2:49 ` Dmitry Torokhov
[not found] ` <20170807182207.348762301bf3d7f8509b1bf7@dk-develop.de>
2017-08-10 14:38 ` Danilo Krummrich
2017-08-11 9:16 ` Linus Walleij
2017-08-11 11:05 ` Danilo Krummrich
2017-08-17 9:09 ` Russell King - ARM Linux
2017-08-17 10:51 ` Danilo Krummrich
2017-08-17 13:01 ` Russell King - ARM Linux
2017-08-17 14:14 ` Danilo Krummrich
2017-08-22 13:35 ` Linus Walleij
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170807182107.6b25804213443850c157c123@dk-develop.de \
--to=danilokrummrich@dk-develop.de \
--cc=devicetree@vger.kernel.org \
--cc=dmitry.torokhov@gmail.com \
--cc=linus.walleij@linaro.org \
--cc=linux-input@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).