* [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
@ 2025-04-16 10:13 Ilpo Järvinen
2025-04-16 10:22 ` Andy Shevchenko
2025-04-16 15:05 ` Rafael J. Wysocki
0 siblings, 2 replies; 11+ messages in thread
From: Ilpo Järvinen @ 2025-04-16 10:13 UTC (permalink / raw)
To: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
Andy Shevchenko, linux-acpi, linux-kernel, linux-pci
Cc: Ilpo Järvinen
Convert open coded resource size calculations to use
resource_set_{range,size}() helpers.
While at it, use SZ_* for size parameter which makes the intent of code
more obvious.
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
arch/x86/kernel/acpi/boot.c | 4 ++--
arch/x86/kernel/amd_nb.c | 3 +--
arch/x86/kernel/apic/apic.c | 3 +--
arch/x86/kernel/apic/io_apic.c | 3 +--
arch/x86/kernel/probe_roms.c | 3 +--
arch/x86/pci/fixup.c | 4 ++--
arch/x86/pci/intel_mid_pci.c | 2 +-
7 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index dae6a73be40e..4490cbc01030 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -21,6 +21,7 @@
#include <linux/pci.h>
#include <linux/efi-bgrt.h>
#include <linux/serial_core.h>
+#include <linux/sizes.h>
#include <linux/pgtable.h>
#include <asm/e820/api.h>
@@ -940,8 +941,7 @@ static int __init acpi_parse_hpet(struct acpi_table_header *table)
snprintf((char *)hpet_res->name, HPET_RESOURCE_NAME_SIZE, "HPET %u",
hpet_tbl->sequence);
- hpet_res->start = hpet_address;
- hpet_res->end = hpet_address + (1 * 1024) - 1;
+ resource_set_range(hpet_res, hpet_address, SZ_1K);
return 0;
}
diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
index 6d12a9b69432..cba336dcb40d 100644
--- a/arch/x86/kernel/amd_nb.c
+++ b/arch/x86/kernel/amd_nb.c
@@ -164,8 +164,7 @@ struct resource *amd_get_mmconfig_range(struct resource *res)
FAM10H_MMIO_CONF_BUSRANGE_MASK;
res->flags = IORESOURCE_MEM;
- res->start = base;
- res->end = base + (1ULL<<(segn_busn_bits + 20)) - 1;
+ resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
return res;
}
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 62584a347931..efd3304ecbb3 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -2640,8 +2640,7 @@ static int __init lapic_insert_resource(void)
return -1;
/* Put local APIC into the resource map. */
- lapic_resource.start = apic_mmio_base;
- lapic_resource.end = lapic_resource.start + PAGE_SIZE - 1;
+ resource_set_range(&lapic_resource, apic_mmio_base, PAGE_SIZE);
insert_resource(&iomem_resource, &lapic_resource);
return 0;
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index eebc360ed1bb..3069885d6421 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2571,8 +2571,7 @@ void __init io_apic_init_mappings(void)
__fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK), ioapic_phys);
idx++;
- ioapic_res->start = ioapic_phys;
- ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
+ resource_set_range(ioapic_res, ioapic_phys, IO_APIC_SLOT_SIZE);
ioapic_res++;
}
}
diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
index cc2c34ba7228..44da85e50c44 100644
--- a/arch/x86/kernel/probe_roms.c
+++ b/arch/x86/kernel/probe_roms.c
@@ -260,8 +260,7 @@ void __init probe_roms(void)
if (!length || start + length > upper || !romchecksum(rom, length))
continue;
- adapter_rom_resources[i].start = start;
- adapter_rom_resources[i].end = start + length - 1;
+ resource_set_range(&adapter_rom_resources[i], start, length);
request_resource(&iomem_resource, &adapter_rom_resources[i]);
start = adapter_rom_resources[i++].end & ~2047UL;
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index efefeb82ab61..94e98e3bf041 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -7,6 +7,7 @@
#include <linux/delay.h>
#include <linux/dmi.h>
#include <linux/pci.h>
+#include <linux/sizes.h>
#include <linux/suspend.h>
#include <linux/vgaarb.h>
#include <asm/amd_node.h>
@@ -347,8 +348,7 @@ static void pci_fixup_video(struct pci_dev *pdev)
if (res->parent)
release_resource(res);
- res->start = 0xC0000;
- res->end = res->start + 0x20000 - 1;
+ resource_set_range(res, 0xC0000, SZ_128K);
res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
IORESOURCE_PCI_FIXED;
dev_info(&pdev->dev, "Video device with shadowed ROM at %pR\n",
diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
index b433b1753016..5e047e802d5d 100644
--- a/arch/x86/pci/intel_mid_pci.c
+++ b/arch/x86/pci/intel_mid_pci.c
@@ -399,7 +399,7 @@ static void pci_fixed_bar_fixup(struct pci_dev *dev)
for (i = 0; i < PCI_STD_NUM_BARS; i++) {
pci_read_config_dword(dev, offset + 8 + (i * 4), &size);
- dev->resource[i].end = dev->resource[i].start + size - 1;
+ resource_set_size(&dev->resource[i], size);
dev->resource[i].flags |= IORESOURCE_PCI_FIXED;
}
}
--
2.39.5
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 10:13 [PATCH 1/1] x86: Use resource_set_{range,size}() helpers Ilpo Järvinen
@ 2025-04-16 10:22 ` Andy Shevchenko
2025-04-16 10:23 ` Andy Shevchenko
2025-04-16 11:53 ` Ilpo Järvinen
2025-04-16 15:05 ` Rafael J. Wysocki
1 sibling, 2 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-04-16 10:22 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, linux-kernel, linux-pci
On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> Convert open coded resource size calculations to use
> resource_set_{range,size}() helpers.
>
> While at it, use SZ_* for size parameter which makes the intent of code
> more obvious.
...
> + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
Then probably
resource_set_range(res, base, BIT_ULL(segn_busn_bits) * SZ_1M);
to follow the same "While at it"?
...
> + resource_set_range(res, 0xC0000, SZ_128K);
> res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> IORESOURCE_PCI_FIXED;
I'm wondering why not DEFINE_RES_MEM() in such cases?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 10:22 ` Andy Shevchenko
@ 2025-04-16 10:23 ` Andy Shevchenko
2025-04-16 11:53 ` Ilpo Järvinen
1 sibling, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-04-16 10:23 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, linux-kernel, linux-pci
On Wed, Apr 16, 2025 at 01:22:21PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
...
> > + resource_set_range(res, 0xC0000, SZ_128K);
> > res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> > IORESOURCE_PCI_FIXED;
>
> I'm wondering why not DEFINE_RES_MEM() in such cases?
For the reference:
1af56ff09e67 ("resource: replace open coded variants of DEFINE_RES_*_NAMED()")
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 10:22 ` Andy Shevchenko
2025-04-16 10:23 ` Andy Shevchenko
@ 2025-04-16 11:53 ` Ilpo Järvinen
2025-04-16 12:03 ` Andy Shevchenko
2025-04-18 7:40 ` Ingo Molnar
1 sibling, 2 replies; 11+ messages in thread
From: Ilpo Järvinen @ 2025-04-16 11:53 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, LKML, linux-pci
[-- Attachment #1: Type: text/plain, Size: 1529 bytes --]
On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> > Convert open coded resource size calculations to use
> > resource_set_{range,size}() helpers.
> >
> > While at it, use SZ_* for size parameter which makes the intent of code
> > more obvious.
>
> ...
>
> > + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
>
> Then probably
>
> resource_set_range(res, base, BIT_ULL(segn_busn_bits) * SZ_1M);
>
> to follow the same "While at it"?
I'll change that now since you brought it up. It did cross my mind to
convert that to * SZ_1M but it seemed to go farther than I wanted with a
simple conversion patch.
I've never liked the abuse of BIT*() for size related shifts though, I
recall I saw somewhere a helper that was better named for size related
operations but I just cannot recall its name and seem to not find that
anymore :-(. But until I come across it once again, I guess I'll have to
settle to BIT*().
> > + resource_set_range(res, 0xC0000, SZ_128K);
> > res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> > IORESOURCE_PCI_FIXED;
>
> I'm wondering why not DEFINE_RES_MEM() in such cases?
I guess you meant DEFINE_RES() as that seems to allow giving custom flags.
However, DEFINE_RES*() will overwrite ->name which seems something that
ought to not be done here.
I found one other case from the same file though which is truly defines
a resource from scratch.
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 11:53 ` Ilpo Järvinen
@ 2025-04-16 12:03 ` Andy Shevchenko
2025-04-16 12:18 ` Ilpo Järvinen
2025-04-18 7:40 ` Ingo Molnar
1 sibling, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2025-04-16 12:03 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, LKML, linux-pci
On Wed, Apr 16, 2025 at 02:53:51PM +0300, Ilpo Järvinen wrote:
> On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> > > + resource_set_range(res, 0xC0000, SZ_128K);
> > > res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> > > IORESOURCE_PCI_FIXED;
> >
> > I'm wondering why not DEFINE_RES_MEM() in such cases?
>
> I guess you meant DEFINE_RES() as that seems to allow giving custom flags.
> However, DEFINE_RES*() will overwrite ->name which seems something that
> ought to not be done here.
Okay, I haven't checked the initial state of name field here, so then
DEFINE_RES_MEM_NAMED()? Or don't we have one?
In any case I would rather see a one assignment for these cases than something
hidden behind proposed conversions.
> I found one other case from the same file though which is truly defines
> a resource from scratch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 12:03 ` Andy Shevchenko
@ 2025-04-16 12:18 ` Ilpo Järvinen
2025-04-16 15:25 ` Andy Shevchenko
0 siblings, 1 reply; 11+ messages in thread
From: Ilpo Järvinen @ 2025-04-16 12:18 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, LKML, linux-pci
[-- Attachment #1: Type: text/plain, Size: 1784 bytes --]
On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> On Wed, Apr 16, 2025 at 02:53:51PM +0300, Ilpo Järvinen wrote:
> > On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> > > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
>
> > > > + resource_set_range(res, 0xC0000, SZ_128K);
> > > > res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> > > > IORESOURCE_PCI_FIXED;
> > >
> > > I'm wondering why not DEFINE_RES_MEM() in such cases?
> >
> > I guess you meant DEFINE_RES() as that seems to allow giving custom flags.
> > However, DEFINE_RES*() will overwrite ->name which seems something that
> > ought to not be done here.
>
> Okay, I haven't checked the initial state of name field here, so then
> DEFINE_RES_MEM_NAMED()? Or don't we have one?
There's pre-existing res->name on it and your suggestion would have
resulted in overwriting it with NULL. res->name seems to be filled earlier
by PCI probe code.
> In any case I would rather see a one assignment for these cases than something
> hidden behind proposed conversions.
TBH, these DEFINE_RES*() helpers are doing hidden things such as
blantantly overwriting ->name which I only realized after I had already
converted to it as per your suggestion.
And yes, it is possible to pass the pre-existing res->name to
DEFINE_RES_NAMED() if that what you insist, though it seems doing it for
the sake of DEFINE_RES*() interface rather than this code wanting to
really define the resource from scratch.
Given the hidden overwriting, please be careful on suggesting
DEFINE_RES*() conversions as it's not as trivial as it seems.
> > I found one other case from the same file though which is truly defines
> > a resource from scratch.
>
>
>
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 12:18 ` Ilpo Järvinen
@ 2025-04-16 15:25 ` Andy Shevchenko
0 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2025-04-16 15:25 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
linux-acpi, LKML, linux-pci
On Wed, Apr 16, 2025 at 03:18:56PM +0300, Ilpo Järvinen wrote:
> On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> > On Wed, Apr 16, 2025 at 02:53:51PM +0300, Ilpo Järvinen wrote:
> > > On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> > > > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
...
> > > > > + resource_set_range(res, 0xC0000, SZ_128K);
> > > > > res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> > > > > IORESOURCE_PCI_FIXED;
> > > >
> > > > I'm wondering why not DEFINE_RES_MEM() in such cases?
> > >
> > > I guess you meant DEFINE_RES() as that seems to allow giving custom flags.
> > > However, DEFINE_RES*() will overwrite ->name which seems something that
> > > ought to not be done here.
> >
> > Okay, I haven't checked the initial state of name field here, so then
> > DEFINE_RES_MEM_NAMED()? Or don't we have one?
>
> There's pre-existing res->name on it and your suggestion would have
> resulted in overwriting it with NULL. res->name seems to be filled earlier
> by PCI probe code.
Okay, then the resource_*() may make more sense, indeed.
> > In any case I would rather see a one assignment for these cases than something
> > hidden behind proposed conversions.
>
> TBH, these DEFINE_RES*() helpers are doing hidden things such as
> blantantly overwriting ->name which I only realized after I had already
> converted to it as per your suggestion.
They are specifically named in capital letters, so main use is in static
assignments, but they are made compound literals, so it's possible to
initialise the on-stack or on-heap at run-time with this be kept in mind.
That's why there is nothing hidden, it implies that the struct is fully
assigned (with the provided fields and some defaults).
> And yes, it is possible to pass the pre-existing res->name to
> DEFINE_RES_NAMED() if that what you insist, though it seems doing it for
> the sake of DEFINE_RES*() interface rather than this code wanting to
> really define the resource from scratch.
>
> Given the hidden overwriting, please be careful on suggesting
> DEFINE_RES*() conversions as it's not as trivial as it seems.
Yeah, seems not everybody aware of the difference with
foo_init_something() vs. FOO_INIT_SOMETHING() :-)
> > > I found one other case from the same file though which is truly defines
> > > a resource from scratch.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 11:53 ` Ilpo Järvinen
2025-04-16 12:03 ` Andy Shevchenko
@ 2025-04-18 7:40 ` Ingo Molnar
2025-04-18 15:15 ` Ilpo Järvinen
1 sibling, 1 reply; 11+ messages in thread
From: Ingo Molnar @ 2025-04-18 7:40 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andy Shevchenko, Rafael J. Wysocki, Len Brown, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Bjorn Helgaas, linux-acpi, LKML, linux-pci
* Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> On Wed, 16 Apr 2025, Andy Shevchenko wrote:
>
> > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> > > Convert open coded resource size calculations to use
> > > resource_set_{range,size}() helpers.
> > >
> > > While at it, use SZ_* for size parameter which makes the intent of code
> > > more obvious.
> >
> > ...
> >
> > > + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
> >
> > Then probably
> >
> > resource_set_range(res, base, BIT_ULL(segn_busn_bits) * SZ_1M);
> >
> > to follow the same "While at it"?
>
> I'll change that now since you brought it up. It did cross my mind to
> convert that to * SZ_1M but it seemed to go farther than I wanted with a
> simple conversion patch.
>
> I've never liked the abuse of BIT*() for size related shifts though,
> I recall I saw somewhere a helper that was better named for size
> related operations but I just cannot recall its name and seem to not
> find that anymore :-(. But until I come across it once again, I guess
> I'll have to settle to BIT*().
BITS_TO_LONGS()?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-18 7:40 ` Ingo Molnar
@ 2025-04-18 15:15 ` Ilpo Järvinen
2025-04-19 14:57 ` Ingo Molnar
0 siblings, 1 reply; 11+ messages in thread
From: Ilpo Järvinen @ 2025-04-18 15:15 UTC (permalink / raw)
To: Ingo Molnar
Cc: Andy Shevchenko, Rafael J. Wysocki, Len Brown, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Bjorn Helgaas, linux-acpi, LKML, linux-pci
[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]
On Fri, 18 Apr 2025, Ingo Molnar wrote:
>
> * Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
>
> > On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> >
> > > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> > > > Convert open coded resource size calculations to use
> > > > resource_set_{range,size}() helpers.
> > > >
> > > > While at it, use SZ_* for size parameter which makes the intent of code
> > > > more obvious.
> > >
> > > ...
> > >
> > > > + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
> > >
> > > Then probably
> > >
> > > resource_set_range(res, base, BIT_ULL(segn_busn_bits) * SZ_1M);
> > >
> > > to follow the same "While at it"?
> >
> > I'll change that now since you brought it up. It did cross my mind to
> > convert that to * SZ_1M but it seemed to go farther than I wanted with a
> > simple conversion patch.
> >
> > I've never liked the abuse of BIT*() for size related shifts though,
> > I recall I saw somewhere a helper that was better named for size
> > related operations but I just cannot recall its name and seem to not
> > find that anymore :-(. But until I come across it once again, I guess
> > I'll have to settle to BIT*().
>
> BITS_TO_LONGS()?
Hi Ingo,
I'm not entiry sure if you're referring to my BIT*() matching unrelated
macros such as BITS_TO_LONGS() (I only meant BIT() and BIT_ULL() which I
thought was clear from the context), or that BITS_TO_LONGS() would be the
solution what I'm looking for.
In case you meant the latter, BITS_TO_LONGS() is not what I'm after.
BIT(n) sets nth bit and what I'm looking for is converting n to power of
two size. Obviously, both are mathematically doing 2^n (or 1 << n) but
they feel conceptually very different things.
--
i.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-18 15:15 ` Ilpo Järvinen
@ 2025-04-19 14:57 ` Ingo Molnar
0 siblings, 0 replies; 11+ messages in thread
From: Ingo Molnar @ 2025-04-19 14:57 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Andy Shevchenko, Rafael J. Wysocki, Len Brown, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Bjorn Helgaas, linux-acpi, LKML, linux-pci
* Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> On Fri, 18 Apr 2025, Ingo Molnar wrote:
>
> >
> > * Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> wrote:
> >
> > > On Wed, 16 Apr 2025, Andy Shevchenko wrote:
> > >
> > > > On Wed, Apr 16, 2025 at 01:13:18PM +0300, Ilpo Järvinen wrote:
> > > > > Convert open coded resource size calculations to use
> > > > > resource_set_{range,size}() helpers.
> > > > >
> > > > > While at it, use SZ_* for size parameter which makes the intent of code
> > > > > more obvious.
> > > >
> > > > ...
> > > >
> > > > > + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
> > > >
> > > > Then probably
> > > >
> > > > resource_set_range(res, base, BIT_ULL(segn_busn_bits) * SZ_1M);
> > > >
> > > > to follow the same "While at it"?
> > >
> > > I'll change that now since you brought it up. It did cross my mind to
> > > convert that to * SZ_1M but it seemed to go farther than I wanted with a
> > > simple conversion patch.
> > >
> > > I've never liked the abuse of BIT*() for size related shifts though,
> > > I recall I saw somewhere a helper that was better named for size
> > > related operations but I just cannot recall its name and seem to not
> > > find that anymore :-(. But until I come across it once again, I guess
> > > I'll have to settle to BIT*().
> >
> > BITS_TO_LONGS()?
>
> Hi Ingo,
>
> I'm not entiry sure if you're referring to my BIT*() matching unrelated
> macros such as BITS_TO_LONGS() (I only meant BIT() and BIT_ULL() which I
> thought was clear from the context), or that BITS_TO_LONGS() would be the
> solution what I'm looking for.
Indeed, I misremembered BITS_TO_LONGS() - now that I looked up its
definition it's definitely not what you wanted... :)
Thanks,
Ingo
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/1] x86: Use resource_set_{range,size}() helpers
2025-04-16 10:13 [PATCH 1/1] x86: Use resource_set_{range,size}() helpers Ilpo Järvinen
2025-04-16 10:22 ` Andy Shevchenko
@ 2025-04-16 15:05 ` Rafael J. Wysocki
1 sibling, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2025-04-16 15:05 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Rafael J. Wysocki, Len Brown, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Bjorn Helgaas,
Andy Shevchenko, linux-acpi, linux-kernel, linux-pci
On Wed, Apr 16, 2025 at 12:13 PM Ilpo Järvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> Convert open coded resource size calculations to use
> resource_set_{range,size}() helpers.
>
> While at it, use SZ_* for size parameter which makes the intent of code
> more obvious.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
For the change in acpi/boot.c
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
> arch/x86/kernel/acpi/boot.c | 4 ++--
> arch/x86/kernel/amd_nb.c | 3 +--
> arch/x86/kernel/apic/apic.c | 3 +--
> arch/x86/kernel/apic/io_apic.c | 3 +--
> arch/x86/kernel/probe_roms.c | 3 +--
> arch/x86/pci/fixup.c | 4 ++--
> arch/x86/pci/intel_mid_pci.c | 2 +-
> 7 files changed, 9 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index dae6a73be40e..4490cbc01030 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -21,6 +21,7 @@
> #include <linux/pci.h>
> #include <linux/efi-bgrt.h>
> #include <linux/serial_core.h>
> +#include <linux/sizes.h>
> #include <linux/pgtable.h>
>
> #include <asm/e820/api.h>
> @@ -940,8 +941,7 @@ static int __init acpi_parse_hpet(struct acpi_table_header *table)
> snprintf((char *)hpet_res->name, HPET_RESOURCE_NAME_SIZE, "HPET %u",
> hpet_tbl->sequence);
>
> - hpet_res->start = hpet_address;
> - hpet_res->end = hpet_address + (1 * 1024) - 1;
> + resource_set_range(hpet_res, hpet_address, SZ_1K);
>
> return 0;
> }
> diff --git a/arch/x86/kernel/amd_nb.c b/arch/x86/kernel/amd_nb.c
> index 6d12a9b69432..cba336dcb40d 100644
> --- a/arch/x86/kernel/amd_nb.c
> +++ b/arch/x86/kernel/amd_nb.c
> @@ -164,8 +164,7 @@ struct resource *amd_get_mmconfig_range(struct resource *res)
> FAM10H_MMIO_CONF_BUSRANGE_MASK;
>
> res->flags = IORESOURCE_MEM;
> - res->start = base;
> - res->end = base + (1ULL<<(segn_busn_bits + 20)) - 1;
> + resource_set_range(res, base, 1ULL << (segn_busn_bits + 20));
> return res;
> }
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index 62584a347931..efd3304ecbb3 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -2640,8 +2640,7 @@ static int __init lapic_insert_resource(void)
> return -1;
>
> /* Put local APIC into the resource map. */
> - lapic_resource.start = apic_mmio_base;
> - lapic_resource.end = lapic_resource.start + PAGE_SIZE - 1;
> + resource_set_range(&lapic_resource, apic_mmio_base, PAGE_SIZE);
> insert_resource(&iomem_resource, &lapic_resource);
>
> return 0;
> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
> index eebc360ed1bb..3069885d6421 100644
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -2571,8 +2571,7 @@ void __init io_apic_init_mappings(void)
> __fix_to_virt(idx) + (ioapic_phys & ~PAGE_MASK), ioapic_phys);
> idx++;
>
> - ioapic_res->start = ioapic_phys;
> - ioapic_res->end = ioapic_phys + IO_APIC_SLOT_SIZE - 1;
> + resource_set_range(ioapic_res, ioapic_phys, IO_APIC_SLOT_SIZE);
> ioapic_res++;
> }
> }
> diff --git a/arch/x86/kernel/probe_roms.c b/arch/x86/kernel/probe_roms.c
> index cc2c34ba7228..44da85e50c44 100644
> --- a/arch/x86/kernel/probe_roms.c
> +++ b/arch/x86/kernel/probe_roms.c
> @@ -260,8 +260,7 @@ void __init probe_roms(void)
> if (!length || start + length > upper || !romchecksum(rom, length))
> continue;
>
> - adapter_rom_resources[i].start = start;
> - adapter_rom_resources[i].end = start + length - 1;
> + resource_set_range(&adapter_rom_resources[i], start, length);
> request_resource(&iomem_resource, &adapter_rom_resources[i]);
>
> start = adapter_rom_resources[i++].end & ~2047UL;
> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index efefeb82ab61..94e98e3bf041 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -7,6 +7,7 @@
> #include <linux/delay.h>
> #include <linux/dmi.h>
> #include <linux/pci.h>
> +#include <linux/sizes.h>
> #include <linux/suspend.h>
> #include <linux/vgaarb.h>
> #include <asm/amd_node.h>
> @@ -347,8 +348,7 @@ static void pci_fixup_video(struct pci_dev *pdev)
> if (res->parent)
> release_resource(res);
>
> - res->start = 0xC0000;
> - res->end = res->start + 0x20000 - 1;
> + resource_set_range(res, 0xC0000, SZ_128K);
> res->flags = IORESOURCE_MEM | IORESOURCE_ROM_SHADOW |
> IORESOURCE_PCI_FIXED;
> dev_info(&pdev->dev, "Video device with shadowed ROM at %pR\n",
> diff --git a/arch/x86/pci/intel_mid_pci.c b/arch/x86/pci/intel_mid_pci.c
> index b433b1753016..5e047e802d5d 100644
> --- a/arch/x86/pci/intel_mid_pci.c
> +++ b/arch/x86/pci/intel_mid_pci.c
> @@ -399,7 +399,7 @@ static void pci_fixed_bar_fixup(struct pci_dev *dev)
>
> for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> pci_read_config_dword(dev, offset + 8 + (i * 4), &size);
> - dev->resource[i].end = dev->resource[i].start + size - 1;
> + resource_set_size(&dev->resource[i], size);
> dev->resource[i].flags |= IORESOURCE_PCI_FIXED;
> }
> }
> --
> 2.39.5
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-04-19 14:57 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-16 10:13 [PATCH 1/1] x86: Use resource_set_{range,size}() helpers Ilpo Järvinen
2025-04-16 10:22 ` Andy Shevchenko
2025-04-16 10:23 ` Andy Shevchenko
2025-04-16 11:53 ` Ilpo Järvinen
2025-04-16 12:03 ` Andy Shevchenko
2025-04-16 12:18 ` Ilpo Järvinen
2025-04-16 15:25 ` Andy Shevchenko
2025-04-18 7:40 ` Ingo Molnar
2025-04-18 15:15 ` Ilpo Järvinen
2025-04-19 14:57 ` Ingo Molnar
2025-04-16 15:05 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).