On Thu, Apr 23, 2020 at 09:20:02AM +0800, Fengping yu wrote: > From: "fengping.yu" <fengping.yu@xxxxxxxxxxxx> > This misses the commit message. It's a show stopper for such patches. Read this [1]. [1]: https://chris.beams.io/posts/git-commit/ ... > +#define KPD_DEBOUNCE_MASK GENMASK(13, 0) > +#define KPD_DEBOUNCE_MAX 256000 Is there any unit in which debounce time is being measured? Add it as a suffix to the definition, if it's possible. ... > +#define BITS_TO_U32(nr) DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(u32)) This already defined in bits.h. ... > + keypad->base = devm_platform_ioremap_resource(pdev, 0); > + if (IS_ERR(keypad->base)) { > + dev_err(&pdev->dev, "Failed to get resource and iomap keypad\n"); This is duplicate noisy message, please remove. > + return PTR_ERR(keypad->base); > + } ... > + writew(keypad->key_debounce * 32 / 1000 & KPD_DEBOUNCE_MASK, Perhaps one pair of parentheses is needed to make logic clear. (Yes, I remember I commented on this in earlier versions where it was many parentheses around above calculations, but you have to use common sense as well) > + keypad->base + KP_DEBOUNCE); -- With Best Regards, Andy Shevchenko
