On Fri, Aug 11, 2023 at 03:31:01PM +0800, wenhua lin wrote: > On Thu, Aug 10, 2023 at 10:01 PM Andy Shevchenko > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > On Thu, Aug 10, 2023 at 08:42:36PM +0800, wenhua lin wrote: > > > On Tue, Aug 8, 2023 at 9:51 PM Andy Shevchenko > > > <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > > > On Tue, Aug 08, 2023 at 03:25:01PM +0800, Wenhua Lin wrote: ... > > > > > + u32 rows_en; /* enabled rows bits */ > > > > > + u32 cols_en; /* enabled cols bits */ > > > > > > > > Why not bitmaps? > > > > > > Bitmap has been used, each bit represents different rows and different columns. > > > > I meant the bitmap type (as of bitmap.h APIs). > > I understand what you mean, I need to study how this bitmap is used. Input subsystem already is using them. ... > > > > > +static int sprd_keypad_parse_dt(struct device *dev) > > > > > > > > dt -> fw > > > > > > I don't quite understand what you mean,。 > > > is it to change the function name to sprd_keypad_parse_fw? > > > > Yes. And make it firmware (which may be ACPI/DT or something else). > > We need to think about how to modify it. As I told already, replace mention of "DT"/"OF" by "firmware" and use device property APIs as per property.h. ... > > > > And I'm wondering if input subsystem already does this for you. > > > > > > I don't quite understand what you mean. > > > > Does input subsystem parse the (some of) device properties already? > > Yes Does it cover what you are parsing here? At least partially... ... > > > > > +err_free: > > > > > + devm_kfree(&pdev->dev, data); > > > > > > > > Huh?! > > > > It's a red flag, and you have no answer to it... > > I realized the problem, the interface using devm_ does not need to do the free. > I will fix this issue in patch v2. The problem is to understand where you can and where you can't use devm_*() in the first place. _Then_ as you said. > > > > > + return ret; ... > > > > > + .owner = THIS_MODULE, > > > > > > > > ~15 years this is not needed. > > > > Where did you get this code from? Time machine? > > > > > > Do you mean the keypad driver is no longer in use? > > > > No, I meant specifically emphasized line. > > The keypad driver code is used on the platform > and has not been submitted to the community. I'm not sure I understand to what you reply here... I'm talking about the "owner" member assignment in the respective data structure. -- With Best Regards, Andy Shevchenko
