[PATCH] video: bridge: add ITE IT66121 DPI-to-HDMI bridge driver

[SHAURYA RANE]

On Fri, Apr 17, 2026 at 12:24 AM Simon Glass <sjg at chromium.org> wrote:

> Please can you add named defines for these bit fields (0x90, 0x89,
> 0x80, 0x10, 0x09, etc.) so it is clear what each AFE setting does. A
> brief comment referencing the programming guide section would also
> help.
>
yes will update that in V2
> > +     /* Preferred DTD pixel clock: bytes 54-55, units of 10 kHz. */
> > +     pclk_khz = ((unsigned long)priv->edid[55] << 8 |
> > +                  priv->edid[54]) * 10;
>
> Please can you use EDID_DETAILED_TIMING_PIXEL_CLOCK() from edid.h (or
> edid_get_timing()) rather than open-coding the EDID parsing.
>
yeah I should have done it is the same approach the anx6345 bridge uses
> > +static int it66121_attach(struct udevice *dev)
> > +{
> > +     struct it66121_priv *priv = dev_get_priv(dev);
> > +     unsigned long pclk_khz;
> > +     u8 status;
> > +     int ret;
> > +
> > +     ret = video_bridge_set_active(dev, true);
> > +     if (ret) {
> > +             debug("IT66121: set_active failed: %d\n", ret);
> > +             return ret;
> > +     }
>
> Just to check: video_bridge_set_active() returns 0 if the GPIOs are
> not specified in DT. If BeaglePlay does not define reset-gpios, this
> returns 0 but does not reset the chip. Is this intentional?
>
Yes, intentional. BeaglePlay doesn't route a reset GPIO to the IT66121
 the chip relies on its internal power-on reset, and we follow up with
the software reset sequence in it66121_hw_init() (writing
IT66121_SW_RST with AREF/VID/AUD/HDCP held in reset, then releasing
them after the AFE is programmed). will add a comment to make this
explicit:
> > +     if (pclk_khz > 0) {
> > +             debug("IT66121: configuring video for %lu kHz pclk\n",
> > +                   pclk_khz);
> > +             ret = it66121_setup_video(dev, pclk_khz);
> > +             if (ret)
> > +                     debug("IT66121: setup_video failed: %d\n", ret);
> > +     }
> > +
> > +     return 0;
>
> If it66121_setup_video() fails you log it but return 0. Should this
> return ret instead? If the failure is intentionally non-fatal, please
> add a comment.
>
Swallowing the error there was wrong. Changed it to return ret on
failure; if video setup fails the bridge isn't usable, so the caller
should see it.
  V2 coming shortly.
  Regards,
  Shaurya