Make ACPI_PTR() use PTR_IF()

The ipu-bridge depends on CONFIG_ACPI, therefore ACPI_PTR(ptr) always yields ptr. Drop all use of ACPI_PTR() in the ipu-bridge.

Signed-off-by Sakari Ailus <[email protected]>

---
 drivers/media/pci/intel/ipu-bridge.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

drivers/media/pci/intel/ipu-bridge.c +8 -10

index f9efa501e8cf..cdd654efd1dd 100644
@@ -17,7 +17,7 @@
 #include <media/ipu-bridge.h>
 #include <media/v4l2-fwnode.h>
 
-#define ADEV_DEV(adev) ACPI_PTR(&((adev)->dev))
+#define ADEV_DEV(adev) &(adev)->dev
 
 /*
  * 92335fcf-3203-4472-af93-7b4453ac29da

@@ -239,12 +239,12 @@ static struct acpi_device *ipu_bridge_get_ivsc_acpi_dev(struct acpi_device *adev
        for (i = 0; i < ARRAY_SIZE(ivsc_acpi_ids); i++) {
                const struct acpi_device_id *acpi_id = &ivsc_acpi_ids[i];
                struct acpi_device *consumer, *ivsc_adev;
-               acpi_handle handle = acpi_device_handle(ACPI_PTR(adev));
+               acpi_handle handle = acpi_device_handle(adev);
 
                for_each_acpi_dev_match(ivsc_adev, acpi_id->id, NULL, -1)
                        /* camera sensor depends on IVSC in DSDT if exist */
                        for_each_acpi_consumer_dev(ivsc_adev, consumer)
