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@gmail.com wrote:

quoted

On Fri, Jan 17, 2020 at 02:35:46AM +0000, Jeff LaBundy wrote:

quoted

+
+	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.

...

quoted

quoted

+MODULE_AUTHOR("Jeff LaBundy [off-list ref]");
+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@gmail.com>

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