Hi Wojciech,
Thank you for the patch.
On Fri, Sep 13, 2024 at 06:51:16AM +0000, Wojciech Siudy (Nokia) wrote:
> From: Wojciech Siudy <wojciech.siudy@xxxxxxxxx>
>
> If the channel selection or deselection times out, it indicates
> a failure in the mux's I2C subsystem. Without sending a reset pulse,
> a power-on-reset of the entire device would be required to restore
> communication.
>
> The datasheet specifies a minimum hold time of 4 ns for the reset
> pulse, but due to the path's capacitance and the mux having its own
> clock, it is recommended to extend this to approximately 1 us.
>
> This option can be enabled using the i2c-mux-timeout-reset property
> in the device tree and should only be used if the reset line is not
> shared with other devices.
>
> Signed-off-by: Wojciech Siudy <wojciech.siudy@xxxxxxxxx>
> ---
> Changelog:
> v2:
> * Removed mail header from the commit log
> * Decreased reset pulse hold time from 10 to 1 ms
> v3:
> * Make this functionality enabled by appropriate property in
> devicetree
> ---
> drivers/i2c/muxes/i2c-mux-pca954x.c | 48 ++++++++++++++++++++++-------
> 1 file changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 6f84018258c4..c9ac0f9c9408 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -110,6 +110,7 @@ struct pca954x {
> u8 last_chan; /* last register value */
> /* MUX_IDLE_AS_IS, MUX_IDLE_DISCONNECT or >= 0 for channel */
> s32 idle_state;
> + u8 timeout_reset;
>
> struct i2c_client *client;
>
> @@ -316,6 +317,30 @@ static u8 pca954x_regval(struct pca954x *data, u8 chan)
> return 1 << chan;
> }
>
> +static void pca954x_reset_deassert(struct pca954x *data)
> +{
> + if (data->reset_cont)
> + reset_control_deassert(data->reset_cont);
> + else
> + gpiod_set_value_cansleep(data->reset_gpio, 0);
> +}
> +
> +static void pca954x_reset_assert(struct pca954x *data)
> +{
> + if (data->reset_cont)
> + reset_control_assert(data->reset_cont);
> + else
> + gpiod_set_value_cansleep(data->reset_gpio, 1);
> +}
> +
> +static void pca954x_reset_mux(struct pca954x *data)
> +{
> + dev_warn(&data->client->dev, "resetting the device\n");
> + pca954x_reset_assert(data);
> + udelay(1);
> + pca954x_reset_deassert(data);
> +}
> +
> static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> {
> struct pca954x *data = i2c_mux_priv(muxc);
> @@ -329,6 +354,8 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> ret = pca954x_reg_write(muxc->parent, client, regval);
> data->last_chan = ret < 0 ? 0 : regval;
> }
> + if (ret == -ETIMEDOUT && (data->reset_cont || data->reset_gpio))
> + pca954x_reset_mux(data);
>
> return ret;
> }
> @@ -338,6 +365,7 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
> struct pca954x *data = i2c_mux_priv(muxc);
> struct i2c_client *client = data->client;
> s32 idle_state;
> + int ret = 0;
>
> idle_state = READ_ONCE(data->idle_state);
> if (idle_state >= 0)
> @@ -347,13 +375,14 @@ static int pca954x_deselect_mux(struct i2c_mux_core *muxc, u32 chan)
> if (idle_state == MUX_IDLE_DISCONNECT) {
> /* Deselect active channel */
> data->last_chan = 0;
> - return pca954x_reg_write(muxc->parent, client,
> - data->last_chan);
> + ret = pca954x_reg_write(muxc->parent, client, data->last_chan);
> + if (ret == -ETIMEDOUT && (data->reset_cont || data->reset_gpio))
> + pca954x_reset_mux(data);
> }
>
> /* otherwise leave as-is */
>
> - return 0;
> + return ret;
> }
>
> static ssize_t idle_state_show(struct device *dev,
> @@ -543,14 +572,6 @@ static int pca954x_get_reset(struct device *dev, struct pca954x *data)
> return 0;
> }
>
> -static void pca954x_reset_deassert(struct pca954x *data)
> -{
> - if (data->reset_cont)
> - reset_control_deassert(data->reset_cont);
> - else
> - gpiod_set_value_cansleep(data->reset_gpio, 0);
> -}
> -
> /*
> * I2C init/probing/exit functions
> */
> @@ -625,6 +646,11 @@ static int pca954x_probe(struct i2c_client *client)
> data->idle_state = MUX_IDLE_DISCONNECT;
> }
>
> + if (device_property_read_bool(dev, "i2c-mux-timeout-reset"))
> + data->timeout_reset = 1;
> + else
> + data->timeout_reset = 0;
timeout_reset is set but never used. Am I missing something ?
> +
> /*
> * Write the mux register at addr to verify
> * that the mux is in fact present. This also
--
Regards,
Laurent Pinchart
