diff mbox series

[v4,13/20] mfd: adp5585: support reset and unlock events

Message ID 20250521-dev-adp5589-fw-v4-13-f2c988d7a7a0@analog.com
State Handled Elsewhere
Headers show
Series mfd: adp5585: support keymap events and drop legacy Input driver | expand

Commit Message

Nuno Sá via B4 Relay May 21, 2025, 1:03 p.m. UTC 
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(-)

Comments

Lee Jones June 12, 2025, 2:55 p.m. UTC | #1 
On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:

> 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.c
> index dcc09c898dd7990b39e21cb2324fa66ae171a802..6737d622a7ed9f280c439399f3709ca8162dee01 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.

> +		if (ev == (ADP5585_GPI_EVENT_START + 5))
> +			return dev_err_probe(adp5585->dev, -EINVAL,
> +					     "Invalid unlock/reset event(%u). R5 not available\n",
> +					     ev);
> +		return 0;
> +	}
> +
> +	return dev_err_probe(adp5585->dev, -EINVAL,
> +			     "Invalid unlock/reset event(%u) for this device\n", ev);
> +}
> +
> +static int adp5589_validate_event(const struct adp5585_dev *adp5585, unsigned int ev)
> +{
> +	if (ev >= ADP5589_KEY_EVENT_START && ev <= ADP5589_KEY_EVENT_END)
> +		return 0;
> +	if (ev >= ADP5589_GPI_EVENT_START && ev <= ADP5589_GPI_EVENT_END)
> +		return 0;
> +
> +	return dev_err_probe(adp5585->dev, -EINVAL,
> +			     "Invalid unlock/reset event(%u) for this device\n",
> +			     ev);

This line break is unnecessary.

> +}
> +
>  static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
>  				       struct regmap_config *regmap_config)
>  {
> @@ -191,6 +236,8 @@ static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
>  		*regmap_config = adp5585_regmap_config_template;
>  		adp5585->id = ADP5585_MAN_ID_VALUE;
>  		adp5585->regs = &adp5585_regs;
> +		if (adp5585->variant == ADP5585_01)
> +			adp5585->has_pin6 = true;
>  		break;
>  	case ADP5589_00:
>  	case ADP5589_01:
> @@ -198,6 +245,8 @@ static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
>  		*regmap_config = adp5589_regmap_config_template;
>  		adp5585->id = ADP5589_MAN_ID_VALUE;
>  		adp5585->regs = &adp5589_regs;
> +		adp5585->has_unlock = true;
> +		adp5585->has_pin6 = true;
>  		break;
>  	default:
>  		return -ENODEV;
> @@ -207,6 +256,168 @@ static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
>  	return 0;
>  }
>  
> +static int adp5585_parse_ev_array(const struct adp5585_dev *adp5585, const char *prop, u32 *events,
> +				  u32 *n_events, u32 max_evs, bool reset_ev)
> +{
> +	struct device *dev = adp5585->dev;
> +	unsigned int ev;
> +	int ret;
> +
> +	/*
> +	 * The device has the capability of handling special events through GPIs or a Keypad:
> +	 *  unlock events: Unlock the keymap until one of the configured events is detected.
> +	 *  reset events: Generate a reset pulse when one of the configured events is detected.
> +	 */
> +	ret = device_property_count_u32(dev, prop);
> +	if (ret < 0)
> +		return 0;
> +
> +	*n_events = ret;
> +
> +	if (!adp5585->has_unlock && !reset_ev)
> +		return dev_err_probe(dev, -EOPNOTSUPP, "Unlock keys not supported\n");
> +
> +	if (*n_events > max_evs)
> +		return dev_err_probe(dev, -EINVAL,
> +				     "Invalid number of keys(%u > %u) for %s\n",
> +				     *n_events, max_evs, prop);
> +
> +	ret = device_property_read_u32_array(dev, prop, events, *n_events);
> +	if (ret)
> +		return ret;
> +
> +	for (ev = 0; ev < *n_events; ev++) {
> +		/* for unlock events, 127 is a wildcard */

As above and throughout the series.

If you define the wildcard magic number you can drop the comment.

> +		if (!reset_ev && events[ev] == 127)
> +			continue;
> +
> +		if (adp5585->id == ADP5585_MAN_ID_VALUE)
> +			ret = adp5585_validate_event(adp5585, events[ev]);
> +		else
> +			ret = adp5589_validate_event(adp5585, events[ev]);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int adp5585_unlock_ev_parse(struct adp5585_dev *adp5585)
> +{
> +	struct device *dev = adp5585->dev;
> +	int ret;
> +
> +	ret = adp5585_parse_ev_array(adp5585, "adi,unlock-events", adp5585->unlock_keys,
> +				     &adp5585->nkeys_unlock, ARRAY_SIZE(adp5585->unlock_keys),
> +				     false);
> +	if (ret)
> +		return ret;
> +	if (!adp5585->nkeys_unlock)
> +		return 0;
> +
> +	ret = device_property_read_u32(dev, "adi,unlock-trigger-sec", &adp5585->unlock_time);
> +	if (!ret) {
> +		if (adp5585->unlock_time > ADP5585_MAX_UNLOCK_TIME_SEC)
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid unlock time(%u > %d)\n",
> +					     adp5585->unlock_time,
> +					     ADP5585_MAX_UNLOCK_TIME_SEC);
> +	}
> +
> +	return 0;
> +}
> +
> +static int adp5585_reset_ev_parse(struct adp5585_dev *adp5585)
> +{
> +	struct device *dev = adp5585->dev;
> +	u32 prop_val;
> +	int ret;
> +
> +	ret = adp5585_parse_ev_array(adp5585, "adi,reset1-events", adp5585->reset1_keys,
> +				     &adp5585->nkeys_reset1,
> +				     ARRAY_SIZE(adp5585->reset1_keys), true);
> +	if (ret)
> +		return ret;
> +
> +	ret = adp5585_parse_ev_array(adp5585, "adi,reset2-events",
> +				     adp5585->reset2_keys,
> +				     &adp5585->nkeys_reset2,
> +				     ARRAY_SIZE(adp5585->reset2_keys), true);
> +	if (ret)
> +		return ret;
> +
> +	if (!adp5585->nkeys_reset1 && !adp5585->nkeys_reset2)
> +		return 0;
> +
> +	if (adp5585->nkeys_reset1 && device_property_read_bool(dev, "adi,reset1-active-high"))
> +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET1_POL, 1);
> +
> +	if (adp5585->nkeys_reset2 && device_property_read_bool(dev, "adi,reset2-active-high"))
> +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET2_POL, 1);
> +
> +	if (device_property_read_bool(dev, "adi,rst-passthrough-enable"))
> +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RST_PASSTHRU_EN, 1);
> +
> +	ret = device_property_read_u32(dev, "adi,reset-trigger-ms", &prop_val);
> +	if (!ret) {
> +		switch (prop_val) {
> +		case 0:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 0);
> +			break;
> +		case 1000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 1);
> +			break;
> +		case 1500:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 2);
> +			break;
> +		case 2000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 3);
> +			break;
> +		case 2500:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 4);
> +			break;
> +		case 3000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 5);
> +			break;
> +		case 3500:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 6);
> +			break;
> +		case 4000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 7);
> +			break;
> +		default:
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid value(%u) for adi,reset-trigger-ms\n",
> +					     prop_val);
> +		}
> +	}
> +
> +	ret = device_property_read_u32(dev, "adi,reset-pulse-width-us",
> +				       &prop_val);

