[UBOOT PATCH] spi: spi-uclass: Fix reduced read speed when parallel config is enabled

[Kummari, Prasad]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Marek Vasut <marex at denx.de>
> Sent: Friday, February 21, 2025 11:28 PM
> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-
> boot at lists.denx.de; tudor.ambarus at linaro.org; j-humphreys at ti.com
> Cc: Simek, Michal <michal.simek at amd.com>; jagan at amarulasolutions.com;
> vigneshr at ti.com; u-kumar1 at ti.com; trini at konsulko.com;
> seanga2 at gmail.com; caleb.connolly at linaro.org; sjg at chromium.org;
> william.zhang at broadcom.com; stefan_b at posteo.net;
> quentin.schulz at cherry.de; Takahiro.Kuwano at infineon.com; p-
> mantena at ti.com; git (AMD-Xilinx) <git at amd.com>; Kummari, Prasad
> <Prasad.Kummari at amd.com>
> Subject: Re: [UBOOT PATCH] spi: spi-uclass: Fix reduced read speed when
> parallel config is enabled
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> On 2/21/25 10:57 AM, Venkatesh Yadav Abbarapu wrote:
> > From: Prasad Kummari <prasad.kummari at amd.com>
> >
> > When SPI_STACKED_PARALLEL is enabled, the chip selects properties are
> > read from the device tree using the reg property and stored in the
> > plat->cs[] array. However, the function dev_read_u32_array() returns
> > zero, causing the execution to enter the else block, print an error
> > message, and return. As a result, the spi-max-frequency property is
> > not read from the device tree, resulting in the use of the default
> > frequency SPI_DEFAULT_SPEED_HZ (100,000). This leads to a lower
> > spi->speed, reducing QSPI DMA read performance.
> >
> > To address this issue, an additional condition has been added to check
> > the return value of reg. If it is nonzero, the function prints an
> > error message and returns. Otherwise, it proceeds to read the
> > spi-max-frequency property from the device tree.
> >
> > Signed-off-by: Prasad Kummari <prasad.kummari at amd.com>
> > Signed-off-by: Venkatesh Yadav Abbarapu
> <venkatesh.abbarapu at amd.com>
> > ---
> >   drivers/spi/spi-uclass.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index
> > d6049753740..a7783b6b6dd 100644
> > --- a/drivers/spi/spi-uclass.c
> > +++ b/drivers/spi/spi-uclass.c
> > @@ -523,8 +523,10 @@ int spi_slave_of_to_plat(struct udevice *dev, struct
> dm_spi_slave_plat *plat)
> >       if (ret == -EOVERFLOW || ret == -FDT_ERR_BADLAYOUT) {
> >               dev_read_u32(dev, "reg", &plat->cs[0]);
> >       } else {
> > -             dev_err(dev, "has no valid 'reg' property (%d)\n", ret);
> > -             return ret;
> > +             if (ret) {
> > +                     dev_err(dev, "has no valid 'reg' property (%d)\n", ret);
> > +                     return ret;
> > +             }
> >       }
> >   #else
> >       plat->cs[0] = dev_read_u32_default(dev, "reg", -1);
>
> Why not simply do this?

Sure, will update!

>
> diff --git a/drivers/spi/spi-uclass.c b/drivers/spi/spi-uclass.c index
> d6049753740..4d9376dbdca 100644
> --- a/drivers/spi/spi-uclass.c
> +++ b/drivers/spi/spi-uclass.c
> @@ -522,7 +522,7 @@ int spi_slave_of_to_plat(struct udevice *dev, struct
> dm_spi_slave_plat *plat)
>
>          if (ret == -EOVERFLOW || ret == -FDT_ERR_BADLAYOUT) {
>                  dev_read_u32(dev, "reg", &plat->cs[0]);
> -       } else {
> +       } else if (ret) {
>                  dev_err(dev, "has no valid 'reg' property (%d)\n", ret);
>                  return ret;
>          }