Skip to contentSkip to conversation

spi: omap3: fix claim/release bus within DM

PATCH New Hannes Schmelzer 4 participants 6 messages Jun 26, 2018 – Jun 28, 2018 SPI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

 drivers/spi/omap3_spi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/spi/omap3_spi.c +7 -7
index 4169abd..41bdab7 100644
@@ -443,9 +443,6 @@ static void spi_reset(struct mcspi *regs)
 static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
 {
 	unsigned int conf;
-
-	spi_reset(priv->regs);
-
 	/*
 	 * setup when switching from (reset default) slave mode
 	 * to single-channel master mode
@@ -480,6 +477,8 @@ int spi_claim_bus(struct spi_slave *slave)
 {
 	struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
+	spi_reset(priv->regs);
+
 	_omap3_spi_claim_bus(priv);
 	_omap3_spi_set_wordlen(priv);
 	_omap3_spi_set_mode(priv);
@@ -492,8 +491,7 @@ void spi_release_bus(struct spi_slave *slave)
 {
 	struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
-	/* Reset the SPI hardware */
-	spi_reset(priv->regs);
+	writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
 }
 
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
@@ -600,8 +598,7 @@ static int omap3_spi_release_bus(struct udevice *dev)
 	struct udevice *bus = dev->parent;
 	struct omap3_spi_priv *priv = dev_get_priv(bus);
 
-	/* Reset the SPI hardware */
-	spi_reset(priv->regs);
+	writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
 
 	return 0;
 }
@@ -634,6 +631,9 @@ static int omap3_spi_probe(struct udevice *dev)
 	else
 		priv->pin_dir = MCSPI_PINDIR_D0_IN_D1_OUT;
 	priv->wordlen = SPI_DEFAULT_WORDLEN;
+
+	spi_reset(priv->regs);
+
 	return 0;
 }
 
Signature · 1 line
2.7.4
#1 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

Quoted 60 lines · Hannes Schmelzer

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

 drivers/spi/omap3_spi.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)
drivers/spi/omap3_spi.c +3 -5
index 4169abd..41bdab7 100644
@@ -443,9 +443,6 @@ static void spi_reset(struct mcspi *regs)
 static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
 {
        unsigned int conf;
-
-       spi_reset(priv->regs);
-
        /*
         * setup when switching from (reset default) slave mode
         * to single-channel master mode
@@ -480,6 +477,8 @@ int spi_claim_bus(struct spi_slave *slave)
 {
        struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
+       spi_reset(priv->regs);
+
        _omap3_spi_claim_bus(priv);
        _omap3_spi_set_wordlen(priv);
        _omap3_spi_set_mode(priv);
@@ -492,8 +491,7 @@ void spi_release_bus(struct spi_slave *slave)
 {
        struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
-       /* Reset the SPI hardware */
-       spi_reset(priv->regs);
+       writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
 }
 
 struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,

drivers/spi/omap3_spi.c static int omap3_spi_release_bus(struct udevice *dev) · line 601

Quoted 6 lines
drivers/spi/omap3_spi.c +1 -2
@@ -600,8 +598,7 @@ static int omap3_spi_release_bus(struct udevice *dev)
        struct udevice *bus = dev->parent;
        struct omap3_spi_priv *priv = dev_get_priv(bus);
 
-       /* Reset the SPI hardware */
-       spi_reset(priv->regs);
+       writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);

This change look compared to what patch is trying to fix. so release doen't require reset? are you sure we need to write only this bit release the bus or do we need to clear?

#2 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

Quoted 64 lines · Jagan Teki
Quoted 63 lines · Hannes Schmelzer

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

  drivers/spi/omap3_spi.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)
drivers/spi/omap3_spi.c +3 -5
index 4169abd..41bdab7 100644
@@ -443,9 +443,6 @@ static void spi_reset(struct mcspi *regs)
  static void _omap3_spi_claim_bus(struct omap3_spi_priv *priv)
  {
         unsigned int conf;
-
-       spi_reset(priv->regs);
-
         /*
          * setup when switching from (reset default) slave mode
          * to single-channel master mode
@@ -480,6 +477,8 @@ int spi_claim_bus(struct spi_slave *slave)
  {
         struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
+       spi_reset(priv->regs);
+
         _omap3_spi_claim_bus(priv);
         _omap3_spi_set_wordlen(priv);
         _omap3_spi_set_mode(priv);
@@ -492,8 +491,7 @@ void spi_release_bus(struct spi_slave *slave)
  {
         struct omap3_spi_priv *priv = to_omap3_spi(slave);
 
-       /* Reset the SPI hardware */
-       spi_reset(priv->regs);
+       writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);
  }
 
  struct spi_slave *spi_setup_slave(unsigned int bus, unsigned int cs,
@@ -600,8 +598,7 @@ static int omap3_spi_release_bus(struct udevice *dev)
         struct udevice *bus = dev->parent;
         struct omap3_spi_priv *priv = dev_get_priv(bus);
 
Quoted 6 lines

drivers/spi/omap3_spi.c void spi_release_bus(struct spi_slave *slave) · line 494

Quoted 3 lines
drivers/spi/omap3_spi.c +1 -2
-       /* Reset the SPI hardware */
-       spi_reset(priv->regs);
+       writel(OMAP3_MCSPI_MODULCTRL_MS, &priv->regs->modulctrl);

This change look compared to what patch is trying to fix. so release doen't require reset? are you sure we need to write only this bit release the bus or do we need to clear?

Yes, this bit switches the mcspi ip core into slave mode, meaning be high impedance on the spi lines, what is exactly the meaning/sense of a bus release.

I know the mcspi ip core very good since i've made already some vxworks driver and even now the
fixes in u-boot for running it with dm.

cheers,
Hannes

#3 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

In reply to Hannes Schmelzer
Quoted 16 lines · Hannes Schmelzer

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

Quoted 6 lines

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

Applied to u-boot-spi/master

#4 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

Quoted 17 lines · Jagan Teki
Quoted 16 lines · Hannes Schmelzer

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

Quoted 7 lines
Quoted 6 lines

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

Applied to u-boot-spi/master

do we have a chance to get into rc3 with this?

cheers,
Hannes

#5 lore raw

PATCH 1/1 spi: omap3: fix claim/release bus within DM

Quoted 19 lines · Hannes Schmelzer
Quoted 17 lines · Jagan Teki
Quoted 16 lines · Hannes Schmelzer

The claim/release bus function must not reset the whole SPI core because settings regarding wordlen, clock-frequency and so on made by set_wordlen, set_mode, set_speed get lost with this action. Resulting in a non-functional SPI.

Without DM the failure didn't came up since after the spi_reset within claim bus all the setup (wordlen, mode, ...) was called, in DM they are called by the spi uclass.

We change now the things as following for having a working SPI instance in DM:

- move the spi_reset(...) to the probe call in DM for having a known hardware state after probe. Without DM we don't have a probe call, so we issue the reset as before during the claim_bus call.

Quoted 8 lines
Quoted 7 lines
Quoted 6 lines

- in release bus we just reset the modulctrl to the reset-value (spi- slave)

---

Applied to u-boot-spi/master

do we have a chance to get into rc3 with this?

I would like to see this and the other fixes in for that, yes, thanks!

Signature · 8 lines
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180628/cb6e7553/attachment.sig>