[PATCH] cadence_qspi: Refactor the flash reset functionality

[Abbarapu, Venkatesh]

Hi Michal,

> -----Original Message-----
> From: Simek, Michal <michal.simek at amd.com>
> Sent: Wednesday, November 6, 2024 5:17 PM
> To: Abbarapu, Venkatesh <venkatesh.abbarapu at amd.com>; u-boot at lists.denx.de
> Cc: 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; git (AMD-
> Xilinx) <git at 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.abbarapu at 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