Re: [PATCH 2/2] regulator: adp5055: Add driver for adp5055

From: Mark Brown
Date: Wed Feb 26 2025 - 06:35:37 EST


On Wed, Feb 26, 2025 at 02:24:58AM +0000, Torreno, Alexis Czezar wrote:

> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Regulator driver for Analog Devices ADP5055
> > > + *
> > > + * Copyright (C) 2025 Analog Devices, Inc.
> > > + */

> > Please make the entire comment block a C++ one so things look more
> > intentional.

> Am not familiar with this, is this where each line use // rather than /**/?

Yes.

> > > +static int adp5055_en_func(struct regulator_dev *dev, int en_val) {
> > > + struct adp5055 *adp5055 = rdev_get_drvdata(dev);

> > Just use the standard GPIO and regmap helpers for this.

> Confused on this, I thought these were standard 'regmap_update_bits' and
> 'gpiod_set_value_cansleep'

You've open coded the operations instead of using the framework helpers,
you shouldn't need to anything other than supply data here.

Attachment: signature.asc
Description: PGP signature