-                               if (ACPI_PTR(consumer->handle) == handle) {
+                               if (consumer->handle == handle) {
                                        acpi_dev_put(consumer);
                                        return ivsc_adev;
                                }

@@ -322,8 +322,7 @@ static int ipu_bridge_read_acpi_buffer(struct acpi_device *adev, char *id,
        acpi_status status;
        int ret = 0;
 
-       status = acpi_evaluate_object(ACPI_PTR(adev->handle),
-                                     id, NULL, &buffer);
+       status = acpi_evaluate_object(adev->handle, id, NULL, &buffer);
        if (ACPI_FAILURE(status))
                return -ENODEV;
 

@@ -379,7 +378,7 @@ static enum v4l2_fwnode_orientation ipu_bridge_parse_orientation(struct acpi_dev
        enum v4l2_fwnode_orientation orientation;
        struct acpi_pld_info *pld = NULL;
 
-       if (!acpi_get_physical_device_location(ACPI_PTR(adev->handle), &pld)) {
+       if (!acpi_get_physical_device_location(adev->handle, &pld)) {
                dev_warn(ADEV_DEV(adev), "_PLD call failed, using default orientation\n");
                return V4L2_FWNODE_ORIENTATION_EXTERNAL;
        }

@@ -785,7 +784,7 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg,
        int ret;
 
        for_each_acpi_dev_match(adev, cfg->hid, NULL, -1) {
-               if (!ACPI_PTR(adev->status.enabled))
+               if (!adev->status.enabled)
                        continue;
 
                if (bridge->n_sensors >= IPU_MAX_PORTS) {

@@ -821,7 +820,7 @@ static int ipu_bridge_connect_sensor(const struct ipu_sensor_config *cfg,
                        goto err_free_swnodes;
                }
 
-               sensor->adev = ACPI_PTR(acpi_dev_get(adev));
+               sensor->adev = acpi_dev_get(adev);
 
                primary = acpi_fwnode_handle(adev);
                primary->secondary = fwnode;

@@ -881,7 +880,7 @@ static int ipu_bridge_ivsc_is_ready(void)
                        &ipu_supported_sensors[i];
 
                for_each_acpi_dev_match(sensor_adev, cfg->hid, NULL, -1) {
-                       if (!ACPI_PTR(sensor_adev->status.enabled))
+                       if (!sensor_adev->status.enabled)
                                continue;
 
                        adev = ipu_bridge_get_ivsc_acpi_dev(sensor_adev);
--

2.47.3

Make ACPI_PTR() use PTR_IF() so __maybe_unused can be dropped from what is being referred to.

Signed-off-by Sakari Ailus <[email protected]>

---
 include/linux/acpi.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

include/linux/acpi.h +2 -4

index 10d6c6c11bdf..347763282f36 100644
@@ -755,7 +755,6 @@ int acpi_device_modalias(struct device *, char *, int);
 
 struct platform_device *acpi_create_platform_device(struct acpi_device *,
                                                    const struct property_entry *);
-#define ACPI_PTR(_ptr) (_ptr)
 
 static inline void acpi_device_set_enumerated(struct acpi_device *adev)
 {

@@ -1073,8 +1072,6 @@ static inline int acpi_dma_configure_id(struct device *dev,
        return 0;
 }
 
-#define ACPI_PTR(_ptr) (NULL)
-
 static inline void acpi_device_set_enumerated(struct acpi_device *adev)
 {
 }

@@ -1152,6 +1149,8 @@ static inline int acpi_mrrm_max_mem_region(void)
 
 #endif /* !CONFIG_ACPI */
 
+#define ACPI_PTR(_ptr) PTR_IF(IS_BUILTIN(CONFIG_ACPI), _ptr)
+
 #ifdef CONFIG_ACPI_HMAT
 int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid,
                                        resource_size_t *size);
--

2.47.3

Quoted 5 lines · Sakari Ailus

Make ACPI_PTR() use PTR_IF() so __maybe_unused can be dropped from what is being referred to.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Rafael J. Wysocki (Intel) <[email protected]>

Quoted 35 lines

---
 include/linux/acpi.h | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

include/linux/acpi.h +2 -4

index 10d6c6c11bdf..347763282f36 100644
@@ -755,7 +755,6 @@ int acpi_device_modalias(struct device *, char *, int);
 
 struct platform_device *acpi_create_platform_device(struct acpi_device *,
                                                    const struct property_entry *);
-#define ACPI_PTR(_ptr) (_ptr)
 
 static inline void acpi_device_set_enumerated(struct acpi_device *adev)
 {

@@ -1073,8 +1072,6 @@ static inline int acpi_dma_configure_id(struct device *dev,
        return 0;
 }
 
-#define ACPI_PTR(_ptr) (NULL)
-
 static inline void acpi_device_set_enumerated(struct acpi_device *adev)
 {
 }

@@ -1152,6 +1149,8 @@ static inline int acpi_mrrm_max_mem_region(void)
 
 #endif /* !CONFIG_ACPI */
 
+#define ACPI_PTR(_ptr) PTR_IF(IS_BUILTIN(CONFIG_ACPI), _ptr)
+
 #ifdef CONFIG_ACPI_HMAT
 int hmat_get_extended_linear_cache_size(struct resource *backing_res, int nid,
                                        resource_size_t *size);
--

Quoted 4 lines · Rafael J. Wysocki (Intel)

Quoted 1 line · Sakari Ailus

Quoted 6 lines

Quoted 4 lines

Make ACPI_PTR() use PTR_IF() so __maybe_unused can be dropped from what is being referred to.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Rafael J. Wysocki (Intel) <[email protected]>

Or do you want me to pick it up?

Quoted 9 lines · Rafael J. Wysocki (Intel)

Quoted 7 lines · Rafael J. Wysocki (Intel)

Quoted 4 lines · Sakari Ailus

Make ACPI_PTR() use PTR_IF() so __maybe_unused can be dropped from what is being referred to.

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Rafael J. Wysocki (Intel) <[email protected]>

Or do you want me to pick it up?

I'm happy to take it.

I wondered if you had an opinion on ACPI_PTR() use in the third patch.

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

Quoted 7 lines · Sakari Ailus

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

NAK.

It doesn't explain why we need that. This also inconsistent with OF type of IDs.

Quoted 9 lines · Andy Shevchenko

Quoted 7 lines · Sakari Ailus

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

Quoted 4 lines

NAK.

It doesn't explain why we need that. This also inconsistent with OF type of IDs.

ACPI IDs are used on ACPI only while compatible strings are used on both ACPI and OF. Thus the ACPI ID table is redundant if CONFIG_ACPI is disabled.

Quoted 15 lines · Sakari Ailus

Quoted 13 lines · Andy Shevchenko

Quoted 7 lines · Sakari Ailus

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

NAK.

It doesn't explain why we need that. This also inconsistent with OF type of IDs.

Quoted 3 lines

ACPI IDs are used on ACPI only while compatible strings are used on both ACPI and OF. Thus the ACPI ID table is redundant if CONFIG_ACPI is disabled.

They may be informative for the cases when on the working say OF system I can get the IDs for ACPI based platforms at run-time.

Also you just save a couple of dozens of bytes. This is peanuts and unneeded churn. Again, here is my formal

NAKed-by Andy Shevchenko <[email protected]>

In case you want to override that, please keep it in the commit message (among other tags). And yes, we have such a tag in the Git history of Linux kernel.

Quoted 16 lines · Andy Shevchenko

Quoted 15 lines · Sakari Ailus

Quoted 13 lines · Andy Shevchenko

Quoted 7 lines · Sakari Ailus

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

NAK.

It doesn't explain why we need that. This also inconsistent with OF type of IDs.

Quoted 6 lines

Quoted 3 lines

ACPI IDs are used on ACPI only while compatible strings are used on both ACPI and OF. Thus the ACPI ID table is redundant if CONFIG_ACPI is disabled.

They may be informative for the cases when on the working say OF system I can get the IDs for ACPI based platforms at run-time.

That's a bit far-fetched I think. Have you ever done that? :-)

Media drivers are probably one of the largest groups of drivers that are as relevant for ACPI as they are for OF.

The of_match_ptr() that used to be present in nearly all media drivers has been largely eliminated in the same context and the only reason for that is that the table is also relevant for ACPI systems.

The case is indeed the
opposite to the ACPI equivalent so I really don't see why we shouldn't just drop it where it's, well, redundant.

Quoted 2 lines

Also you just save a couple of dozens of bytes. This is peanuts and unneeded churn. Again, here is my formal

I recall you often comment on struct field ordering to save, well, typically less than dozens of bytes, too.

Quoted 1 line

NAKed-by Andy Shevchenko <[email protected]>

Quoted 2 lines

In case you want to override that, please keep it in the commit message (among other tags). And yes, we have such a tag in the Git history of Linux kernel.

Quoted 17 lines · Sakari Ailus

Quoted 16 lines · Andy Shevchenko

Quoted 15 lines · Sakari Ailus

Quoted 13 lines · Andy Shevchenko

Quoted 7 lines · Sakari Ailus

Add ACPI_PTR() to the ACPI ID tables where missing. The patch has been produced with the following command:

git grep -lP 'acpi_match_table.*=\s*(?!ACPI_PTR)' \
	drivers/media/|while read i; do
	perl -i -pe 's/\.acpi_match_table\s*=\s*\K(\w+),/ACPI_PTR($1),/' $i;

done

NAK.

It doesn't explain why we need that. This also inconsistent with OF type of IDs.

Quoted 8 lines

Quoted 6 lines

Quoted 3 lines

ACPI IDs are used on ACPI only while compatible strings are used on both ACPI and OF. Thus the ACPI ID table is redundant if CONFIG_ACPI is disabled.

They may be informative for the cases when on the working say OF system I can get the IDs for ACPI based platforms at run-time.

That's a bit far-fetched I think. Have you ever done that? :-)

I have not working with OF platforms, but I'm pretty much see value of this possibility.

Quoted 6 lines

Media drivers are probably one of the largest groups of drivers that are as relevant for ACPI as they are for OF.

The of_match_ptr() that used to be present in nearly all media drivers has been largely eliminated in the same context and the only reason for that is that the table is also relevant for ACPI systems.

So, if we ever will have a code to use ACPI IDs in OF enumeration, this will have the same argument.

Quoted 3 lines

The case is indeed the
opposite to the ACPI equivalent so I really don't see why we shouldn't just drop it where it's, well, redundant.

The use of the unneeded macro makes code harder to follow. Still ID tables are useful on their own.

Quoted 6 lines

Quoted 3 lines

Also you just save a couple of dozens of bytes. This is peanuts and unneeded churn. Again, here is my formal

I recall you often comment on struct field ordering to save, well, typically less than dozens of bytes, too.

There is a difference, the ID tables are from RO sections, the structures are runtime and grow linearly as many objects of a type are created. Among two the priority of reducing runtime memory footprint has a priority over the RO sections.

Quoted 2 lines

Quoted 2 lines

NAKed-by Andy Shevchenko <[email protected]>

The tag still stays. But nobody prevents media maintainers to do what they want :-)

Quoted 3 lines

Quoted 3 lines

In case you want to override that, please keep it in the commit message (among other tags). And yes, we have such a tag in the Git history of Linux kernel.