[PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[Andy Shevchenko]

Replace printk(KERN_WARNING) by pr_warn().

While at it, use %pa for the resource_size_t variables.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 kernel/resource.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 4c5e80b92f2f..ab32b015bd50 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -888,7 +888,7 @@ void insert_resource_expand_to_fit(struct resource *root, struct resource *new)
 		if (conflict->end > new->end)
 			new->end = conflict->end;
 
-		printk("Expanded resource %s due to conflict with %s\n", new->name, conflict->name);
+		pr_info("Expanded resource %s due to conflict with %s\n", new->name, conflict->name);
 	}
 	write_unlock(&resource_lock);
 }
@@ -1283,9 +1283,7 @@ void __release_region(struct resource *parent, resource_size_t start,
 
 	write_unlock(&resource_lock);
 
-	printk(KERN_WARNING "Trying to free nonexistent resource "
-		"<%016llx-%016llx>\n", (unsigned long long)start,
-		(unsigned long long)end);
+	pr_warn("Trying to free nonexistent resource <%pa-%pa>\n", &start, &end);
 }
 EXPORT_SYMBOL(__release_region);
 
@@ -1658,6 +1656,7 @@ __setup("reserve=", reserve_setup);
 int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
 {
 	struct resource *p = &iomem_resource;
+	resource_size_t end = addr + size - 1;
 	int err = 0;
 	loff_t l;
 
@@ -1667,12 +1666,12 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
 		 * We can probably skip the resources without
 		 * IORESOURCE_IO attribute?
 		 */
-		if (p->start >= addr + size)
+		if (p->start > end)
 			continue;
 		if (p->end < addr)
 			continue;
 		if (PFN_DOWN(p->start) <= PFN_DOWN(addr) &&
-		    PFN_DOWN(p->end) >= PFN_DOWN(addr + size - 1))
+		    PFN_DOWN(p->end) >= PFN_DOWN(end))
 			continue;
 		/*
 		 * if a resource is "BUSY", it's not a hardware resource
@@ -1683,10 +1682,8 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
 		if (p->flags & IORESOURCE_BUSY)
 			continue;
 
-		printk(KERN_WARNING "resource sanity check: requesting [mem %#010llx-%#010llx], which spans more than %s %pR\n",
-		       (unsigned long long)addr,
-		       (unsigned long long)(addr + size - 1),
-		       p->name, p);
+		pr_warn("resource sanity check: requesting [mem %pa-%pa], which spans more than %s %pR\n",
+			&addr, &end, p->name, p);
 		err = -1;
 		break;
 	}
-- 
2.35.1

[PATCH v1 2/2] resource: Convert DEFINE_RES_NAMED() to be compound literal

[Andy Shevchenko]

Currently DEFINE_RES_NAMED() can only be used to fill the static data.
In some cases it would be convenient to use it as right value in the
assignment operation. But it can't be done as is, because compiler has
no clue about the data layout. Converting it to be a compound literal
allows the above mentioned usage.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/ioport.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 27642ca15d93..67d3fb2133b6 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -155,7 +155,7 @@ enum {
 
 /* helpers to define resources */
 #define DEFINE_RES_NAMED(_start, _size, _name, _flags)			\
-	{								\
+(struct resource) {							\
 		.start = (_start),					\
 		.end = (_start) + (_size) - 1,				\
 		.name = (_name),					\
-- 
2.35.1

Re: [PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[Rafael J. Wysocki]

On Wed, Nov 9, 2022 at 11:51 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Replace printk(KERN_WARNING) by pr_warn().
>
> While at it, use %pa for the resource_size_t variables.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

This is Greg's stuff I think.

> ---
>  kernel/resource.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 4c5e80b92f2f..ab32b015bd50 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -888,7 +888,7 @@ void insert_resource_expand_to_fit(struct resource *root, struct resource *new)
>                 if (conflict->end > new->end)
>                         new->end = conflict->end;
>
> -               printk("Expanded resource %s due to conflict with %s\n", new->name, conflict->name);
> +               pr_info("Expanded resource %s due to conflict with %s\n", new->name, conflict->name);
>         }
>         write_unlock(&resource_lock);
>  }
> @@ -1283,9 +1283,7 @@ void __release_region(struct resource *parent, resource_size_t start,
>
>         write_unlock(&resource_lock);
>
> -       printk(KERN_WARNING "Trying to free nonexistent resource "
> -               "<%016llx-%016llx>\n", (unsigned long long)start,
> -               (unsigned long long)end);
> +       pr_warn("Trying to free nonexistent resource <%pa-%pa>\n", &start, &end);
>  }
>  EXPORT_SYMBOL(__release_region);
>
> @@ -1658,6 +1656,7 @@ __setup("reserve=", reserve_setup);
>  int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
>  {
>         struct resource *p = &iomem_resource;
> +       resource_size_t end = addr + size - 1;

And this change could be mentioned in the changelog too.

>         int err = 0;
>         loff_t l;
>
> @@ -1667,12 +1666,12 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
>                  * We can probably skip the resources without
>                  * IORESOURCE_IO attribute?
>                  */
> -               if (p->start >= addr + size)
> +               if (p->start > end)
>                         continue;
>                 if (p->end < addr)
>                         continue;
>                 if (PFN_DOWN(p->start) <= PFN_DOWN(addr) &&
> -                   PFN_DOWN(p->end) >= PFN_DOWN(addr + size - 1))
> +                   PFN_DOWN(p->end) >= PFN_DOWN(end))
>                         continue;
>                 /*
>                  * if a resource is "BUSY", it's not a hardware resource
> @@ -1683,10 +1682,8 @@ int iomem_map_sanity_check(resource_size_t addr, unsigned long size)
>                 if (p->flags & IORESOURCE_BUSY)
>                         continue;
>
> -               printk(KERN_WARNING "resource sanity check: requesting [mem %#010llx-%#010llx], which spans more than %s %pR\n",
> -                      (unsigned long long)addr,
> -                      (unsigned long long)(addr + size - 1),
> -                      p->name, p);
> +               pr_warn("resource sanity check: requesting [mem %pa-%pa], which spans more than %s %pR\n",
> +                       &addr, &end, p->name, p);
>                 err = -1;
>                 break;
>         }
> --
> 2.35.1
>

Re: [PATCH v1 2/2] resource: Convert DEFINE_RES_NAMED() to be compound literal

[Rafael J. Wysocki]

On Wed, Nov 9, 2022 at 11:50 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> Currently DEFINE_RES_NAMED() can only be used to fill the static data.
> In some cases it would be convenient to use it as right value in the
> assignment operation. But it can't be done as is, because compiler has
> no clue about the data layout. Converting it to be a compound literal
> allows the above mentioned usage.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>

> ---
>  include/linux/ioport.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index 27642ca15d93..67d3fb2133b6 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -155,7 +155,7 @@ enum {
>
>  /* helpers to define resources */
>  #define DEFINE_RES_NAMED(_start, _size, _name, _flags)                 \
> -       {                                                               \
> +(struct resource) {                                                    \
>                 .start = (_start),                                      \
>                 .end = (_start) + (_size) - 1,                          \
>                 .name = (_name),                                        \
> --
> 2.35.1
>

Re: [PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[Andy Shevchenko]

On Wed, Nov 09, 2022 at 12:12:03PM +0100, Rafael J. Wysocki wrote:
> On Wed, Nov 9, 2022 at 11:51 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> >
> > Replace printk(KERN_WARNING) by pr_warn().
> >
> > While at it, use %pa for the resource_size_t variables.
> >
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
> This is Greg's stuff I think.

OK.

...

> > +       resource_size_t end = addr + size - 1;
> 
> And this change could be mentioned in the changelog too.

And it's (indirectly). This is to have a variable to be consistent in the
print. But I can add more explicit mention.

Thank you for a review!

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[Greg Kroah-Hartman]

On Wed, Nov 09, 2022 at 01:26:47PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 09, 2022 at 12:12:03PM +0100, Rafael J. Wysocki wrote:
> > On Wed, Nov 9, 2022 at 11:51 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > >
> > > Replace printk(KERN_WARNING) by pr_warn().
> > >
> > > While at it, use %pa for the resource_size_t variables.
> > >
> > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > 
> > This is Greg's stuff I think.
> 
> OK.
> 
> ...
> 
> > > +       resource_size_t end = addr + size - 1;
> > 
> > And this change could be mentioned in the changelog too.
> 
> And it's (indirectly). This is to have a variable to be consistent in the
> print. But I can add more explicit mention.

Please do.

Re: [PATCH v1 1/2] resource: Replace printk(KERN_WARNING) by pr_warn()

[Andy Shevchenko]

On Wed, Nov 09, 2022 at 02:37:01PM +0100, Greg Kroah-Hartman wrote:
> On Wed, Nov 09, 2022 at 01:26:47PM +0200, Andy Shevchenko wrote:
> > On Wed, Nov 09, 2022 at 12:12:03PM +0100, Rafael J. Wysocki wrote:
> > > On Wed, Nov 9, 2022 at 11:51 AM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:

...

> > > > +       resource_size_t end = addr + size - 1;
> > > 
> > > And this change could be mentioned in the changelog too.
> > 
> > And it's (indirectly). This is to have a variable to be consistent in the
> > print. But I can add more explicit mention.
> 
> Please do.

v2 has been sent, thank you for review!

-- 
With Best Regards,
Andy Shevchenko