On Tue, Jun 18, 2024 at 04:59:52PM +0300, Dzmitry Sankouski wrote:
> index 000000000000..3c58a1bd2262
> --- /dev/null
> +++ b/drivers/regulator/s2dos05-regulator.c
> @@ -0,0 +1,362 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * s2dos05.c - Regulator driver for the Samsung s2dos05
> + *
Please make the entire comment a C++ one so things look more
intentional.
> +static int s2m_enable(struct regulator_dev *rdev)
> +{
> + struct s2dos05_data *info = rdev_get_drvdata(rdev);
> + struct regmap *regmap = info->regmap;
> +
> + return regmap_update_bits(regmap, rdev->desc->enable_reg,
> + rdev->desc->enable_mask,
> + rdev->desc->enable_mask);
> +}
Please use the generic regmap helpers rather than open coding them.
> + reg_np = of_get_child_by_name(dev->parent->of_node, "regulators");
> + if (!reg_np) {
> + dev_err(dev, "could not find regulators sub-node\n");
> + return -EINVAL;
> + }
> +
> + err = of_regulator_match(dev, reg_np, rdata, rdev_num);
> + of_node_put(reg_np);
Use of_match for this rather than open coding.
> + s2dos05 = devm_kzalloc(dev, sizeof(struct s2dos05_data),
> + GFP_KERNEL);
> + rdata = kcalloc(rdev_num, sizeof(*rdata), GFP_KERNEL);
> + if (!rdata)
> + return -ENOMEM;
Mixing devm_ and regular allocations seems likely to go wrong, please be
consistent.
Attachment:
signature.asc
Description: PGP signature
