Re: [PATCH v2 02/17] mfd: adp5585: enable oscilator during probe

["Nuno Sá" <noname.nuno@gmail.com>]

On Mon, 2025-04-21 at 11:57 +0300, Laurent Pinchart wrote:
> Hi Nuno,
> 
> Thank you for the patch.
> 
> On Tue, Apr 15, 2025 at 03:49:18PM +0100, Nuno Sá via B4 Relay wrote:
> > From: Nuno Sá <nuno.sa@analog.com>
> > 
> > Make sure to enable the oscillator in the top device. This will allow to
> > not control this in the child PWM device as that would not work with
> > future support for keyboard matrix where the oscillator needs to be
> > always enabled (and so cannot be disabled by disabling PWM).
> 
> Setting this bit unconditionally increases power consumption. It should
> only be set when needed.

Well, not sure if the effort for that pays off... The only usecase were it would
make sense to do that would be for PWM. For the other devices (and assuming I'm
right with the GPI case) we need this always set.

- Nuno Sá

> 
> > Signed-off-by: Nuno Sá <nuno.sa@analog.com>
> > ---
> >  drivers/mfd/adp5585.c | 16 ++++++++++++++++
> >  1 file changed, 16 insertions(+)
> > 
> > diff --git a/drivers/mfd/adp5585.c b/drivers/mfd/adp5585.c
> > index
> > 160e0b38106a6d78f7d4b7c866cb603d96ea673e..f17b5f2474cac6a403556694066f438288
> > 264a49 100644
> > --- a/drivers/mfd/adp5585.c
> > +++ b/drivers/mfd/adp5585.c
> > @@ -110,6 +110,13 @@ static const struct regmap_config
> > adp5585_regmap_configs[] = {
> >  	},
> >  };
> >  
> > +static void adp5585_osc_disable(void *data)
> > +{
> > +	const struct adp5585_dev *adp5585 = data;
> > +
> > +	regmap_write(adp5585->regmap, ADP5585_GENERAL_CFG, 0);
> > +}
> > +
> >  static int adp5585_i2c_probe(struct i2c_client *i2c)
> >  {
> >  	const struct regmap_config *regmap_config;
> > @@ -138,6 +145,15 @@ static int adp5585_i2c_probe(struct i2c_client *i2c)
> >  		return dev_err_probe(&i2c->dev, -ENODEV,
> >  				     "Invalid device ID 0x%02x\n", id);
> >  
> > +	ret = regmap_set_bits(adp5585->regmap, ADP5585_GENERAL_CFG,
> > +			      ADP5585_OSC_EN);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = devm_add_action_or_reset(&i2c->dev, adp5585_osc_disable,
> > adp5585);
> > +	if (ret)
> > +		return ret;
> > +
> >  	ret = devm_mfd_add_devices(&i2c->dev, PLATFORM_DEVID_AUTO,
> >  				   adp5585_devs, ARRAY_SIZE(adp5585_devs),
> >  				   NULL, 0, NULL);