Re: [PATCH 15/16] fpga: machxo2: extend erase timeout for machxo2 FPGA

[Johannes Zink <j.zink@pengutronix.de>]

Hi Yilun, 

On Mon, 2022-08-29 at 22:57 +0800, Xu Yilun wrote:
> On 2022-08-29 at 12:51:19 +0200, Johannes Zink wrote:
> > Hi Yilun, 
> > 
> > On Mon, 2022-08-29 at 17:26 +0800, Xu Yilun wrote:
> > > On 2022-08-25 at 16:13:42 +0200, Johannes Zink wrote:
> > > > Measurements showed that some FPGAs take significantly longer
> > > > than
> > > > the
> > > > default wait function supplied. The datasheet inidicates up to
> > > > 30
> > > > seconds erase times for some MachXO2 FPGAs, depending on the
> > > > number
> > > > of
> > > > LUTs (and the corresponding configuration flash size).
> > > > 
> > > > Signed-off-by: Johannes Zink <j.zink@pengutronix.de>
> > > > ---

[snip]
 
> > > > +static inline int machxo2_wait_until_not_busy_timeout(struct
> > > > machxo2_common_priv *priv)
> > > > +{
> > > > +       int ret, pollret;
> > > > +       u32 status = MACHXO2_BUSY;
> > > > +
> > > > +       pollret = read_poll_timeout(priv->get_status, ret,
> > > > +                                   (ret && ret != -EAGAIN) ||
> > > > !(status & MACHXO2_BUSY),
> > > > +                                   MACHXO2_ERASE_USEC_SLEEP,
> > > > MACHXO2_MAX_ERASE_USEC,
> > > > +                                   true, priv, &status);
> > > 
> > > Why just taking care of erase timeout? I see the busy wait in
> > > many
> > > places.
> > > 
> > 
> > Erasing the flash memory takes significantly longer than the other
> > operations (up to 30s), which is why I decided to use this separate
> > implementation. For other commands the fpga indicates no-more-busy
> > much
> > faster than for the erase_flash command.
> 
> It is almost always better to have a relatively measureable timeout,
> unless it is really time critical. Apparently spi/i2c transfer is not
> time critical itself. So since you have implemented a better
> function,
> why not use it?
> 
> Thanks,
> Yilun

That's a fair point. I will look in the datasheet how long the timeouts
on the other operations should be set and will replace the former busy
wait implementation in v2. I would not expect any breakage from that.

Best regards
Johannes

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