Re: [PATCH v10] staging: fbtft: add tearing signal detect

On Thu, 28 Jan 2021 12:15:28 +0100
Geert Uytterhoeven [off-list ref] wrote:

Hi Carlis,

On Thu, Jan 28, 2021 at 12:03 PM carlis [off-list ref]
wrote:

quoted

On Thu, 28 Jan 2021 10:42:54 +0100
Geert Uytterhoeven [off-list ref] wrote:  

quoted

On Thu, Jan 28, 2021 at 7:53 AM Kari Argillander
[off-list ref] wrote:  

quoted

On Thu, Jan 28, 2021 at 09:42:58AM +0800, carlis wrote:  

quoted

On Thu, 28 Jan 2021 00:32:22 +0200
Kari Argillander [off-list ref] wrote:  

quoted

quoted

 #include "fbtft.h"

 #define DRVNAME "fb_st7789v"

@@ -66,6 +69,32 @@ enum st7789v_command {
 #define MADCTL_MX BIT(6) /* bitmask for column address

order */ #define MADCTL_MY BIT(7) /* bitmask for page
address order */

+#define SPI_PANEL_TE_TIMEOUT     400 /* msecs */
+static struct mutex te_mutex;/* mutex for set te gpio irq
status */  

Space after ;  

hi, i have fix it in the patch v11  

quoted

Yeah sorry. I accidentally review wrong patch. But mostly stuff
are still relevant.
 

quoted

quoted

quoted

@@ -82,6 +111,33 @@ enum st7789v_command {
  */
 static int init_display(struct fbtft_par *par)
 {
+ int rc;
+ struct device *dev = par->info->device;
+
+ par->gpio.te = devm_gpiod_get_index_optional(dev, "te",

0, GPIOD_IN);
+ if (IS_ERR(par->gpio.te)) {
+         rc = PTR_ERR(par->gpio.te);
+         dev_err(par->info->device, "Failed to request te
gpio: %d\n", rc);
+         return rc;
+ }  

You request with optinal and you still want to error out? We
could just continue and not care about that error. User
will be happier if device still works somehow.  

devm_gpiod_get_index_optional() returns NULL, not an error, if the
GPIO is not found.  So if IS_ERR() is the right check.

And checks for -EPROBE_DEFER can be handled automatically
by using dev_err_probe() instead of dev_err().
 

hi, i fix it like below!?
        par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
        GPIOD_IN); if (IS_ERR(par->gpio.te)) {
                rc = PTR_ERR(par->gpio.te);
                dev_err_probe(par->info->device, rc, "Failed to
request te gpio\n"); return rc;
        }
        if (par->gpio.te) {
                init_completion(&spi_panel_te);
                rc = devm_request_irq(dev,
                                      gpiod_to_irq(par->gpio.te),
                                     spi_panel_te_handler,
        IRQF_TRIGGER_RISING, "TE_GPIO", par);
                if (rc) {
                        dev_err(par->info->device, "TE request_irq
        failed.\n"); return rc;  

dev_err_probe()

quoted

                }

                disable_irq_nosync(gpiod_to_irq(par->gpio.te));
        } else {
                dev_info(par->info->device, "%s:%d, TE gpio not
                specified\n", __func__, __LINE__);
        }  

Gr{oetje,eeting}s,

                        Geert

hi,i will fix it like below:


	par->gpio.te = devm_gpiod_get_index_optional(dev, "te", 0,
	GPIOD_IN); if (IS_ERR(par->gpio.te))
		return dev_err_probe(par->info->device,
	PTR_ERR(par->gpio.te), "Failed to request te gpio\n");

	if (par->gpio.te) {
		init_completion(&spi_panel_te);
		rc = devm_request_irq(dev,
				      gpiod_to_irq(par->gpio.te),
				     spi_panel_te_handler,
	IRQF_TRIGGER_RISING, "TE_GPIO", par);
		if (IS_ERR(rc))
			return dev_err_probe(par->info->device,
	PTR_ERR(rc), "TE request_irq failed.\n");

		disable_irq_nosync(gpiod_to_irq(par->gpio.te));
	} else {
		dev_info(par->info->device, "%s:%d, TE gpio not
		specified\n", __func__, __LINE__);
	}


regards,
zhangxuezhi