Odd line break.

> +	if (!ret) {
> +		switch (prop_val) {
> +		case 500:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 0);
> +			break;
> +		case 1000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 1);
> +			break;
> +		case 2000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 2);
> +			break;
> +		case 10000:
> +			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 3);
> +			break;
> +		default:
> +			return dev_err_probe(dev, -EINVAL,
> +					     "Invalid value(%u) for adi,reset-pulse-width-us\n",
> +					     prop_val);
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static void adp5585_remove_devices(void *dev)
>  {
>  	mfd_remove_devices(dev);
> @@ -307,6 +518,59 @@ static int adp5585_setup(struct adp5585_dev *adp5585)
>  	unsigned int reg_val, i;
>  	int ret;
>  
> +	/* Configure the device with reset and unlock events */
> +	for (i = 0; i < adp5585->nkeys_unlock; i++) {
> +		ret = regmap_write(adp5585->regmap, ADP5589_UNLOCK1 + i,
> +				   adp5585->unlock_keys[i] | ADP5589_UNLOCK_EV_PRESS);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (adp5585->nkeys_unlock) {
> +		ret = regmap_update_bits(adp5585->regmap, ADP5589_UNLOCK_TIMERS,
> +					 ADP5589_UNLOCK_TIMER, adp5585->unlock_time);
> +		if (ret)
> +			return ret;
> +
> +		ret = regmap_set_bits(adp5585->regmap, ADP5589_LOCK_CFG, ADP5589_LOCK_EN);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < adp5585->nkeys_reset1; i++) {
> +		ret = regmap_write(adp5585->regmap, regs->reset1_event_a + i,
> +				   adp5585->reset1_keys[i] | ADP5585_RESET_EV_PRESS);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (i = 0; i < adp5585->nkeys_reset2; i++) {
> +		ret = regmap_write(adp5585->regmap, regs->reset2_event_a + i,
> +				   adp5585->reset2_keys[i] | ADP5585_RESET_EV_PRESS);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (adp5585->nkeys_reset1 || adp5585->nkeys_reset2) {
> +		ret = regmap_write(adp5585->regmap, regs->reset_cfg, adp5585->reset_cfg);
> +		if (ret)
> +			return ret;
> +
> +		reg_val = 0;

Initialisation during declaration is preferred.

> +		/* If there's a reset1 event, then R4 is used as an output for the reset signal */
> +		if (adp5585->nkeys_reset1)
> +			reg_val = ADP5585_R4_EXTEND_CFG_RESET1;
> +		/* If there's a reset2 event, then C4 is used as an output for the reset signal */
> +		if (adp5585->nkeys_reset2)
> +			reg_val |= ADP5585_C4_EXTEND_CFG_RESET2;
> +
> +		ret = regmap_update_bits(adp5585->regmap, regs->ext_cfg,
> +					 ADP5585_C4_EXTEND_CFG_MASK | ADP5585_R4_EXTEND_CFG_MASK,
> +					 reg_val);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	/* Clear any possible event by reading all the FIFO entries */
>  	for (i = 0; i < ADP5585_EV_MAX; i++) {
>  		ret = regmap_read(adp5585->regmap, ADP5585_FIFO_1 + i, &reg_val);
> @@ -351,7 +615,11 @@ static int adp5585_parse_fw(struct adp5585_dev *adp5585)
>  					     "Invalid value(%u) for poll-interval\n", prop_val);
>  	}
>  
> -	return 0;
> +	ret = adp5585_unlock_ev_parse(adp5585);
> +	if (ret)
> +		return ret;
> +
> +	return adp5585_reset_ev_parse(adp5585);
>  }
>  
>  static void adp5585_irq_disable(void *data)
> diff --git a/include/linux/mfd/adp5585.h b/include/linux/mfd/adp5585.h
> index b6baf87907a567fe975f8b24f3c36753e6145066..5a1de5ae4bb144ed49a03a4e9e93eb614abe9fa3 100644
> --- a/include/linux/mfd/adp5585.h
> +++ b/include/linux/mfd/adp5585.h
> @@ -68,6 +68,7 @@
>  #define ADP5585_GPIO_DIRECTION_A	0x27
>  #define ADP5585_GPIO_DIRECTION_B	0x28
>  #define ADP5585_RESET1_EVENT_A		0x29
> +#define		ADP5585_RESET_EV_PRESS		BIT(7)
>  #define ADP5585_RESET1_EVENT_B		0x2a
>  #define ADP5585_RESET1_EVENT_C		0x2b
>  #define ADP5585_RESET2_EVENT_A		0x2c
> @@ -118,6 +119,13 @@
>  #define ADP5585_MAX_REG			ADP5585_INT_EN
>  
>  #define ADP5585_PIN_MAX			11
> +#define ADP5585_MAX_UNLOCK_TIME_SEC	7
> +#define ADP5585_KEY_EVENT_START		1
> +#define ADP5585_KEY_EVENT_END		25
> +#define ADP5585_GPI_EVENT_START		37
> +#define ADP5585_GPI_EVENT_END		47
> +#define ADP5585_ROW5_KEY_EVENT_START	1
> +#define ADP5585_ROW5_KEY_EVENT_END	30
>  
>  /* ADP5589 */
>  #define		ADP5589_MAN_ID_VALUE		0x10
> @@ -128,6 +136,20 @@
>  #define ADP5589_GPO_DATA_OUT_A		0x2a
>  #define ADP5589_GPO_OUT_MODE_A		0x2d
>  #define		ADP5589_GPIO_DIRECTION_A	0x30
> +#define ADP5589_UNLOCK1			0x33
> +#define		ADP5589_UNLOCK_EV_PRESS		BIT(7)
> +#define ADP5589_UNLOCK_TIMERS		0x36
> +#define		ADP5589_UNLOCK_TIMER		GENMASK(2, 0)
> +#define ADP5589_LOCK_CFG		0x37
> +#define		ADP5589_LOCK_EN			BIT(0)
> +#define ADP5589_RESET1_EVENT_A		0x38
> +#define ADP5589_RESET2_EVENT_A		0x3B
> +#define ADP5589_RESET_CFG		0x3D
> +#define		ADP5585_RESET2_POL		BIT(7)
> +#define		ADP5585_RESET1_POL		BIT(6)
> +#define		ADP5585_RST_PASSTHRU_EN		BIT(5)
> +#define		ADP5585_RESET_TRIG_TIME		GENMASK(4, 2)
> +#define		ADP5585_PULSE_WIDTH		GENMASK(1, 0)
>  #define ADP5589_PWM_OFFT_LOW		0x3e
>  #define ADP5589_PWM_ONT_LOW		0x40
>  #define ADP5589_PWM_CFG			0x42
> @@ -138,6 +160,10 @@
>  #define ADP5589_MAX_REG			ADP5589_INT_EN
>  
>  #define ADP5589_PIN_MAX			19
> +#define ADP5589_KEY_EVENT_START		1
> +#define ADP5589_KEY_EVENT_END		88
> +#define ADP5589_GPI_EVENT_START		97
> +#define ADP5589_GPI_EVENT_END		115
>  
>  struct regmap;
>  
> @@ -158,6 +184,9 @@ struct adp5585_regs {
>  	unsigned int ext_cfg;
>  	unsigned int int_en;
>  	unsigned int poll_ptime_cfg;
> +	unsigned int reset_cfg;
> +	unsigned int reset1_event_a;
> +	unsigned int reset2_event_a;
>  };
>  
>  struct adp5585_dev {
> @@ -167,8 +196,18 @@ struct adp5585_dev {
>  	struct blocking_notifier_head event_notifier;
>  	enum adp5585_variant variant;
>  	unsigned int id;
> +	bool has_unlock;
> +	bool has_pin6;
>  	int irq;
>  	unsigned int ev_poll_time;
> +	unsigned int unlock_time;
> +	unsigned int unlock_keys[2];
> +	unsigned int nkeys_unlock;
> +	unsigned int reset1_keys[3];
> +	unsigned int nkeys_reset1;
> +	unsigned int reset2_keys[2];
> +	unsigned int nkeys_reset2;
> +	u8 reset_cfg;
>  };
>  
>  #endif
> 
> -- 
> 2.49.0
> 
>
Nuno Sá June 13, 2025, 9:48 a.m. UTC | #2 
On Thu, 2025-06-12 at 15:55 +0100, Lee Jones wrote:
> On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:
> 
> > 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.c
> > index
> > dcc09c898dd7990b39e21cb2324fa66ae171a802..6737d622a7ed9f280c439399f3709ca816
> > 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.

- Nuno Sá
> 
> > +		if (ev == (ADP5585_GPI_EVENT_START + 5))
> > +			return dev_err_probe(adp5585->dev, -EINVAL,
> > +					     "Invalid unlock/reset
> > event(%u). R5 not available\n",
> > +					     ev);
> > +		return 0;
> > +	}
> > +
> > +	return dev_err_probe(adp5585->dev, -EINVAL,
> > +			     "Invalid unlock/reset event(%u) for this
> > device\n", ev);
> > +}
> > +
> > +static int adp5589_validate_event(const struct adp5585_dev *adp5585,
> > unsigned int ev)
> > +{
> > +	if (ev >= ADP5589_KEY_EVENT_START && ev <= ADP5589_KEY_EVENT_END)
> > +		return 0;
> > +	if (ev >= ADP5589_GPI_EVENT_START && ev <= ADP5589_GPI_EVENT_END)
> > +		return 0;
> > +
> > +	return dev_err_probe(adp5585->dev, -EINVAL,
> > +			     "Invalid unlock/reset event(%u) for this
> > device\n",
> > +			     ev);
> 
> This line break is unnecessary.
> 
> > +}
> > +
> >  static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
> >  				       struct regmap_config *regmap_config)
> >  {
> > @@ -191,6 +236,8 @@ static int adp5585_fill_variant_config(struct
> > adp5585_dev *adp5585,
> >  		*regmap_config = adp5585_regmap_config_template;
> >  		adp5585->id = ADP5585_MAN_ID_VALUE;
> >  		adp5585->regs = &adp5585_regs;
> > +		if (adp5585->variant == ADP5585_01)
> > +			adp5585->has_pin6 = true;
> >  		break;
> >  	case ADP5589_00:
> >  	case ADP5589_01:
> > @@ -198,6 +245,8 @@ static int adp5585_fill_variant_config(struct
> > adp5585_dev *adp5585,
> >  		*regmap_config = adp5589_regmap_config_template;
> >  		adp5585->id = ADP5589_MAN_ID_VALUE;
> >  		adp5585->regs = &adp5589_regs;
> > +		adp5585->has_unlock = true;
> > +		adp5585->has_pin6 = true;
> >  		break;
> >  	default:
> >  		return -ENODEV;
> > @@ -207,6 +256,168 @@ static int adp5585_fill_variant_config(struct
> > adp5585_dev *adp5585,
> >  	return 0;
> >  }
> >  
> > +static int adp5585_parse_ev_array(const struct adp5585_dev *adp5585, const
> > char *prop, u32 *events,
> > +				  u32 *n_events, u32 max_evs, bool
> > reset_ev)
> > +{
> > +	struct device *dev = adp5585->dev;
> > +	unsigned int ev;
> > +	int ret;
> > +
> > +	/*
> > +	 * The device has the capability of handling special events through
> > GPIs or a Keypad:
> > +	 *  unlock events: Unlock the keymap until one of the configured
> > events is detected.
> > +	 *  reset events: Generate a reset pulse when one of the configured
> > events is detected.
> > +	 */
> > +	ret = device_property_count_u32(dev, prop);
> > +	if (ret < 0)
> > +		return 0;
> > +
> > +	*n_events = ret;
> > +
> > +	if (!adp5585->has_unlock && !reset_ev)
> > +		return dev_err_probe(dev, -EOPNOTSUPP, "Unlock keys not
> > supported\n");
> > +
> > +	if (*n_events > max_evs)
> > +		return dev_err_probe(dev, -EINVAL,
> > +				     "Invalid number of keys(%u > %u) for
> > %s\n",
> > +				     *n_events, max_evs, prop);
> > +
> > +	ret = device_property_read_u32_array(dev, prop, events, *n_events);
> > +	if (ret)
> > +		return ret;
> > +
> > +	for (ev = 0; ev < *n_events; ev++) {
> > +		/* for unlock events, 127 is a wildcard */
> 
> As above and throughout the series.
> 
> If you define the wildcard magic number you can drop the comment.
> 
> > +		if (!reset_ev && events[ev] == 127)
> > +			continue;
> > +
> > +		if (adp5585->id == ADP5585_MAN_ID_VALUE)
> > +			ret = adp5585_validate_event(adp5585, events[ev]);
> > +		else
> > +			ret = adp5589_validate_event(adp5585, events[ev]);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int adp5585_unlock_ev_parse(struct adp5585_dev *adp5585)
> > +{
> > +	struct device *dev = adp5585->dev;
> > +	int ret;
> > +
> > +	ret = adp5585_parse_ev_array(adp5585, "adi,unlock-events", adp5585-
> > >unlock_keys,
> > +				     &adp5585->nkeys_unlock,
> > ARRAY_SIZE(adp5585->unlock_keys),
> > +				     false);
> > +	if (ret)
> > +		return ret;
> > +	if (!adp5585->nkeys_unlock)
> > +		return 0;
> > +
> > +	ret = device_property_read_u32(dev, "adi,unlock-trigger-sec",
> > &adp5585->unlock_time);
> > +	if (!ret) {
> > +		if (adp5585->unlock_time > ADP5585_MAX_UNLOCK_TIME_SEC)
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "Invalid unlock time(%u >
> > %d)\n",
> > +					     adp5585->unlock_time,
> > +					     ADP5585_MAX_UNLOCK_TIME_SEC);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int adp5585_reset_ev_parse(struct adp5585_dev *adp5585)
> > +{
> > +	struct device *dev = adp5585->dev;
> > +	u32 prop_val;
> > +	int ret;
> > +
> > +	ret = adp5585_parse_ev_array(adp5585, "adi,reset1-events", adp5585-
> > >reset1_keys,
> > +				     &adp5585->nkeys_reset1,
> > +				     ARRAY_SIZE(adp5585->reset1_keys),
> > true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = adp5585_parse_ev_array(adp5585, "adi,reset2-events",
> > +				     adp5585->reset2_keys,
> > +				     &adp5585->nkeys_reset2,
> > +				     ARRAY_SIZE(adp5585->reset2_keys),
> > true);
> > +	if (ret)
> > +		return ret;
> > +
> > +	if (!adp5585->nkeys_reset1 && !adp5585->nkeys_reset2)
> > +		return 0;
> > +
> > +	if (adp5585->nkeys_reset1 && device_property_read_bool(dev,
> > "adi,reset1-active-high"))
> > +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET1_POL, 1);
> > +
> > +	if (adp5585->nkeys_reset2 && device_property_read_bool(dev,
> > "adi,reset2-active-high"))
> > +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET2_POL, 1);
> > +
> > +	if (device_property_read_bool(dev, "adi,rst-passthrough-enable"))
> > +		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RST_PASSTHRU_EN,
> > 1);
> > +
> > +	ret = device_property_read_u32(dev, "adi,reset-trigger-ms",
> > &prop_val);
> > +	if (!ret) {
> > +		switch (prop_val) {
> > +		case 0:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 0);
> > +			break;
> > +		case 1000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 1);
> > +			break;
> > +		case 1500:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 2);
> > +			break;
> > +		case 2000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 3);
> > +			break;
> > +		case 2500:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 4);
> > +			break;
> > +		case 3000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 5);
> > +			break;
> > +		case 3500:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 6);
> > +			break;
> > +		case 4000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_RESET_TRIG_TIME, 7);
> > +			break;
> > +		default:
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "Invalid value(%u) for
> > adi,reset-trigger-ms\n",
> > +					     prop_val);
> > +		}
> > +	}
> > +
> > +	ret = device_property_read_u32(dev, "adi,reset-pulse-width-us",
> > +				       &prop_val);
> 
> Odd line break.
> 
> > +	if (!ret) {
> > +		switch (prop_val) {
> > +		case 500:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_PULSE_WIDTH, 0);
> > +			break;
> > +		case 1000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_PULSE_WIDTH, 1);
> > +			break;
> > +		case 2000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_PULSE_WIDTH, 2);
> > +			break;
> > +		case 10000:
> > +			adp5585->reset_cfg |=
> > FIELD_PREP(ADP5585_PULSE_WIDTH, 3);
> > +			break;
> > +		default:
> > +			return dev_err_probe(dev, -EINVAL,
> > +					     "Invalid value(%u) for
> > adi,reset-pulse-width-us\n",
> > +					     prop_val);
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  static void adp5585_remove_devices(void *dev)
> >  {
> >  	mfd_remove_devices(dev);
> > @@ -307,6 +518,59 @@ static int adp5585_setup(struct adp5585_dev *adp5585)
> >  	unsigned int reg_val, i;
> >  	int ret;
> >  
> > +	/* Configure the device with reset and unlock events */
> > +	for (i = 0; i < adp5585->nkeys_unlock; i++) {
> > +		ret = regmap_write(adp5585->regmap, ADP5589_UNLOCK1 + i,
> > +				   adp5585->unlock_keys[i] |
> > ADP5589_UNLOCK_EV_PRESS);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	if (adp5585->nkeys_unlock) {
> > +		ret = regmap_update_bits(adp5585->regmap,
> > ADP5589_UNLOCK_TIMERS,
> > +					 ADP5589_UNLOCK_TIMER, adp5585-
> > >unlock_time);
> > +		if (ret)
> > +			return ret;
> > +
> > +		ret = regmap_set_bits(adp5585->regmap, ADP5589_LOCK_CFG,
> > ADP5589_LOCK_EN);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	for (i = 0; i < adp5585->nkeys_reset1; i++) {
> > +		ret = regmap_write(adp5585->regmap, regs->reset1_event_a +
> > i,
> > +				   adp5585->reset1_keys[i] |
> > ADP5585_RESET_EV_PRESS);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	for (i = 0; i < adp5585->nkeys_reset2; i++) {
> > +		ret = regmap_write(adp5585->regmap, regs->reset2_event_a +
> > i,
> > +				   adp5585->reset2_keys[i] |
> > ADP5585_RESET_EV_PRESS);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	if (adp5585->nkeys_reset1 || adp5585->nkeys_reset2) {
> > +		ret = regmap_write(adp5585->regmap, regs->reset_cfg,
> > adp5585->reset_cfg);
> > +		if (ret)
> > +			return ret;
> > +
> > +		reg_val = 0;
> 
> Initialisation during declaration is preferred.
> 
> > +		/* If there's a reset1 event, then R4 is used as an output
> > for the reset signal */
> > +		if (adp5585->nkeys_reset1)
> > +			reg_val = ADP5585_R4_EXTEND_CFG_RESET1;
> > +		/* If there's a reset2 event, then C4 is used as an output
> > for the reset signal */
> > +		if (adp5585->nkeys_reset2)
> > +			reg_val |= ADP5585_C4_EXTEND_CFG_RESET2;
> > +
> > +		ret = regmap_update_bits(adp5585->regmap, regs->ext_cfg,
> > +					 ADP5585_C4_EXTEND_CFG_MASK |
> > ADP5585_R4_EXTEND_CFG_MASK,
> > +					 reg_val);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> >  	/* Clear any possible event by reading all the FIFO entries */
> >  	for (i = 0; i < ADP5585_EV_MAX; i++) {
> >  		ret = regmap_read(adp5585->regmap, ADP5585_FIFO_1 + i,
> > &reg_val);
> > @@ -351,7 +615,11 @@ static int adp5585_parse_fw(struct adp5585_dev
> > *adp5585)
> >  					     "Invalid value(%u) for poll-
> > interval\n", prop_val);
> >  	}
> >  
> > -	return 0;
> > +	ret = adp5585_unlock_ev_parse(adp5585);
> > +	if (ret)
> > +		return ret;
> > +
> > +	return adp5585_reset_ev_parse(adp5585);
> >  }
> >  
> >  static void adp5585_irq_disable(void *data)
> > diff --git a/include/linux/mfd/adp5585.h b/include/linux/mfd/adp5585.h
> > index
> > b6baf87907a567fe975f8b24f3c36753e6145066..5a1de5ae4bb144ed49a03a4e9e93eb614a
> > be9fa3 100644
> > --- a/include/linux/mfd/adp5585.h
> > +++ b/include/linux/mfd/adp5585.h
> > @@ -68,6 +68,7 @@
> >  #define ADP5585_GPIO_DIRECTION_A	0x27
> >  #define ADP5585_GPIO_DIRECTION_B	0x28
> >  #define ADP5585_RESET1_EVENT_A		0x29
> > +#define		ADP5585_RESET_EV_PRESS		BIT(7)
> >  #define ADP5585_RESET1_EVENT_B		0x2a
> >  #define ADP5585_RESET1_EVENT_C		0x2b
> >  #define ADP5585_RESET2_EVENT_A		0x2c
> > @@ -118,6 +119,13 @@
> >  #define ADP5585_MAX_REG			ADP5585_INT_EN
> >  
> >  #define ADP5585_PIN_MAX			11
> > +#define ADP5585_MAX_UNLOCK_TIME_SEC	7
> > +#define ADP5585_KEY_EVENT_START		1
> > +#define ADP5585_KEY_EVENT_END		25
> > +#define ADP5585_GPI_EVENT_START		37
> > +#define ADP5585_GPI_EVENT_END		47
> > +#define ADP5585_ROW5_KEY_EVENT_START	1
> > +#define ADP5585_ROW5_KEY_EVENT_END	30
> >  
> >  /* ADP5589 */
> >  #define		ADP5589_MAN_ID_VALUE		0x10
> > @@ -128,6 +136,20 @@
> >  #define ADP5589_GPO_DATA_OUT_A		0x2a
> >  #define ADP5589_GPO_OUT_MODE_A		0x2d
> >  #define		ADP5589_GPIO_DIRECTION_A	0x30
> > +#define ADP5589_UNLOCK1			0x33
> > +#define		ADP5589_UNLOCK_EV_PRESS		BIT(7)
> > +#define ADP5589_UNLOCK_TIMERS		0x36
> > +#define		ADP5589_UNLOCK_TIMER		GENMASK(2, 0)
> > +#define ADP5589_LOCK_CFG		0x37
> > +#define		ADP5589_LOCK_EN			BIT(0)
> > +#define ADP5589_RESET1_EVENT_A		0x38
> > +#define ADP5589_RESET2_EVENT_A		0x3B
> > +#define ADP5589_RESET_CFG		0x3D
> > +#define		ADP5585_RESET2_POL		BIT(7)
> > +#define		ADP5585_RESET1_POL		BIT(6)
> > +#define		ADP5585_RST_PASSTHRU_EN		BIT(5)
> > +#define		ADP5585_RESET_TRIG_TIME		GENMASK(4,
> > 2)
> > +#define		ADP5585_PULSE_WIDTH		GENMASK(1, 0)
> >  #define ADP5589_PWM_OFFT_LOW		0x3e
> >  #define ADP5589_PWM_ONT_LOW		0x40
> >  #define ADP5589_PWM_CFG			0x42
> > @@ -138,6 +160,10 @@
> >  #define ADP5589_MAX_REG			ADP5589_INT_EN
> >  
> >  #define ADP5589_PIN_MAX			19
> > +#define ADP5589_KEY_EVENT_START		1
> > +#define ADP5589_KEY_EVENT_END		88
> > +#define ADP5589_GPI_EVENT_START		97
> > +#define ADP5589_GPI_EVENT_END		115
> >  
> >  struct regmap;
> >  
> > @@ -158,6 +184,9 @@ struct adp5585_regs {
> >  	unsigned int ext_cfg;
> >  	unsigned int int_en;
> >  	unsigned int poll_ptime_cfg;
> > +	unsigned int reset_cfg;
> > +	unsigned int reset1_event_a;
> > +	unsigned int reset2_event_a;
> >  };
> >  
> >  struct adp5585_dev {
> > @@ -167,8 +196,18 @@ struct adp5585_dev {
> >  	struct blocking_notifier_head event_notifier;
> >  	enum adp5585_variant variant;
> >  	unsigned int id;
> > +	bool has_unlock;
> > +	bool has_pin6;
> >  	int irq;
> >  	unsigned int ev_poll_time;
> > +	unsigned int unlock_time;
> > +	unsigned int unlock_keys[2];
> > +	unsigned int nkeys_unlock;
> > +	unsigned int reset1_keys[3];
> > +	unsigned int nkeys_reset1;
> > +	unsigned int reset2_keys[2];
> > +	unsigned int nkeys_reset2;
> > +	u8 reset_cfg;
> >  };
> >  
> >  #endif
> > 
> > -- 
> > 2.49.0
> > 
> >
Lee Jones June 13, 2025, 1:07 p.m. UTC | #3 
On Fri, 13 Jun 2025, Nuno Sá wrote:

> On Thu, 2025-06-12 at 15:55 +0100, Lee Jones wrote:
> > On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:
> > 
> > > 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.c
> > > index
> > > dcc09c898dd7990b39e21cb2324fa66ae171a802..6737d622a7ed9f280c439399f3709ca816
> > > 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?

> - Nuno Sá
> > 
> > > +		if (ev == (ADP5585_GPI_EVENT_START + 5))
> > > +			return dev_err_probe(adp5585->dev, -EINVAL,
> > > +					     "Invalid unlock/reset
> > > event(%u). R5 not available\n",
> > > +					     ev);
> > > +		return 0;
> > > +	}

[...]
Nuno Sá June 13, 2025, 1:13 p.m. UTC | #4 
On Fri, 2025-06-13 at 14:07 +0100, Lee Jones wrote:
> On Fri, 13 Jun 2025, Nuno Sá wrote:
> 
> > On Thu, 2025-06-12 at 15:55 +0100, Lee Jones wrote:
> > > On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:
> > > 
> > > > 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.c
> > > > index
> > > > 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.

- Nuno Sá

> 
> > - Nuno Sá
> > > 
> > > > +		if (ev == (ADP5585_GPI_EVENT_START + 5))
> > > > +			return dev_err_probe(adp5585->dev, -EINVAL,
> > > > +					     "Invalid unlock/reset
> > > > event(%u). R5 not available\n",
> > > > +					     ev);
> > > > +		return 0;
> > > > +	}
> 
> [...]
Lee Jones June 13, 2025, 1:27 p.m. UTC | #5 
On Fri, 13 Jun 2025, Nuno Sá wrote:

> On Fri, 2025-06-13 at 14:07 +0100, Lee Jones wrote:
> > On Fri, 13 Jun 2025, Nuno Sá wrote:
> > 
> > > On Thu, 2025-06-12 at 15:55 +0100, Lee Jones wrote:
> > > > On Wed, 21 May 2025, Nuno Sá via B4 Relay wrote:
> > > > 
> > > > > 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.c
> > > > > index
> > > > > 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.
diff mbox series

Patch

diff --git a/drivers/mfd/adp5585.c b/drivers/mfd/adp5585.c
index dcc09c898dd7990b39e21cb2324fa66ae171a802..6737d622a7ed9f280c439399f3709ca8162dee01 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 */
+		if (ev == (ADP5585_GPI_EVENT_START + 5))
+			return dev_err_probe(adp5585->dev, -EINVAL,
+					     "Invalid unlock/reset event(%u). R5 not available\n",
+					     ev);
+		return 0;
+	}
+
+	return dev_err_probe(adp5585->dev, -EINVAL,
+			     "Invalid unlock/reset event(%u) for this device\n", ev);
+}
+
+static int adp5589_validate_event(const struct adp5585_dev *adp5585, unsigned int ev)
+{
+	if (ev >= ADP5589_KEY_EVENT_START && ev <= ADP5589_KEY_EVENT_END)
+		return 0;
+	if (ev >= ADP5589_GPI_EVENT_START && ev <= ADP5589_GPI_EVENT_END)
+		return 0;
+
+	return dev_err_probe(adp5585->dev, -EINVAL,
+			     "Invalid unlock/reset event(%u) for this device\n",
+			     ev);
+}
+
 static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
 				       struct regmap_config *regmap_config)
 {
@@ -191,6 +236,8 @@  static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
 		*regmap_config = adp5585_regmap_config_template;
 		adp5585->id = ADP5585_MAN_ID_VALUE;
 		adp5585->regs = &adp5585_regs;
+		if (adp5585->variant == ADP5585_01)
+			adp5585->has_pin6 = true;
 		break;
 	case ADP5589_00:
 	case ADP5589_01:
@@ -198,6 +245,8 @@  static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
 		*regmap_config = adp5589_regmap_config_template;
 		adp5585->id = ADP5589_MAN_ID_VALUE;
 		adp5585->regs = &adp5589_regs;
+		adp5585->has_unlock = true;
+		adp5585->has_pin6 = true;
 		break;
 	default:
 		return -ENODEV;
@@ -207,6 +256,168 @@  static int adp5585_fill_variant_config(struct adp5585_dev *adp5585,
 	return 0;
 }
 
+static int adp5585_parse_ev_array(const struct adp5585_dev *adp5585, const char *prop, u32 *events,
+				  u32 *n_events, u32 max_evs, bool reset_ev)
+{
+	struct device *dev = adp5585->dev;
+	unsigned int ev;
+	int ret;
+
+	/*
+	 * The device has the capability of handling special events through GPIs or a Keypad:
+	 *  unlock events: Unlock the keymap until one of the configured events is detected.
+	 *  reset events: Generate a reset pulse when one of the configured events is detected.
+	 */
+	ret = device_property_count_u32(dev, prop);
+	if (ret < 0)
+		return 0;
+
+	*n_events = ret;
+
+	if (!adp5585->has_unlock && !reset_ev)
+		return dev_err_probe(dev, -EOPNOTSUPP, "Unlock keys not supported\n");
+
+	if (*n_events > max_evs)
+		return dev_err_probe(dev, -EINVAL,
+				     "Invalid number of keys(%u > %u) for %s\n",
+				     *n_events, max_evs, prop);
+
+	ret = device_property_read_u32_array(dev, prop, events, *n_events);
+	if (ret)
+		return ret;
+
+	for (ev = 0; ev < *n_events; ev++) {
+		/* for unlock events, 127 is a wildcard */
+		if (!reset_ev && events[ev] == 127)
+			continue;
+
+		if (adp5585->id == ADP5585_MAN_ID_VALUE)
+			ret = adp5585_validate_event(adp5585, events[ev]);
+		else
+			ret = adp5589_validate_event(adp5585, events[ev]);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
+}
+
+static int adp5585_unlock_ev_parse(struct adp5585_dev *adp5585)
+{
+	struct device *dev = adp5585->dev;
+	int ret;
+
+	ret = adp5585_parse_ev_array(adp5585, "adi,unlock-events", adp5585->unlock_keys,
+				     &adp5585->nkeys_unlock, ARRAY_SIZE(adp5585->unlock_keys),
+				     false);
+	if (ret)
+		return ret;
+	if (!adp5585->nkeys_unlock)
+		return 0;
+
+	ret = device_property_read_u32(dev, "adi,unlock-trigger-sec", &adp5585->unlock_time);
+	if (!ret) {
+		if (adp5585->unlock_time > ADP5585_MAX_UNLOCK_TIME_SEC)
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid unlock time(%u > %d)\n",
+					     adp5585->unlock_time,
+					     ADP5585_MAX_UNLOCK_TIME_SEC);
+	}
+
+	return 0;
+}
+
+static int adp5585_reset_ev_parse(struct adp5585_dev *adp5585)
+{
+	struct device *dev = adp5585->dev;
+	u32 prop_val;
+	int ret;
+
+	ret = adp5585_parse_ev_array(adp5585, "adi,reset1-events", adp5585->reset1_keys,
+				     &adp5585->nkeys_reset1,
+				     ARRAY_SIZE(adp5585->reset1_keys), true);
+	if (ret)
+		return ret;
+
+	ret = adp5585_parse_ev_array(adp5585, "adi,reset2-events",
+				     adp5585->reset2_keys,
+				     &adp5585->nkeys_reset2,
+				     ARRAY_SIZE(adp5585->reset2_keys), true);
+	if (ret)
+		return ret;
+
+	if (!adp5585->nkeys_reset1 && !adp5585->nkeys_reset2)
+		return 0;
+
+	if (adp5585->nkeys_reset1 && device_property_read_bool(dev, "adi,reset1-active-high"))
+		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET1_POL, 1);
+
+	if (adp5585->nkeys_reset2 && device_property_read_bool(dev, "adi,reset2-active-high"))
+		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET2_POL, 1);
+
+	if (device_property_read_bool(dev, "adi,rst-passthrough-enable"))
+		adp5585->reset_cfg |= FIELD_PREP(ADP5585_RST_PASSTHRU_EN, 1);
+
+	ret = device_property_read_u32(dev, "adi,reset-trigger-ms", &prop_val);
+	if (!ret) {
+		switch (prop_val) {
+		case 0:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 0);
+			break;
+		case 1000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 1);
+			break;
+		case 1500:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 2);
+			break;
+		case 2000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 3);
+			break;
+		case 2500:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 4);
+			break;
+		case 3000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 5);
+			break;
+		case 3500:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 6);
+			break;
+		case 4000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_RESET_TRIG_TIME, 7);
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid value(%u) for adi,reset-trigger-ms\n",
+					     prop_val);
+		}
+	}
+
+	ret = device_property_read_u32(dev, "adi,reset-pulse-width-us",
+				       &prop_val);
+	if (!ret) {
+		switch (prop_val) {
+		case 500:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 0);
+			break;
+		case 1000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 1);
+			break;
+		case 2000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 2);
+			break;
+		case 10000:
+			adp5585->reset_cfg |= FIELD_PREP(ADP5585_PULSE_WIDTH, 3);
+			break;
+		default:
+			return dev_err_probe(dev, -EINVAL,
+					     "Invalid value(%u) for adi,reset-pulse-width-us\n",
+					     prop_val);
+		}
+	}
+
+	return 0;
+}
+
 static void adp5585_remove_devices(void *dev)
 {
 	mfd_remove_devices(dev);
@@ -307,6 +518,59 @@  static int adp5585_setup(struct adp5585_dev *adp5585)
 	unsigned int reg_val, i;
 	int ret;
 
+	/* Configure the device with reset and unlock events */
+	for (i = 0; i < adp5585->nkeys_unlock; i++) {
+		ret = regmap_write(adp5585->regmap, ADP5589_UNLOCK1 + i,
+				   adp5585->unlock_keys[i] | ADP5589_UNLOCK_EV_PRESS);
+		if (ret)
+			return ret;
+	}
+
+	if (adp5585->nkeys_unlock) {
+		ret = regmap_update_bits(adp5585->regmap, ADP5589_UNLOCK_TIMERS,
+					 ADP5589_UNLOCK_TIMER, adp5585->unlock_time);
+		if (ret)
+			return ret;
+
+		ret = regmap_set_bits(adp5585->regmap, ADP5589_LOCK_CFG, ADP5589_LOCK_EN);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < adp5585->nkeys_reset1; i++) {
+		ret = regmap_write(adp5585->regmap, regs->reset1_event_a + i,
+				   adp5585->reset1_keys[i] | ADP5585_RESET_EV_PRESS);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < adp5585->nkeys_reset2; i++) {
+		ret = regmap_write(adp5585->regmap, regs->reset2_event_a + i,
+				   adp5585->reset2_keys[i] | ADP5585_RESET_EV_PRESS);
+		if (ret)
+			return ret;
+	}
+
+	if (adp5585->nkeys_reset1 || adp5585->nkeys_reset2) {
+		ret = regmap_write(adp5585->regmap, regs->reset_cfg, adp5585->reset_cfg);
+		if (ret)
+			return ret;
+
+		reg_val = 0;
+		/* If there's a reset1 event, then R4 is used as an output for the reset signal */
+		if (adp5585->nkeys_reset1)
+			reg_val = ADP5585_R4_EXTEND_CFG_RESET1;
+		/* If there's a reset2 event, then C4 is used as an output for the reset signal */
+		if (adp5585->nkeys_reset2)
+			reg_val |= ADP5585_C4_EXTEND_CFG_RESET2;
+
+		ret = regmap_update_bits(adp5585->regmap, regs->ext_cfg,
+					 ADP5585_C4_EXTEND_CFG_MASK | ADP5585_R4_EXTEND_CFG_MASK,
+					 reg_val);
+		if (ret)
+			return ret;
+	}
+
 	/* Clear any possible event by reading all the FIFO entries */
 	for (i = 0; i < ADP5585_EV_MAX; i++) {
 		ret = regmap_read(adp5585->regmap, ADP5585_FIFO_1 + i, &reg_val);
@@ -351,7 +615,11 @@  static int adp5585_parse_fw(struct adp5585_dev *adp5585)
 					     "Invalid value(%u) for poll-interval\n", prop_val);
 	}
 
