Hi Michael, Thank you for the patch. On Tue, Jun 11, 2019 at 03:04:07PM +0100, Michael Drake wrote: > If the alive check detects a transition to the alive state, > the device configuration is rewritten. This seems like a big hack. You will have at the very least to explain why this is needed, and why you can't configure the device in response to drm_bridge operation calls. > Signed-off-by: Michael Drake <michael.drake@xxxxxxxxxxxxxxx> > Cc: Patrick Glaser <pglaser@xxxxxxxxx> > Cc: Nate Case <ncase@xxxxxxxxx> > --- > drivers/gpu/drm/bridge/ti948.c | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/ti948.c b/drivers/gpu/drm/bridge/ti948.c > index 86daa3701b91..b5c766711c4b 100644 > --- a/drivers/gpu/drm/bridge/ti948.c > +++ b/drivers/gpu/drm/bridge/ti948.c > @@ -132,6 +132,8 @@ struct ti948_reg_val { > * @reg_names: Array of regulator names, or NULL. > * @regs: Array of regulators, or NULL. > * @reg_count: Number of entries in reg_names and regs arrays. > + * @alive_check: Context for the alive checking work item. > + * @alive: Whether the device is alive or not (alive_check). > */ > struct ti948_ctx { > struct i2c_client *i2c; > @@ -141,6 +143,8 @@ struct ti948_ctx { > const char **reg_names; > struct regulator **regs; > size_t reg_count; > + struct delayed_work alive_check; > + bool alive; > }; > > static bool ti948_readable_reg(struct device *dev, unsigned int reg) > @@ -346,6 +350,8 @@ static int ti948_power_on(struct ti948_ctx *ti948) > if (ret != 0) > return ret; > > + ti948->alive = true; > + > msleep(500); > > return 0; > @@ -356,6 +362,8 @@ static int ti948_power_off(struct ti948_ctx *ti948) > int i; > int ret; > > + ti948->alive = false; > + > for (i = ti948->reg_count; i > 0; i--) { > dev_info(&ti948->i2c->dev, "Disabling %s regulator\n", > ti948->reg_names[i - 1]); > @@ -388,8 +396,17 @@ static void ti948_alive_check(struct work_struct *work) > { > struct delayed_work *dwork = to_delayed_work(work); > struct ti948_ctx *ti948 = delayed_work_to_ti948_ctx(dwork); > + int ret = ti948_device_check(ti948); > > - dev_info(&ti948->i2c->dev, "%s Alive check!\n", __func__); > + if (ti948->alive == false && ret == 0) { > + dev_info(&ti948->i2c->dev, "Device has come back to life!\n"); > + ti948_write_config_seq(ti948); > + ti948->alive = true; > + > + } else if (ti948->alive == true && ret != 0) { > + dev_info(&ti948->i2c->dev, "Device has stopped responding\n"); > + ti948->alive = false; > + } > > /* Reschedule ourself for the next check. */ > schedule_delayed_work(&ti948->alive_check, TI948_ALIVE_CHECK_DELAY); -- Regards, Laurent Pinchart