Re: [PATCH v2] pinctrl: at91-pio4: implement .get_multiple and .set_multiple

[Alexandre Belloni]

On 06/09/2019 09:46:02+0000, David Laight wrote:
> From: Alexandre Belloni
> > Sent: 06 September 2019 10:12
> > On 06/09/2019 09:05:36+0000, David Laight wrote:
> > > From: Alexandre Belloni
> > > > Implement .get_multiple and .set_multiple to allow reading or setting
> > > > multiple pins simultaneously. Pins in the same bank will all be switched at
> > > > the same time, improving synchronization and performances.
> > >
> > > Actually it won't 'improve synchronisation', instead it will lead to
> > > random synchronisation errors and potential metastability if one
> > > pin is used as a clock and another as data, or if the code is reading
> > > a free-flowing counter.
> > >
> > 
> > It does improve gpio switching synchronisation when they are in the same
> > bank as it will remove the 250ns delay. Of course, if you need this
> > delay between clk and data, then the consumer driver should ensure the
> > delay is present.
> 
> With multiple requests the output pin changes will always be in the
> same order and will be separated by (say) 250ns.
> This is a guaranteed synchronisation.
> 
> If you change multiple pins with the same 'iowrite()' then the pins
> will change at approximately the same time.
> But the actual order will depend on internal device delays (which
> may depend on the actual silicon and temperature).
> You then have to take account of varying track lengths and the
> target devices input stage properties before knowing which change
> arrives first.
> The delays might be sub-nanosecond, but they matter if you are
> talking about synchronisation.
> 

And my point is that this means that your gpio consumer driver is buggy
if it doesn't do multiple requests if it requires a delay between two
pin changes.

> IIRC both SMBus and I2C now quote 0ns setup time.
> Changing both clock and data with the same IOW isn't enough to
> guarantee this.
> (In practise the I2C setup time required by a device is probably
> slightly negative (In order to support 0ns inputs) so a very small
> -ve setup will (mostly) work.)

I'm not sure what is your point exactly as this patch doesn't break any
existing use cases.

-- 
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com