The 10/27/2021 12:41, Peter Rosin wrote:
>
> Hi!
Hi Peter,
>
> I'm sorry for the slow response...
>
> On 2021-10-13 16:10, Horatiu Vultur wrote:
> > Use select-delay property to add a delay once the mux state is changed.
> > This is required on some platforms to allow the GPIO signals to get
> > stabilized.
> >
> > Signed-off-by: Lars Povlsen <lars.povlsen@xxxxxxxxxxxxx>
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@xxxxxxxxxxxxx>
> > ---
> > drivers/i2c/muxes/i2c-mux-gpio.c | 7 +++++++
> > 1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
> > index bac415a52b78..1cc69eb67221 100644
> > --- a/drivers/i2c/muxes/i2c-mux-gpio.c
> > +++ b/drivers/i2c/muxes/i2c-mux-gpio.c
> > @@ -13,6 +13,8 @@
> > #include <linux/slab.h>
> > #include <linux/bits.h>
> > #include <linux/gpio/consumer.h>
> > +#include <linux/delay.h>
> > +
> > /* FIXME: stop poking around inside gpiolib */
> > #include "../../gpio/gpiolib.h"
> >
> > @@ -20,6 +22,7 @@ struct gpiomux {
> > struct i2c_mux_gpio_platform_data data;
> > int ngpios;
> > struct gpio_desc **gpios;
> > + int select_delay;
> > };
> >
> > static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
> > @@ -29,6 +32,8 @@ static void i2c_mux_gpio_set(const struct gpiomux *mux, unsigned val)
> > values[0] = val;
> >
> > gpiod_set_array_value_cansleep(mux->ngpios, mux->gpios, NULL, values);
> > + if (mux->select_delay)
> > + udelay(mux->select_delay);
>
> Use fsleep(mux->select_delay) if you don't know how long the delay really
> is.
>
> However, you needlessly invoke the delay even if you do not actually change
> the state of the mux. In order to fix that, you need to keep track of the
> current state of the mux, but that's a chunk of boring code to write. If you
> instead switch to using a mux-gpio from the mux subsystem and point an
> i2c-mux-gpmux to that instance, you get that for free, and you can make simple
> changes to the i2c-mux-gpmux driver to get this sorted properly, basically
> exactly as this patch but with this
>
> - ret = mux_control_select(mux->control, chan->channel);
> + ret = mux_control_select_delay(mux->control, chan->channel,
> + mux->delay_us);
>
> instead of the udelay/fsleep in this patch. That will invoke the requested
> delay, but only if too little time has gone by since the latest state change.
Thanks for the advice! I will change to use i2c-mux-gpmux and make the
changes there.
>
> That interface (mux_control_select_delay) is brand new though, but available
> in linux-next and scheduled for the next merge window. But, since I fumbled
> this series it's a bit late for this merge window anyway (sorry again) so
> that should not be an issue.
No worries, I will try to send a new version.
>
> Cheers,
> Peter
>
> > }
> >
> > static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
> > @@ -153,6 +158,8 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
> > if (fwnode_property_read_u32(dev->fwnode, "idle-state", &mux->data.idle))
> > mux->data.idle = I2C_MUX_GPIO_NO_IDLE;
> >
> > + fwnode_property_read_u32(dev->fwnode, "select-delay", &mux->select_delay);
> > +
> > return 0;
> > }
> >
> >
--
/Horatiu
