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.