[PATCH v12] staging: fbtft: add tearing signal detect

[Andy Shevchenko]

On Fri, Jan 29, 2021 at 7:01 AM carlis <zhangxuezhi3 at gmail.com> wrote:
> On Thu, 28 Jan 2021 16:33:02 +0200
> Andy Shevchenko <andy.shevchenko at gmail.com> wrote:
> > On Thu, Jan 28, 2021 at 2:58 PM Carlis <zhangxuezhi3 at gmail.com> wrote:
> >
> > Thanks for your contribution, my comments below.
> >
> > > From: zhangxuezhi <zhangxuezhi1 at yulong.com>
> >
> > You probably have to configure your Git to use the same account for
> > author and committer.
>
> hi,you mean like below:
>         Carlis <zhangxuezhi1 at yulong.com>
> ?

I meant that you shouldn't probably have a From: line in the commit message.

...

> hi, i have modified it according to your suggestion like below:

Please, go again thru my comments and comments from others and
carefully address all of them everywhere in your contribution. If you
have questions, ask them in reply in the corresponding context.

...

> /**
>  * init_tearing_effect_line() - init tearing effect line

>  *

For example, above was commented on and hasn't been addressed here.

>  * @par: FBTFT parameter object
>  *
>  * Return: 0 on success, < 0 if error occurred.
>  */
> static int init_tearing_effect_line(struct fbtft_par *par)
> {
>         struct device *dev = par->info->device;
>         struct gpio_desc *te;
>         int rc;
>
>         te = gpiod_get_optional(dev, "te", GPIOD_IN);
>         if (IS_ERR(te))
>                 return dev_err_probe(dev, PTR_ERR(te), "Failed to
>         request te GPIO\n");
>

>         if (te) {

This one is not like I suggested.

>                 par->irq_te = gpiod_to_irq(te);
>                 gpiod_put(te);
>

>                 if (par->irq_te) {

This is wrong.

>                         rc = devm_request_irq(dev,
>                                               par->irq_te,
>                 panel_te_handler,
>                                               IRQF_TRIGGER_RISING,
>                 "TE_GPIO", par);

Try to use less LOCs.

>                         if (rc)
>                                 return dev_err_probe(dev, rc, "TE IRQ
>                 request failed.\n");
>
>                         disable_irq_nosync(par->irq_te);
>                         init_completion(&par->panel_te);

>                 } else {
>                         return dev_err_probe(dev, par->irq_te, "gpiod
>                         to TE IRQ failed.\n");
>                 }

Again, it is not what had been suggested.

>         }
>
>         return 0;
> }

The rest is better, but we will see later on when you submit a new
version (And I feel it won't be last).

-- 
With Best Regards,
Andy Shevchenko