ср, 8 лип. 2026 р. о 16:45 Advait Dhamorikar <[email protected]> пише:
>
> > Hey there! Thanks for your contribution, it is pretty solid but there
> > are a few places that can be improved.
> Hi Svyatoslav, thanks for your inputs!
>
> > > +config VIDEO_AMOLED_CHIPWEALTH_CH13726A
> >
> > May you please rename AMOLED > PANEL
>
> Most of the drivers say `CONFIG_VIDEO_LCD_XXXX`
> Since this driver is an AMOLED I thought of using that.
> There also don't seem to be any existing setups named `CONFG_VIDEO_PANEL_XXX`.
> Should I still rename it to VIDEO_PANEL_CHIPWEALTH_CH13726A?
>
Yeah, other configs IMHO should be renamed to PANEL and ideally
separated into a dedicated folder.
> > > +static int ch13726a_dcs_write_buf(struct udevice *dev, const void *data,
> > > + size_t len)
> > > +{
> > > + struct mipi_dsi_panel_plat *plat = dev_get_plat(dev);
> > > + int ret;
> > > +
> > > + ret = mipi_dsi_dcs_write_buffer(plat->device, data, len);
> > > + if (ret < 0)
> > > + log_err("failed to write dcs sequence: %d\n", ret);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +#define dcs_write_seq(dev, cmd, seq...) \
> > > +({ \
> > > + const u8 d[] = { cmd, seq }; \
> > > + \
> > > + ch13726a_dcs_write_buf(dev, d, ARRAY_SIZE(d)); \
> > > +})
> > > +
>
> > You can fold this.
>
> I'll fold this into one single #define altogether.
>
> > > +struct ch13726a_panel_priv {
> > > + struct udevice *reg;
> > > + struct udevice *backlight;
> > > + struct gpio_desc reset;
> > > +};
> >
> > You are lacking supplies here: vdd1v2, vddio, vdd and avdd
> >
> > U-Boot can handle them so you should include control over them.
>
> My board has the input rails permanently powered, so the current
> driver doesn't request any regulators
> I'll add support for the vdd, vddio, vdd1v2, and avdd supplies. Should
> these be treated as
> optional (-ENOENT) and guarded with CONFIG_IS_ENABLED(DM_REGULATOR)?
Yes, that would be acceptable.
> I couldn't find other panel drivers that consistently follow one approach,
> so I'd like to match the preferred U-Boot style.
>
> Best regards,
> Advait
