On Sun, Jan 19, 2020 at 10:40:31PM +0000, Jeff LaBundy wrote:
> On Fri, Jan 17, 2020 at 01:33:30PM -0800, dmitry.torokhov@xxxxxxxxx wrote:
> > On Fri, Jan 17, 2020 at 02:35:46AM +0000, Jeff LaBundy wrote:
> > > +
> > > + ret = device_property_read_u32_array(&pdev->dev, "linux,keycodes",
> > > + iqs62x_keys->keycode,
> > > + iqs62x_keys->keycodemax);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "Failed to read keycodes: %d\n", ret);
> > > + return ret;
> > > + }
> >
> > I wonder why you can't simply use
> >
> > error = device_property_read_u32_array(&pdev->dev, "linux,keycodes",
> > iqs62x_keys->keycode,
> > IQS62X_NUM_KEYS);
> >
> > Are you concerned with someone trying to set up keys that are not
> > actually exposed later via EVOCSKEYCODES and that is why you are
> > limiting keycodemax?
>
> When I try this, I find that device_property_read_u32_array returns -EOVERFLOW
> for arrays with fewer than IQS62X_NUM_KEYS elements. To avoid forcing users to
> pad the array all the way out to IQS62X_NUM_KEYS in the case of simple channel
> assignments (like those in the example bindings), keycodemax must be passed to
> device_property_read_u32_array which means it must be limited before-hand. The
> same method seems to be used in other drivers as well (e.g. mpr121_touchkey).
Ah, indeed, ignore me here please.
...
> > > +MODULE_AUTHOR("Jeff LaBundy <jeff@xxxxxxxxxxx>");
> > > +MODULE_DESCRIPTION("Azoteq IQS620A/621/622/624/625 Keys and Switches");
> > > +MODULE_LICENSE("GPL");
> > > +MODULE_ALIAS("platform:" IQS62X_DRV_NAME_KEYS);
> >
> > Otherwise
> >
> > Acked-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> >
> > I suppose it will be merged through MFD?
>
> That's the plan; Lee confirmed this would be OK once the series is ready. Just
> as a heads up, I expect minor changes to this and other patches as iqs62x.h is
> hardened (e.g. "iqs62x->map" --> "iqs62x->regmap"). I assume you're OK with me
> keeping your Acked-by unless there are major changes, but let me know if you'd
> prefer I didn't.
Yes, please keep Acked-by unless there is significant rework.
Thanks.
--
Dmitry
