On Fri, Nov 08, 2024 at 05:35:43PM +0800, Ramon Cristopher M. Calam wrote: > Add ADI LT8722 full bridge DC/DC converter driver. > +++ b/drivers/regulator/lt8722-regulator.c > @@ -0,0 +1,701 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Analog Devices LT8722 Ultracompact Full Bridge Driver with SPI driver > + * Please make the entire comment a C++ one so things look more intentional. > +static int lt8722_reg_read(struct spi_device *spi, u8 reg, u32 *val) > +{ > +static int lt8722_reg_write(struct spi_device *spi, u8 reg, u32 val) > +{ You can use these as reg_read() and reg_write() operations in regmap which will allow the driver to use all the standard helpers and vastly reduce the size of the driver. > +static int lt8722_parse_fw(struct lt8722_chip_info *chip, > + struct regulator_init_data *init_data) > +{ > + int ret; > + > + /* Override the min_uV constraint with the minimum output voltage */ > + init_data->constraints.min_uV = LT8722_MIN_VOUT; Any modification of the constraints by the driver is a bug. Adjust the information the driver provides about the voltages it supports if you need to do this. > +static int lt8722_is_enabled(struct regulator_dev *rdev) > +{ > + struct lt8722_chip_info *chip = rdev_get_drvdata(rdev); > + int ret; > + u32 reg_val; > + bool en_req, en_pin; > + > + ret = lt8722_reg_read(chip->spi, LT8722_SPIS_COMMAND, ®_val); > + if (ret < 0) > + return ret; > + > + en_req = FIELD_GET(LT8722_EN_REQ_MASK, reg_val); > + en_pin = gpiod_get_value(chip->en_gpio); > + > + return en_req && en_pin; > +} Always adjusting both the GPIO and register all the time like this is pointless, it turns the GPIO into just pure overhead. Just use the standard support for setting enables via registrers and GPIOs. When there's a GPIO leave the register permanently enabld. > + chip->en_gpio = devm_gpiod_get(&spi->dev, "enable", GPIOD_OUT_LOW); > + if (IS_ERR(chip->en_gpio)) > + return PTR_ERR(chip->en_gpio); Presumably this is optional, it could just be tied off.
Attachment:
signature.asc
Description: PGP signature