[PATCH v1 0/2] acpi: Remove acpi_release_memory()

[PATCH v1 0/2] acpi: Remove acpi_release_memory()

[Heikki Krogerus]

Hi,

It seems there never were and there never will be actual devices that
expose the UCSI ACPI mailbox interface. There are now PD controllers
that support the UCSI interface, but they do not use the ACPI mailbox.

So there is no point in mapping the mailbox with ioremap(), we can
just use memremap(). That should make it possible to also remove the
function acpi_release_memory(). That function was only there to make
it possible to use ioremap() in the UCSI ACPI driver.

thanks,

Heikki Krogerus (2):
  usb: typec: ucsi: acpi: Map the mailbox with memremap()
  acpi: Remove the helper for deactivating memory region

 drivers/acpi/osl.c                 | 86 ------------------------------
 drivers/usb/typec/ucsi/ucsi_acpi.c | 19 ++-----
 include/linux/acpi.h               |  3 --
 3 files changed, 4 insertions(+), 104 deletions(-)

-- 
2.35.1

[PATCH v1 1/2] usb: typec: ucsi: acpi: Map the mailbox with memremap()

[Heikki Krogerus]

The UCSI mailbox is always in main memory.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/usb/typec/ucsi/ucsi_acpi.c | 19 ++++---------------
 1 file changed, 4 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
