mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Rasmus Villemoes <ravi@prevas.dk>
To: Colin Foster <colin.foster@in-advantage.com>, Lee Jones <lee@kernel.org>
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Felix Blix Everberg <felix.blix@prevas.dk>,
	Rasmus Villemoes <ravi@prevas.dk>
Subject: [PATCH 3/8] mfd: ocelot: rework SPI (re-)initialization after chip reset
Date: Wed, 19 Mar 2025 13:30:53 +0100	[thread overview]
Message-ID: <20250319123058.452202-4-ravi@prevas.dk> (raw)
In-Reply-To: <20250319123058.452202-1-ravi@prevas.dk>

As the comments in ocelot-spi.c explain, after a chip reset, the
CFGSTAT register must be written again setting the appropriate number
of padding bytes; otherwise reads are not reliable.

However, the way the code is currently structured violates that: After
the BIT_SOFT_CHIP_RST is written, ocelot_chip_reset() immediately
enters a readx_poll_timeout().

Since the code in ocelot-spi.c implementing those reads still insert
the padding bytes in the spi transfer, any read done after the reset
is effected, including the "final" read that should confirm the
self-clearing has happened, is most likely garbage:

Say we are using 1 padding byte; after the
reset is completed, the device doesn't wait for 8 cycles before
sending data (because CFGSTAT.IF_CFG has been reset to 0), so the
result is going to be something like 24 bits from the actual register,
shifted 8 bits, followed by 8 bits of whatever state the MISO line is
in when not being driven explicitly.

Now, some of the registers blocks (DEVCPU_QS and DEVCPU_ORG) are
documented to have access times of 0.1us, but the reset register is
located in DEVCPU_GCB, thus has access time 1us, so it seems there is
no way around doing the reads of the SOFT_RST register with the
correct value set in CFGSTAT.IF_CFG.

So rework the code to let the underlying bus define an ->init_bus
callback that the core can call after setting the soft reset bit, and
before polling that bit for being clear.

Note that we do have somewhat of a catch-22: We cannot read back the
REG_GCB_SOFT_RST register to know if reset is completed until
CFGSTAT.IF_CFG is reinitialized, but even if we are always allowed to
write to that register, if it is possible to write it "too soon",
before reset is completed, that re-initialization might be in
vain. The data sheet is unfortunately silent on how much time a soft
reset might take, and I simply assume that the re-initialization can
be done after 100us.

This also serves as preparation for implementing mdio management,
lifting some of the initialization steps from ocelot-spi.c to
ocelot-core.c.

Signed-off-by: Rasmus Villemoes <ravi@prevas.dk>
---
 drivers/mfd/ocelot-core.c | 8 ++++++++
 drivers/mfd/ocelot-spi.c  | 9 +--------
 drivers/mfd/ocelot.h      | 2 ++
 3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/ocelot-core.c b/drivers/mfd/ocelot-core.c
index 78b5fe15efdd2..9caab83138e59 100644
--- a/drivers/mfd/ocelot-core.c
+++ b/drivers/mfd/ocelot-core.c
@@ -110,6 +110,14 @@ int ocelot_chip_reset(struct device *dev)
 	if (ret)
 		return ret;
 
+	if (ddata->init_bus) {
+		fsleep(VSC7512_GCB_RST_SLEEP_US);
+		ret = ddata->init_bus(dev);
+		if (ret)
+			return dev_err_probe(dev, ret,
+					     "Error initializing bus after reset\n");
+	}
+
 	return readx_poll_timeout(ocelot_gcb_chip_rst_status, ddata, val, !val,
 				  VSC7512_GCB_RST_SLEEP_US, VSC7512_GCB_RST_TIMEOUT_US);
 }
