Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Sun, 25 Sep 2022 18:30:02 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     Sascha Hauer <s.hauer@pengutronix.de>,
        Rob Herring <robh@kernel.org>,
        Rob Herring <robh+dt@kernel.org>,
        Linus Walleij <linus.walleij@linaro.org>
Cc:     linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org, Bartosz Golaszewski <brgl@bgdev.pl>,
        kernel@pengutronix.de
Subject: Re: [PATCH v3 1/2] gpio: Add gpio latch driver
Message-ID: <20220925153002.makz5eijmwzrgqqs@mobilestation>
References: <20220914071306.3254881-1-s.hauer@pengutronix.de>
 <20220914071306.3254881-2-s.hauer@pengutronix.de>
 <20220914140310.5ffrnsy63piegdyr@mobilestation>
 <20220922133105.GN986@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220922133105.GN986@pengutronix.de>
Precedence: bulk

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:
> > > +				    unsigned int offset, bool val)
> > > +{
> > > +	int latch = offset / priv->n_latched_gpios;
> > > +	int i;
> > > +
> > > +	assign_bit(offset, priv->shadow, val);
> > > +
> > 
> > > +	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?
> > 
> > > +
> > > +	set(priv->clk_gpios->desc[latch], 1);
> > 
> > -> duration?
> > 
> > > +	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 |