On 12/16/25 18:08, Patrice CHOTARD wrote:
> 
> 
> On 12/9/25 13:19, Raphael Gallais-Pou wrote:
>> Drivers should extract device-tree data before probing via the
>> .of_to_plat hook.
>>
>> Implement it for stm32_ltdc driver.  No functional change.
>>
>> Signed-off-by: Raphael Gallais-Pou <[email protected]>
>> ---
>> This patch has been tested on stm32mp135f-dk and stm32mp257f-ev1.
>> ---
>>  drivers/video/stm32/stm32_ltdc.c | 218 
>> +++++++++++++++++++++------------------
>>  1 file changed, 117 insertions(+), 101 deletions(-)
>>
>> diff --git a/drivers/video/stm32/stm32_ltdc.c 
>> b/drivers/video/stm32/stm32_ltdc.c
>> index 
>> 834bfb625d2d34a44bd8edff1c92af6dec344c20..1c55fc94b3aa67dbcb822684c2c9a128b159e8a6
>>  100644
>> --- a/drivers/video/stm32/stm32_ltdc.c
>> +++ b/drivers/video/stm32/stm32_ltdc.c
>> @@ -22,8 +22,17 @@
>>  #include <linux/bitops.h>
>>  #include <linux/printk.h>
>>  
>> -struct stm32_ltdc_priv {
>> +struct stm32_ltdc_plat {
>>      void __iomem *regs;
>> +    struct udevice *bridge;
>> +    struct udevice *panel;
>> +    struct reset_ctl rst;
>> +    struct clk pclk;
>> +    struct clk bclk;
>> +};
>> +
>> +struct stm32_ltdc_priv {
>> +    struct display_timing timings;
>>      enum video_log2_bpp l2bpp;
>>      u32 bg_col_argb;
>>      const u32 *layer_regs;
>> @@ -364,18 +373,20 @@ static bool has_alpha(u32 fmt)
>>      }
>>  }
>>  
>> -static void stm32_ltdc_enable(struct stm32_ltdc_priv *priv)
>> +static void stm32_ltdc_enable(void __iomem *regs)
>>  {
>>      /* Reload configuration immediately & enable LTDC */
>> -    setbits_le32(priv->regs + LTDC_SRCR, SRCR_IMR);
>> -    setbits_le32(priv->regs + LTDC_GCR, GCR_LTDCEN);
>> +    setbits_le32(regs + LTDC_SRCR, SRCR_IMR);
>> +    setbits_le32(regs + LTDC_GCR, GCR_LTDCEN);
>>  }
>>  
>> -static void stm32_ltdc_set_mode(struct stm32_ltdc_priv *priv,
>> -                            struct display_timing *timings)
>> +static void stm32_ltdc_set_mode(struct udevice *dev)
>>  {
>> -    void __iomem *regs = priv->regs;
>> +    struct stm32_ltdc_plat *plat = dev_get_plat(dev);
>> +    struct stm32_ltdc_priv *priv = dev_get_priv(dev);
>> +    struct display_timing *timings = &priv->timings;
>>      u32 hsync, vsync, acc_hbp, acc_vbp, acc_act_w, acc_act_h;
>> +    void __iomem *regs = plat->regs;
>>      u32 total_w, total_h;
>>      u32 val;
>>  
>> @@ -422,12 +433,14 @@ static void stm32_ltdc_set_mode(struct stm32_ltdc_priv 
>> *priv,
>>                      GCR_HSPOL | GCR_VSPOL | GCR_DEPOL | GCR_PCPOL, val);
>>  
>>      /* Overall background color */
>> -    writel(priv->bg_col_argb, priv->regs + LTDC_BCCR);
>> +    writel(priv->bg_col_argb, regs + LTDC_BCCR);
>>  }
>>  
>> -static void stm32_ltdc_set_layer1(struct stm32_ltdc_priv *priv, ulong 
>> fb_addr)
>> +static void stm32_ltdc_set_layer1(struct udevice *dev, ulong fb_addr)
>>  {
>> -    void __iomem *regs = priv->regs;
>> +    struct stm32_ltdc_plat *plat = dev_get_plat(dev);
>> +    struct stm32_ltdc_priv *priv = dev_get_priv(dev);
>> +    void __iomem *regs = plat->regs;
>>      u32 x0, x1, y0, y1;
>>      u32 pitch_in_bytes;
>>      u32 line_length;
>> @@ -493,7 +506,7 @@ static void stm32_ltdc_set_layer1(struct stm32_ltdc_priv 
>> *priv, ulong fb_addr)
>>      writel(fb_addr, regs + LTDC_L1CFBAR);
>>  
>>      /* Enable layer 1 */
>> -    setbits_le32(priv->regs + LTDC_L1CR, LXCR_LEN);
>> +    setbits_le32(regs + LTDC_L1CR, LXCR_LEN);
>>  }
>>  
>>  static int stm32_ltdc_get_remote_device(struct udevice *dev, ofnode ep_node,
>> @@ -618,53 +631,99 @@ static inline int stm32_ltdc_alloc_fb(struct udevice 
>> *dev)
>>  }
>>  #endif
>>  
>> -static int stm32_ltdc_probe(struct udevice *dev)
>> +static int stm32_ltdc_of_to_plat(struct udevice *dev)
>>  {
>> -    struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> -    struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>> +    struct stm32_ltdc_plat *plat = dev_get_plat(dev);
>>      struct stm32_ltdc_priv *priv = dev_get_priv(dev);
>> -    struct udevice *bridge = NULL;
>> -    struct udevice *panel = NULL;
>> -    struct display_timing timings;
>> -    struct clk pclk, bclk;
>> -    struct reset_ctl rst;
>>      ofnode node, port;
>> -    ulong rate;
>>      int ret;
>>  
>> -    priv->regs = dev_read_addr_ptr(dev);
>> -    if (!priv->regs) {
>> +    plat->regs = dev_read_addr_ptr(dev);
>> +    if (!plat->regs) {
>>              dev_err(dev, "ltdc dt register address error\n");
>>              return -EINVAL;
>>      }
>>  
>> -    ret = clk_get_by_name(dev, "bus", &bclk);
>> -    if (ret) {
>> -            if (ret != -ENODATA) {
>> -                    dev_err(dev, "bus clock get error %d\n", ret);
>> -                    return ret;
>> -            }
>> -    } else {
>> -            ret = clk_enable(&bclk);
>> -            if (ret) {
>> -                    dev_err(dev, "bus clock enable error %d\n", ret);
>> -                    return ret;
>> -            }
>> +    ret = clk_get_by_name(dev, "bus", &plat->bclk);
>> +    if (ret && ret != -ENODATA) {
>> +            dev_err(dev, "bus clock get error %d\n", ret);
>> +            return ret;
>>      }
>>  
>> -    ret = clk_get_by_name(dev, "lcd", &pclk);
>> +    ret = clk_get_by_name(dev, "lcd", &plat->pclk);
>>      if (ret) {
>>              dev_err(dev, "peripheral clock get error %d\n", ret);
>>              return ret;
>>      }
>>  
>> -    ret = clk_enable(&pclk);
>> +    /*
>> +     * Try all the ports until one working.
>> +     *
>> +     * This is done in two times. First is checks for the
>> +     * UCLASS_VIDEO_BRIDGE available, and then for this bridge
>> +     * it scans for a UCLASS_PANEL.
>> +     */
>> +    port = dev_read_subnode(dev, "port");
>> +    if (!ofnode_valid(port)) {
>> +            dev_err(dev, "%s(%s): 'port' subnode not found\n",
>> +                    __func__, dev_read_name(dev));
>> +            return -EINVAL;
>> +    }
>> +
>> +    for (node = ofnode_first_subnode(port);
>> +         ofnode_valid(node);
>> +         node = dev_read_next_subnode(node)) {
>> +            ret = stm32_ltdc_display_init(dev, &node, &plat->panel, 
>> &plat->bridge);
>> +            if (ret)
>> +                    dev_dbg(dev, "Device failed ret=%d\n", ret);
>> +            else
>> +                    break;
>> +    }
>> +
>> +    /* Sanity check */
>> +    if (ret)
>> +            return ret;
>> +
>> +    ret = panel_get_display_timing(plat->panel, &priv->timings);
>> +    if (ret) {
>> +            ret = ofnode_decode_display_timing(dev_ofnode(plat->panel),
>> +                                               0, &priv->timings);
>> +            if (ret) {
>> +                    dev_err(dev, "decode display timing error %d\n", ret);
>> +                    return ret;
>> +            }
>> +    }
>> +
>> +    ret = reset_get_by_index(dev, 0, &plat->rst);
>> +    if (ret)
>> +            dev_err(dev, "missing ltdc hardware reset\n");
>> +
>> +    return ret;
>> +}
>> +
>> +static int stm32_ltdc_probe(struct udevice *dev)
>> +{
>> +    struct video_uc_plat *uc_plat = dev_get_uclass_plat(dev);
>> +    struct video_priv *uc_priv = dev_get_uclass_priv(dev);
>> +    struct stm32_ltdc_plat *plat = dev_get_plat(dev);
>> +    struct stm32_ltdc_priv *priv = dev_get_priv(dev);
>> +    struct display_timing *timings = &priv->timings;
>> +    ulong rate;
>> +    int ret;
>> +
>> +    ret = clk_enable(&plat->bclk);
>> +    if (ret) {
>> +            dev_err(dev, "bus clock enable error %d\n", ret);
>> +            return ret;
>> +    }
>> +
>> +    ret = clk_enable(&plat->pclk);
>>      if (ret) {
>>              dev_err(dev, "peripheral clock enable error %d\n", ret);
>>              return ret;
>>      }
>>  
>> -    priv->hw_version = readl(priv->regs + LTDC_IDR);
>> +    priv->hw_version = readl(plat->regs + LTDC_IDR);
>>      dev_dbg(dev, "%s: LTDC hardware 0x%x\n", __func__, priv->hw_version);
>>  
>>      switch (priv->hw_version) {
>> @@ -686,67 +745,22 @@ static int stm32_ltdc_probe(struct udevice *dev)
>>              return -ENODEV;
>>      }
>>  
>> -    /*
>> -     * Try all the ports until one working.
>> -     *
>> -     * This is done in two times. First is checks for the
>> -     * UCLASS_VIDEO_BRIDGE available, and then for this bridge
>> -     * it scans for a UCLASS_PANEL.
>> -     */
>> -
>> -    port = dev_read_subnode(dev, "port");
>> -    if (!ofnode_valid(port)) {
>> -            dev_err(dev, "%s(%s): 'port' subnode not found\n",
>> -                    __func__, dev_read_name(dev));
>> -            return -EINVAL;
>> -    }
>> -
>> -    for (node = ofnode_first_subnode(port);
>> -         ofnode_valid(node);
>> -         node = dev_read_next_subnode(node)) {
>> -            ret = stm32_ltdc_display_init(dev, &node, &panel, &bridge);
>> -            if (ret)
>> -                    dev_dbg(dev, "Device failed ret=%d\n", ret);
>> -            else
>> -                    break;
>> -    }
>> -
>> -    /* Sanity check */
>> -    if (ret)
>> -            return ret;
>> -
>> -    ret = panel_get_display_timing(panel, &timings);
>> -    if (ret) {
>> -            ret = ofnode_decode_display_timing(dev_ofnode(panel),
>> -                                               0, &timings);
>> -            if (ret) {
>> -                    dev_err(dev, "decode display timing error %d\n", ret);
>> -                    return ret;
>> -            }
>> -    }
>> -
>> -    rate = clk_set_rate(&pclk, timings.pixelclock.typ);
>> +    rate = clk_set_rate(&plat->pclk, timings->pixelclock.typ);
>>      if (IS_ERR_VALUE(rate))
>>              dev_warn(dev, "fail to set pixel clock %d hz, ret=%ld\n",
>> -                     timings.pixelclock.typ, rate);
>> +                     timings->pixelclock.typ, rate);
>>  
>>      dev_dbg(dev, "Set pixel clock req %d hz get %ld hz\n",
>> -            timings.pixelclock.typ, rate);
>> -
>> -    ret = reset_get_by_index(dev, 0, &rst);
>> -    if (ret) {
>> -            dev_err(dev, "missing ltdc hardware reset\n");
>> -            return ret;
>> -    }
>> +            timings->pixelclock.typ, rate);
>>  
>>      /* Reset */
>> -    reset_deassert(&rst);
>> +    reset_deassert(&plat->rst);
>>  
>>      if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
>> -            if (bridge) {
>> -                    ret = video_bridge_attach(bridge);
>> +            if (plat->bridge) {
>> +                    ret = video_bridge_attach(plat->bridge);
>>                      if (ret) {
>> -                            dev_err(bridge, "fail to attach bridge\n");
>> +                            dev_err(plat->bridge, "fail to attach 
>> bridge\n");
>>                              return ret;
>>                      }
>>              }
>> @@ -757,8 +771,8 @@ static int stm32_ltdc_probe(struct udevice *dev)
>>      priv->bg_col_argb = 0xFFFFFFFF; /* white no transparency */
>>      priv->crop_x = 0;
>>      priv->crop_y = 0;
>> -    priv->crop_w = timings.hactive.typ;
>> -    priv->crop_h = timings.vactive.typ;
>> +    priv->crop_w = timings->hactive.typ;
>> +    priv->crop_h = timings->vactive.typ;
>>      priv->alpha = 0xFF;
>>  
>>      ret = stm32_ltdc_alloc_fb(dev);
>> @@ -766,30 +780,30 @@ static int stm32_ltdc_probe(struct udevice *dev)
>>              return ret;
>>  
>>      dev_dbg(dev, "%dx%d %dbpp frame buffer at 0x%lx\n",
>> -            timings.hactive.typ, timings.vactive.typ,
>> +            timings->hactive.typ, timings->vactive.typ,
>>              VNBITS(priv->l2bpp), uc_plat->base);
>>      dev_dbg(dev, "crop %d,%d %dx%d bg 0x%08x alpha %d\n",
>>              priv->crop_x, priv->crop_y, priv->crop_w, priv->crop_h,
>>              priv->bg_col_argb, priv->alpha);
>>  
>>      /* Configure & start LTDC */
>> -    stm32_ltdc_set_mode(priv, &timings);
>> -    stm32_ltdc_set_layer1(priv, uc_plat->base);
>> -    stm32_ltdc_enable(priv);
>> +    stm32_ltdc_set_mode(dev);
>> +    stm32_ltdc_set_layer1(dev, uc_plat->base);
>> +    stm32_ltdc_enable(plat->regs);
>>  
>> -    uc_priv->xsize = timings.hactive.typ;
>> -    uc_priv->ysize = timings.vactive.typ;
>> +    uc_priv->xsize = timings->hactive.typ;
>> +    uc_priv->ysize = timings->vactive.typ;
>>      uc_priv->bpix = priv->l2bpp;
>>  
>> -    if (!bridge) {
>> -            ret = panel_enable_backlight(panel);
>> +    if (!plat->bridge) {
>> +            ret = panel_enable_backlight(plat->panel);
>>              if (ret) {
>>                      dev_err(dev, "panel %s enable backlight error %d\n",
>> -                            panel->name, ret);
>> +                            plat->panel->name, ret);
>>                      return ret;
>>              }
>>      } else if (IS_ENABLED(CONFIG_VIDEO_BRIDGE)) {
>> -            ret = video_bridge_set_backlight(bridge, 80);
>> +            ret = video_bridge_set_backlight(plat->bridge, 80);
>>              if (ret) {
>>                      dev_err(dev, "fail to set backlight\n");
>>                      return ret;
>> @@ -827,7 +841,9 @@ U_BOOT_DRIVER(stm32_ltdc) = {
>>      .name                   = "stm32_display",
>>      .id                     = UCLASS_VIDEO,
>>      .of_match               = stm32_ltdc_ids,
>> +    .of_to_plat             = stm32_ltdc_of_to_plat,
>>      .probe                  = stm32_ltdc_probe,
>>      .bind                   = stm32_ltdc_bind,
>> +    .plat_auto      = sizeof(struct stm32_ltdc_plat),
>>      .priv_auto      = sizeof(struct stm32_ltdc_priv),
>>  };
>>
>> ---
>> base-commit: c98b6a6dcd1fc7bcba081fc2353954e33de5053c
>> change-id: 20251209-ltdc-48abd0de6852
>>
>> Best regards,
> 
> Hi Raphael
> 
> Reviewed-by: Patrice Chotard <[email protected]>
> 
> Thanks
> Patrice
> 

Applied to u-boot-stm32/master

Thanks
Patrice

Reply via email to