| Message ID | 20230711095920.264308-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | platform/x86/amd: pmc: Use release_mem_region() to undo request_mem_region_muxed() | expand |
On Tue, Jul 11, 2023 at 11:59:20AM +0200, Hans de Goede wrote: > Muxed (mem) regions will wait in request_mem_region_muxed() if the region > is busy (in use by another consumer) during the call. > > In order to wake-up possibly waiting other consumers of the region, > it must be released by a release_mem_region() call, which will actually > wake up any waiters. > > release_mem_region() also frees the resource created by > request_mem_region_muxed(), avoiding the need for the unmatched kfree(). Seems reasonable to me. Do we need to have a Fixes tag? Reviewed-by: Andy Shevchenko <andy@kernel.org> > Cc: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > Mario, can you ask one of the reporters with a machine which needs the quirk > to test this ? > --- > drivers/platform/x86/amd/pmc-quirks.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc-quirks.c b/drivers/platform/x86/amd/pmc-quirks.c > index 362e7c0097d7..ad702463a65d 100644 > --- a/drivers/platform/x86/amd/pmc-quirks.c > +++ b/drivers/platform/x86/amd/pmc-quirks.c > @@ -11,7 +11,6 @@ > #include <linux/dmi.h> > #include <linux/io.h> > #include <linux/ioport.h> > -#include <linux/slab.h> > > #include "pmc.h" > > @@ -135,12 +134,10 @@ static const struct dmi_system_id fwbug_list[] = { > */ > static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) > { > - struct resource *res; > void __iomem *addr; > u8 val; > > - res = request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80"); > - if (!res) > + if (!request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80")) > return; > > addr = ioremap(s2idle_bug_mmio, 1); > @@ -152,8 +149,7 @@ static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) > > iounmap(addr); > cleanup_resource: > - release_resource(res); > - kfree(res); > + release_mem_region(s2idle_bug_mmio, 1); > } > > void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev) > -- > 2.41.0 >
On 7/11/23 06:13, Andy Shevchenko wrote: > On Tue, Jul 11, 2023 at 11:59:20AM +0200, Hans de Goede wrote: >> Muxed (mem) regions will wait in request_mem_region_muxed() if the region >> is busy (in use by another consumer) during the call. >> >> In order to wake-up possibly waiting other consumers of the region, >> it must be released by a release_mem_region() call, which will actually >> wake up any waiters. >> >> release_mem_region() also frees the resource created by >> request_mem_region_muxed(), avoiding the need for the unmatched kfree(). > > Seems reasonable to me. > Do we need to have a Fixes tag? > Reviewed-by: Andy Shevchenko <andy@kernel.org> Reviewed-by: Mario Limonciello <mario.limonciello@amd.com> I've gotten a positive test report with this as well, thanks! > >> Cc: Mario Limonciello <mario.limonciello@amd.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> Mario, can you ask one of the reporters with a machine which needs the quirk >> to test this ? >> --- >> drivers/platform/x86/amd/pmc-quirks.c | 8 ++------ >> 1 file changed, 2 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/platform/x86/amd/pmc-quirks.c b/drivers/platform/x86/amd/pmc-quirks.c >> index 362e7c0097d7..ad702463a65d 100644 >> --- a/drivers/platform/x86/amd/pmc-quirks.c >> +++ b/drivers/platform/x86/amd/pmc-quirks.c >> @@ -11,7 +11,6 @@ >> #include <linux/dmi.h> >> #include <linux/io.h> >> #include <linux/ioport.h> >> -#include <linux/slab.h> >> >> #include "pmc.h" >> >> @@ -135,12 +134,10 @@ static const struct dmi_system_id fwbug_list[] = { >> */ >> static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) >> { >> - struct resource *res; >> void __iomem *addr; >> u8 val; >> >> - res = request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80"); >> - if (!res) >> + if (!request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80")) >> return; >> >> addr = ioremap(s2idle_bug_mmio, 1); >> @@ -152,8 +149,7 @@ static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) >> >> iounmap(addr); >> cleanup_resource: >> - release_resource(res); >> - kfree(res); >> + release_mem_region(s2idle_bug_mmio, 1); >> } >> >> void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev) >> -- >> 2.41.0 >> >
Hi, On 7/11/23 11:59, Hans de Goede wrote: > Muxed (mem) regions will wait in request_mem_region_muxed() if the region > is busy (in use by another consumer) during the call. > > In order to wake-up possibly waiting other consumers of the region, > it must be released by a release_mem_region() call, which will actually > wake up any waiters. > > release_mem_region() also frees the resource created by > request_mem_region_muxed(), avoiding the need for the unmatched kfree(). > > Cc: Mario Limonciello <mario.limonciello@amd.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Mario, Andy, thank you for the reviews, I've applied this patch to my fixes branch: https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=fixes I will include this patch in my next fixes pull-req to Linus for the current kernel development cycle. Regards, Hans > --- > Mario, can you ask one of the reporters with a machine which needs the quirk > to test this ? > --- > drivers/platform/x86/amd/pmc-quirks.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/platform/x86/amd/pmc-quirks.c b/drivers/platform/x86/amd/pmc-quirks.c > index 362e7c0097d7..ad702463a65d 100644 > --- a/drivers/platform/x86/amd/pmc-quirks.c > +++ b/drivers/platform/x86/amd/pmc-quirks.c > @@ -11,7 +11,6 @@ > #include <linux/dmi.h> > #include <linux/io.h> > #include <linux/ioport.h> > -#include <linux/slab.h> > > #include "pmc.h" > > @@ -135,12 +134,10 @@ static const struct dmi_system_id fwbug_list[] = { > */ > static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) > { > - struct resource *res; > void __iomem *addr; > u8 val; > > - res = request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80"); > - if (!res) > + if (!request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80")) > return; > > addr = ioremap(s2idle_bug_mmio, 1); > @@ -152,8 +149,7 @@ static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) > > iounmap(addr); > cleanup_resource: > - release_resource(res); > - kfree(res); > + release_mem_region(s2idle_bug_mmio, 1); > } > > void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev)
diff --git a/drivers/platform/x86/amd/pmc-quirks.c b/drivers/platform/x86/amd/pmc-quirks.c index 362e7c0097d7..ad702463a65d 100644 --- a/drivers/platform/x86/amd/pmc-quirks.c +++ b/drivers/platform/x86/amd/pmc-quirks.c @@ -11,7 +11,6 @@ #include <linux/dmi.h> #include <linux/io.h> #include <linux/ioport.h> -#include <linux/slab.h> #include "pmc.h" @@ -135,12 +134,10 @@ static const struct dmi_system_id fwbug_list[] = { */ static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) { - struct resource *res; void __iomem *addr; u8 val; - res = request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80"); - if (!res) + if (!request_mem_region_muxed(s2idle_bug_mmio, 1, "amd_pmc_pm80")) return; addr = ioremap(s2idle_bug_mmio, 1); @@ -152,8 +149,7 @@ static void amd_pmc_skip_nvme_smi_handler(u32 s2idle_bug_mmio) iounmap(addr); cleanup_resource: - release_resource(res); - kfree(res); + release_mem_region(s2idle_bug_mmio, 1); } void amd_pmc_process_restore_quirks(struct amd_pmc_dev *dev)
Muxed (mem) regions will wait in request_mem_region_muxed() if the region is busy (in use by another consumer) during the call. In order to wake-up possibly waiting other consumers of the region, it must be released by a release_mem_region() call, which will actually wake up any waiters. release_mem_region() also frees the resource created by request_mem_region_muxed(), avoiding the need for the unmatched kfree(). Cc: Mario Limonciello <mario.limonciello@amd.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- Mario, can you ask one of the reporters with a machine which needs the quirk to test this ? --- drivers/platform/x86/amd/pmc-quirks.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)