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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux