@@ -1170,6 +1170,56 @@ const struct sja1105_dynamic_table_ops sja1110_dyn_ops[BLK_IDX_MAX_DYN] = {
},
};
+#define SJA1105_DYNAMIC_CONFIG_SLEEP_US 10
+#define SJA1105_DYNAMIC_CONFIG_TIMEOUT_US 100000
+
+static int
+sja1105_dynamic_config_poll_valid(struct sja1105_private *priv,
+ struct sja1105_dyn_cmd *cmd,
+ const struct sja1105_dynamic_table_ops *ops)
+{
+ u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {};
+ int rc;
+
+ /* We don't _need_ to read the full entry, just the command area which
+ * is a fixed SJA1105_SIZE_DYN_CMD. But our cmd_packing() API expects a
+ * buffer that contains the full entry too. Additionally, our API
+ * doesn't really know how many bytes into the buffer does the command
+ * area really begin. So just read back the whole entry.
+ */
+ rc = sja1105_xfer_buf(priv, SPI_READ, ops->addr, packed_buf,
+ ops->packed_size);
+ if (rc)
+ return rc;
+
+ /* Unpack the command structure, and return it to the caller in case it
+ * needs to perform further checks on it (VALIDENT).
+ */
+ memset(cmd, 0, sizeof(*cmd));
+ ops->cmd_packing(packed_buf, cmd, UNPACK);
+
+ /* Hardware hasn't cleared VALID => still working on it */
+ return cmd->valid ? -EAGAIN : 0;
+}
+
+/* Poll the dynamic config entry's control area until the hardware has
+ * cleared the VALID bit, which means we have confirmation that it has
+ * finished processing the command.
+ */
+static int
+sja1105_dynamic_config_wait_complete(struct sja1105_private *priv,
+ struct sja1105_dyn_cmd *cmd,
+ const struct sja1105_dynamic_table_ops *ops)
+{
+ int rc;
+
+ return read_poll_timeout(sja1105_dynamic_config_poll_valid,
+ rc, rc != -EAGAIN,
+ SJA1105_DYNAMIC_CONFIG_SLEEP_US,
+ SJA1105_DYNAMIC_CONFIG_TIMEOUT_US,
+ false, priv, cmd, ops);
+}
+
/* Provides read access to the settings through the dynamic interface
* of the switch.
* @blk_idx is used as key to select from the sja1105_dynamic_table_ops.
@@ -1196,7 +1246,6 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv,
struct sja1105_dyn_cmd cmd = {0};
/* SPI payload buffer */
u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {0};
- int retries = 3;
int rc;
if (blk_idx >= BLK_IDX_MAX_DYN)
@@ -1239,28 +1288,12 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv,
if (rc < 0)
return rc;
- /* Loop until we have confirmation that hardware has finished
- * processing the command and has cleared the VALID field
- */
- do {
- memset(packed_buf, 0, ops->packed_size);
-
- /* Retrieve the read operation's result */
- rc = sja1105_xfer_buf(priv, SPI_READ, ops->addr, packed_buf,
- ops->packed_size);
- if (rc < 0)
- return rc;
-
- cmd = (struct sja1105_dyn_cmd) {0};
- ops->cmd_packing(packed_buf, &cmd, UNPACK);
-
- if (!cmd.valident && !(ops->access & OP_VALID_ANYWAY))
- return -ENOENT;
- cpu_relax();
- } while (cmd.valid && --retries);
+ rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops);
+ if (rc < 0)
+ return rc;
- if (cmd.valid)
- return -ETIMEDOUT;
+ if (!cmd.valident && !(ops->access & OP_VALID_ANYWAY))
+ return -ENOENT;
/* Don't dereference possibly NULL pointer - maybe caller
* only wanted to see whether the entry existed or not.
@@ -1321,6 +1354,10 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv,
if (rc < 0)
return rc;
+ rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops);
+ if (rc < 0)
+ return rc;
+
cmd = (struct sja1105_dyn_cmd) {0};
ops->cmd_packing(packed_buf, &cmd, UNPACK);
if (cmd.errors)
The hardware manual says that software should attempt a new dynamic config access (be it a a write or a read-back) only while the VALID bit is cleared. The VALID bit is set by software to 1, and it remains set as long as the hardware is still processing the request. Currently the driver only polls for the command completion only for reads, because that's when we need the actual data read back. Writes have been more or less "asynchronous", although this has never been an observable issue. This change makes sja1105_dynamic_config_write poll the VALID bit as well, to absolutely ensure that a follow-up access to the static config finds the VALID bit cleared. So VALID means "work in progress", while VALIDENT means "entry being read is valid". On reads we check the VALIDENT bit too, while on writes that bit is not always defined. So we need to factor it out of the loop, and make the loop provide back the unpacked command structure, so that sja1105_dynamic_config_read can check the VALIDENT bit. The change also attempts to convert the open-coded loop to use the read_poll_timeout macro, since I know this will come up during review. It's more code, but hey, it uses read_poll_timeout! Tested on SJA1105T, SJA1105S, SJA1110A. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- v1->v5: none .../net/dsa/sja1105/sja1105_dynamic_config.c | 81 ++++++++++++++----- 1 file changed, 59 insertions(+), 22 deletions(-)