[U-Boot] [PATCH v2 5/8] spi: cadence_qspi: add reset handling

[Simon Goldschmidt]

On Thu, Feb 21, 2019 at 11:14 PM Marek Vasut <marex at denx.de> wrote:
>
> On 2/21/19 11:04 PM, Simon Goldschmidt wrote:
> >
> >
> > Am Do., 21. Feb. 2019, 22:56 hat Marek Vasut <marex at denx.de
> > <mailto:marex at denx.de>> geschrieben:
> >
> >     On 2/21/19 10:43 PM, Simon Goldschmidt wrote:
> >     > This adds reset handling to the cadence qspi driver.
> >     >
> >     > For backwards compatibility, only a warning is printed when failing to
> >     > get reset handles.
> >     >
> >     > Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt at gmail.com
> >     <mailto:simon.k.r.goldschmidt at gmail.com>>
> >     > ---
> >     >
> >     > Changes in v2:
> >     > - add .remove callback to release the resets
> >     >
> >     >  drivers/spi/cadence_qspi.c | 16 ++++++++++++++++
> >     >  drivers/spi/cadence_qspi.h |  4 ++++
> >     >  2 files changed, 20 insertions(+)
> >     >
> >     > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> >     > index 11fce9c4fe..3bfa0201c4 100644
> >     > --- a/drivers/spi/cadence_qspi.c
> >     > +++ b/drivers/spi/cadence_qspi.c
> >     > @@ -8,6 +8,7 @@
> >     >  #include <dm.h>
> >     >  #include <fdtdec.h>
> >     >  #include <malloc.h>
> >     > +#include <reset.h>
> >     >  #include <spi.h>
> >     >  #include <linux/errno.h>
> >     >  #include "cadence_qspi.h"
> >     > @@ -154,10 +155,17 @@ static int cadence_spi_probe(struct udevice
> >     *bus)
> >     >  {
> >     >       struct cadence_spi_platdata *plat = bus->platdata;
> >     >       struct cadence_spi_priv *priv = dev_get_priv(bus);
> >     > +     int ret;
> >     >
> >     >       priv->regbase = plat->regbase;
> >     >       priv->ahbbase = plat->ahbbase;
> >     >
> >     > +     ret = reset_get_bulk(bus, &priv->resets);
> >     > +     if (ret)
> >     > +             dev_warn(bus, "Can't get reset: %d\n", ret);
> >     > +     else
> >     > +             reset_deassert_bulk(&priv->resets);
> >     > +
> >     >       if (!priv->qspi_is_init) {
> >     >               cadence_qspi_apb_controller_init(plat);
> >     >               priv->qspi_is_init = 1;
> >     > @@ -166,6 +174,13 @@ static int cadence_spi_probe(struct udevice *bus)
> >     >       return 0;
> >     >  }
> >     >
> >     > +static int cadence_spi_remove(struct udevice *dev)
> >     > +{
> >     > +     struct cadence_spi_priv *priv = dev_get_priv(dev);
> >     > +
> >     > +     return reset_release_bulk(&priv->resets);
> >     > +}
> >     > +
> >     >  static int cadence_spi_set_mode(struct udevice *bus, uint mode)
> >     >  {
> >     >       struct cadence_spi_priv *priv = dev_get_priv(bus);
> >     > @@ -342,4 +357,5 @@ U_BOOT_DRIVER(cadence_spi) = {
> >     >       .platdata_auto_alloc_size = sizeof(struct cadence_spi_platdata),
> >     >       .priv_auto_alloc_size = sizeof(struct cadence_spi_priv),
> >     >       .probe = cadence_spi_probe,
> >     > +     .remove = cadence_spi_remove,
> >
> >     .remove() only ever gets executed for drivers setting DM_FLAG_OS_PREPARE
> >     flag. Fix this in the other drivers too.
> >
> >
> > Ehrm, I haven't checked, but is this common practice? Why doesn't it
> > always get called?
>
> That's how the code behaves. Probably to speed up booting the kernel on
> devices which don't need to be torn down.

What surprises me is that the OS_PREPARE flag is used only for one spi driver
and for 'mmc_blk' (but this is really new). Is it still the right
thing to do? How could
this be one of the first drivers releasing its reset before boot? :-)

Regards,
Simon