[PATCH] regmap: add WARN_ONCE when invalid mask is provided to regmap_field_init()

[Simon Glass]

Hi Matt,

On Thu, 8 Sept 2022 at 19:39, Matt Ranostay <mranostay at ti.com> wrote:
>
> In regmap_field_init() when a invalid mask is provided it still
> initializes without any warnings of it being incorrect.
>
> An example of this is when the LSB is greater than MSB a mask of zero
> is produced.
>
> WARN_ONCE() is not ideal for this but requires less changes to core regmap
> code.
>
> Based on: https://lore.kernel.org/all/20220708013125.313892-1-mranostay@ti.com/
>
> Signed-off-by: Matt Ranostay <mranostay at ti.com>
> ---
>  drivers/core/regmap.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> index 5f98f85cfce..3100a58af2c 100644
> --- a/drivers/core/regmap.c
> +++ b/drivers/core/regmap.c
> @@ -20,6 +20,7 @@
>  #include <linux/compat.h>
>  #include <linux/err.h>
>  #include <linux/bitops.h>
> +#include <linux/bug.h>
>
>  /*
>   * Internal representation of a regmap field. Instead of storing the MSB and
> @@ -649,6 +650,8 @@ static void regmap_field_init(struct regmap_field *rm_field,
>         rm_field->reg = reg_field.reg;
>         rm_field->shift = reg_field.lsb;
>         rm_field->mask = GENMASK(reg_field.msb, reg_field.lsb);
> +
> +       WARN_ONCE(rm_field->mask == 0, "invalid empty mask defined\n");
>  }
>
>  struct regmap_field *devm_regmap_field_alloc(struct udevice *dev,
> --
> 2.37.2
>

I think it would be better to return an error and check it in the
caller, with a log_debug() here. We don't want to increase code size
for a programming error, particularly in SPL.

Regards,
Simon