Re: [PATCH v8 2/2] drm/panel: Add Sitronix ST7701 panel driver

[Sam Ravnborg]

Hi Jagan.

Thanks for being persistent and keep follow-up on this driver.

On Thu, Jan 24, 2019 at 11:58:44PM +0530, Jagan Teki wrote:
> ST7701 designed for small and medium sizes of TFT LCD display, is
> capable of supporting up to 480RGBX864 in resolution. It provides
> several system interfaces like MIPI/RGB/SPI.
> 
> Currently added support for Techstar TS8550B which is ST7701 based
> 480x854, 2-lane MIPI DSI LCD panel.
> 
> Driver now registering mipi_dsi device, but indeed it can extendable
> for RGB if any requirement trigger in future.
> 
> Signed-off-by: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> ---

> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7701.c b/drivers/gpu/drm/panel/panel-sitronix-st7701.c
> new file mode 100644
> index 000000000000..499dd8a34a6d
> --- /dev/null
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7701.c
> @@ -0,0 +1,429 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2019, Amarula Solutions.
> + * Author: Jagan Teki <jagan@xxxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#include <linux/backlight.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +
> +#include <linux/gpio/consumer.h>
> +#include <linux/regulator/consumer.h>
> +
> +#include <drm/drmP.h>
For new drivers please do not use drmP.h.
We are slowly going away from this and in drm-misc.git
the drmP.h file is no longer needed for anything.


> +	st7701->backlight = devm_of_find_backlight(&dsi->dev);
> +	if (IS_ERR(st7701->backlight))
> +		return PTR_ERR(st7701->backlight);

> +static int st7701_dsi_remove(struct mipi_dsi_device *dsi)
> +{
> +	struct st7701 *st7701 = mipi_dsi_get_drvdata(dsi);
> +
> +	mipi_dsi_detach(dsi);
> +	drm_panel_remove(&st7701->panel);
> +
> +	if (st7701->backlight)
> +		put_device(&st7701->backlight->dev);
> +
> +	return 0;
> +}

Please address this comment from Noralf:
"This happens automatically on driver detach if devm_of_find_backlight()
is used"

In other words, no need to do anything with backlight in st7701_dsi_remove()

With these details fixed you can add back my:
Reviewed-by: Sam Ravnborg <sam@xxxxxxxxxxxx>