diff mbox series

spi: omap3_spi: Fix speed and mode selection

Message ID 20201129072305.17019-1-vigneshr@ti.com
State Accepted
Delegated to: Lokesh Vutla
Headers show
Series spi: omap3_spi: Fix speed and mode selection | expand

Commit Message

Vignesh Raghavendra Nov. 29, 2020, 7:23 a.m. UTC
McSPI IP provides per CS specific speed and mode selection. Therefore it
is possible to apply these settings only after CS is known. But
set_speed and set_mode can be called without bus being claimed, this
would lead driver to set up wrong CS (or previously used CS).

Fix this by apply set_speed and set_mode only if bus is already claimed.

Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
---
 drivers/spi/omap3_spi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

Comments

Vignesh Raghavendra Dec. 1, 2020, 10:15 a.m. UTC | #1
+Miquel,

On 11/29/20 12:53 PM, Vignesh Raghavendra wrote:
> McSPI IP provides per CS specific speed and mode selection. Therefore it
> is possible to apply these settings only after CS is known. But
> set_speed and set_mode can be called without bus being claimed, this
> would lead driver to set up wrong CS (or previously used CS).
> 
> Fix this by apply set_speed and set_mode only if bus is already claimed.
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  drivers/spi/omap3_spi.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c
> index 56cb217486..fe73362945 100644
> --- a/drivers/spi/omap3_spi.c
> +++ b/drivers/spi/omap3_spi.c
> @@ -37,6 +37,8 @@ struct omap3_spi_priv {
>  	unsigned int mode;
>  	unsigned int wordlen;
>  	unsigned int pin_dir:1;
> +
> +	bool bus_claimed;
>  };
>  
>  static void omap3_spi_write_chconf(struct omap3_spi_priv *priv, int val)
> @@ -372,6 +374,8 @@ static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
>  	conf |= OMAP3_MCSPI_MODULCTRL_SINGLE;
>  
>  	writel(conf, &priv->regs->modulctrl);
> +
> +	priv->bus_claimed = true;
>  }
>  
>  static int omap3_spi_claim_bus(struct udevice *dev)
> @@ -381,9 +385,12 @@ static int omap3_spi_claim_bus(struct udevice *dev)
>  	struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
>  
>  	priv->cs = slave_plat->cs;
> -	priv->freq = slave_plat->max_hz;
> +	if (!priv->freq)
> +		priv->freq = slave_plat->max_hz;
>  
>  	_omap3_spi_claim_bus(priv);
> +	_omap3_spi_set_speed(priv);
> +	_omap3_spi_set_mode(priv);
>  
>  	return 0;
>  }
> @@ -395,6 +402,8 @@ static int omap3_spi_release_bus(struct udevice *dev)
>  
>  	writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
>  
> +	priv->bus_claimed = false;
> +
>  	return 0;
>  }
>  
> @@ -440,7 +449,8 @@ static int omap3_spi_set_speed(struct udevice *dev, unsigned int speed)
>  	struct omap3_spi_priv *priv = dev_get_priv(dev);
>  
>  	priv->freq = speed;
> -	_omap3_spi_set_speed(priv);
> +	if (priv->bus_claimed)
> +		_omap3_spi_set_speed(priv);
>  
>  	return 0;
>  }
> @@ -451,7 +461,8 @@ static int omap3_spi_set_mode(struct udevice *dev, uint mode)
>  
>  	priv->mode = mode;
>  
> -	_omap3_spi_set_mode(priv);
> +	if (priv->bus_claimed)
> +		_omap3_spi_set_mode(priv);
>  
>  	return 0;
>  }
>
Miquel Raynal Dec. 1, 2020, 10:19 a.m. UTC | #2
Hi Vignesh,

Vignesh Raghavendra <vigneshr@ti.com> wrote on Tue, 1 Dec 2020 15:45:27
+0530:

> +Miquel,
> 
> On 11/29/20 12:53 PM, Vignesh Raghavendra wrote:
> > McSPI IP provides per CS specific speed and mode selection. Therefore it
> > is possible to apply these settings only after CS is known. But
> > set_speed and set_mode can be called without bus being claimed, this
> > would lead driver to set up wrong CS (or previously used CS).
> > 
> > Fix this by apply set_speed and set_mode only if bus is already claimed.

I was experiencing the following issue: any first sf probe 1:1 call
would fail while any other attempt would work.

This patch fixes it.

Tested-by: Miquel Raynal <miquel.raynal@bootlin.com>