diff --git a/drivers/mfd/ocelot-spi.c b/drivers/mfd/ocelot-spi.c
index 97e4061e3dff7..37828dd3ee95e 100644
--- a/drivers/mfd/ocelot-spi.c
+++ b/drivers/mfd/ocelot-spi.c
@@ -215,6 +215,7 @@ static int ocelot_spi_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	spi_set_drvdata(spi, ddata);
+	ddata->init_bus = ocelot_spi_initialize;
 	ddata->init_regmap = ocelot_spi_init_regmap;
 
 	if (spi->max_speed_hz <= 500000) {
@@ -264,14 +265,6 @@ static int ocelot_spi_probe(struct spi_device *spi)
 	if (err)
 		return dev_err_probe(dev, err, "Error resetting device\n");
 
-	/*
-	 * A chip reset will clear the SPI configuration, so it needs to be done
-	 * again before we can access any registers.
-	 */
-	err = ocelot_spi_initialize(dev);
-	if (err)
-		return dev_err_probe(dev, err, "Error initializing SPI bus after reset\n");
-
 	err = ocelot_core_init(dev);
 	if (err)
 		return dev_err_probe(dev, err, "Error initializing Ocelot core\n");
diff --git a/drivers/mfd/ocelot.h b/drivers/mfd/ocelot.h
index 4f602611a5a86..5aa6589b9038e 100644
--- a/drivers/mfd/ocelot.h
+++ b/drivers/mfd/ocelot.h
@@ -12,6 +12,7 @@ struct resource;
 
 /**
  * struct ocelot_ddata - Private data for an external Ocelot chip
+ * @init_bus:		Bus-specific initialization callback (optional).
  * @init_regmap:	Bus-specific callback for initializing regmap.
  * @gcb_regmap:		General Configuration Block regmap. Used for
  *			operations like chip reset.
@@ -25,6 +26,7 @@ struct resource;
  *			data of a SPI read operation.
  */
 struct ocelot_ddata {
+	int (*init_bus)(struct device *dev);
 	struct regmap * (*init_regmap)(struct device *dev, const struct resource *res);
 	struct regmap *gcb_regmap;
 	struct regmap *cpuorg_regmap;
-- 
2.49.0


  parent reply	other threads:[~2025-03-19 12:31 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-19 12:30 [PATCH 0/8] mfd: ocelot: add support for MDIO managed switch Rasmus Villemoes
2025-03-19 12:30 ` [PATCH 1/8] mfd: ocelot: refactor bus-specific regmap initialization Rasmus Villemoes
2025-03-19 20:08   ` Colin Foster
2025-03-21 11:41     ` Lee Jones
2025-03-21 12:39       ` Rasmus Villemoes
2025-03-28  8:37         ` Lee Jones
2025-03-19 12:30 ` [PATCH 2/8] mfd: ocelot: move SPI specific macros to ocelot-spi.c Rasmus Villemoes
2025-03-19 20:11   ` Colin Foster
2025-03-19 12:30 ` Rasmus Villemoes [this message]
2025-03-19 22:08   ` [PATCH 3/8] mfd: ocelot: rework SPI (re-)initialization after chip reset Colin Foster
2025-03-20 11:17     ` Rasmus Villemoes
2025-03-22 13:36       ` Colin Foster
2025-03-25 15:35         ` Rasmus Villemoes
2025-03-19 12:30 ` [PATCH 4/8] mfd: ocelot: lift chip reset logic to ocelot-core.c Rasmus Villemoes
2025-03-19 22:44   ` Colin Foster
2025-03-19 12:30 ` [PATCH 5/8] mfd: ocelot: make ocelot_chip_init() static Rasmus Villemoes
2025-03-19 22:45   ` Colin Foster
2025-03-19 12:30 ` [PATCH 6/8] mfd: ocelot: correct Kconfig dependency Rasmus Villemoes
2025-03-19 22:48   ` Colin Foster
2025-03-19 12:30 ` [PATCH 7/8] mfd: ocelot: enable support for mdio management Rasmus Villemoes
2025-03-19 12:30 ` [PATCH 8/8] dt-bindings: mfd: ocelot: mention MDIO management and add example Rasmus Villemoes
2025-03-19 13:24   ` Rob Herring (Arm)
2025-03-20 14:25     ` Rasmus Villemoes
2025-03-19 19:55 ` [PATCH 0/8] mfd: ocelot: add support for MDIO managed switch Colin Foster
2025-06-30 15:14 ` Colin Foster
2025-07-01 10:52   ` Rasmus Villemoes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250319123058.452202-4-ravi@prevas.dk \
    --to=ravi@prevas.dk \
    --cc=colin.foster@in-advantage.com \
    --cc=devicetree@vger.kernel.org \
    --cc=felix.blix@prevas.dk \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®