Re: [PATCH v4 1/2] drm/panel: Add support for Truly NT35597 panel

[Sean Paul <seanpaul@chromium.org>]

On Wed, May 30, 2018 at 09:47:25AM -0600, Jordan Crouse wrote:
> On Fri, May 25, 2018 at 05:27:51PM -0700, Abhinav Kumar wrote:
> > Add support for Truly NT35597 panel used
> > in MSM reference platforms.
> > 
> > This panel supports both single DSI and dual DSI
> > modes.
> > 
> > However, this patch series adds support only for
> > dual DSI mode.
> > 
> > Changes in v4:
> > - Fix the license identifier
> > - Fix formatting issues for the regulator loads
> > - Fix error messages and return code
> > 
> > Signed-off-by: Archit Taneja <architt@codeaurora.org>
> > Signed-off-by: Abhinav Kumar <abhinavk@codeaurora.org>
> > ---
> >  drivers/gpu/drm/panel/Kconfig               |   8 +
> >  drivers/gpu/drm/panel/Makefile              |   1 +
> >  drivers/gpu/drm/panel/panel-truly-nt35597.c | 576 ++++++++++++++++++++++++++++
> >  3 files changed, 585 insertions(+)
> >  create mode 100644 drivers/gpu/drm/panel/panel-truly-nt35597.c
> > 
> > diff --git a/drivers/gpu/drm/panel/Kconfig b/drivers/gpu/drm/panel/Kconfig
> > index 25682ff..2fcd9b1 100644
> > --- a/drivers/gpu/drm/panel/Kconfig
> > +++ b/drivers/gpu/drm/panel/Kconfig
> > @@ -177,4 +177,12 @@ config DRM_PANEL_SITRONIX_ST7789V
> >  	  Say Y here if you want to enable support for the Sitronix
> >  	  ST7789V controller for 240x320 LCD panels
> >  
> > +config DRM_PANEL_TRULY_NT35597_WQXGA
> > +	tristate "Truly WQXGA"
> > +	depends on OF
> > +	depends on DRM_MIPI_DSI
> > +	select VIDEOMODE_HELPERS
> > +	help
> > +	  Say Y here if you want to enable support for Truly NT35597 WQXGA Dual DSI
> > +	  Video Mode panel
> >  endmenu
> > diff --git a/drivers/gpu/drm/panel/Makefile b/drivers/gpu/drm/panel/Makefile
> > index f26efc1..056ea93 100644
> > --- a/drivers/gpu/drm/panel/Makefile
> > +++ b/drivers/gpu/drm/panel/Makefile
> > @@ -18,3 +18,4 @@ obj-$(CONFIG_DRM_PANEL_SEIKO_43WVF1G) += panel-seiko-43wvf1g.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LQ101R1SX01) += panel-sharp-lq101r1sx01.o
> >  obj-$(CONFIG_DRM_PANEL_SHARP_LS043T1LE01) += panel-sharp-ls043t1le01.o
> >  obj-$(CONFIG_DRM_PANEL_SITRONIX_ST7789V) += panel-sitronix-st7789v.o
> > +obj-$(CONFIG_DRM_PANEL_TRULY_NT35597_WQXGA) += panel-truly-nt35597.o
> > diff --git a/drivers/gpu/drm/panel/panel-truly-nt35597.c b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> > new file mode 100644
> > index 0000000..a57cbf0
> > --- /dev/null
> > +++ b/drivers/gpu/drm/panel/panel-truly-nt35597.c
> > @@ -0,0 +1,576 @@
> > +// SPDX-License-Identifier: GPL-2.0
> 
> I guess it is up to Sean and Rob if they want to accept // comments for SPDX.
> I'm not sure there is a hard and fast rule about it.
> 

Hard meh from me. Looks like there are plenty of examples of both in drm.

Sean

> > +/*
> > + * Copyright (c) 2018, The Linux Foundation. All rights reserved.
> > + */
> > +
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/pinctrl/consumer.h>
> > +
> > +#include <video/mipi_display.h>
> > +#include <video/of_videomode.h>
> > +#include <video/videomode.h>
> > +
> > +#include <drm/drmP.h>
> > +#include <drm/drm_panel.h>
> > +#include <drm/drm_mipi_dsi.h>
> > +
> > +static const char * const regulator_names[] = {
> > +	"vdda",
> > +	"vdispp",
> > +	"vdispn"
> 
> The reason why the coding style insists on commas for the last member of an
> listy is that if you added another item the resulting patch would
> only need one line instead of three. For example:
>   
> 	+   "foo",
> 
> instead of:
> 
> 	- "vdispn"
> 	+ "vdispn",
> 	+ "foo",
> 
> <snip>
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel