[u-boot][PATCH 2/2] spi: atmel-quadspi: Add verbose debug facilities to monitor register accesses

[Jagan Teki]

On Mon, Mar 23, 2020 at 6:38 PM <Tudor.Ambarus at microchip.com> wrote:
>
> On Friday, March 20, 2020 11:37:59 AM EET Tudor.Ambarus at microchip.com wrote:
> > From: Tudor Ambarus <tudor.ambarus at microchip.com>
> >
> > This feature should not be enabled in release but can be useful for
> > developers who need to monitor register accesses at some specific places.
> >
> > Helped me identify a bug in u-boot, by comparing the register accesses
> > from the u-boot driver with the ones from its linux variant.
> >
> > Signed-off-by: Tudor Ambarus <tudor.ambarus at microchip.com>
> > ---
> >  drivers/spi/atmel-quadspi.c | 114 ++++++++++++++++++++++++++++++------
> >  1 file changed, 96 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/spi/atmel-quadspi.c b/drivers/spi/atmel-quadspi.c
> > index 4099ee87993d..3de367e6a0c8 100644
> > --- a/drivers/spi/atmel-quadspi.c
> > +++ b/drivers/spi/atmel-quadspi.c
> > @@ -148,6 +148,7 @@ struct atmel_qspi {
> >       void __iomem *mem;
> >       resource_size_t mmap_size;
> >       const struct atmel_qspi_caps *caps;
> > +     struct udevice *dev;
> >       ulong bus_clk_rate;
> >       u32 mr;
> >  };
> > @@ -169,6 +170,81 @@ static const struct atmel_qspi_mode atmel_qspi_modes[]
> > = { { 4, 4, 4, QSPI_IFR_WIDTH_QUAD_CMD },
> >  };
> >
> > +#ifdef VERBOSE_DEBUG
> > +static const char *atmel_qspi_reg_name(u32 offset, char *tmp, size_t sz)
> > +{
> > +     switch (offset) {
> > +     case QSPI_CR:
> > +             return "CR";
> > +     case QSPI_MR:
> > +             return "MR";
> > +     case QSPI_RD:
> > +             return "MR";
> > +     case QSPI_TD:
> > +             return "TD";
> > +     case QSPI_SR:
> > +             return "SR";
> > +     case QSPI_IER:
> > +             return "IER";
> > +     case QSPI_IDR:
> > +             return "IDR";
> > +     case QSPI_IMR:
> > +             return "IMR";
> > +     case QSPI_SCR:
> > +             return "SCR";
> > +     case QSPI_IAR:
> > +             return "IAR";
> > +     case QSPI_ICR:
> > +             return "ICR/WICR";
> > +     case QSPI_IFR:
> > +             return "IFR";
> > +     case QSPI_RICR:
> > +             return "RICR";
> > +     case QSPI_SMR:
> > +             return "SMR";
> > +     case QSPI_SKR:
> > +             return "SKR";
> > +     case QSPI_WPMR:
> > +             return "WPMR";
> > +     case QSPI_WPSR:
> > +             return "WPSR";
> > +     case QSPI_VERSION:
> > +             return "VERSION";
> > +     default:
> > +             snprintf(tmp, sz, "0x%02x", offset);
> > +             break;
> > +     }
> > +
> > +     return tmp;
> > +}
> > +#endif /* VERBOSE_DEBUG */
> > +
> > +static u32 atmel_qspi_read(struct atmel_qspi *aq, u32 offset)
> > +{
> > +     u32 value = readl(aq->regs + offset);
> > +
> > +#ifdef VERBOSE_DEBUG
> > +     char tmp[8];
>
> The largest string that I print is "ICR/WICR" which is 8bytes, but I didn't
> count the trailing null space, so the array should better be increased to 16
> bytes, to avoid truncation.
>
> > +
> > +     dev_vdbg(aq->dev, "read 0x%08x from %s\n", value,
> > +              atmel_qspi_reg_name(offset, tmp, sizeof(tmp)));
> > +#endif /* VERBOSE_DEBUG */
> > +
> > +     return value;
> > +}
> > +
> > +static void atmel_qspi_write(u32 value, struct atmel_qspi *aq, u32 offset)
> > +{
> > +#ifdef VERBOSE_DEBUG
> > +     char tmp[8];
>
> ditto
>
> Jagan, if the rest looks good, would you do this change when applying? Let me
> know if I have to resubmit, or if there are any suggestions.

Yes and Applied to u-boot-spi/master