* [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid @ 2022-05-18 23:36 Marek Vasut 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Marek Vasut @ 2022-05-18 23:36 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Laurent Pinchart, Jonas Karlman, Neil Armstrong, robert.foss, Maxime Ripard, Sam Ravnborg Handle empty data-lanes = < >; property, which translates to dsi_lanes = 0 as invalid. Fixes: bbfd3190b6562 ("drm/bridge: tc358767: Add DSI-to-DPI mode support") 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> --- drivers/gpu/drm/bridge/tc358767.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 7366968dd7b21..84e6b5aa8dd1f 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -1902,7 +1902,7 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc) of_node_put(host_node); of_node_put(endpoint); - if (dsi_lanes < 0 || dsi_lanes > 4) + if (dsi_lanes <= 0 || dsi_lanes > 4) return -EINVAL; if (!host) -- 2.35.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported 2022-05-18 23:36 [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Marek Vasut @ 2022-05-18 23:36 ` Marek Vasut 2022-05-19 7:46 ` Andrzej Hajda 2022-05-19 7:52 ` Lucas Stach 2022-05-18 23:36 ` [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled Marek Vasut ` (2 subsequent siblings) 3 siblings, 2 replies; 10+ messages in thread From: Marek Vasut @ 2022-05-18 23:36 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Laurent Pinchart, Jonas Karlman, Neil Armstrong, robert.foss, Maxime Ripard, Sam Ravnborg The DSI-to-e(DP) mode is now supported, update the driver comment to reflect this. No functional change. Fixes: 3080c21a043ab ("drm/bridge: tc358767: Add DSI-to-(e)DP mode support") 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> --- drivers/gpu/drm/bridge/tc358767.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index 84e6b5aa8dd1f..a8bb6de9e600a 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -6,7 +6,7 @@ * The following modes are supported: * DPI->(e)DP -- supported * DSI->DPI .... supported - * DSI->(e)DP .. NOT supported + * DSI->(e)DP .. supported * * Copyright (C) 2016 CogentEmbedded Inc * Author: Andrey Gusakov <andrey.gusakov@cogentembedded.com> -- 2.35.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut @ 2022-05-19 7:46 ` Andrzej Hajda 2022-05-19 7:52 ` Lucas Stach 1 sibling, 0 replies; 10+ messages in thread From: Andrzej Hajda @ 2022-05-19 7:46 UTC (permalink / raw) To: Marek Vasut, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart On 19.05.2022 01:36, Marek Vasut wrote: > The DSI-to-e(DP) mode is now supported, update the driver comment > to reflect this. No functional change. > > Fixes: 3080c21a043ab ("drm/bridge: tc358767: Add DSI-to-(e)DP mode support") > 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> > --- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 84e6b5aa8dd1f..a8bb6de9e600a 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -6,7 +6,7 @@ > * The following modes are supported: > * DPI->(e)DP -- supported > * DSI->DPI .... supported > - * DSI->(e)DP .. NOT supported > + * DSI->(e)DP .. supported Since everything is supported(?) this message becomes useless, or can be shortened. With or w/o: Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > * > * Copyright (C) 2016 CogentEmbedded Inc > * Author: Andrey Gusakov <andrey.gusakov@cogentembedded.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut 2022-05-19 7:46 ` Andrzej Hajda @ 2022-05-19 7:52 ` Lucas Stach 1 sibling, 0 replies; 10+ messages in thread From: Lucas Stach @ 2022-05-19 7:52 UTC (permalink / raw) To: Marek Vasut, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart Am Donnerstag, dem 19.05.2022 um 01:36 +0200 schrieb Marek Vasut: > The DSI-to-e(DP) mode is now supported, update the driver comment > to reflect this. No functional change. > > Fixes: 3080c21a043ab ("drm/bridge: tc358767: Add DSI-to-(e)DP mode support") > 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> > --- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 84e6b5aa8dd1f..a8bb6de9e600a 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -6,7 +6,7 @@ > * The following modes are supported: > * DPI->(e)DP -- supported > * DSI->DPI .... supported > - * DSI->(e)DP .. NOT supported > + * DSI->(e)DP .. supported Couldn't we just drop the status behind the modes when the line above already states that those modes are supported, which is now true for all of them? But I then also don't care too much about it, so if you like your version better, feel free to commit with: Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > * > * Copyright (C) 2016 CogentEmbedded Inc > * Author: Andrey Gusakov <andrey.gusakov@cogentembedded.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled 2022-05-18 23:36 [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Marek Vasut 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut @ 2022-05-18 23:36 ` Marek Vasut 2022-05-19 7:57 ` Lucas Stach 2022-05-19 7:44 ` [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Andrzej Hajda 2022-05-19 7:50 ` Lucas Stach 3 siblings, 1 reply; 10+ messages in thread From: Marek Vasut @ 2022-05-18 23:36 UTC (permalink / raw) To: dri-devel Cc: Marek Vasut, Laurent Pinchart, Jonas Karlman, Neil Armstrong, robert.foss, Maxime Ripard, Sam Ravnborg 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> --- drivers/gpu/drm/bridge/tc358767.c | 51 ++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c index a8bb6de9e600a..4b15acbc73c4e 100644 --- a/drivers/gpu/drm/bridge/tc358767.c +++ b/drivers/gpu/drm/bridge/tc358767.c @@ -2055,10 +2055,24 @@ 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; + } + + 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)) - return PTR_ERR(tc->sd_gpio); + if (IS_ERR(tc->sd_gpio)) { + ret = PTR_ERR(tc->sd_gpio); + goto err_clk; + } if (tc->sd_gpio) { gpiod_set_value_cansleep(tc->sd_gpio, 0); @@ -2067,26 +2081,21 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) /* Reset GPIO is optional */ tc->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); - if (IS_ERR(tc->reset_gpio)) - return PTR_ERR(tc->reset_gpio); + if (IS_ERR(tc->reset_gpio)) { + ret = PTR_ERR(tc->reset_gpio); + goto err_clk; + } if (tc->reset_gpio) { gpiod_set_value_cansleep(tc->reset_gpio, 1); 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); dev_err(dev, "Failed to initialize regmap: %d\n", ret); - return ret; + goto err_clk; } ret = of_property_read_u32(dev->of_node, "toshiba,hpd-pin", @@ -2096,7 +2105,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) } else { if (tc->hpd_pin < 0 || tc->hpd_pin > 1) { dev_err(dev, "failed to parse HPD number\n"); - return ret; + goto err_clk; } } @@ -2110,7 +2119,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) "tc358767-irq", tc); if (ret) { dev_err(dev, "failed to register dp interrupt\n"); - return ret; + goto err_clk; } tc->have_irq = true; @@ -2119,12 +2128,13 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) ret = regmap_read(tc->regmap, TC_IDREG, &tc->rev); if (ret) { dev_err(tc->dev, "can not read device ID: %d\n", ret); - return ret; + goto err_clk; } if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) { dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev); - return -EINVAL; + ret = -EINVAL; + goto err_clk; } tc->assr = (tc->rev == 0x6601); /* Enable ASSR for eDP panels */ @@ -2164,7 +2174,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) if (tc->bridge.type != DRM_MODE_CONNECTOR_DPI) { /* (e)DP output */ ret = tc_aux_link_setup(tc); if (ret) - return ret; + goto err_clk; } tc->bridge.of_node = dev->of_node; @@ -2176,11 +2186,15 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) ret = tc_mipi_dsi_host_attach(tc); if (ret) { drm_bridge_remove(&tc->bridge); - return ret; + goto err_clk; } } return 0; + +err_clk: + clk_disable_unprepare(tc->refclk); + return ret; } static int tc_remove(struct i2c_client *client) @@ -2188,6 +2202,7 @@ static int tc_remove(struct i2c_client *client) struct tc_data *tc = i2c_get_clientdata(client); drm_bridge_remove(&tc->bridge); + clk_disable_unprepare(tc->refclk); return 0; } -- 2.35.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled 2022-05-18 23:36 ` [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled Marek Vasut @ 2022-05-19 7:57 ` Lucas Stach 2022-05-19 12:05 ` Marek Vasut 0 siblings, 1 reply; 10+ messages in thread From: Lucas Stach @ 2022-05-19 7:57 UTC (permalink / raw) To: Marek Vasut, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart Am Donnerstag, dem 19.05.2022 um 01:36 +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> > 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> > --- > drivers/gpu/drm/bridge/tc358767.c | 51 ++++++++++++++++++++----------- > 1 file changed, 33 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index a8bb6de9e600a..4b15acbc73c4e 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -2055,10 +2055,24 @@ 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; > + } > + > + 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)) > - return PTR_ERR(tc->sd_gpio); > + if (IS_ERR(tc->sd_gpio)) { > + ret = PTR_ERR(tc->sd_gpio); > + goto err_clk; > + } > > if (tc->sd_gpio) { > gpiod_set_value_cansleep(tc->sd_gpio, 0); > @@ -2067,26 +2081,21 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > > /* Reset GPIO is optional */ > tc->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > - if (IS_ERR(tc->reset_gpio)) > - return PTR_ERR(tc->reset_gpio); > + if (IS_ERR(tc->reset_gpio)) { > + ret = PTR_ERR(tc->reset_gpio); > + goto err_clk; > + } > > if (tc->reset_gpio) { > gpiod_set_value_cansleep(tc->reset_gpio, 1); > 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); > dev_err(dev, "Failed to initialize regmap: %d\n", ret); > - return ret; > + goto err_clk; > } > > ret = of_property_read_u32(dev->of_node, "toshiba,hpd-pin", > @@ -2096,7 +2105,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > } else { > if (tc->hpd_pin < 0 || tc->hpd_pin > 1) { > dev_err(dev, "failed to parse HPD number\n"); > - return ret; > + goto err_clk; > } > } > > @@ -2110,7 +2119,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > "tc358767-irq", tc); > if (ret) { > dev_err(dev, "failed to register dp interrupt\n"); > - return ret; > + goto err_clk; > } > > tc->have_irq = true; > @@ -2119,12 +2128,13 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > ret = regmap_read(tc->regmap, TC_IDREG, &tc->rev); > if (ret) { > dev_err(tc->dev, "can not read device ID: %d\n", ret); > - return ret; > + goto err_clk; > } > > if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) { > dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev); > - return -EINVAL; > + ret = -EINVAL; > + goto err_clk; > } > > tc->assr = (tc->rev == 0x6601); /* Enable ASSR for eDP panels */ > @@ -2164,7 +2174,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > if (tc->bridge.type != DRM_MODE_CONNECTOR_DPI) { /* (e)DP output */ > ret = tc_aux_link_setup(tc); > if (ret) > - return ret; > + goto err_clk; > } > > tc->bridge.of_node = dev->of_node; > @@ -2176,11 +2186,15 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) > ret = tc_mipi_dsi_host_attach(tc); > if (ret) { > drm_bridge_remove(&tc->bridge); > - return ret; > + goto err_clk; > } > } > > return 0; > + > +err_clk: > + clk_disable_unprepare(tc->refclk); > + return ret; That's a lot of error paths to disable a effectively always-on clock when something goes wrong. Could you maybe simplify this by using devm_add_action_or_reset for the clock disable? Seems like lots of watchdog drivers do this already if you need some inspiration. Regards, Lucas > } > > static int tc_remove(struct i2c_client *client) > @@ -2188,6 +2202,7 @@ static int tc_remove(struct i2c_client *client) > struct tc_data *tc = i2c_get_clientdata(client); > > drm_bridge_remove(&tc->bridge); > + clk_disable_unprepare(tc->refclk); > > return 0; > } ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled 2022-05-19 7:57 ` Lucas Stach @ 2022-05-19 12:05 ` Marek Vasut 0 siblings, 0 replies; 10+ messages in thread From: Marek Vasut @ 2022-05-19 12:05 UTC (permalink / raw) To: Lucas Stach, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart On 5/19/22 09:57, Lucas Stach wrote: > Am Donnerstag, dem 19.05.2022 um 01:36 +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> >> 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> >> --- >> drivers/gpu/drm/bridge/tc358767.c | 51 ++++++++++++++++++++----------- >> 1 file changed, 33 insertions(+), 18 deletions(-) >> >> diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c >> index a8bb6de9e600a..4b15acbc73c4e 100644 >> --- a/drivers/gpu/drm/bridge/tc358767.c >> +++ b/drivers/gpu/drm/bridge/tc358767.c >> @@ -2055,10 +2055,24 @@ 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; >> + } >> + >> + 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)) >> - return PTR_ERR(tc->sd_gpio); >> + if (IS_ERR(tc->sd_gpio)) { >> + ret = PTR_ERR(tc->sd_gpio); >> + goto err_clk; >> + } >> >> if (tc->sd_gpio) { >> gpiod_set_value_cansleep(tc->sd_gpio, 0); >> @@ -2067,26 +2081,21 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> >> /* Reset GPIO is optional */ >> tc->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); >> - if (IS_ERR(tc->reset_gpio)) >> - return PTR_ERR(tc->reset_gpio); >> + if (IS_ERR(tc->reset_gpio)) { >> + ret = PTR_ERR(tc->reset_gpio); >> + goto err_clk; >> + } >> >> if (tc->reset_gpio) { >> gpiod_set_value_cansleep(tc->reset_gpio, 1); >> 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); >> dev_err(dev, "Failed to initialize regmap: %d\n", ret); >> - return ret; >> + goto err_clk; >> } >> >> ret = of_property_read_u32(dev->of_node, "toshiba,hpd-pin", >> @@ -2096,7 +2105,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> } else { >> if (tc->hpd_pin < 0 || tc->hpd_pin > 1) { >> dev_err(dev, "failed to parse HPD number\n"); >> - return ret; >> + goto err_clk; >> } >> } >> >> @@ -2110,7 +2119,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> "tc358767-irq", tc); >> if (ret) { >> dev_err(dev, "failed to register dp interrupt\n"); >> - return ret; >> + goto err_clk; >> } >> >> tc->have_irq = true; >> @@ -2119,12 +2128,13 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> ret = regmap_read(tc->regmap, TC_IDREG, &tc->rev); >> if (ret) { >> dev_err(tc->dev, "can not read device ID: %d\n", ret); >> - return ret; >> + goto err_clk; >> } >> >> if ((tc->rev != 0x6601) && (tc->rev != 0x6603)) { >> dev_err(tc->dev, "invalid device ID: 0x%08x\n", tc->rev); >> - return -EINVAL; >> + ret = -EINVAL; >> + goto err_clk; >> } >> >> tc->assr = (tc->rev == 0x6601); /* Enable ASSR for eDP panels */ >> @@ -2164,7 +2174,7 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> if (tc->bridge.type != DRM_MODE_CONNECTOR_DPI) { /* (e)DP output */ >> ret = tc_aux_link_setup(tc); >> if (ret) >> - return ret; >> + goto err_clk; >> } >> >> tc->bridge.of_node = dev->of_node; >> @@ -2176,11 +2186,15 @@ static int tc_probe(struct i2c_client *client, const struct i2c_device_id *id) >> ret = tc_mipi_dsi_host_attach(tc); >> if (ret) { >> drm_bridge_remove(&tc->bridge); >> - return ret; >> + goto err_clk; >> } >> } >> >> return 0; >> + >> +err_clk: >> + clk_disable_unprepare(tc->refclk); >> + return ret; > > That's a lot of error paths to disable a effectively always-on clock > when something goes wrong. Could you maybe simplify this by using > devm_add_action_or_reset for the clock disable? Seems like lots of > watchdog drivers do this already if you need some inspiration. I sent V2 as a separate patch. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid 2022-05-18 23:36 [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Marek Vasut 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut 2022-05-18 23:36 ` [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled Marek Vasut @ 2022-05-19 7:44 ` Andrzej Hajda 2022-05-19 7:50 ` Lucas Stach 3 siblings, 0 replies; 10+ messages in thread From: Andrzej Hajda @ 2022-05-19 7:44 UTC (permalink / raw) To: Marek Vasut, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart On 19.05.2022 01:36, Marek Vasut wrote: > Handle empty data-lanes = < >; property, which translates to > dsi_lanes = 0 as invalid. > > Fixes: bbfd3190b6562 ("drm/bridge: tc358767: Add DSI-to-DPI mode support") > 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> Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com> Regards Andrzej > --- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 7366968dd7b21..84e6b5aa8dd1f 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1902,7 +1902,7 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc) > of_node_put(host_node); > of_node_put(endpoint); > > - if (dsi_lanes < 0 || dsi_lanes > 4) > + if (dsi_lanes <= 0 || dsi_lanes > 4) > return -EINVAL; > > if (!host) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid 2022-05-18 23:36 [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Marek Vasut ` (2 preceding siblings ...) 2022-05-19 7:44 ` [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Andrzej Hajda @ 2022-05-19 7:50 ` Lucas Stach 2022-05-19 9:45 ` Marek Vasut 3 siblings, 1 reply; 10+ messages in thread From: Lucas Stach @ 2022-05-19 7:50 UTC (permalink / raw) To: Marek Vasut, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart Am Donnerstag, dem 19.05.2022 um 01:36 +0200 schrieb Marek Vasut: > Handle empty data-lanes = < >; property, which translates to > dsi_lanes = 0 as invalid. > I'm having a hard time imagining a situation where on would add a empty data-lanes property to the DT. But then 0 DSI lanes is clearly a broken configuration, so: Reviewed-by: Lucas Stach <l.stach@pengutronix.de> > Fixes: bbfd3190b6562 ("drm/bridge: tc358767: Add DSI-to-DPI mode support") > 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> > --- > drivers/gpu/drm/bridge/tc358767.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/bridge/tc358767.c b/drivers/gpu/drm/bridge/tc358767.c > index 7366968dd7b21..84e6b5aa8dd1f 100644 > --- a/drivers/gpu/drm/bridge/tc358767.c > +++ b/drivers/gpu/drm/bridge/tc358767.c > @@ -1902,7 +1902,7 @@ static int tc_mipi_dsi_host_attach(struct tc_data *tc) > of_node_put(host_node); > of_node_put(endpoint); > > - if (dsi_lanes < 0 || dsi_lanes > 4) > + if (dsi_lanes <= 0 || dsi_lanes > 4) > return -EINVAL; > > if (!host) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid 2022-05-19 7:50 ` Lucas Stach @ 2022-05-19 9:45 ` Marek Vasut 0 siblings, 0 replies; 10+ messages in thread From: Marek Vasut @ 2022-05-19 9:45 UTC (permalink / raw) To: Lucas Stach, dri-devel Cc: Neil Armstrong, Jonas Karlman, robert.foss, Maxime Ripard, Sam Ravnborg, Laurent Pinchart On 5/19/22 09:50, Lucas Stach wrote: > Am Donnerstag, dem 19.05.2022 um 01:36 +0200 schrieb Marek Vasut: >> Handle empty data-lanes = < >; property, which translates to >> dsi_lanes = 0 as invalid. >> > I'm having a hard time imagining a situation where on would add a empty > data-lanes property to the DT. Like when you make a typo for example . ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-05-19 12:05 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-18 23:36 [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Marek Vasut 2022-05-18 23:36 ` [PATCH 2/3] drm/bridge: tc358767: Report DSI-to-(e)DP as supported Marek Vasut 2022-05-19 7:46 ` Andrzej Hajda 2022-05-19 7:52 ` Lucas Stach 2022-05-18 23:36 ` [PATCH 3/3] drm/bridge: tc358767: Make sure Refclk clock are enabled Marek Vasut 2022-05-19 7:57 ` Lucas Stach 2022-05-19 12:05 ` Marek Vasut 2022-05-19 7:44 ` [PATCH 1/3] drm/bridge: tc358767: Handle dsi_lanes == 0 as invalid Andrzej Hajda 2022-05-19 7:50 ` Lucas Stach 2022-05-19 9:45 ` Marek Vasut
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.