index 6771f05e32c29..7455e3aff2be0 100644
--- a/drivers/usb/typec/ucsi/ucsi_acpi.c
+++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
@@ -19,7 +19,7 @@
 struct ucsi_acpi {
 	struct device *dev;
 	struct ucsi *ucsi;
-	void __iomem *base;
+	void *base;
 	struct completion complete;
 	unsigned long flags;
 	guid_t guid;
@@ -51,7 +51,7 @@ static int ucsi_acpi_read(struct ucsi *ucsi, unsigned int offset,
 	if (ret)
 		return ret;
 
-	memcpy(val, (const void __force *)(ua->base + offset), val_len);
+	memcpy(val, ua->base + offset, val_len);
 
 	return 0;
 }
@@ -61,7 +61,7 @@ static int ucsi_acpi_async_write(struct ucsi *ucsi, unsigned int offset,
 {
 	struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
 
-	memcpy((void __force *)(ua->base + offset), val, val_len);
+	memcpy(ua->base + offset, val, val_len);
 
 	return ucsi_acpi_dsm(ua, UCSI_DSM_FUNC_WRITE);
 }
@@ -132,18 +132,7 @@ static int ucsi_acpi_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	/* This will make sure we can use ioremap() */
-	status = acpi_release_memory(ACPI_HANDLE(&pdev->dev), res, 1);
-	if (ACPI_FAILURE(status))
-		return -ENOMEM;
-
-	/*
-	 * NOTE: The memory region for the data structures is used also in an
-	 * operation region, which means ACPI has already reserved it. Therefore
-	 * it can not be requested here, and we can not use
-	 * devm_ioremap_resource().
-	 */
-	ua->base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+	ua->base = devm_memremap(&pdev->dev, res->start, resource_size(res), MEMREMAP_WB);
 	if (!ua->base)
 		return -ENOMEM;
 
-- 
2.35.1

[PATCH v1 2/2] acpi: Remove the helper for deactivating memory region

[Heikki Krogerus]

There are no more users for acpi_release_memory().

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
 drivers/acpi/osl.c   | 86 --------------------------------------------
 include/linux/acpi.h |  3 --
 2 files changed, 89 deletions(-)

diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 7a70c4bfc23c6..3269a888fb7a9 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -36,7 +36,6 @@
 #include <linux/io-64-nonatomic-lo-hi.h>
 
 #include "acpica/accommon.h"
-#include "acpica/acnamesp.h"
 #include "internal.h"
 
 /* Definitions for ACPI_DEBUG_PRINT() */
@@ -1496,91 +1495,6 @@ int acpi_check_region(resource_size_t start, resource_size_t n,
 }
 EXPORT_SYMBOL(acpi_check_region);
 
-static acpi_status acpi_deactivate_mem_region(acpi_handle handle, u32 level,
-					      void *_res, void **return_value)
-{
-	struct acpi_mem_space_context **mem_ctx;
-	union acpi_operand_object *handler_obj;
-	union acpi_operand_object *region_obj2;
-	union acpi_operand_object *region_obj;
-	struct resource *res = _res;
-	acpi_status status;
-
-	region_obj = acpi_ns_get_attached_object(handle);
-	if (!region_obj)
-		return AE_OK;
-
-	handler_obj = region_obj->region.handler;
-	if (!handler_obj)
-		return AE_OK;
-
-	if (region_obj->region.space_id != ACPI_ADR_SPACE_SYSTEM_MEMORY)
-		return AE_OK;
-
-	if (!(region_obj->region.flags & AOPOBJ_SETUP_COMPLETE))
-		return AE_OK;
-
-	region_obj2 = acpi_ns_get_secondary_object(region_obj);
-	if (!region_obj2)
-		return AE_OK;
-
-	mem_ctx = (void *)&region_obj2->extra.region_context;
-
-	if (!(mem_ctx[0]->address >= res->start &&
-	      mem_ctx[0]->address < res->end))
-		return AE_OK;
-
-	status = handler_obj->address_space.setup(region_obj,
-						  ACPI_REGION_DEACTIVATE,
-						  NULL, (void **)mem_ctx);
-	if (ACPI_SUCCESS(status))
-		region_obj->region.flags &= ~(AOPOBJ_SETUP_COMPLETE);
-
-	return status;
-}
-
-/**
- * acpi_release_memory - Release any mappings done to a memory region
- * @handle: Handle to namespace node
- * @res: Memory resource
- * @level: A level that terminates the search
- *
- * Walks through @handle and unmaps all SystemMemory Operation Regions that
- * overlap with @res and that have already been activated (mapped).
- *
- * This is a helper that allows drivers to place special requirements on memory
- * region that may overlap with operation regions, primarily allowing them to
- * safely map the region as non-cached memory.
- *
- * The unmapped Operation Regions will be automatically remapped next time they
- * are called, so the drivers do not need to do anything else.
- */
-acpi_status acpi_release_memory(acpi_handle handle, struct resource *res,
-				u32 level)
-{
-	acpi_status status;
-
-	if (!(res->flags & IORESOURCE_MEM))
-		return AE_TYPE;
-
-	status = acpi_walk_namespace(ACPI_TYPE_REGION, handle, level,
-				     acpi_deactivate_mem_region, NULL,
-				     res, NULL);
-	if (ACPI_FAILURE(status))
-		return status;
-
-	/*
-	 * Wait for all of the mappings queued up for removal by
-	 * acpi_deactivate_mem_region() to actually go away.
-	 */
-	synchronize_rcu();
-	rcu_barrier();
-	flush_scheduled_work();
-
-	return AE_OK;
-}
-EXPORT_SYMBOL_GPL(acpi_release_memory);
-
 /*
  * Let drivers know whether the resource checks are effective
  */
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index d7136d13aa442..fadda404bcc9b 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -520,9 +520,6 @@ int acpi_check_resource_conflict(const struct resource *res);
 int acpi_check_region(resource_size_t start, resource_size_t n,
 		      const char *name);
 
-acpi_status acpi_release_memory(acpi_handle handle, struct resource *res,
-				u32 level);
-
 int acpi_resources_are_enforced(void);
 
 #ifdef CONFIG_HIBERNATION
-- 
2.35.1

Re: [PATCH v1 0/2] acpi: Remove acpi_release_memory()

[Greg Kroah-Hartman]

On Mon, Apr 25, 2022 at 02:45:42PM +0300, Heikki Krogerus wrote:
> Hi,
> 
> It seems there never were and there never will be actual devices that
> expose the UCSI ACPI mailbox interface. There are now PD controllers
> that support the UCSI interface, but they do not use the ACPI mailbox.
> 
> So there is no point in mapping the mailbox with ioremap(), we can
> just use memremap(). That should make it possible to also remove the
> function acpi_release_memory(). That function was only there to make
> it possible to use ioremap() in the UCSI ACPI driver.
> 
> thanks,
> 
> Heikki Krogerus (2):
>   usb: typec: ucsi: acpi: Map the mailbox with memremap()
>   acpi: Remove the helper for deactivating memory region
> 
>  drivers/acpi/osl.c                 | 86 ------------------------------
>  drivers/usb/typec/ucsi/ucsi_acpi.c | 19 ++-----
>  include/linux/acpi.h               |  3 --
>  3 files changed, 4 insertions(+), 104 deletions(-)
> 
> -- 
> 2.35.1
> 

Look good to me, Rafael, want me to take this through the USB tree?  Or
if you want to take it through the USB tree, here's my reviewed-by:
either is fine with me.

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Re: [PATCH v1 0/2] acpi: Remove acpi_release_memory()

[Rafael J. Wysocki]

On Tue, Apr 26, 2022 at 1:43 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Apr 25, 2022 at 02:45:42PM +0300, Heikki Krogerus wrote:
> > Hi,
> >
> > It seems there never were and there never will be actual devices that
> > expose the UCSI ACPI mailbox interface. There are now PD controllers
> > that support the UCSI interface, but they do not use the ACPI mailbox.
> >
> > So there is no point in mapping the mailbox with ioremap(), we can
> > just use memremap(). That should make it possible to also remove the
> > function acpi_release_memory(). That function was only there to make
> > it possible to use ioremap() in the UCSI ACPI driver.
> >
> > thanks,
> >
> > Heikki Krogerus (2):
> >   usb: typec: ucsi: acpi: Map the mailbox with memremap()
> >   acpi: Remove the helper for deactivating memory region
> >
> >  drivers/acpi/osl.c                 | 86 ------------------------------
> >  drivers/usb/typec/ucsi/ucsi_acpi.c | 19 ++-----
> >  include/linux/acpi.h               |  3 --
> >  3 files changed, 4 insertions(+), 104 deletions(-)
> >
> > --
> > 2.35.1
> >
>
> Look good to me, Rafael, want me to take this through the USB tree?  Or
> if you want to take it through the USB tree, here's my reviewed-by:
> either is fine with me.
>
> Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I've taken this into the ACPI tree (for 5.19) with your R-by, thanks!