linux-kernel - Re: [PATCH v2 2/2] drm/tiny: Add driver for Sharp Memory LCD

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3b6297d6-f954-499a-9504-233b27ff686a@kernel.org>
Date: Sat, 27 Jul 2024 11:03:19 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Alex Lanzano <lanzano.alex@...il.com>, mehdi.djait@...tlin.com,
 christophe.jaillet@...adoo.fr,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>
Cc: dri-devel@...ts.freedesktop.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] drm/tiny: Add driver for Sharp Memory LCD

On 26/07/2024 21:44, Alex Lanzano wrote:
> Add support for the monochrome Sharp Memory LCDs.
> 
> Signed-off-by: Alex Lanzano <lanzano.alex@...il.com>
> Co-developed-by: Mehdi Djait <mehdi.djait@...tlin.com>
> Signed-off-by: Mehdi Djait <mehdi.djait@...tlin.com>
> ---
>  MAINTAINERS                         |   7 +
>  drivers/gpu/drm/tiny/Kconfig        |  20 +
>  drivers/gpu/drm/tiny/Makefile       |   1 +
>  drivers/gpu/drm/tiny/sharp-memory.c | 726 ++++++++++++++++++++++++++++
>  4 files changed, 754 insertions(+)
>  create mode 100644 drivers/gpu/drm/tiny/sharp-memory.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 71b739b40921..b5b08247a994 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7123,6 +7123,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/display/panel/samsung,s6d7aa0.yaml
>  F:	drivers/gpu/drm/panel/panel-samsung-s6d7aa0.c
>  
> +DRM DRIVER FOR SHARP MEMORY LCD
> +M:	Alex Lanzano <lanzano.alex@...il.com>
> +S:	Maintained
> +T:	git git://anongit.freedesktop.org/drm/drm-misc


Do you have drm-misc commit rights? If not, drop it. There is no point
to put some other maintainer's tree in your entry. Git tree is already
present in the maintainer's entry who is going to apply the patches.


> +F:	Documentation/devicetree/bindings/display/sharp,sharp-memory.yaml

If you rename the file in your patchset, you must rename it EVERYWHERE.


> +F:	drivers/gpu/drm/tiny/sharp-memory.c
> +
>  DRM DRIVER FOR SITRONIX ST7586 PANELS


...

> +	smd->spi = spi;
> +	drm = &smd->drm;
> +	ret = drmm_mode_config_init(drm);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "Failed to initialize drm config\n");
> +
> +	smd->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_HIGH);
> +	if (smd->enable_gpio == NULL)
> +		dev_warn(&spi->dev, "Enable gpio not defined\n");
> +
> +	/*
> +	 * VCOM is a signal that prevents DC bias from being built up in
> +	 * the panel resulting in pixels being forever stuck in one state.
> +	 *
> +	 * This driver supports three different methods to generate this
> +	 * signal depending on EXTMODE pin:
> +	 *
> +	 * software (EXTMODE = L) - This mode uses a kthread to
> +	 * periodically send a "maintain display" message to the display,
> +	 * toggling the vcom bit on and off with each message
> +	 *
> +	 * external (EXTMODE = H) - This mode relies on an external
> +	 * clock to generate the signal on the EXTCOMM pin
> +	 *
> +	 * pwm (EXTMODE = H) - This mode uses a pwm device to generate
> +	 * the signal on the EXTCOMM pin
> +	 *
> +	 */
> +	smd->vcom = 0;
> +	if (device_property_read_string(&spi->dev, "vcom-mode", &vcom_mode_str))
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Unable to find vcom-mode node in device tree\n");
> +
> +	if (!strcmp("software", vcom_mode_str)) {
> +		smd->vcom_mode = SHARP_MEMORY_SOFTWARE_VCOM;
> +
> +	} else if (!strcmp("external", vcom_mode_str)) {
> +		smd->vcom_mode = SHARP_MEMORY_EXTERNAL_VCOM;
> +
> +	} else if (!strcmp("pwm", vcom_mode_str)) {
> +		smd->vcom_mode = SHARP_MEMORY_PWM_VCOM;
> +		ret = sharp_memory_init_pwm_vcom_signal(smd);
> +		if (ret)
> +			return dev_err_probe(dev, ret,
> +					     "Failed to initialize external COM signal\n");
> +	} else {
> +		return dev_err_probe(dev, -EINVAL, "Invalid value set for vcom-mode\n");
> +	}
> +
> +	drm->mode_config.funcs = &sharp_memory_mode_config_funcs;
> +
> +	/* Set the DRM display mode depending on panel model */
> +	model = (uintptr_t)spi_get_device_match_data(spi);
> +	switch (model) {
> +	case LS013B7DH03:
> +		smd->mode = &sharp_memory_ls013b7dh03_mode;
> +		break;
> +
> +	case LS010B7DH04:
> +		smd->mode = &sharp_memory_ls010b7dh04_mode;
> +		break;
> +
> +	case LS011B7DH03:
> +		smd->mode = &sharp_memory_ls011b7dh03_mode;
> +		break;
> +
> +	case LS012B7DD01:
> +		smd->mode = &sharp_memory_ls012b7dd01_mode;
> +		break;
> +
> +	case LS013B7DH05:
> +		smd->mode = &sharp_memory_ls013b7dh05_mode;
> +		break;
> +
> +	case LS018B7DH02:
> +		smd->mode = &sharp_memory_ls018b7dh02_mode;
> +		break;
> +
> +	case LS027B7DH01:
> +		fallthrough;
> +	case LS027B7DH01A:
> +		smd->mode = &sharp_memory_ls027b7dh01_mode;
> +		break;
> +
> +	case LS032B7DD02:
> +		smd->mode = &sharp_memory_ls032b7dd02_mode;
> +		break;
> +
> +	case LS044Q7DH01:
> +		smd->mode = &sharp_memory_ls044q7dh01_mode;
> +		break;

This is over-complicated. Just store the mode in device match data.


> +
> +	default:
> +		return dev_err_probe(&spi->dev, -EINVAL, "Invalid DRM display mode\n");
> +	}


Best regards,
Krzysztof