The MAX9286 implements an I2C mux which we maintain in an open state
while we are streaming from the cameras.
The development code for the MAX9286 uses an integer value with
arbitrary state values to control these state transitions. This is
highlighted with a FIXME and is difficult to interpret the meaning of the
values 0, 1, 2.
Introduce a new function call max9286_i2c_mux_open() to make it clear
when a component opens all the mux channels, and update the usage
in s_stream() to max9286_i2c_mux_close() the mux on stop stream.
We previously had missed an occasion to sleep after an update to the I2C
Fwd/Rev channels, so all writes to this configuration register are moved
to a helper: max9286_i2c_mux_configure() which guarantees the delay.
Signed-off-by: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
---
drivers/media/i2c/max9286.c | 74 ++++++++++++++++++-------------------
1 file changed, 37 insertions(+), 37 deletions(-)
diff --git a/drivers/media/i2c/max9286.c b/drivers/media/i2c/max9286.c
index 5b8dfa652d50..b34fb31c6db5 100644
--- a/drivers/media/i2c/max9286.c
+++ b/drivers/media/i2c/max9286.c
@@ -144,7 +144,7 @@ struct max9286_priv {
struct media_pad pads[MAX9286_N_PADS];
struct regulator *regulator;
bool poc_enabled;
- int streaming;
+ int mux_state;
struct i2c_mux_core *mux;
unsigned int mux_channel;
@@ -221,57 +221,59 @@ static int max9286_write(struct max9286_priv *priv, u8 reg, u8 val)
* I2C Multiplexer
*/
-static int max9286_i2c_mux_close(struct max9286_priv *priv)
+enum max9286_i2c_mux_state {
+ MAX9286_MUX_CLOSED = 0,
+ MAX9286_MUX_OPEN,
+};
+
+static void max9286_i2c_mux_configure(struct max9286_priv *priv, u8 conf)
+{
+ max9286_write(priv, 0x0a, conf);
+
+ /*
+ * We must sleep after any change to the forward or reverse channel
+ * configuration.
+ */
+ usleep_range(3000, 5000);
+}
+
+static void max9286_i2c_mux_open(struct max9286_priv *priv)
+{
+ /* Open all channels on the MAX9286 */
+ max9286_i2c_mux_configure(priv, 0xff);
+
+ priv->mux_state = MAX9286_MUX_OPEN;
+}
+
+static void max9286_i2c_mux_close(struct max9286_priv *priv)
{
- /* FIXME: See note in max9286_i2c_mux_select() */
- if (priv->streaming)
- return 0;
/*
* Ensure that both the forward and reverse channel are disabled on the
* mux, and that the channel ID is invalidated to ensure we reconfigure
- * on the next select call.
+ * on the next max9286_i2c_mux_select() call.
*/
- priv->mux_channel = -1;
- max9286_write(priv, 0x0a, 0x00);
- usleep_range(3000, 5000);
+ max9286_i2c_mux_configure(priv, 0x00);
- return 0;
+ priv->mux_state = MAX9286_MUX_CLOSED;
+ priv->mux_channel = -1;
}
static int max9286_i2c_mux_select(struct i2c_mux_core *muxc, u32 chan)
{
struct max9286_priv *priv = i2c_mux_priv(muxc);
- /*
- * FIXME: This state keeping is a hack and do the job. It should
- * be should be reworked. One option to consider is that once all
- * cameras are programmed the mux selection logic should be disabled
- * and all all reverse and forward channels enable all the time.
- *
- * In any case this logic with a int that have two states should be
- * reworked!
- */
- if (priv->streaming == 1) {
- max9286_write(priv, 0x0a, 0xff);
- priv->streaming = 2;
- return 0;
- } else if (priv->streaming == 2) {
+ /* channel select is disabled when configured in the opened state. */
+ if (priv->mux_state == MAX9286_MUX_OPEN)
return 0;
- }
if (priv->mux_channel == chan)
return 0;
priv->mux_channel = chan;
- max9286_write(priv, 0x0a,
- MAX9286_FWDCCEN(chan) | MAX9286_REVCCEN(chan));
-
- /*
- * We must sleep after any change to the forward or reverse channel
- * configuration.
- */
- usleep_range(3000, 5000);
+ max9286_i2c_mux_configure(priv,
+ MAX9286_FWDCCEN(chan) |
+ MAX9286_REVCCEN(chan));
return 0;
}
@@ -441,8 +443,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
int ret;
if (enable) {
- /* FIXME: See note in max9286_i2c_mux_select() */
- priv->streaming = 1;
+ max9286_i2c_mux_open(priv);
/* Start all cameras. */
for_each_source(priv, source) {
@@ -490,8 +491,7 @@ static int max9286_s_stream(struct v4l2_subdev *sd, int enable)
for_each_source(priv, source)
v4l2_subdev_call(source->sd, video, s_stream, 0);
- /* FIXME: See note in max9286_i2c_mux_select() */
- priv->streaming = 0;
+ max9286_i2c_mux_close(priv);
}
return 0;
--
2.20.1
