Re: [PATCH v4 13/20] mfd: adp5585: support reset and unlock events
From: Lee Jones <lee@kernel.org>
Date: 2025-06-13 13:27:50
Also in:
linux-gpio, linux-input, linux-pwm
On Fri, 13 Jun 2025, Nuno Sá wrote:
On Fri, 2025-06-13 at 14:07 +0100, Lee Jones wrote:quoted
On Fri, 13 Jun 2025, Nuno Sá wrote:quoted
On Thu, 2025-06-12 at 15:55 +0100, Lee Jones wrote:quoted
On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:quoted
From: Nuno Sá <nuno.sa@analog.com> The ADP558x family of devices can be programmed to respond to some especial events, In case of the unlock events, one can lock the keypad and use KEYS or GPIs events to unlock it. For the reset events, one can again use a combinations of GPIs/KEYs in order to generate an event that will trigger the device to generate an output reset pulse. Signed-off-by: Nuno Sá <nuno.sa@analog.com> --- drivers/mfd/adp5585.c | 270 +++++++++++++++++++++++++++++++++++++++++++- include/linux/mfd/adp5585.h | 39 +++++++ 2 files changed, 308 insertions(+), 1 deletion(-)diff --git a/drivers/mfd/adp5585.c b/drivers/mfd/adp5585.cindex dcc09c898dd7990b39e21cb2324fa66ae171a802..6737d622a7ed9f280c439399f3709c a816 2dee01 100644--- a/drivers/mfd/adp5585.c +++ b/drivers/mfd/adp5585.c@@ -170,6 +170,9 @@ static const struct adp5585_regs adp5585_regs = {.int_en = ADP5585_INT_EN, .gen_cfg = ADP5585_GENERAL_CFG, .poll_ptime_cfg = ADP5585_POLL_PTIME_CFG, + .reset_cfg = ADP5585_RESET_CFG, + .reset1_event_a = ADP5585_RESET1_EVENT_A, + .reset2_event_a = ADP5585_RESET2_EVENT_A, }; static const struct adp5585_regs adp5589_regs = {@@ -177,8 +180,50 @@ static const struct adp5585_regs adp5589_regs = {.int_en = ADP5589_INT_EN, .gen_cfg = ADP5589_GENERAL_CFG, .poll_ptime_cfg = ADP5589_POLL_PTIME_CFG, + .reset_cfg = ADP5589_RESET_CFG, + .reset1_event_a = ADP5589_RESET1_EVENT_A, + .reset2_event_a = ADP5589_RESET2_EVENT_A, }; +static int adp5585_validate_event(const struct adp5585_dev *adp5585, unsigned int ev) +{ + if (adp5585->has_pin6) { + if (ev >= ADP5585_ROW5_KEY_EVENT_START && ev <= ADP5585_ROW5_KEY_EVENT_END) + return 0; + if (ev >= ADP5585_GPI_EVENT_START && ev <= ADP5585_GPI_EVENT_END) + return 0; + + return dev_err_probe(adp5585->dev, -EINVAL, + "Invalid unlock/reset event(%u) for this device\n", ev); + } + + if (ev >= ADP5585_KEY_EVENT_START && ev <= ADP5585_KEY_EVENT_END) + return 0; + if (ev >= ADP5585_GPI_EVENT_START && ev <= ADP5585_GPI_EVENT_END) { + /* if it's GPI6 */You have to tell us why this is a problem. Nit: Comments should start with an upper case char.The error message kind of states the problem :). But I'll put it in the comment.I don't think it does. Remember, people reading this do not know the H/W as well as you do. How is GPI6 even related to R5?Yeah, you might be right. GPI6 is the same pin as R5. In a variation of the chip we have this extra pin (though the datasheet refers to it as R5) that can either be used as part of the keypad or a GPIO. In the other variants, it's a reset pin. The check is making sure we're not trying to configure an unlock/reset event on a pin that do not exist. But I get your point, for me it's clear that R5 == GPI6. Me not being consistent in the comments/messages won't help anyone reading the code.
You nailed it! Thanks for understanding. -- Lee Jones [李琼斯]