[PATCH] regmap: add WARN_ONCE when invalid mask is provided to regmap_field_init()
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@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,
Hi Matt,
On Thu, 8 Sept 2022 at 19:39, Matt Ranostay mranostay@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@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
participants (2)
-
Matt Ranostay -
Simon Glass