On Fri, Nov 20, 2020 at 12:14:55PM +0000, Adam Ward wrote:

> @@ -1,7 +1,21 @@
>  // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (C) 2020 Axis Communications AB */
> +/* Copyright (C) 2020 Axis Communications AB
> + *
> + * DA9121 Single-channel dual-phase 10A buck converter

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

> +     node = of_get_child_by_name(chip->dev->of_node, "regulators");
> +     if (!node) {
> +             dev_err(chip->dev, "Regulators node not found\n");
> +             return ERR_PTR(-ENODEV);
> +     }
> +
> +     num = of_regulator_match(chip->dev, node, da9121_matches,
> +                              ARRAY_SIZE(da9121_matches));

Use of_parse_cb().

> +     /* interrupt assumptions require at least one buck to be configured */
> +     if (num == 0) {
> +             dev_err(chip->dev, "Did not match any regulators in the DT\n");
> +             return ERR_PTR(-EINVAL);
> +     }

The physical presence of the regulator is not going to change based on
the DT.

> +             if (variant_parameters[chip->variant_id].num_bucks == 2) {
> +                     uint32_t ripple_cancel;
> +                     uint32_t reg = (i ? DA9xxx_REG_BUCK_BUCK2_7
> +                                       : DA9121_REG_BUCK_BUCK1_7);

Please write normal conditional statements to improve legibility.

Attachment: signature.asc
Description: PGP signature

Reply via email to