> > 
> > Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> > ---
> >  drivers/spi/omap3_spi.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c
> > index 56cb217486..fe73362945 100644
> > --- a/drivers/spi/omap3_spi.c
> > +++ b/drivers/spi/omap3_spi.c
> > @@ -37,6 +37,8 @@ struct omap3_spi_priv {
> >  	unsigned int mode;
> >  	unsigned int wordlen;
> >  	unsigned int pin_dir:1;
> > +
> > +	bool bus_claimed;
> >  };
> >  
> >  static void omap3_spi_write_chconf(struct omap3_spi_priv *priv, int val)
> > @@ -372,6 +374,8 @@ static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
> >  	conf |= OMAP3_MCSPI_MODULCTRL_SINGLE;
> >  
> >  	writel(conf, &priv->regs->modulctrl);
> > +
> > +	priv->bus_claimed = true;
> >  }
> >  
> >  static int omap3_spi_claim_bus(struct udevice *dev)
> > @@ -381,9 +385,12 @@ static int omap3_spi_claim_bus(struct udevice *dev)
> >  	struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
> >  
> >  	priv->cs = slave_plat->cs;
> > -	priv->freq = slave_plat->max_hz;
> > +	if (!priv->freq)
> > +		priv->freq = slave_plat->max_hz;
> >  
> >  	_omap3_spi_claim_bus(priv);
> > +	_omap3_spi_set_speed(priv);
> > +	_omap3_spi_set_mode(priv);
> >  
> >  	return 0;
> >  }
> > @@ -395,6 +402,8 @@ static int omap3_spi_release_bus(struct udevice *dev)
> >  
> >  	writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
> >  
> > +	priv->bus_claimed = false;
> > +
> >  	return 0;
> >  }
> >  
> > @@ -440,7 +449,8 @@ static int omap3_spi_set_speed(struct udevice *dev, unsigned int speed)
> >  	struct omap3_spi_priv *priv = dev_get_priv(dev);
> >  
> >  	priv->freq = speed;
> > -	_omap3_spi_set_speed(priv);
> > +	if (priv->bus_claimed)
> > +		_omap3_spi_set_speed(priv);
> >  
> >  	return 0;
> >  }
> > @@ -451,7 +461,8 @@ static int omap3_spi_set_mode(struct udevice *dev, uint mode)
> >  
> >  	priv->mode = mode;
> >  
> > -	_omap3_spi_set_mode(priv);
> > +	if (priv->bus_claimed)
> > +		_omap3_spi_set_mode(priv);
> >  
> >  	return 0;
> >  }


Thanks,
Miquèl
Lokesh Vutla Dec. 23, 2020, 8:16 a.m. UTC | #3
On 29/11/20 12:53 pm, Vignesh Raghavendra wrote:
> McSPI IP provides per CS specific speed and mode selection. Therefore it
> is possible to apply these settings only after CS is known. But
> set_speed and set_mode can be called without bus being claimed, this
> would lead driver to set up wrong CS (or previously used CS).
> 
> Fix this by apply set_speed and set_mode only if bus is already claimed.
> 
> Signed-off-by: Vignesh Raghavendra <vigneshr@ti.com>
> ---
>  drivers/spi/omap3_spi.c | 17 ++++++++++++++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 


Applied to u-boot-ti/for-next

Thanks and regards,
Lokesh
diff mbox series

Patch

diff --git a/drivers/spi/omap3_spi.c b/drivers/spi/omap3_spi.c
index 56cb217486..fe73362945 100644
--- a/drivers/spi/omap3_spi.c
+++ b/drivers/spi/omap3_spi.c
@@ -37,6 +37,8 @@  struct omap3_spi_priv {
 	unsigned int mode;
 	unsigned int wordlen;
 	unsigned int pin_dir:1;
+
+	bool bus_claimed;
 };
 
 static void omap3_spi_write_chconf(struct omap3_spi_priv *priv, int val)
@@ -372,6 +374,8 @@  static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
 	conf |= OMAP3_MCSPI_MODULCTRL_SINGLE;
 
 	writel(conf, &priv->regs->modulctrl);
+
+	priv->bus_claimed = true;
 }
 
 static int omap3_spi_claim_bus(struct udevice *dev)
@@ -381,9 +385,12 @@  static int omap3_spi_claim_bus(struct udevice *dev)
 	struct dm_spi_slave_platdata *slave_plat = dev_get_parent_platdata(dev);
 
 	priv->cs = slave_plat->cs;
-	priv->freq = slave_plat->max_hz;
+	if (!priv->freq)
+		priv->freq = slave_plat->max_hz;
 
 	_omap3_spi_claim_bus(priv);
+	_omap3_spi_set_speed(priv);
+	_omap3_spi_set_mode(priv);
 
 	return 0;
 }
@@ -395,6 +402,8 @@  static int omap3_spi_release_bus(struct udevice *dev)
 
 	writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
 
+	priv->bus_claimed = false;
+
 	return 0;
 }
 
@@ -440,7 +449,8 @@  static int omap3_spi_set_speed(struct udevice *dev, unsigned int speed)
 	struct omap3_spi_priv *priv = dev_get_priv(dev);
 
 	priv->freq = speed;
-	_omap3_spi_set_speed(priv);
+	if (priv->bus_claimed)
+		_omap3_spi_set_speed(priv);
 
 	return 0;
 }
@@ -451,7 +461,8 @@  static int omap3_spi_set_mode(struct udevice *dev, uint mode)
 
 	priv->mode = mode;
 
-	_omap3_spi_set_mode(priv);
+	if (priv->bus_claimed)
+		_omap3_spi_set_mode(priv);
 
 	return 0;
 }