diff mbox series

dm: regmap: Disable range checks in SPL

Message ID 20221015143919.771214-1-sjg@chromium.org
State Accepted
Delegated to: Simon Glass
Headers show
Series dm: regmap: Disable range checks in SPL | expand

Commit Message

Simon Glass Oct. 15, 2022, 2:39 p.m. UTC
A recent change to regmap breaks building of phycore-rk3288 for me. The
difference is only a few bytes. Somehow CI seems to pass, even though it
fails when I run docker locally. But it prevents me from sending any more
pull requests.

In any case this board is clearly near the limit. We could revert the
offending change, but it is needed for sandbox tests.

Instead, add a way to drop the range checks in SPL, since they end up
doing nothing if everything is working as expected.

This makes phycore-rk3288 build again for me and reduces the size of SPL
slightly for a number of boards.

Signed-off-by: Simon Glass <sjg@chromium.org>
Fixes: 947d4f132b4 ("regmap: fix range checks")
---

 drivers/core/regmap.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

Comments

Simon Glass Oct. 15, 2022, 6:40 p.m. UTC | #1
Hi,

On Sat, 15 Oct 2022 at 08:39, Simon Glass <sjg@chromium.org> wrote:
>
> A recent change to regmap breaks building of phycore-rk3288 for me. The
> difference is only a few bytes. Somehow CI seems to pass, even though it
> fails when I run docker locally. But it prevents me from sending any more
> pull requests.
>
> In any case this board is clearly near the limit. We could revert the
> offending change, but it is needed for sandbox tests.
>
> Instead, add a way to drop the range checks in SPL, since they end up
> doing nothing if everything is working as expected.
>
> This makes phycore-rk3288 build again for me and reduces the size of SPL
> slightly for a number of boards.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Fixes: 947d4f132b4 ("regmap: fix range checks")
> ---
>
>  drivers/core/regmap.c | 22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> index 5ccbf9abb8a..ab02ddba443 100644
> --- a/drivers/core/regmap.c
> +++ b/drivers/core/regmap.c
> @@ -4,6 +4,8 @@
>   * Written by Simon Glass <sjg@chromium.org>
>   */
>
> +#define LOG_CATEGORY   LOGC_DM
> +
>  #include <common.h>
>  #include <dm.h>
>  #include <errno.h>
> @@ -36,6 +38,21 @@ struct regmap_field {
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +/**
> + * do_range_check() - Control whether range checks are done
> + *
> + * Returns: true to do range checks, false to skip
> + *
> + * This is used to reduce code size on SPL where range checks are known not to
> + * be needed
> + *
> + * Add this to the top of the file to enable them: #define LOG_DEBUG
> + */
> +static inline bool do_range_check(void)
> +{
> +       return _LOG_DEBUG || !IS_ENABLED(CONFIG_SPL);
> +}
> +
>  /**
>   * regmap_alloc() - Allocate a regmap with a given number of ranges.
>   *
> @@ -391,7 +408,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>         struct regmap_range *range;
>         void *ptr;
>
> -       if (range_num >= map->range_count) {
> +       if (do_range_check() && range_num >= map->range_count) {
>                 debug("%s: range index %d larger than range count\n",
>                       __func__, range_num);
>                 return -ERANGE;
> @@ -399,7 +416,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
>         range = &map->ranges[range_num];
>
>         offset <<= map->reg_offset_shift;
> -       if (offset + val_len > range->size || offset + val_len < offset) {
> +       if (do_range_check() &&
> +           (offset + val_len > range->size || offset + val_len < offset)) {
>                 debug("%s: offset/size combination invalid\n", __func__);
>                 return -ERANGE;
>         }
> --
> 2.38.0.413.g74048e4d9e-goog
>

If no objections, I'll pull this in right away (e.g. Monday) as I am
blocked from doing a pull request due to phycore-rk3288.

Regards,
Simon
Simon Glass Oct. 18, 2022, 11:29 a.m. UTC | #2
On Sat, 15 Oct 2022 at 12:40, Simon Glass <sjg@chromium.org> wrote:
>
> Hi,
>
> On Sat, 15 Oct 2022 at 08:39, Simon Glass <sjg@chromium.org> wrote:
> >
> > A recent change to regmap breaks building of phycore-rk3288 for me. The
> > difference is only a few bytes. Somehow CI seems to pass, even though it
> > fails when I run docker locally. But it prevents me from sending any more
> > pull requests.
> >
> > In any case this board is clearly near the limit. We could revert the
> > offending change, but it is needed for sandbox tests.
> >
> > Instead, add a way to drop the range checks in SPL, since they end up
> > doing nothing if everything is working as expected.
> >
> > This makes phycore-rk3288 build again for me and reduces the size of SPL
> > slightly for a number of boards.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > Fixes: 947d4f132b4 ("regmap: fix range checks")
> > ---
> >
> >  drivers/core/regmap.c | 22 ++++++++++++++++++++--
> >  1 file changed, 20 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
> > index 5ccbf9abb8a..ab02ddba443 100644
> > --- a/drivers/core/regmap.c
> > +++ b/drivers/core/regmap.c
> > @@ -4,6 +4,8 @@
> >   * Written by Simon Glass <sjg@chromium.org>
> >   */
> >
> > +#define LOG_CATEGORY   LOGC_DM
> > +
> >  #include <common.h>
> >  #include <dm.h>
> >  #include <errno.h>
> > @@ -36,6 +38,21 @@ struct regmap_field {
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> > +/**
> > + * do_range_check() - Control whether range checks are done
> > + *
> > + * Returns: true to do range checks, false to skip
> > + *
> > + * This is used to reduce code size on SPL where range checks are known not to
> > + * be needed
> > + *
> > + * Add this to the top of the file to enable them: #define LOG_DEBUG
> > + */
> > +static inline bool do_range_check(void)
> > +{
> > +       return _LOG_DEBUG || !IS_ENABLED(CONFIG_SPL);
> > +}
> > +
> >  /**
> >   * regmap_alloc() - Allocate a regmap with a given number of ranges.
> >   *
> > @@ -391,7 +408,7 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> >         struct regmap_range *range;
> >         void *ptr;
> >
> > -       if (range_num >= map->range_count) {
> > +       if (do_range_check() && range_num >= map->range_count) {
> >                 debug("%s: range index %d larger than range count\n",
> >                       __func__, range_num);
> >                 return -ERANGE;
> > @@ -399,7 +416,8 @@ int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
> >         range = &map->ranges[range_num];
> >
> >         offset <<= map->reg_offset_shift;
> > -       if (offset + val_len > range->size || offset + val_len < offset) {
> > +       if (do_range_check() &&
> > +           (offset + val_len > range->size || offset + val_len < offset)) {
> >                 debug("%s: offset/size combination invalid\n", __func__);
> >                 return -ERANGE;
> >         }
> > --
> > 2.38.0.413.g74048e4d9e-goog
> >
>
> If no objections, I'll pull this in right away (e.g. Monday) as I am
> blocked from doing a pull request due to phycore-rk3288.

Applied to u-boot-dm.
diff mbox series

Patch

diff --git a/drivers/core/regmap.c b/drivers/core/regmap.c
index 5ccbf9abb8a..ab02ddba443 100644
--- a/drivers/core/regmap.c
+++ b/drivers/core/regmap.c
@@ -4,6 +4,8 @@ 
  * Written by Simon Glass <sjg@chromium.org>
  */
 
+#define LOG_CATEGORY	LOGC_DM
+
 #include <common.h>
 #include <dm.h>
 #include <errno.h>
@@ -36,6 +38,21 @@  struct regmap_field {
 
 DECLARE_GLOBAL_DATA_PTR;
 
+/**
+ * do_range_check() - Control whether range checks are done
+ *
+ * Returns: true to do range checks, false to skip
+ *
+ * This is used to reduce code size on SPL where range checks are known not to
+ * be needed
+ *
+ * Add this to the top of the file to enable them: #define LOG_DEBUG
+ */
+static inline bool do_range_check(void)
+{
+	return _LOG_DEBUG || !IS_ENABLED(CONFIG_SPL);
+}
+
 /**
  * regmap_alloc() - Allocate a regmap with a given number of ranges.
  *
@@ -391,7 +408,7 @@  int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
 	struct regmap_range *range;
 	void *ptr;
 
-	if (range_num >= map->range_count) {
+	if (do_range_check() && range_num >= map->range_count) {
 		debug("%s: range index %d larger than range count\n",
 		      __func__, range_num);
 		return -ERANGE;
@@ -399,7 +416,8 @@  int regmap_raw_read_range(struct regmap *map, uint range_num, uint offset,
 	range = &map->ranges[range_num];
 
 	offset <<= map->reg_offset_shift;
-	if (offset + val_len > range->size || offset + val_len < offset) {
+	if (do_range_check() &&
+	    (offset + val_len > range->size || offset + val_len < offset)) {
 		debug("%s: offset/size combination invalid\n", __func__);
 		return -ERANGE;
 	}