-	return 0;
+	ret = adp5585_unlock_ev_parse(adp5585);
+	if (ret)
+		return ret;
+
+	return adp5585_reset_ev_parse(adp5585);
 }
 
 static void adp5585_irq_disable(void *data)
diff --git a/include/linux/mfd/adp5585.h b/include/linux/mfd/adp5585.h
index b6baf87907a567fe975f8b24f3c36753e6145066..5a1de5ae4bb144ed49a03a4e9e93eb614abe9fa3 100644
--- a/include/linux/mfd/adp5585.h
+++ b/include/linux/mfd/adp5585.h
@@ -68,6 +68,7 @@ 
 #define ADP5585_GPIO_DIRECTION_A	0x27
 #define ADP5585_GPIO_DIRECTION_B	0x28
 #define ADP5585_RESET1_EVENT_A		0x29
+#define		ADP5585_RESET_EV_PRESS		BIT(7)
 #define ADP5585_RESET1_EVENT_B		0x2a
 #define ADP5585_RESET1_EVENT_C		0x2b
 #define ADP5585_RESET2_EVENT_A		0x2c
@@ -118,6 +119,13 @@ 
 #define ADP5585_MAX_REG			ADP5585_INT_EN
 
 #define ADP5585_PIN_MAX			11
+#define ADP5585_MAX_UNLOCK_TIME_SEC	7
+#define ADP5585_KEY_EVENT_START		1
+#define ADP5585_KEY_EVENT_END		25
+#define ADP5585_GPI_EVENT_START		37
+#define ADP5585_GPI_EVENT_END		47
+#define ADP5585_ROW5_KEY_EVENT_START	1
+#define ADP5585_ROW5_KEY_EVENT_END	30
 
 /* ADP5589 */
 #define		ADP5589_MAN_ID_VALUE		0x10
