Re: [PATCH v2 6/6] drm/panel: Add Ilitek ILI9341 DBI panel driver

Hi Paul,

just a drive-by comment:

On Sat, Aug 22, 2020 at 6:33 PM Paul Cercueil paul@crapouillou.net wrote:

•   gpiod_set_value_cansleep(priv->reset_gpiod, 0);
  

•   usleep_range(20, 1000);
  

•   gpiod_set_value_cansleep(priv->reset_gpiod, 1);
  

This implies that the reset line is active low.

I would specify in the DT GPIO handle that it is active low and invert the above.

So:

reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;

gpiod_set_value_cansleep(priv->reset_gpiod, 1); usleep_range(20, 1000); gpiod_set_value_cansleep(priv->reset_gpiod, 0);

•   priv->reset_gpiod = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
  

•   if (IS_ERR(priv->reset_gpiod)) {
  

•           dev_err(dev, "Couldn't get our reset GPIO\n");
  

•           return PTR_ERR(priv->reset_gpiod);
  

•   }
  

This would then fetch the GPIO as asserted (device in reset) unless changed, but that may be the right thing to do actually.

+static const struct ili9341_pdata yx240qv29_pdata = {

•   .mode = { DRM_SIMPLE_MODE(240, 320, 37, 49) },
  

•   .width_mm = 0, // TODO
  

•   .height_mm = 0, // TODO
  

When nothing else works and data sheets are incomplete I just take out a ruler and measure on the actual device.

Yours, Linus Walleij