Den 22.07.2021 04.07, skrev Dillon Min: > Hi Noralf > > Thanks for your time to review my patch. > > On Thu, 22 Jul 2021 at 01:42, Noralf Trønnes <noralf@xxxxxxxxxxx> wrote: >> >> >> >> Den 21.07.2021 09.41, skrev dillon.minfei@xxxxxxxxx: >>> From: Dillon Min <dillon.minfei@xxxxxxxxx> >>> >>> This driver combine tiny/ili9341.c mipi_dbi_interface driver >>> with mipi_dpi_interface driver, can support ili9341 with serial >>> mode or parallel rgb interface mode by register configuration. >>> >>> Cc: Linus Walleij <linus.walleij@xxxxxxxxxx> >>> Signed-off-by: Dillon Min <dillon.minfei@xxxxxxxxx> >>> --- >> >>> +static const struct of_device_id ili9341_of_match[] = { >>> + { >>> + .compatible = "st,sf-tc240t-9370-t", >>> + .data = &ili9341_stm32f429_disco_data, >>> + }, >>> + { >>> + /* porting from tiny/ili9341.c >>> + * for original mipi dbi compitable >>> + */ >>> + .compatible = "adafruit,yx240qv29", >> >> I don't understand this, now there will be 2 drivers that support the >> same display? > > There is no reason to create two drivers to support the same display. > > To support only-dbi and dbi+dpi panel at drm/panel or drm/tiny both > fine with me. > >> >> AFAICT drm/tiny/ili9341.c is just copied into this driver, is the plan >> to remove the tiny/ driver? If so I couldn't see this mentioned anywhere. > > Yes, I'd like to merge the code from drm/tiny/ili9341.c to this driver > (to make a single driver to support different bus). > > I have two purpose to extend the feature drm/tiny/ili9341.c > > - keep compatible = "adafruit,yx240qv29", add bus mode dts bindings (panel_bus) > to define the interface which host wants to use. such as > panel_bus="dbi" or "rgb" > or "i80" for this case, i will add dpi code to drm/tiny/ili9341.c. > > - merge tiny/ili9341.c to this driver,remove drm/tiny/ili9341.c, add > new dts compatible > string to support other interfaces. just like what i'm doing now. > > I have no idea about your plan on drm/tiny drivers, actually some of > these panels under > the diny folder can support both dbi and dbi+dpi (much faster, need > more pins). no > doubt the requirement to support dpi is always there. > > What is your preference? > I have no plans for tiny/, it's just a place to put tiny DRM drivers of all sorts. Whether or not to have "full" DRM drivers in panel/ is up to Sam and Laurent I guess, currently there's only drm_panel drivers in there. I have no objections to doing that though. I just wanted to make sure we don't have 2 drivers for the same display. Noralf. > Thanks & Regards > Dillon > >> >> Noralf. >> >>> + .data = NULL, >>> + }, >>> +}; >>> +MODULE_DEVICE_TABLE(of, ili9341_of_match);