| Message ID | 20220519114751.69852-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/bridge: tc358767: Make sure Refclk clock are enabled | expand |
Am Donnerstag, dem 19.05.2022 um 13:47 +0200 schrieb Marek Vasut: > The Refclk may be supplied by SoC clock output instead of crystal > oscillator, make sure the clock are enabled before any other action > is performed with the bridge chip, otherwise it may either fail to > operate at all, or miss reset GPIO toggle. > > Fixes: 7caff0fc4296e ("drm/bridge: tc358767: Add DPI to eDP bridge driver") > Signed-off-by: Marek Vasut <marex@denx.de> Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > --- > V2: Use devm_add_action_or_reset() to add clock disable hook instead > of wall of failpath > --- > drivers/gpu/drm/bridge/tc358767.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 45ea829d56601..b2ef01303be23 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -2033,6 +2033,13 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) > return -EINVAL; > } > > +static void tc_clk_disable(void *data) > +{ > + struct clk *refclk = data; > + > + clk_disable_unprepare(refclk); > +} > + > static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > { > struct device *dev = &client->dev; > @@ -2049,6 +2056,22 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > if (ret) > return ret; > > + tc->refclk = devm_clk_get(dev, "ref"); > + if (IS_ERR(tc->refclk)) { > + ret = PTR_ERR(tc->refclk); > + dev_err(dev, "Failed to get refclk: %d\n", ret); > + return ret; > + } > + > + ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); > + if (ret) > + return ret; > + > + clk_prepare_enable(tc->refclk); > + > + /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */ > + usleep_range(10, 15); > + > /* Shut down GPIO is optional */ > tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH); > if (IS_ERR(tc->sd_gpio)) > @@ -2069,13 +2092,6 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > usleep_range(5000, 10000); > } > > - tc->refclk = devm_clk_get(dev, "ref"); > - if (IS_ERR(tc->refclk)) { > - ret = PTR_ERR(tc->refclk); > - dev_err(dev, "Failed to get refclk: %d\n", ret); > - return ret; > - } > - > tc->regmap = devm_regmap_init_i2c(client, &tc_regmap_config); > if (IS_ERR(tc->regmap)) { > ret = PTR_ERR(tc->regmap);
On Thu, May 19, 2022 at 01:47:51PM +0200, Marek Vasut wrote: > The Refclk may be supplied by SoC clock output instead of crystal > oscillator, make sure the clock are enabled before any other action > is performed with the bridge chip, otherwise it may either fail to > operate at all, or miss reset GPIO toggle. > > Fixes: 7caff0fc4296e ("drm/bridge: tc358767: Add DPI to eDP bridge driver") > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jonas Karlman <jonas@kwiboo.se> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> > Cc: Lucas Stach <l.stach@pengutronix.de> > Cc: Marek Vasut <marex@denx.de> > Cc: Maxime Ripard <maxime@cerno.tech> > Cc: Neil Armstrong <narmstrong@baylibre.com> > Cc: Robert Foss <robert.foss@linaro.org> > Cc: Sam Ravnborg <sam@ravnborg.org> > --- > V2: Use devm_add_action_or_reset() to add clock disable hook instead > of wall of failpath > --- > drivers/gpu/drm/bridge/tc358767.c | 30 +++++++++++++++++++++++------- > 1 file changed, 23 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 45ea829d56601..b2ef01303be23 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -2033,6 +2033,13 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) > return -EINVAL; > } > > +static void tc_clk_disable(void *data) > +{ > + struct clk *refclk = data; > + > + clk_disable_unprepare(refclk); > +} > + > static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > { > struct device *dev = &client->dev; > @@ -2049,6 +2056,22 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > if (ret) > return ret; > > + tc->refclk = devm_clk_get(dev, "ref"); > + if (IS_ERR(tc->refclk)) { > + ret = PTR_ERR(tc->refclk); > + dev_err(dev, "Failed to get refclk: %d\n", ret); > + return ret; > + } > + > + ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); > + if (ret) > + return ret; if devm_add_action_or_reset fails, tc_clk_disable will be called resulting in an unbalanced enable count for refclk. it should be called after the clk_prepare_enable() call. Maxime
diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 45ea829d56601..b2ef01303be23 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -2033,6 +2033,13 @@ static int tc_probe_bridge_endpoint(struct tc_data *tc) return -EINVAL; } +static void tc_clk_disable(void *data) +{ + struct clk *refclk = data; + + clk_disable_unprepare(refclk); +} + static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) { struct device *dev = &client->dev; @@ -2049,6 +2056,22 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) if (ret) return ret; + tc->refclk = devm_clk_get(dev, "ref"); + if (IS_ERR(tc->refclk)) { + ret = PTR_ERR(tc->refclk); + dev_err(dev, "Failed to get refclk: %d\n", ret); + return ret; + } + + ret = devm_add_action_or_reset(dev, tc_clk_disable, tc->refclk); + if (ret) + return ret; + + clk_prepare_enable(tc->refclk); + + /* tRSTW = 100 cycles , at 13 MHz that is ~7.69 us */ + usleep_range(10, 15); + /* Shut down GPIO is optional */ tc->sd_gpio = devm_gpiod_get_optional(dev, "shutdown", GPIOD_OUT_HIGH); if (IS_ERR(tc->sd_gpio)) @@ -2069,13 +2092,6 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) usleep_range(5000, 10000); } - tc->refclk = devm_clk_get(dev, "ref"); - if (IS_ERR(tc->refclk)) { - ret = PTR_ERR(tc->refclk); - dev_err(dev, "Failed to get refclk: %d\n", ret); - return ret; - } - tc->regmap = devm_regmap_init_i2c(client, &tc_regmap_config); if (IS_ERR(tc->regmap)) { ret = PTR_ERR(tc->regmap);
The Refclk may be supplied by SoC clock output instead of crystal oscillator, make sure the clock are enabled before any other action is performed with the bridge chip, otherwise it may either fail to operate at all, or miss reset GPIO toggle. Fixes: 7caff0fc4296e ("drm/bridge: tc358767: Add DPI to eDP bridge driver") Signed-off-by: Marek Vasut <marex@denx.de> Cc: Jonas Karlman <jonas@kwiboo.se> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com> Cc: Lucas Stach <l.stach@pengutronix.de> Cc: Marek Vasut <marex@denx.de> Cc: Maxime Ripard <maxime@cerno.tech> Cc: Neil Armstrong <narmstrong@baylibre.com> Cc: Robert Foss <robert.foss@linaro.org> Cc: Sam Ravnborg <sam@ravnborg.org> --- V2: Use devm_add_action_or_reset() to add clock disable hook instead of wall of failpath --- drivers/gpu/drm/bridge/tc358767.c | 30 +++++++++++++++++++++++------- 1 file changed, 23 insertions(+), 7 deletions(-)