Re: [PATCH v3 1/2] gpio: Add gpio latch driver

To += @Rob, @Linus

On Thu, Sep 22, 2022 at 03:31:05PM +0200, Sascha Hauer wrote:

On Wed, Sep 14, 2022 at 05:03:10PM +0300, Serge Semin wrote:

quoted

quoted

+				    unsigned int offset, bool val)
+{
+	int latch = offset / priv->n_latched_gpios;
+	int i;
+
+	assign_bit(offset, priv->shadow, val);
+

quoted

+	for (i = 0; i < priv->n_latched_gpios; i++)
+		set(priv->latched_gpios->desc[i],
+		    test_bit(latch * priv->n_latched_gpios + i, priv->shadow));

-> duration?

quoted

+
+	set(priv->clk_gpios->desc[latch], 1);

-> duration?

quoted

+	set(priv->clk_gpios->desc[latch], 0);

I am pretty much sure there must be some duration between the actions
above *. See for instance the tw and (tsu + th) timing requirements in
the next edge-triggered flip-flops:
https://www.ti.com/lit/ds/symlink/sn74lv74a.pdf?ts=1663163389954&ref_url=https%253A%252F%252Fwww.google.com%252F

The durations are normally small (ns or a bit smaller) but still need
to be added anyway.

Note since the durations are device-specific an additional DT-property array
with durations should be added too.

Do you think a fixed udelay(1) would be enough for now? Bigger delays
shouldn't be needed and smaller delays expand to udelay(1) anyway on
architectures not providing an architecture specific ndelay().

I am sure you shouldn't assume what the particular architecture
provide and what it doesn't. When it comes to the GPIOs the switching
timings can have a critical value in a lot of applications (like i2c
bitbang, or real-time systems). There is no point in waiting for
micro seconds in the fast-path like this when there is only a few
nano seconds delay required.

I couldn't find any generic ready-to-use DT-property for this case.
So IMO instead the next properties would work:
1. "setup-duration-ns" - data input timing after which the clock input
can be asserted (Tsu in the hw-manual above).
2. "clock-duration-ns" - clock input timing for which the CLK signal
must be kept asserted so the device would perceive the input
states, the outputs would be updated and the clock signal could be
driven back to low (Tw including Th in the hw-manual above).

@Rob, @Linus, any suggestion regarding the properties and their naming?

-Serge

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |