| Message ID | 20250512-dev-adp5589-fw-v3-4-092b14b79a88@analog.com |
|---|---|
| State | New |
| Headers | show |
| Series | mfd: adp5585: support keymap events and drop legacy Input driver | expand |
Hi Nuno, Thank you for the patch. On Mon, May 12, 2025 at 01:38:56PM +0100, Nuno Sá via B4 Relay wrote: > From: Nuno Sá <nuno.sa@analog.com> > > The adp5585 is a Multi Function Device that can also be a gpio > controller and as it turns out, when OSC_EN is not set, we can't > reliably read the gpio value when it's configured as input. Hence, > OSC_EN will be set during probe by the parent device (and cleared on > unbind). > > Moreover, we'll add support for the keymap matrix (input device) which > definitely needs OSC_EN to be set and so, we cannot afford that disabling > the PWM output also breaks the keymap events generation. I think you can squash this with 03/22 if you send a new version. Moving the OSC_EN bit handling from the PWM child driver to the MFD driver is a single logical change. > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > --- > drivers/pwm/pwm-adp5585.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/drivers/pwm/pwm-adp5585.c b/drivers/pwm/pwm-adp5585.c > index 40472ac5db6410a33e4f790fe8e6c23b517502be..c8821035b7c1412a55a642e6e8a46b66e693a5af 100644 > --- a/drivers/pwm/pwm-adp5585.c > +++ b/drivers/pwm/pwm-adp5585.c > @@ -62,7 +62,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, > int ret; > > if (!state->enabled) { > - regmap_clear_bits(regmap, ADP5585_GENERAL_CFG, ADP5585_OSC_EN); > regmap_clear_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); > return 0; > } > @@ -100,10 +99,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, > if (ret) > return ret; > > - ret = regmap_set_bits(regmap, ADP5585_GENERAL_CFG, ADP5585_OSC_EN); > - if (ret) > - return ret; > - > return regmap_set_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); > } >
On Tue, 2025-05-13 at 17:26 +0200, Laurent Pinchart wrote: > Hi Nuno, > > Thank you for the patch. > > On Mon, May 12, 2025 at 01:38:56PM +0100, Nuno Sá via B4 Relay wrote: > > From: Nuno Sá <nuno.sa@analog.com> > > > > The adp5585 is a Multi Function Device that can also be a gpio > > controller and as it turns out, when OSC_EN is not set, we can't > > reliably read the gpio value when it's configured as input. Hence, > > OSC_EN will be set during probe by the parent device (and cleared on > > unbind). > > > > Moreover, we'll add support for the keymap matrix (input device) which > > definitely needs OSC_EN to be set and so, we cannot afford that disabling > > the PWM output also breaks the keymap events generation. > > I think you can squash this with 03/22 if you send a new version. Moving > the OSC_EN bit handling from the PWM child driver to the MFD driver is a > single logical change. Will do... happy to reduce the number of patches in the series > > > Signed-off-by: Nuno Sá <nuno.sa@analog.com> > > --- > > drivers/pwm/pwm-adp5585.c | 5 ----- > > 1 file changed, 5 deletions(-) > > > > diff --git a/drivers/pwm/pwm-adp5585.c b/drivers/pwm/pwm-adp5585.c > > index > > 40472ac5db6410a33e4f790fe8e6c23b517502be..c8821035b7c1412a55a642e6e8a46b66e6 > > 93a5af 100644 > > --- a/drivers/pwm/pwm-adp5585.c > > +++ b/drivers/pwm/pwm-adp5585.c > > @@ -62,7 +62,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, > > int ret; > > > > if (!state->enabled) { > > - regmap_clear_bits(regmap, ADP5585_GENERAL_CFG, > > ADP5585_OSC_EN); > > regmap_clear_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); > > return 0; > > } > > @@ -100,10 +99,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, > > if (ret) > > return ret; > > > > - ret = regmap_set_bits(regmap, ADP5585_GENERAL_CFG, ADP5585_OSC_EN); > > - if (ret) > > - return ret; > > - > > return regmap_set_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); > > } > >
diff --git a/drivers/pwm/pwm-adp5585.c b/drivers/pwm/pwm-adp5585.c index 40472ac5db6410a33e4f790fe8e6c23b517502be..c8821035b7c1412a55a642e6e8a46b66e693a5af 100644 --- a/drivers/pwm/pwm-adp5585.c +++ b/drivers/pwm/pwm-adp5585.c @@ -62,7 +62,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, int ret; if (!state->enabled) { - regmap_clear_bits(regmap, ADP5585_GENERAL_CFG, ADP5585_OSC_EN); regmap_clear_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); return 0; } @@ -100,10 +99,6 @@ static int pwm_adp5585_apply(struct pwm_chip *chip, if (ret) return ret; - ret = regmap_set_bits(regmap, ADP5585_GENERAL_CFG, ADP5585_OSC_EN); - if (ret) - return ret; - return regmap_set_bits(regmap, ADP5585_PWM_CFG, ADP5585_PWM_EN); }