чт, 20 квіт. 2023 р. о 19:35 Simon Glass <s...@chromium.org> пише: > > Hi Svyatoslav, > > On Thu, 20 Apr 2023 at 17:53, Svyatoslav Ryhel <clamo...@gmail.com> wrote: > > > > чт, 20 квіт. 2023 р. о 01:41 Simon Glass <s...@chromium.org> пише: > > > > > > Hi Svyatoslav, > > > > > > On Wed, 19 Apr 2023 at 13:17, Svyatoslav Ryhel <clamo...@gmail.com> wrote: > > > > > > > > Driver adds support for panels with Renesas R69328 IC > > > > > > > > Currently supported compatible is: > > > > - jdi,dx12d100vm0eaa > > > > > > > > Tested-by: Andreas Westman Dorcsak <hed...@yahoo.com> # LG P880 T30 > > > > Tested-by: Svyatoslav Ryhel <clamo...@gmail.com> # LG P895 T30 > > > > Signed-off-by: Svyatoslav Ryhel <clamo...@gmail.com> > > > > --- > > > > drivers/video/Kconfig | 9 ++ > > > > drivers/video/Makefile | 1 + > > > > drivers/video/renesas-r69328.c | 232 +++++++++++++++++++++++++++++++++ > > > > 3 files changed, 242 insertions(+) > > > > create mode 100644 drivers/video/renesas-r69328.c > > > > > > Reviewed-by: Simon Glass <s...@chromium.org> > > > > > > Please see below > > > > > > [..] > > > > > > > +static int renesas_r69328_enable_backlight(struct udevice *dev) > > > > +{ > > > > + struct renesas_r69328_priv *priv = dev_get_priv(dev); > > > > + int ret; > > > > + > > > > + ret = dm_gpio_set_value(&priv->enable_gpio, 1); > > > > + if (ret) { > > > > + printf("%s: error changing enable-gpios (%d)\n", > > > > __func__, ret); > > > > > > If you use log_err() then the function name is should automatically if > > > CONFIG_LOGF_FUNC is enabled > > > > > > > + return ret; > > > > + } > > > > + mdelay(5); > > > > + > > > > + ret = dm_gpio_set_value(&priv->reset_gpio, 0); > > > > + if (ret) { > > > > + printf("%s: error changing reset-gpios (%d)\n", > > > > __func__, ret); > > > > + return ret; > > > > + } > > > > + mdelay(5); > > > > + > > > > + ret = dm_gpio_set_value(&priv->reset_gpio, 1); > > > > + if (ret) { > > > > + printf("%s: error changing reset-gpios (%d)\n", > > > > __func__, ret); > > > > + return ret; > > > > + } > > > > + > > > > + mdelay(5); > > > > > > Please add a comment about the delays so people know how they were > > > chosen. E.g. it might be in the datasheet. > > > > > > > I would love to add reasonable arguments for these delays but unfortunately > > I am not LG developer and there are no public datasheets available for > > either > > of the panels. I have used the downstream kernel as a reference and built it > > to a more modern look. In case someone can provide datasheets for this panel > > or for a similar one with the same controller, this driver may be completed > > and > > further extended for new panels. As for now it was extensively tested on > > p895 > > and p880 respectively and did not fail any time. > > A comment like that in the source code would go a long way to help > someone looking at these delays later. >
Ok, I will add some comments on the next iteration if such will be needed. > Regards, > Simon