On Fri, 13 Jul 2018 14:11:10 +0200
Yannick Fertré yannick.fer...@st.com wrote:
> Manage a bridge insert between the display controller & a panel.
>
> Signed-off-by: Yannick Fertré <yannick.fer...@st.com>
> ---
> drivers/video/stm32/stm32_ltdc.c | 154
> +++++++++++++++++++++++----------------
> 1 file changed, 92 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/video/stm32/stm32_ltdc.c
> b/drivers/video/stm32/stm32_ltdc.c
> index dc6c889..4a1db5e 100644
> --- a/drivers/video/stm32/stm32_ltdc.c
> +++ b/drivers/video/stm32/stm32_ltdc.c
...
> @@ -330,96 +328,128 @@ static int stm32_ltdc_probe(struct udevice *dev)
> struct video_uc_platdata *uc_plat = dev_get_uclass_platdata(dev);
> struct video_priv *uc_priv = dev_get_uclass_priv(dev);
> struct stm32_ltdc_priv *priv = dev_get_priv(dev);
> - struct udevice *panel;
> +#ifdef CONFIG_VIDEO_BRIDGE
> + struct udevice *bridge = NULL;
> +#endif
Please drop ifdef here. See below.
...
> - rate = clk_set_rate(&pclk, priv->timing.pixelclock.typ);
> - if (rate < 0) {
> - debug("%s: fail to set pixel clock %d hz %d hz\n",
> - __func__, priv->timing.pixelclock.typ, rate);
> - return rate;
> +#ifdef CONFIG_VIDEO_BRIDGE
> + ret = uclass_get_device(UCLASS_VIDEO_BRIDGE, 0, &bridge);
> + if (ret)
> + debug("No video bridge, or no backlight on bridge\n");
> +
> + if (bridge) {
> + ret = video_bridge_attach(bridge);
> + if (ret) {
> + dev_err(dev, "fail to attach bridge\n");
> + return ret;
> + }
> }
> -
> - debug("%s: Set pixel clock req %d hz get %d hz\n", __func__,
> - priv->timing.pixelclock.typ, rate);
> -
> +#endif
Can we do without ifdefs and use IS_ENABLED() instead ?
E.g. like:
if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
ret = uclass_get_device(UCLASS_VIDEO_BRIDGE, 0, &bridge);
...
}
...
> +#ifdef CONFIG_VIDEO_BRIDGE
> + if (bridge) {
> + ret = video_bridge_set_backlight(bridge, 80);
> + if (ret) {
> + dev_err(dev, "fail to set backlight\n");
> + return ret;
> + }
> + } else {
> + ret = panel_enable_backlight(panel);
> + if (ret) {
> + dev_err(dev, "panel %s enable backlight error %d\n",
> + panel->name, ret);
> + return ret;
> + }
> + }
> +#else
> + ret = panel_enable_backlight(panel);
> + if (ret) {
> + dev_err(dev, "panel %s enable backlight error %d\n",
> + panel->name, ret);
> + return ret;
> + }
> +#endif
Please reduce code duplication:
if (!bridge) {
ret = panel_enable_backlight(panel);
if (ret) {
dev_err(dev, "panel %s enable backlight error %d\n",
panel->name, ret);
return ret;
}
} else if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
ret = video_bridge_set_backlight(bridge, 80);
if (ret) {
dev_err(dev, "fail to set backlight\n");
return ret;
}
}
--
Anatolij
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
