Commit df405910 authored by Vladimir Oltean's avatar Vladimir Oltean Committed by David S. Miller

net: dsa: sja1105: wait for dynamic config command completion on writes too

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: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 232deb3f
...@@ -1170,6 +1170,56 @@ const struct sja1105_dynamic_table_ops sja1110_dyn_ops[BLK_IDX_MAX_DYN] = { ...@@ -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 /* Provides read access to the settings through the dynamic interface
* of the switch. * of the switch.
* @blk_idx is used as key to select from the sja1105_dynamic_table_ops. * @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, ...@@ -1196,7 +1246,6 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv,
struct sja1105_dyn_cmd cmd = {0}; struct sja1105_dyn_cmd cmd = {0};
/* SPI payload buffer */ /* SPI payload buffer */
u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {0}; u8 packed_buf[SJA1105_MAX_DYN_CMD_SIZE] = {0};
int retries = 3;
int rc; int rc;
if (blk_idx >= BLK_IDX_MAX_DYN) if (blk_idx >= BLK_IDX_MAX_DYN)
...@@ -1239,28 +1288,12 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv, ...@@ -1239,28 +1288,12 @@ int sja1105_dynamic_config_read(struct sja1105_private *priv,
if (rc < 0) if (rc < 0)
return rc; return rc;
/* Loop until we have confirmation that hardware has finished rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops);
* processing the command and has cleared the VALID field if (rc < 0)
*/ return rc;
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);
if (cmd.valid) if (!cmd.valident && !(ops->access & OP_VALID_ANYWAY))
return -ETIMEDOUT; return -ENOENT;
/* Don't dereference possibly NULL pointer - maybe caller /* Don't dereference possibly NULL pointer - maybe caller
* only wanted to see whether the entry existed or not. * only wanted to see whether the entry existed or not.
...@@ -1321,6 +1354,10 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv, ...@@ -1321,6 +1354,10 @@ int sja1105_dynamic_config_write(struct sja1105_private *priv,
if (rc < 0) if (rc < 0)
return rc; return rc;
rc = sja1105_dynamic_config_wait_complete(priv, &cmd, ops);
if (rc < 0)
return rc;
cmd = (struct sja1105_dyn_cmd) {0}; cmd = (struct sja1105_dyn_cmd) {0};
ops->cmd_packing(packed_buf, &cmd, UNPACK); ops->cmd_packing(packed_buf, &cmd, UNPACK);
if (cmd.errors) if (cmd.errors)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment