Hi, On Tue, Oct 10, 2023 at 5:14 AM Cong Yang <yangcong5@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx> wrote: > > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > new file mode 100644 > index 000000000000..e095ad91c4bc > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili9882t.c > @@ -0,0 +1,762 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Panels based on the Ilitek ILI9882T display controller. > + */ > +#include <linux/delay.h> > +#include <linux/gpio/consumer.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> nit: remove include of linux/of_device.h since you don't use any of the functions declared there. > +#include <linux/regulator/consumer.h> > + > +#include <drm/drm_connector.h> > +#include <drm/drm_crtc.h> > +#include <drm/drm_mipi_dsi.h> > +#include <drm/drm_panel.h> > + > +#include <video/mipi_display.h> > + > +/* > + * Use this descriptor struct to describe different panels using the > + * Ilitek ILI9882T display controller. > + */ > +struct panel_desc { > + const struct drm_display_mode *modes; > + unsigned int bpc; > + > + /** > + * @width_mm: width of the panel's active display area > + * @height_mm: height of the panel's active display area > + */ > + struct { > + unsigned int width_mm; > + unsigned int height_mm; > + } size; > + > + unsigned long mode_flags; > + enum mipi_dsi_pixel_format format; > + const struct panel_init_cmd *init_cmds; > + unsigned int init_cmd_length; Remove "init_cmd_length" since it's now unused. > +static void ili9882t_remove(struct mipi_dsi_device *dsi) > +{ > + struct ili9882t *ili = mipi_dsi_get_drvdata(dsi); > + int ret; > + > + > + ret = mipi_dsi_detach(dsi); nit: remove extra blank line above. Other than nits, this looks good to me now. Reviewed-by: Douglas Anderson <dianders@xxxxxxxxxxxx>