Messages in this thread | | | Date | Thu, 10 Aug 2017 16:38:10 +0200 | From | Danilo Krummrich <> | Subject | Re: [PATCH] serio: PS2 gpio bit banging driver for the serio bus |
| |
Hi Linus,
On 2017-08-07 18:22, Danilo Krummrich wrote: >> > +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. > I couldn't find any guarantee that the mode and tx_byte change is implicitly covered by a barrier in this case. E.g. the bcm2835 driver does not make sure stores are completed before the particular interrupt is enabled, except by the fact that writel on ARM contains a wmb(). But this is nothing to rely on. (Please tell me if I miss something.) Therefore I would like to keep this barrier and replace it with smp_wmb() if you are fine with that.
Regards, Danilo
| |