-----Original Message-----
From: Andy Shevchenko <redacted>
Sent: Friday, July 8, 2022 4:56 PM
To: Sa, Nuno <Nuno.Sa@analog.com>
Cc: devicetree <redacted>; open list:GPIO
SUBSYSTEM [off-list ref]; linux-input <linux-
input@vger.kernel.org>; Dmitry Torokhov
[off-list ref]; Bartosz Golaszewski
[off-list ref]; Hennerich, Michael
[off-list ref]; Rob Herring
[off-list ref]; Krzysztof Kozlowski
[off-list ref]; Linus Walleij
[off-list ref]
Subject: Re: [PATCH 04/10] input: keyboard: adp5588-keys: add
support for fw properties

[External]

On Fri, Jul 8, 2022 at 11:37 AM Nuno Sá [off-list ref] wrote:

quoted

Use firmware properties (eg: OF) to get the device specific
configuration. This change just replaces the platform data since there
was no platform using it and so, it makes no sense having both.

Special note to the PULL-UP disable setting that is now supported as
part of the gpio subsystem (using 'set_config()' callback).

...

quoted

+#define ADP5588_DEVICE_ID_MASK 0xF
+
+ /* Configuration Register1 */
+#define ADP5588_AUTO_INC       (1 << 7)
+#define ADP5588_GPIEM_CFG      (1 << 6)
+#define ADP5588_OVR_FLOW_M     (1 << 5)
+#define ADP5588_INT_CFG                (1 << 4)
+#define ADP5588_OVR_FLOW_IEN   (1 << 3)
+#define ADP5588_K_LCK_IM       (1 << 2)
+#define ADP5588_GPI_IEN                (1 << 1)
+#define ADP5588_KE_IEN         (1 << 0)

Okay, you add something in the wrong form and then fix it in the other
patch in the very same series? Please no ping-pong type of changes.
Squash / rebase your series accordingly.

Well, I thought to just copy it as it was on the platform file and then just fix
it with the rest of the coding styles changes. But I'm fine in fixing it already
in this patch. In fact, there's a lot of defines that are not used (it's just
defining the complete register map) so I can as well get rid of all the stuff 
that is not used anywhere in the driver.

...

quoted

-       ret = adp5588_write(client, KP_GPIO2, KP_SEL(pdata->cols) &

0xFF);

quoted

+       ret = adp5588_write(client, KP_GPIO2, KP_SEL(kpad->cols) &

0xFF);

Do you need these " & 0xFF" parts?

Not sure but probably not. I just kept as it was...

...

quoted

+               /*
+                * fw properties keys start from 0 but on the device they

Firmware

ack...

- Nuno Sá