@@ -128,6 +136,20 @@ 
 #define ADP5589_GPO_DATA_OUT_A		0x2a
 #define ADP5589_GPO_OUT_MODE_A		0x2d
 #define		ADP5589_GPIO_DIRECTION_A	0x30
+#define ADP5589_UNLOCK1			0x33
+#define		ADP5589_UNLOCK_EV_PRESS		BIT(7)
+#define ADP5589_UNLOCK_TIMERS		0x36
+#define		ADP5589_UNLOCK_TIMER		GENMASK(2, 0)
+#define ADP5589_LOCK_CFG		0x37
+#define		ADP5589_LOCK_EN			BIT(0)
+#define ADP5589_RESET1_EVENT_A		0x38
+#define ADP5589_RESET2_EVENT_A		0x3B
+#define ADP5589_RESET_CFG		0x3D
+#define		ADP5585_RESET2_POL		BIT(7)
+#define		ADP5585_RESET1_POL		BIT(6)
+#define		ADP5585_RST_PASSTHRU_EN		BIT(5)
+#define		ADP5585_RESET_TRIG_TIME		GENMASK(4, 2)
+#define		ADP5585_PULSE_WIDTH		GENMASK(1, 0)
 #define ADP5589_PWM_OFFT_LOW		0x3e
 #define ADP5589_PWM_ONT_LOW		0x40
 #define ADP5589_PWM_CFG			0x42
@@ -138,6 +160,10 @@ 
 #define ADP5589_MAX_REG			ADP5589_INT_EN
 
 #define ADP5589_PIN_MAX			19
+#define ADP5589_KEY_EVENT_START		1
+#define ADP5589_KEY_EVENT_END		88
+#define ADP5589_GPI_EVENT_START		97
+#define ADP5589_GPI_EVENT_END		115
 
 struct regmap;
 
@@ -158,6 +184,9 @@  struct adp5585_regs {
 	unsigned int ext_cfg;
 	unsigned int int_en;
 	unsigned int poll_ptime_cfg;
+	unsigned int reset_cfg;
+	unsigned int reset1_event_a;
+	unsigned int reset2_event_a;
 };
 
 struct adp5585_dev {
@@ -167,8 +196,18 @@  struct adp5585_dev {
 	struct blocking_notifier_head event_notifier;
 	enum adp5585_variant variant;
 	unsigned int id;
+	bool has_unlock;
+	bool has_pin6;
 	int irq;
 	unsigned int ev_poll_time;
+	unsigned int unlock_time;
+	unsigned int unlock_keys[2];
+	unsigned int nkeys_unlock;
+	unsigned int reset1_keys[3];
+	unsigned int nkeys_reset1;
+	unsigned int reset2_keys[2];
+	unsigned int nkeys_reset2;
+	u8 reset_cfg;
 };
 
 #endif