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

Reply via email to