Re: [PATCH v2] Input: goodix - Fix compilation when ACPI support is disabled

[Hans de Goede <hdegoede@redhat.com>]

Hi,

On 4/1/20 3:45 AM, Dmitry Torokhov wrote:
> Hi Hans,
> 
> On Wed, Mar 25, 2020 at 04:02:46PM +0100, Hans de Goede wrote:
>> acpi_evaluate_object() and acpi_execute_simple_method() are not part of
>> the group of ACPI related functions which get stubbed by
>> include/linux/acpi.h when ACPI support is disabled, so the
>> IRQ_PIN_ACCESS_ACPI_METHOD handling code must be disabled through
>> an #ifdef when ACPI support is not enabled.
>>
>> For consistency also #ifdef out the IRQ_PIN_ACCESS_ACPI_GPIO code
>> and use the same #if condition as which is used to replace
>> goodix_add_acpi_gpio_mappings with a stub.
> 
> I am not big fun of multiple #ifdefs sprinkled through the code, can we
> do more straightforward stubs, like below?

The solution you suggested is fine with me.

Regards,

Hans


p.s.

For reference here is Dmitry's solution once again, manually copied
because my email client cut it of as being part of the signature:


Input: goodix - fix compilation when ACPI support is disabled

From: Hans de Goede <hdegoede@redhat.com>

acpi_evaluate_object() and acpi_execute_simple_method() are not part of
the group of ACPI related functions which get stubbed by
include/linux/acpi.h when ACPI support is disabled, so the
IRQ_PIN_ACCESS_ACPI_METHOD handling code must be stubbed out.

For consistency use the same #if condition as which is used to replace
goodix_add_acpi_gpio_mappings with a stub.

Fixes: c5fca485320e ("Input: goodix - add support for controlling the IRQ pin through ACPI methods")
Reported-by: kbuild test robot <lkp@intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
[dtor: stubbed out the ACPI method accessors]
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
  drivers/input/touchscreen/goodix.c |   55 +++++++++++++++++++++++++++---------
  1 file changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 47f812b804c8..02c75ea385e0 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -63,6 +63,11 @@
  #define MAX_CONTACTS_LOC	5
  #define TRIGGER_LOC		6

+/* Our special handling for GPIO accesses through ACPI is x86 specific */
+#if defined CONFIG_X86 && defined CONFIG_ACPI
+#define ACPI_GPIO_SUPPORT
+#endif
+
  struct goodix_ts_data;

  enum goodix_irq_pin_access_method {
@@ -600,12 +605,42 @@ static int goodix_send_cfg(struct goodix_ts_data *ts, const u8 *cfg, int len)
  	return 0;
  }

-static int goodix_irq_direction_output(struct goodix_ts_data *ts,
-				       int value)
+#ifdef ACPI_GPIO_SUPPORT
+static int goodix_pin_acpi_direction_input(struct goodix_ts_data *ts)
  {
-	struct device *dev = &ts->client->dev;
+	acpi_handle handle = ACPI_HANDLE(&ts->client->dev);
  	acpi_status status;

+	status = acpi_evaluate_object(handle, "INTI", NULL, NULL);
+	return ACPI_SUCCESS(status) ? 0 : -EIO;
+}
+
+static int goodix_pin_acpi_output_method(struct goodix_ts_data *ts, int value)
+{
+	acpi_handle handle = ACPI_HANDLE(&ts->client->dev);
+	acpi_status status;
+
+	status = acpi_execute_simple_method(handle, "INTO", value);
+	return ACPI_SUCCESS(status) ? 0 : -EIO;
+}
+#else
+static int goodix_pin_acpi_direction_input(struct goodix_ts_data *ts)
+{
+	dev_err(&ts->client->dev,
+		"%s called on device without ACPI support\n", __func__);
+	return -EINVAL;
+}
+
+static int goodix_pin_acpi_output_method(struct goodix_ts_data *ts, int value)
+{
+	dev_err(&ts->client->dev,
+		"%s called on device without ACPI support\n", __func__);
+	return -EINVAL;
+}
+#endif
+
+static int goodix_irq_direction_output(struct goodix_ts_data *ts, int value)
+{
  	switch (ts->irq_pin_access_method) {
  	case IRQ_PIN_ACCESS_NONE:
  		dev_err(&ts->client->dev,
@@ -621,9 +656,7 @@ static int goodix_irq_direction_output(struct goodix_ts_data *ts,
  		 */
  		return gpiod_direction_output_raw(ts->gpiod_int, value);
  	case IRQ_PIN_ACCESS_ACPI_METHOD:
-		status = acpi_execute_simple_method(ACPI_HANDLE(dev),
-						    "INTO", value);
-		return ACPI_SUCCESS(status) ? 0 : -EIO;
+		return goodix_pin_acpi_output_method(ts, value);
  	}

  	return -EINVAL; /* Never reached */
@@ -631,9 +664,6 @@ static int goodix_irq_direction_output(struct goodix_ts_data *ts,

  static int goodix_irq_direction_input(struct goodix_ts_data *ts)
  {
-	struct device *dev = &ts->client->dev;
-	acpi_status status;
-
  	switch (ts->irq_pin_access_method) {
  	case IRQ_PIN_ACCESS_NONE:
  		dev_err(&ts->client->dev,
@@ -641,12 +671,11 @@ static int goodix_irq_direction_input(struct goodix_ts_data *ts)
  			__func__);
  		return -EINVAL;
  	case IRQ_PIN_ACCESS_GPIO:
+		return gpiod_direction_input(ts->gpiod_int);
  	case IRQ_PIN_ACCESS_ACPI_GPIO:
  		return gpiod_direction_input(ts->gpiod_int);
  	case IRQ_PIN_ACCESS_ACPI_METHOD:
-		status = acpi_evaluate_object(ACPI_HANDLE(dev), "INTI",
-					      NULL, NULL);
-		return ACPI_SUCCESS(status) ? 0 : -EIO;
+		return goodix_pin_acpi_direction_input(ts);
  	}

  	return -EINVAL; /* Never reached */
@@ -710,7 +739,7 @@ static int goodix_reset(struct goodix_ts_data *ts)
  	return 0;
  }

-#if defined CONFIG_X86 && defined CONFIG_ACPI
+#ifdef ACPI_GPIO_SUPPORT
  #include <asm/cpu_device_id.h>
  #include <asm/intel-family.h>