Hi Michal,
> -----Original Message-----
> From: Simek, Michal <michal.si...@amd.com>
> Sent: Wednesday, November 6, 2024 5:17 PM
> To: Abbarapu, Venkatesh <venkatesh.abbar...@amd.com>; u-boot@lists.denx.de
> Cc: ja...@amarulasolutions.com; vigne...@ti.com; u-kum...@ti.com;
> tr...@konsulko.com; sean...@gmail.com; caleb.conno...@linaro.org;
> s...@chromium.org; william.zh...@broadcom.com; stefa...@posteo.net; git (AMD-
> Xilinx) <g...@amd.com>
> Subject: Re: [PATCH] cadence_qspi: Refactor the flash reset functionality
>
>
>
> On 10/28/24 10:24, Venkatesh Yadav Abbarapu wrote:
> > As the flash reset is handled in spi nor core, removing the flash
> > reset functionality. As the configuration like tristate and hysterisis
> > need to be enabled by the cdo. Handle the flash reset only for mini
> > u-boot case.
> >
> > Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbar...@amd.com>
> > ---
> > drivers/spi/cadence_ospi_versal.c | 43 -------------------------------
> > drivers/spi/cadence_qspi.c | 7 +++--
> > 2 files changed, 5 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/spi/cadence_ospi_versal.c
> > b/drivers/spi/cadence_ospi_versal.c
> > index 222f828f54..de51e4c37f 100644
> > --- a/drivers/spi/cadence_ospi_versal.c
> > +++ b/drivers/spi/cadence_ospi_versal.c
> > @@ -125,48 +125,6 @@ int cadence_qspi_apb_wait_for_dma_cmplt(struct
> cadence_spi_priv *priv)
> > return 0;
> > }
> >
> > -#if defined(CONFIG_DM_GPIO)
> > -int cadence_qspi_versal_flash_reset(struct udevice *dev) -{
> > - struct gpio_desc gpio;
> > - u32 reset_gpio;
> > - int ret;
> > -
> > - /* request gpio and set direction as output set to 1 */
> > - ret = gpio_request_by_name(dev, "reset-gpios", 0, &gpio,
> > - GPIOD_IS_OUT | GPIOD_IS_OUT_ACTIVE);
> > - if (ret) {
> > - printf("%s: unable to reset ospi flash device", __func__);
> > - return ret;
> > - }
> > -
> > - reset_gpio = PMIO_NODE_ID_BASE + gpio.offset;
> > -
> > - /* Request for pin */
> > - xilinx_pm_request(PM_PINCTRL_REQUEST, reset_gpio, 0, 0, 0, NULL);
> > -
> > - /* Enable hysteresis in cmos receiver */
> > - xilinx_pm_request(PM_PINCTRL_CONFIG_PARAM_SET, reset_gpio,
> > - PM_PINCTRL_CONFIG_SCHMITT_CMOS,
> > - PM_PINCTRL_INPUT_TYPE_SCHMITT, 0, NULL);
> > -
> > - /* Disable Tri-state */
> > - xilinx_pm_request(PM_PINCTRL_CONFIG_PARAM_SET, reset_gpio,
> > - PM_PINCTRL_CONFIG_TRI_STATE,
> > - PM_PINCTRL_TRI_STATE_DISABLE, 0, NULL);
> > - udelay(1);
> > -
> > - /* Set value 0 to pin */
> > - dm_gpio_set_value(&gpio, 0);
> > - udelay(1);
> > -
> > - /* Set value 1 to pin */
> > - dm_gpio_set_value(&gpio, 1);
> > - udelay(1);
> > -
> > - return 0;
> > -}
> > -#else
> > int cadence_qspi_versal_flash_reset(struct udevice *dev)
> > {
> > /* CRP WPROT */
> > @@ -209,7 +167,6 @@ int cadence_qspi_versal_flash_reset(struct udevice
> > *dev)
> >
> > return 0;
> > }
> > -#endif
> >
> > void cadence_qspi_apb_enable_linear_mode(bool enable)
> > {
> > diff --git a/drivers/spi/cadence_qspi.c b/drivers/spi/cadence_qspi.c
> > index 9c466f8695..f8c3fe8428 100644
> > --- a/drivers/spi/cadence_qspi.c
> > +++ b/drivers/spi/cadence_qspi.c
> > @@ -258,8 +258,11 @@ static int cadence_spi_probe(struct udevice *bus)
> > if (priv->read_delay >= 0)
> > priv->read_delay = -1;
> >
> > - /* Reset ospi flash device */
> > - return cadence_qspi_versal_flash_reset(bus);
> > + if (!CONFIG_IS_ENABLED(DM_GPIO))
>
> I think this is not the right thing to do here.
> Because there could be boards which requires to do some other GPIO handling
> out
> of spi core. I think this patch is introducing limitation for nothing.
>
> I would keep origin code and just move this condition to
> drivers/spi/cadence_ospi_versal.c
This function cadence_qspi_versal_flash_reset() is specific to AMD only.
Thanks
Venkatesh
>
> M
