| 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 |
+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; > } >
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
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 --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; }
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(-)